844669 - Reduce start-up memory consumption hit caused by bug 831354

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Theme and Visual Design, defect)

Description

[Nicholas Nethercote [inactive]]

Bug 831354 added two fonts.  This apparently made text easier to read, but increased the amount of memory in the about:memory's "freetype" bucket at start-up (according to AWSY) from 0.60 to 2.18 MiB, i.e. by 1.58 MiB.

This was known and deemed acceptable... but it's quite a chunk when we're trying to get Firefox to work well on 256 MiB phones.

Some questions that jkew might know the answers to:
- How many fonts did we have before that?
- Are these new fonts especially large?
- Can they be shrunk at all?
- Do we know how the freetype memory is divided up?

Comment 1

[Jonathan Kew [:jfkthame]]

Previously, we weren't shipping any fonts as part of the fennec package (ignoring the reader-mode fonts, which were only loaded "on demand" through @font-face).

So at startup, we'd be using the fonts that are installed as part of the device's OS; we'll build a list of all the fonts (typically about 30 or 40 faces, I think, depending on Android version), but initially we'll probably only need to instantiate Roboto or Droid Sans, and maybe Droid Serif, perhaps in both regular and bold. These are fairly small fonts (less than 200K per file).

With the added fonts (two families, containing a total of 10 faces), we're now going to instantiate Open Sans and Charis SIL instead - or rather, because of bug 754215, we probably instantiate these *as well as* the Droid fonts, as those are still present in our fallback list.

Open Sans is not all that much larger than Droid Sans (220K per face, roughly), though the fact that it comes with separate italic faces for each weight means that we may well be instantiating a larger number of distinct faces. Charis SIL, however, is -much- larger than Droid Serif - it's about 1.6MB per face. In large part, this is because it is a far more comprehensive font, with over 4600 glyphs compared to Droid's 900.

I have not investigated freetype's memory handling; depending how much of the font data it decides to load into memory up-front, it may well be that making any use whatsoever of a large font like Charis causes a significant memory hit. If this is a serious concern, I could try to look into it in more detail and see if, for example, we could make it lazier about loading the font data.

Comment 2

[Nicholas Nethercote [inactive]]

> If this is a serious concern, I could try to look into it in
> more detail and see if, for example, we could make it lazier about loading
> the font data.

If you could spend at least a short time to see what you can learn, that would be very helpful.  Thanks.

Comment 3

[Jonathan Kew [:jfkthame]]

It looks like there are a whole bunch of things going on here! And yes, we'll be able to do some things to mitigate the issue. Some notes on findings so far:

1. Freetype memory consumption is significantly higher on the first startup after a new build is installed, because on first startup we read details of all fonts, and once we've done that we hold on to the FT_Face objects we created. We probably should release those as soon as we've loaded the font details we need, and then re-create them only if the fonts are actually used.

1a. The above only applies to first-run, since we cache the font list in the startup cache and thus avoid actually reading all the font files on subsequent startups.

1b. However, at least in my local build, access to the startup cache was failing, so that we were hitting the "first-run" case every time. I'm not sure why that was. After completely uninstalling the browser and reinstalling, I have not been able to reproduce that problem. (I suspected it might have been related to bug 842262, but attempting to reproduce the behavior by applying and removing the patch there did *not* confirm that, so perhaps my installation was broken in some other way.)

2. Opening "about:memory" is itself problematic, because the "box-drawing" characters it uses are not present in the default text fonts we're using; as a result, they hit the global font fallback code path, which requires us to load character maps for all the fonts. We can mitigate this by implementing GetCommonFallbackFonts in gfxAndroidPlatform, so that we'll try likely fonts such as Droid Sans Fallback before resorting to the read-all-fonts option.

3. When we do decide to read a font's character map, we create an FT_Face in order to access the data, and that FT_Face is then permanently held open. Given that in many cases, we read the cmap but then never actually instantiate and use the font, we should not be keeping those FT_Face objects around. This will make much of the cmap-reading memory usage become transient instead of permanent.

With these issues fixed, I think you'll see the freetype "bucket" drop back to a much smaller size. (How do I see these details on AWSY, btw? Just loading http://www.arewesmallyet.com/ doesn't show anything about Android...)

Comment 4

[Andrew McCreight [:mccr8]]

By "AWSY", Nick actually means https://areweslimyet.com/  He assumes a greater awareness of what that stands for than is the case. ;)  From there, click on "mobile", then you'll need to zoom in a little, then you can click on an individual circle to get a detailed about:memory breakdown.

