Closed
Bug 1490402
Opened 7 years ago
Closed 7 years ago
Use UTF-8 strings for font family & face names, etc in the system font list
Categories
(Core :: Layout: Text and Fonts, enhancement, P3)
Core
Layout: Text and Fonts
Tracking
()
RESOLVED
FIXED
mozilla64
Tracking | Status | |
---|---|---|
firefox64 | --- | fixed |
People
(Reporter: jfkthame, Assigned: jfkthame)
References
Details
Attachments
(1 file)
The system font list (gfxPlatformFontList subclass, and the gfxFontFamily and gfxFontEntry objects that hang off it, via various hashtables etc) store a lot of names. These are currently all 16-bit (UTF-16) strings, and account for dozens of KB of RAM in every process.
While font names can in principle contain arbitrary Unicode characters, in practice they are almost universally limited to plain ASCII. Therefore, we should be able to use 8-bit (UTF-8) strings to store them, and (to a first approximation) halve the associated memory footprint.
(I've been intending to switch to UTF-8 names anyway as part of moving a lot of the font-list data into shared memory for Fission; so switching the string storage in the existing code will help ease that migration, in addition to the immediate RAM usage reduction.)
Assignee | ||
Comment 1•7 years ago
|
||
Pushed a try job to see how this fares: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8b0657e056e7b5a905db76695181c89e614f4364
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → jfkthame
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•7 years ago
|
||
The memory saving this provides will obviously depend on the repertoire of fonts installed on a given system, but as a rough guideline I'm seeing savings of around 20KB or somewhat more (per process) across the three main desktop platforms. The memory in question here is labelled "font-list" in verbose about:memory reports.
Assignee | ||
Comment 3•7 years ago
|
||
Comment on attachment 9008130 [details] [diff] [review]
Use UTF-8 strings (in place of UTF-16) for font family/face names in the system font list, to reduce memory footprint
Sorry about the size of the patch... but most of it is boring mechanical changes of nsString -> nsCString etc. Aside from that, utf8/16 conversions are introduced where the font list code interacts with the outside world, and some that we needed previously are removed.
One further field we could consider changing is the type of FontFamilyName::mName (in gfxFontFamily.h), but it looks like making this utf8 will require some tweaks on the stylo side as well, so I've left it for now. Maybe a followup.
Attachment #9008130 -
Flags: review?(lsalzman)
Comment 4•7 years ago
|
||
(In reply to Jonathan Kew (:jfkthame) from comment #3)
> One further field we could consider changing is the type of
> FontFamilyName::mName (in gfxFontFamily.h), but it looks like making this
> utf8 will require some tweaks on the stylo side as well, so I've left it for
> now. Maybe a followup.
Just wanted to mention that this may be desirable given rust strings are generally utf-8. We use atoms (nsAtom) for font families in the style system. Changing that to be an atom would allow the style system and font code to share the allocation, which would be nice I think.
Updated•7 years ago
|
Attachment #9008130 -
Flags: review?(lsalzman) → review+
Assignee | ||
Comment 5•7 years ago
|
||
Hmm, tryserver says this doesn't quite work on Win7, which indicates the GDI part is buggy: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d850c8d11d7dc175bf0fc39f3e8294ed05e1eee5. The problem is failing to convert back to 16-bit strings when copying names into the GDI logfont structure. I'll fix & re-test before landing.
Pushed by jkew@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/588fe70e5905
Use UTF-8 strings (in place of UTF-16) for font family/face names in the system font list, to reduce memory footprint. r=lsalzman
Comment 7•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox64:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
You need to log in
before you can comment on or make changes to this bug.
Description
•