876070 - re-enable graphite2 font shaping on mobile

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect)

Description

[Jonathan Kew [:jfkthame]]

In a followup to bug 700023, we disabled Graphite shaping on Android due to the runtime RAM footprint (bug 846832), as the default serif font we're shipping with fennec includes Graphite tables and so gets routed via the graphite2 shaper.

The refactoring of font/face management in bug 847344, together with the internal size optimizations in graphite 1.2.2 (bug 876069), should greatly reduce the memory impact of using graphite. As such, it's time to try re-enabling it on trunk, and see what AWSY has to say.

Comment 1

[Jonathan Kew [:jfkthame]]

Attachment: re-enable graphite shaping on mobile by default.

Comment 2

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

I'm creating try builds without [1] and with [2] this change and will run them through AWSY to see what the impact is.

[1] https://tbpl.mozilla.org/?tree=Try&rev=df07c66a2b37
[2] https://tbpl.mozilla.org/?tree=Try&rev=825c81d2b85b

Comment 3

[Jonathan Kew [:jfkthame]]

Thanks, it'll be good to see what that shows. Note that the graphite update in bug 876069 (awaiting jdaggett's review) should also reduce memory footprint. I've pushed an additional try build [3] that includes both this change -and- the graphite update; could you please include this in your comparison as well?

[3] https://tbpl.mozilla.org/?tree=Try&rev=f9605aab527e

Comment 4

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

I forgot I needed ARMv6 builds for proper comparison. I re-pushed all three to try and tested those on the AWSY harness and got the following data:

Try cset 695749d9db11 (M-C cset 2b36716b4d27)
Samples: 4
Average: 148491264.00
Stddev: 74245636.68
Max: 148541440
Min: 148451328

Try cset c789aee6bf15 (above + graphite enabled)
Samples: 4
Average: 148496384.00
Stddev: 74248192.99
Max: 148516864
Min: 148459520

Try cset 8e145111d8dc (above + the change in comment 3)
Samples: 4
Average: 148478976.00
Stddev: 74239491.01
Max: 148520960
Min: 148430848

So it looks like there is no significant increase in startup resident memory usage by turning on graphite (goes up by ~5k), and the change in comment 3 reduces it by ~18k. I also looked at the memory usage after opening a bunch of tabs and there I saw about a 2% increase in resident memory usage from turning on graphite, which is within the normal deviation range of the data. So I have no objections to turning this on.

Comment 5

[Jonathan Kew [:jfkthame]]

Thanks for the testing; looks like the memory-footprint fixes have worked as intended.

https://hg.mozilla.org/integration/mozilla-inbound/rev/0f9247a41a5e

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0f9247a41a5e

Comment 7

[Amir Aharoni]

Thank you for working on this! When is this supposed to be released in a stable version?

Comment 8

[Jonathan Kew [:jfkthame]]

It should be in Firefox 24 for Android (as per the "target milestone" field above). FF24 is currently in the Beta channel, and will go to release in about a month.