Comment 5

[Jonathan Kew [:jfkthame]]

Ah, thanks ... "S" = slim, not small. Excuse my confusion! Yes, that's much more useful.

Comment 6

[Kartikaya Gupta (email:[e-mail])]

(In reply to Jonathan Kew (:jfkthame) from comment #3)
> 2. Opening "about:memory" is itself problematic, because the "box-drawing"
> characters it uses are not present in the default text fonts we're using; as
> a result, they hit the global font fallback code path, which requires us to
> load character maps for all the fonts. We can mitigate this by implementing
> GetCommonFallbackFonts in gfxAndroidPlatform, so that we'll try likely fonts
> such as Droid Sans Fallback before resorting to the read-all-fonts option.

You can also obtain the details of the about:memory dump in a JSON file without rendering it in the browser (that's what the data obtained on AWSY comes from). Details at https://wiki.mozilla.org/Mobile/Fennec/Android#about:memory

Comment 7

[Jonathan Kew [:jfkthame]]

Attachment: pt 1 - don't keep freetype faces for all fonts around after building the font list on first-run

Comment 8

[Jonathan Kew [:jfkthame]]

Attachment: pt 2 - use a transient FT_Face to read font tables such as cmap, if the font entry does not already have an existing face

Comment 9

[Jonathan Kew [:jfkthame]]

Attachment: pt 3 - implement a simple GetCommonFallbackFonts for Android, to reduce the chances of hitting the global fallback codepath and loading all cmaps

Comment 10

[Jonathan Kew [:jfkthame]]

Tryserver build with these three patches in progress at https://tbpl.mozilla.org/?tree=Try&rev=542eb0405bd3. In my local testing, this brings the freetype memory usage after startup down to pre-bug 831354 levels, or even somewhat less.

Comment 11

[Jonathan Kew [:jfkthame]]

Attachment: pt 3 - implement a simple GetCommonFallbackFonts for Android, to reduce the chances of hitting the global fallback codepath and loading all cmaps

Oops, stupid typo in previous version :( ... this version should work.

Comment 12

[Jonathan Kew [:jfkthame]]

https://tbpl.mozilla.org/?tree=Try&rev=13212b17ad3e

Comment 13

[Nicholas Nethercote [inactive]]

> 1. Freetype memory consumption is significantly higher on the first startup
> after a new build is installed, because on first startup we read details of
> all fonts

Ok.  That's probably affecting the AWSY results, since I assume we do one build, one run, for each mozilla-inbound push.  I wonder if that's skewing desktop AWSY runs as well.

> 2. Opening "about:memory" is itself problematic, because the "box-drawing"
> characters it uses are not present in the default text fonts we're using; as
> a result, they hit the global font fallback code path, which requires us to
> load character maps for all the fonts.

As kats said, we don't load about:memory but access the underlying memory reporters directly.  So this won't be a factor in the AWSY numbers.


Thanks for investigating!  Sorry about the "AWSY" confusion, it appears that arewesmallyet.com is tracking Firefox's binary size, or something like that.  Huh.

Comment 14

[Jonathan Kew [:jfkthame]]

(In reply to Nicholas Nethercote [:njn] from comment #13)
> > 1. Freetype memory consumption is significantly higher on the first startup
> > after a new build is installed, because on first startup we read details of
> > all fonts
> 
> Ok.  That's probably affecting the AWSY results, since I assume we do one
> build, one run, for each mozilla-inbound push.  I wonder if that's skewing
> desktop AWSY runs as well.

As far as fonts are concerned, at least, it shouldn't be a factor on desktop, as the use of the startupcache for font info is (currently) mobile-only. But of course there might be other areas where the use (or not) of startupcache is significant.

Comment 15

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 718596 [details] [diff] [review]
pt 2 - use a transient FT_Face to read font tables such as cmap, if the font entry does not already have an existing face

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

::: gfx/thebes/gfxFT2FontList.cpp
@@ +120,5 @@
> +    }
> +
> +    operator FT_Face() { return mFace; }
> +
> +    FT_Face Forget() {

nit, lowercase "f" to be consistent with nsAutoPtr

Comment 16

[Jonathan Kew [:jfkthame]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6e5e18863db6
https://hg.mozilla.org/integration/mozilla-inbound/rev/d3e55590d252
https://hg.mozilla.org/integration/mozilla-inbound/rev/14493f3ee798

Comment 17

[Nicholas Nethercote [inactive]]

jkew: any estimate on the difference these patches make?  I don't see much change on AWSY/mobile.

Comment 18

[Jonathan Kew [:jfkthame]]

In my local testing, they reduced the freetype bucket immediately after startup from about 2.5MB to 0.6MB, IIRC. They're not likely to make any significant difference to longer-term memory use, as all (or most) of the available fonts will eventually get loaded, but they should reduce the number that are loaded and held in memory immediately on startup.

(Do they show on AWSY yet? The newest mobile results I see there are nearly a week old...)

Comment 19

[Nicholas Nethercote [inactive]]

(In reply to Jonathan Kew (:jfkthame) from comment #18)
> In my local testing, they reduced the freetype bucket immediately after
> startup from about 2.5MB to 0.6MB, IIRC.

Good.

> (Do they show on AWSY yet? The newest mobile results I see there are nearly
> a week old...)

True.  But if you zoom in there's a big drop, which I assumed was caused by the graphite pref disabling in bug 700023, but that only just landed...

Comment 20

[Kartikaya Gupta (email:[e-mail])]

Yeah the AWSY/mobile data are on hold at the moment while I fix the heap-unclassified data and regenerate backlog. The last results *are* from about a week ago, and it'll take a couple of days to catch up probably once I get it going again.

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6e5e18863db6
https://hg.mozilla.org/mozilla-central/rev/d3e55590d252
https://hg.mozilla.org/mozilla-central/rev/14493f3ee798

Comment 22

[Geoff Brown [:gbrown]]

Talos approves:

(from dev-tree-management Digest, Vol 51, Issue 8)

Improvement: mobile - Tp4 Mobile NoChrome (Main RSS) - Android 2.2 (Native) - 2.36% decrease
--------------------------------------------------------------------------------------------
    Previous: avg 93508093.333 stddev 739686.056 of 30 runs up to revision 55477ae32280
    New     : avg 91300680.000 stddev 379142.740 of 5 runs since revision 9f18d05d1ba1
    Change  : -2207413.333 (2.36% / z=2.984)
    Graph   : http://mzl.la/103qH5v

Comment 23

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 718594 [details] [diff] [review]
pt 1 - don't keep freetype faces for all fonts around after building the font list on first-run

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 831354 
User impact if declined: we use more memory
Testing completed (on m-c, etc.): on nightly
Risk to taking this patch (and alternatives if risky): 
String or UUID changes made by this patch:

Comment 24

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 718596 [details] [diff] [review]
pt 2 - use a transient FT_Face to read font tables such as cmap, if the font entry does not already have an existing face

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 831354 
User impact if declined: we use more memory
Testing completed (on m-c, etc.): on nightly
Risk to taking this patch (and alternatives if risky): 
String or UUID changes made by this patch:

Comment 25

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 718637 [details] [diff] [review]
pt 3 - implement a simple GetCommonFallbackFonts for Android, to reduce the chances of hitting the global fallback codepath and loading all cmaps

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 831354 
User impact if declined: we use more memory
Testing completed (on m-c, etc.): on nightly
Risk to taking this patch (and alternatives if risky): 
String or UUID changes made by this patch:

Comment 26

[bhavana bajaj [:bajaj]]

Comment on attachment 718594 [details] [diff] [review]
pt 1 - don't keep freetype faces for all fonts around after building the font list on first-run

Helps avoid extra memory usage due to landing of new fonts in Bug 831354

Comment 27

[Jonathan Kew [:jfkthame]]

Transplanted patches to aurora:

https://hg.mozilla.org/releases/mozilla-aurora/rev/abf2efa55070
https://hg.mozilla.org/releases/mozilla-aurora/rev/b661c3303b3c
https://hg.mozilla.org/releases/mozilla-aurora/rev/41d7a2a0776b

Comment 28

[Kartikaya Gupta (email:[e-mail])]

AWSY confirms that these patches reduced "fresh start" memory usage by ~2.86 MiB and "fresh start +30s" memory usage by ~2.97 MiB. Thanks!

72854a54aac64519d1f8a098499766b3da2066e6 100.80MiB Δ 248.00KiB
14493f3ee798f16f22fe7630c56f4c7fc4e79a1d 97.95MiB Δ -2.86MiB

72854a54aac64519d1f8a098499766b3da2066e6 110.34MiB Δ 24.00KiB
14493f3ee798f16f22fe7630c56f4c7fc4e79a1d 107.37MiB Δ -2.97MiB