All users were logged out of Bugzilla on October 13th, 2018

Downloaded font resources take up excessive memory

RESOLVED FIXED in Firefox 49

Status

()

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: jfkthame, Assigned: jfkthame)

Tracking

unspecified
mozilla49
Points:
---

Firefox Tracking Flags

(firefox49 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

3 years ago
When we download, decode and sanitize @font-face resources, we generally end up with quite a bit of over-allocation. The OTS decoder/sanitizer cannot know in advance how large the final font data will be, so it uses an ExpandingMemoryStream which grows by leaps and bounds... so for example, each of the Fira fonts used on b.m.o ends up with a sanitized data block that has around 100K or so of unused space.

So we can reduce the persistent RAM requirements of downloaded fonts by reallocating the data to a properly-sized block, and get rid of the clownshoes.
(Assignee)

Comment 1

3 years ago
Created attachment 8747194 [details] [diff] [review]
Reallocate sanitized user font data into an appropriately-sized block, to reduce ongoing RAM footprint

Seems worth trimming the clownshoes here. With a few font-heavy sites loaded, the total slop can fairly easily run into megabytes.
Attachment #8747194 - Flags: review?(jmuizelaar)
(Assignee)

Updated

3 years ago
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Comment on attachment 8747194 [details] [diff] [review]
Reallocate sanitized user font data into an appropriately-sized block, to reduce ongoing RAM footprint

Review of attachment 8747194 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/thebes/gfxUserFontSet.cpp
@@ +266,1 @@
>  }

It seems like this would be better as a method ExpandingMemoryStream. Something like ShrinkToLength() perhaps.
Attachment #8747194 - Flags: review?(jmuizelaar) → review-
(Assignee)

Comment 3

3 years ago
Created attachment 8747301 [details] [diff] [review]
Reallocate sanitized user font data into an appropriately-sized block, to reduce ongoing RAM footprint

OK, here's an option I was considering: encapsulate the shrink-to-fit within the .forget() method. This is the only place where the ExpandingMemoryStream's buffer gets exposed to the outside world, so we can simply fix it up here and the client code needn't care what was going on internally while the stream was being written.
Attachment #8747301 - Flags: review?(jmuizelaar)
(Assignee)

Updated

3 years ago
Attachment #8747194 - Attachment is obsolete: true
Attachment #8747301 - Flags: review?(jmuizelaar) → review+

Comment 5

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/a619c5e024c6
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox49: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in before you can comment on or make changes to this bug.