1935149 - 14.04 - 8.15% wikipedia ContentfulSpeedIndex / wikipedia fcp + 20 more (Windows) regression on Thu November 28 2024

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect)

Description

[Treeherder Bug Filer]

Perfherder has detected a browsertime performance regression from push 91e11316d48cf2231d097057e123a01bd78aa7cb. As author of one of the patches included in that push, we need your help to address this regression.

Regressions:

Ratio	Test	Platform	Options	Absolute values (old vs new)	Performance Profiles
14%	wikipedia ContentfulSpeedIndex	windows11-64-shippable-qr	cold fission webrender	474.96 -> 541.65	Before/After
14%	wikipedia FirstVisualChange	windows11-64-shippable-qr	cold fission webrender	474.96 -> 541.49	Before/After
14%	wikipedia PerceptualSpeedIndex	windows11-64-shippable-qr	cold fission webrender	477.20 -> 543.98	Before/After
14%	wikipedia SpeedIndex	windows11-64-shippable-qr	cold fission webrender	480.67 -> 547.87	Before/After
14%	wikipedia loadtime	windows11-64-shippable-qr	bytecode-cached fission warm webrender	219.15 -> 249.01	Before/After
13%	google-slides ContentfulSpeedIndex	windows11-64-shippable-qr	bytecode-cached fission warm webrender	457.68 -> 519.16	Before/After
12%	wikipedia largestContentfulPaint	windows11-64-shippable-qr	cold fission webrender	531.26 -> 594.92	Before/After
11%	wikipedia ContentfulSpeedIndex	windows11-64-shippable-qr	bytecode-cached cold fission webrender	490.49 -> 546.69	Before/After
11%	wikipedia PerceptualSpeedIndex	windows11-64-shippable-qr	bytecode-cached cold fission webrender	492.61 -> 549.01	Before/After
11%	wikipedia FirstVisualChange	windows11-64-shippable-qr	bytecode-cached cold fission webrender	490.49 -> 546.52	Before/After
...	...	...	...	...	...
10%	wikipedia LastVisualChange	windows11-64-shippable-qr	fission warm webrender	570.35 -> 624.56	Before/After
9%	wikipedia fcp	windows11-64-shippable-qr	bytecode-cached cold fission webrender	346.82 -> 377.97	Before/After
9%	wikipedia loadtime	windows11-64-shippable-qr	bytecode-cached cold fission webrender	639.90 -> 695.22	Before/After
9%	wikipedia LastVisualChange	windows11-64-shippable-qr	bytecode-cached cold fission webrender	1,015.28 -> 1,101.84	Before/After
8%	wikipedia fcp	windows11-64-shippable-qr	cold fission webrender	344.40 -> 372.47	Before/After

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests. Please follow our guide to handling regression bugs and let us know your plans within 3 business days, or the patch(es) may be backed out in accordance with our regression policy.

If you need the profiling jobs you can trigger them yourself from treeherder job view or ask a sheriff to do that for you.

You can run all of these tests on try with ./mach try perf --alert 42944

The following documentation link provides more information about this command.

For more information on performance sheriffing please see our FAQ.

If you have any questions, please do not hesitate to reach out to [e-mail].

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1930292

Comment 2

[Emilio Cobos Álvarez [:emilio]]

I suspect this is the page being very sensible to viewport size... Which my patch changes slightly.

Do you know how do we set up these tests? Do they use the default window size or do we try to get a consistent viewport size?

Comment 3

[Emilio Cobos Álvarez [:emilio]]

I confirmed that my patch changes the viewport size slightly, and I'm moderately sure given this only affects wikipedia effectively that this is the cause...

That said, I see something worth digging in the profiles. Jonathan, I see two massive reflows on the bad load, while only one on the good load. Looking at them, they are forced by gfxPlatformFontList::ForceGlobalReflowLocked, but they come from two different messages:

