1583005 - Major performance regression with gfx.e10s.font-list.shared enabled

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect)

Description

[Mats Palmgren (inactive)]

STR

1. create a fresh profile
2. start Nightly (note that initial built-in page loads quickly)
3. go to about:config and set gfx.e10s.font-list.shared to true
4. restart browser

ACTUAL RESULTS
Initial built-in page (all pages really) loads very slowly.
All pages takes ~10 seconds to render anything.

This is a regression in the last week or so.
Nightly 71.0a1 (2019-09-21) (64-bit) on Linux.

Comment 1

[Jonathan Kew [:jfkthame]]

Huh, that's curious.... I wonder if one of the recent font-list bugfixes is causing unexpected levels of cache churn or something. Pinpointing when it regressed would be a first step.

Comment 2

[Jonathan Kew [:jfkthame]]

This appears to be a regression from Lee's patches in bug 1547063, though I'm not yet sure exactly why. Maybe profiling will show a smoking gun....

Comment 3

[Jonathan Kew [:jfkthame]]

According to https://perfht.ml/31Jocez, we seem to be spending a crazy-large amount of time (43 seconds, in this profile) doing fontconfig stuff from within gtk, called from nsLookAndFeel::EnsureInit. It seems like fontconfig may be deciding it needs to reinitialize its whole configuration on every call, or something like that.

It's not obvious to me why this would be dependent on the gfx.e10s.font-list.shared setting, but apparently it is. Lee, any thoughts on what might be going on here?

Comment 4

[Jonathan Kew [:jfkthame]]

OK, it looks to me like we need to ensure that each content process has done something to cause fontconfig initialization before we start making gtk calls from nsLookAndFeel. Otherwise (for reasons I don't really understand) we run into this pathological situation when gtk wants to touch font-related stuff.

Turning on the shared font list triggers this because the content processes no longer do any fontconfig stuff during InitSharedFontListForPlatform. It looks like we can avoid the issue by hoisting the setting of mLastConfig = FcConfigGetCurrent() up to where it happens in all processes (not just the parent); this should be pretty harmless. (It also means that if we eventually want access to the configuration in child processes, which we may need for better fontconfig integration anyhow, it'll be there for us.)

Comment 5

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1583005 - Ensure the fontconfig configuration is initialized in all processes (by calling FcConfigGetCurrent) before potential gtk access. r?lsalzman

Comment 6

[Lee Salzman [:lsalzman]]

Well, just looking at the backtrace and the Fontconfig source code, even though this seems like a very unlikely scenario, this could happen:

FcConfigSubstituteWithPat calls FcConfigGetCurrent/FcConfigEnsure, and FcConfigEnsure in the multi-threaded case has a retry/spin-lock guarding the initialization of this, which could hypothetically be where the FcConfigDestroy comes from. One way to test this would be to call FcConfigInit (which calls FcConfigEnsure) earlier in the loading process to make sure that the default FcConfig gets initialized without any contention.

One of the changes I made in my Cairo cleanups was to remove the querying of a dummy Cairo-FT font to get the FT_Library, since now we do all FT_Face creation outside of Cairo, we just create our own instance of an FT_Library. Doing the call to FcConfigInit around where that used to be seems like a good way to test this hypothesis?

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/745fe3b9c450
Ensure the fontconfig configuration is initialized in all processes (by calling FcConfigGetCurrent) before potential gtk access. r=lsalzman

Comment 8

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/745fe3b9c450