1613996 - One-second reflow on https://policies.google.com/privacy?hl=en-CA due to font information lookup

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect)

Description

[Markus Stange [:mstange]]

Here's a profile of loading https://policies.google.com/privacy?hl=en-CA in Fenix on a Moto G5: https://perfht.ml/2ODutEd

A long time is spent in GlobalFontFallback (unfortunately I don't know for which char), in WhichPrefFontSupportsChar, and in other things.

Comment 1

[Jonathan Kew [:jfkthame]]

That's not great... I wonder if it's a result of the language-selector control, which includes things like Ethiopic that probably aren't supported by standard fonts on many devices.

(I thought I'd seen the target character being logged to the profiler in situations like this - does the labelling at https://searchfox.org/mozilla-central/rev/623de665034eee43a54ff02939b61385ffd5990d/gfx/thebes/gfxFontEntry.cpp#1743-1750 not work on Android?)

Comment 2

[Emilio Cobos Álvarez (:emilio)]

(In reply to Jonathan Kew (:jfkthame) from comment #1)

(I thought I'd seen the target character being logged to the profiler in situations like this - does the labelling at https://searchfox.org/mozilla-central/rev/623de665034eee43a54ff02939b61385ffd5990d/gfx/thebes/gfxFontEntry.cpp#1743-1750 not work on Android?)

Doesn't that AUTO_PROFILER_DYNAMIC_* thing instantly return? Shouldn't it be at the function-level scope?

Comment 3

[Markus Stange [:mstange]]

Agreed, this got broken in bug 1578329. I'll file a bug.

Comment 4

[Markus Stange [:mstange]]

(filed bug 1614013)

Comment 5

[Jonathan Kew [:jfkthame]]

Ah, right - I should've noticed that didn't make sense!

As for the actual issue here, I wonder if we can improve things by bypassing freetype when we need to read the character map of a font, but don't have an FT_Face on hand -- it might be faster to access the file directly just to read the cmap than to actually instantiate an FT_Face. I'll try to experiment with this.

Comment 6

[Jonathan Kew [:jfkthame]]

Markus, I've pushed a try build at https://treeherder.mozilla.org/#/jobs?repo=try&revision=b3fa21c482a04f5373cd800fed0002650dba9007 with a patch that attempts to avoid instantiating an FT_Font just to read cmap information, but I don't have a suitable setup to reproduce and test this issue to see if it helps. If you could give it a try and see if the profile shows any difference, that would be awesome -- thanks.

Comment 7

[Jonathan Kew [:jfkthame]]

Updated try build with a tidier version of the patch:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3b5b02ce61f0057f320188b96c04a19c010ecc41.

Comment 8

[Markus Stange [:mstange]]

This is great! It seems to reduce the time spent in FindFontForChar on that page by 600ms for me.
The reflow time has decreased from 1.3 seconds to 0.7 seconds.

Profile before: https://perfht.ml/3bqMFuK (~783ms in FindFontForChar (666 of 1166 samples over a true period of 1371ms))
Profile after: https://perfht.ml/2SrlcQI (~145ms in FindFontForChar (124 of 566 samples over a true period of 666ms))

Comment 9

[Jonathan Kew [:jfkthame]]

Awesome -- that's better than I dared hope for! OK, I'll put up a patch for review.

Comment 10

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1613996 - Try to avoid instantiating an FT_Face just to read the font's charmap for FindFontForChar. r=lsalzman

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/153d0220e353
Try to avoid instantiating an FT_Face just to read the font's charmap for FindFontForChar. r=lsalzman

Comment 12

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/153d0220e353