• One from RecvFontListChanged, from here.
• Another one from RecvRebuildFontList a bit later (from LoadCMapsRunnable if I'm reading it correctly).

The bad load also gets these, they just happen to arrive in the same rendering loop tick so they don't do the work twice.

My understanding is that the first one is only sent from CancelInitOtherFamilyNamesTask()... I wonder if we can avoid it if we're going to run something else again? That'd probably fix the regression here...

Comment 4

[Greg Mierzwinski [:sparky]]

We use an orange paint/frame to determine the viewport that we should analyze for the visual metrics. So if browser viewport changes, then the analysis uses a slightly different viewport size. In the original video artifacts you can see that orange frame painted at the start of the test.

Comment 5

[Jonathan Kew [:jfkthame]]

I guess it ought to help if CancelInitOtherFamilyNamesTask() were to check for the existence of a non-null mLoadCmapsRunnable, and skip calling BroadcastFontListChanged() if one is present.

I pushed this to tryserver and it appears to have the desired effect: https://treeherder.mozilla.org/perfherder/compare?originalProject=try&originalRevision=19586e7ec5277c3d839b8f2d9f2ec0691fcfeb30&newProject=try&newRevision=92a3a64669ba2fce6f003f3abc0056aac6c525a2&framework=13&page=1&filter=wikipedia&showOnlyImportant=1

Let me think a bit about whether that seems ok to do in general, or if it's liable to bite us in some other way...

Comment 6

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1935149 - Streamline IPC messaging related to font-list updates.

Merge the FontListChanged message sent by the parent when the list is updated
with the more general ForceGlobalReflow. Avoid forcing an update from the
InitOtherFamilyNames task if there is also a LoadCmaps operation in progress;
just ensure it will include the NeedsReframe flag in its message instead.

This should not change observable behavior, but aims to reduce the risk of
font data updates resulting in redundant multiple reflows.

Comment 7

[Jonathan Kew [:jfkthame]]

PerfCompare for the above patch suggests it should give a nice improvement:
https://perf.compare/compare-results?baseRev=668e3ce5815c85d613f4a92e980a696fadcc4448&baseRepo=try&newRev=061f0cc292d1292f3848c8a9986d786ecc46a050&newRepo=try&framework=13&filter_confidence=high

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e1a2b6301950
Streamline IPC messaging related to font-list updates. r=layout-reviewers,emilio

Comment 9

[Cristina Horotan [:chorotan]]

Backed out for causing multiple mochitest failures.

Backout link

Push with failures

Failure log browser_launched_app_save_directory.js
Also causing failure spike in:
Failure log test_autocomplete_tab_between_fields.html
Failure log browser_devices_get_user_media_unprompted_access.js
Failure log browser_navigator_clipboard_clickjacking.js
Failure log browser_bug822367.js

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4de847c4b4f9
Streamline IPC messaging related to font-list updates. r=layout-reviewers,emilio

Comment 11

[Narcis Beleuzu [:NarcisB]]

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

Comment 12

[Florin Bilt]

(In reply to Narcis Beleuzu [:NarcisB] from comment #11)

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

Perfherder has detected a browsertime performance change from push 4de847c4b4f9799abdf6302e7c0a915e9e11a7e8.

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)	Performance Profiles
17%	wikipedia largestContentfulPaint	windows11-64-shippable-qr	cold fission webrender	583.07 -> 485.33	Before/After
16%	wikipedia largestContentfulPaint	windows11-64-shippable-qr	bytecode-cached cold fission webrender	593.42 -> 500.21	Before/After
14%	wikipedia loadtime	windows11-64-shippable-qr	bytecode-cached cold fission webrender	689.50 -> 592.32	Before/After
13%	wikipedia loadtime	windows11-64-shippable-qr	cold fission webrender	674.15 -> 584.77	Before/After
13%	wikipedia PerceptualSpeedIndex	windows11-64-shippable-qr	bytecode-cached cold fission webrender	535.25 -> 466.99	Before/After
...	...	...	...	...	...
10%	wikipedia LastVisualChange	windows11-64-shippable-qr	bytecode-cached cold fission webrender	1,091.98 -> 978.60	Before/After

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.

If you need the profiling jobs you can trigger them yourself from treeherder job view or ask a sheriff to do that for you.

You can run these tests on try with ./mach try perf --alert 43357

For more information on performance sheriffing please see our FAQ.

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:jfkthame, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval. Also, don't forget to request an uplift for the patches in the regression caused by this fix.
• If no, please set status-firefox135 to wontfix.

For more information, please visit BugBot documentation.