1900623 - 32.09 - 8.33% perf_reftest style-attr-1.html / perf_reftest_singletons link-style-cache-1.html + 9 more (Linux, OSX, Windows) regression on Wed May 29 2024

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Andrej (:aglavic)]

Perfherder has detected a talos performance regression from push acfd5e328aa9a8fa1bcceaa7130e9e028990326b. 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)
32%	perf_reftest style-attr-1.html	linux1804-64-shippable-qr	e10s fission stylo webrender	2.68 -> 3.53
26%	perf_reftest style-attr-1.html	linux1804-64-shippable-qr	e10s fission stylo webrender-sw	2.66 -> 3.35
22%	perf_reftest_singletons style-attr-1.html	linux1804-64-shippable-qr	e10s fission stylo webrender	3.37 -> 4.12
18%	perf_reftest style-attr-1.html	windows10-64-shippable-qr	e10s fission stylo webrender	2.49 -> 2.94
14%	perf_reftest style-attr-1.html	macosx1015-64-shippable-qr	e10s fission stylo webrender	3.61 -> 4.11
13%	perf_reftest style-attr-1.html	macosx1015-64-shippable-qr	e10s fission stylo webrender-sw	3.62 -> 4.08
13%	perf_reftest_singletons style-attr-1.html	macosx1015-64-shippable-qr	e10s fission stylo webrender	4.11 -> 4.63
11%	perf_reftest_singletons link-style-cache-1.html	macosx1015-64-shippable-qr	e10s fission stylo webrender	258.70 -> 286.11
10%	perf_reftest_singletons link-style-cache-1.html	linux1804-64-shippable-qr	e10s fission stylo webrender	304.72 -> 336.08
10%	perf_reftest_singletons link-style-cache-1.html	linux1804-64-shippable-qr	e10s fission stylo webrender	304.15 -> 335.07
8%	perf_reftest_singletons link-style-cache-1.html	windows10-64-shippable-qr	e10s fission stylo webrender	299.05 -> 323.97

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 these tests on try with ./mach try perf --alert 586

For more information on performance sheriffing please see our FAQ.

Comment 1

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1900623 - Fix perf regression from bug 1899272. r=#style

Move the uncommon code-path out of line.

Comment 2

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

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

Comment 3

[Emilio Cobos Álvarez [:emilio]]

https://treeherder.mozilla.org/perfherder/compare?originalProject=try&originalRevision=2d467c8d57697580fd6b9a60174164d9f0cbe27f&newProject=try&newRevision=5027b687b652e7d5035fe404038f1cc3f46375b2&page=1

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3a84ab9eef63
Fix perf regression from bug 1899272. r=firefox-style-system-reviewers,boris

Comment 5

[Sandor Molnar[:smolnar]]

Backed out for causing build bustages

Backout link: https://hg.mozilla.org/integration/autoland/rev/a0c259ed52184cb6f2ea388a14c9e1b48d1fab1c

Push with failures

Failure log -> error[E0308]: mismatched types

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fc972e51f7a1
Fix perf regression from bug 1899272. r=firefox-style-system-reviewers,boris

Comment 7

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/fc972e51f7a1

Comment 8

[Mike Hommey [:glandium]]

So, fun fact... this seems to not have covered the full regression for style-attr-1 on mac (talking about the one I was looking at), and, more interestingly, the combination of bug 1899272 and this one make it so that backing out the clang + rust upgrade doesn't fix bug 1899324 anymore.

Comment 9

[Emilio Cobos Álvarez [:emilio]]

Hmm, you're right. on Linux and windows it is much closer to the baseline values... Regarding bug 1899324... I don't think the regression can be due to matches_complex_selector_internal. The point of the test is that we don't run selector-matching when changing the style attribute, but due to how it's structured (100k declarations) it is really sensible to the code generation on this loop.

I think I'd be ok taking that regression (the point of the test is not regressing that higher-level optimization), but if you can figure out what makes that loop slower on mac that'd be awesome. I would've expected most of the time being spent in just cache misses walking the rule tree but seems that might not be the case?

Comment 10

[Mike Hommey [:glandium]]

I don't think the regression can be due to matches_complex_selector_internal

Well, profiles say most of the time is spent there... I think I've burnt too much time on this already, I'll just give up at this point.

Comment 11

[Emilio Cobos Álvarez [:emilio]]

Yeah, but it's spent there in time that's not measured: https://searchfox.org/mozilla-central/rev/346ac9335cbc33e3124a4541c938a0dc455e785e/testing/talos/talos/tests/perf-reftest/style-attr-1.html#14 is the expensive call that should spend a lot of time in selector-matching, but perf_start happens the line after (we want to make sure we don't do all that work after an incremental change).

Comment 12

[Andrej (:aglavic)]

(In reply to Pulsebot from comment #6)

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fc972e51f7a1
Fix perf regression from bug 1899272. r=firefox-style-system-reviewers,boris

Thanks for the work, we're seeing improvements from our side :)

Perfherder has detected a talos performance change from push 5aa202ae3b6bbd64fc97ce2ac2f0dc56f912b634.

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
18%	perf_reftest style-attr-1.html	linux1804-64-shippable-qr	e10s fission stylo webrender	3.44 -> 2.80
17%	perf_reftest style-attr-1.html	linux1804-64-shippable-qr	e10s fission stylo webrender-sw	3.47 -> 2.89
16%	perf_reftest_singletons style-attr-1.html	linux1804-64-shippable-qr	e10s fission stylo webrender	4.15 -> 3.51
8%	perf_reftest style-attr-1.html	macosx1015-64-shippable-qr	e10s fission stylo webrender	4.11 -> 3.78
7%	perf_reftest_singletons link-style-cache-1.html	linux1804-64-shippable-qr	e10s fission stylo webrender	334.46 -> 309.47
...	...	...	...	...
6%	perf_reftest_singletons link-style-cache-1.html	macosx1015-64-shippable-qr	e10s fission stylo webrender	286.83 -> 269.08

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 670

For more information on performance sheriffing please see our FAQ.