1678116 - Slow layout in gfxMacFontFamily::FindStyleVariations .SF NS after upgrade to Big Sur

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Asif Youssuff]

From https://www.reddit.com/r/firefox/comments/jud0ei/firefox_82_seems_slow_after_upgrading_to_macos_11/gcprx0j/

1. Open Firefox
2. Start typing a search term and hit enter.
3. The moment between hitting a search and showing you the search result is very slow. (This is the PROBLEM). In the report it's called Layout.

Profile: https://share.firefox.dev/2UCPUaW

Comment 1

[Asif Youssuff]

This profile: https://share.firefox.dev/3lM3vst seems to show the same issue (from https://www.reddit.com/r/firefox/comments/jud0ei/firefox_82_seems_slow_after_upgrading_to_macos_11/gcuzoua/).

Comment 2

[Jonathan Kew [:jfkthame]]

ni? myself to see if I can reproduce this.

Comment 3

[Markus Stange [:mstange]]

(In reply to Asif Youssuff from comment #0)

Profile: https://share.firefox.dev/2UCPUaW

Wow, 1.5 seconds! Something's clearly wrong here.

(In reply to Asif Youssuff from comment #1)

This profile: https://share.firefox.dev/3lM3vst seems to show the same issue (from https://www.reddit.com/r/firefox/comments/jud0ei/firefox_82_seems_slow_after_upgrading_to_macos_11/gcuzoua/).

Yeah, in this profile it takes ~580ms, but the bigger problem there is slow network. I'll reply on Reddit.

Comment 4

[Jonathan Kew [:jfkthame]]

Is this reproducible in 84beta? From a quick look at the code, I wonder if bug 1672842 will avoid this.

Comment 5

[Jonathan Kew [:jfkthame]]

Hmm. Still seems to be an issue on Beta as well.

Comment 6

[Jonathan Kew [:jfkthame]]

OK, there are a couple of things to note here.

First, this code (FindStyleVariations) only runs if gfx.e10s.font-list.shared is false, so we don't see this stack on Nightly or early Beta where that pref is true by default. Sounds good....? But no, actually we instead see an even longer reflow with the bulk of the time being spent in the InitOtherFamilyNames method, which blocks on a sync call to the parent process. So it's still bad.

However, the good news is that the patches awaiting review in bug 1669855 will help, by making InitOtherFamilyNames non-blocking; this eliminates the hugely-long reflow here.

As for the case where gfx.e10s.font-list.shared is false (late-Beta and Release): the first patch in bug 1672842 prevented us hitting this case in the parent process, as the system font (.SF NS) now gets initialized separately via a custom code path, because Core Text doesn't want to let us treat it like a normal font family. However, content processes still end up trying to initialize it like a normal font, based on the font-family list that is passed from the parent during content-proc startup, and that's where we hit the slow case.

Fortunately, we can just let the content process use the same short-circuit initialization of the system font family as the parent process is using. That avoids the incredibly slow case we're hitting here.

Comment 7

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1678116 - On recent macOS systems, avoid expensive initialization of the system font family in content processes. r=jrmuizel

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4bf72f03219a
On recent macOS systems, avoid expensive initialization of the system font family in content processes. r=jrmuizel

Comment 9

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/4bf72f03219a

Comment 10

[Jonathan Kew [:jfkthame]]

Comment on attachment 9189036 [details]
Bug 1678116 - On recent macOS systems, avoid expensive initialization of the system font family in content processes. r=jrmuizel

Beta/Release Uplift Approval Request

• User impact if declined: Very slow initial reflow (jank) for users on macOS Big Sur when the site uses the Mac system font family.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): We already have a special code path to handle the system font in the parent process, this just allows content to use that same code.
• String changes made/needed:

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9189036 [details]
Bug 1678116 - On recent macOS systems, avoid expensive initialization of the system font family in content processes. r=jrmuizel

Approved for 84.0b4.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/2c1c1cd55ec8