1790595 - Performance regression in `StyleSheetManager._getMediaRules`

Status: RESOLVED FIXED

(DevTools :: Style Editor, defect, P2)

Description

[Nicolas Chevobbe [:nchevobbe]]

Patch from Bug 1788030 caused a performance regression on the styleeditor https://treeherder.mozilla.org/perfherder/alerts?id=35326

Regressions:

Ratio	Test	Platform	Options	Absolute values (old vs new)
14%	damp custom.styleeditor.open.DAMP	windows10-64-shippable-qr	e10s fission stylo webrender	771.45 -> 879.01
13%	damp custom.styleeditor.open.DAMP	linux1804-64-shippable-qr	e10s fission stylo webrender	801.07 -> 904.38
12%	damp custom.styleeditor.open.DAMP	linux1804-64-shippable-qr	e10s fission stylo webrender-sw	789.05 -> 887.52
12%	damp custom.styleeditor.open.DAMP	macosx1015-64-shippable-qr	e10s fission stylo webrender-sw	723.97 -> 812.24
12%	damp custom.styleeditor.open.DAMP	windows10-64-shippable-qr	e10s fission stylo webrender-sw	785.04 -> 880.69

Comment 1

[Nicolas Chevobbe [:nchevobbe]]

Looks like this can be reverted https://treeherder.mozilla.org/perfherder/comparesubtest?originalProject=try&newProject=try&newRevision=91cbe0e42a3be7366d347bebda0577151559f67b&originalSignature=2916453&newSignature=2916453&framework=12&originalRevision=95a58a60ccc191739dc9c9345d265eac404cc2d5&page=1

Comment 2

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

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

Comment 3

[Nicolas Chevobbe [:nchevobbe]]

Attachment: Bug 1790595 - [devtools] Fix performance regression in _getMediaRules. r=jdescottes.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c2224b54c672
[devtools] Fix performance regression in _getMediaRules. r=jdescottes.

Comment 5

[Marian-Vasile Laza]

Backed out changeset c2224b54c672 (Bug 1790595) for causing mochitest failures on test_styles-applied.html.
Backout link
Push with failures <--> c3
Failure Log

Comment 6

[Nicolas Chevobbe [:nchevobbe]]

not sure what's happening
TV jobs seem fine with the test https://treeherder.mozilla.org/jobs?repo=try&revision=e06b69265768dc19d39297f396cb12d58313fd30 , and I was able to run the whole folder without issue

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e2e5c02c98e3
[devtools] Fix performance regression in _getMediaRules. r=jdescottes.

Comment 8

[Marian-Vasile Laza]

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

Comment 9

[Nicolas Chevobbe [:nchevobbe]]

Comment on attachment 9294427 [details]
Bug 1790595 - [devtools] Fix performance regression in _getMediaRules. r=jdescottes.

Beta/Release Uplift Approval Request

• User impact if declined: performance regression when opening DevTools StyleEditor panel
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• 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): devtools panel performance fix for a function already covered by automated tests
• String changes made/needed:
• Is Android affected?: Yes

Comment 10

[Pascal Chevrel:pascalc]

Comment on attachment 9294427 [details]
Bug 1790595 - [devtools] Fix performance regression in _getMediaRules. r=jdescottes.

106 perf regression, LGTM, approved for 106 beta 2, thanks.

Comment 11

[Pascal Chevrel:pascalc]

https://hg.mozilla.org/releases/mozilla-beta/rev/e4771050777f

Comment 12

[Julian Descottes [:jdescottes]]

== Change summary for alert #35431 (as of Tue, 20 Sep 2022 21:02:41 GMT) ==

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
30%	damp custom.styleeditor.reload.DAMP	windows10-64-shippable-qr	e10s fission stylo webrender-sw	812.95 -> 571.96
29%	damp custom.styleeditor.open.DAMP	macosx1015-64-shippable-qr	e10s fission stylo webrender-sw	810.23 -> 574.69
29%	damp custom.styleeditor.open.DAMP	macosx1015-64-shippable-qr	e10s fission stylo webrender	808.70 -> 575.16
27%	damp custom.styleeditor.open.DAMP	linux1804-64-shippable-qr	e10s fission stylo webrender-sw	882.07 -> 642.88
27%	damp custom.styleeditor.open.DAMP	linux1804-64-shippable-qr	e10s fission stylo webrender	893.87 -> 653.82
...	...	...	...	...
23%	damp custom.styleeditor.reload.DAMP	linux1804-64-shippable-qr	e10s fission stylo webrender	739.50 -> 572.25

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=35431

Comment 13

[Julian Descottes [:jdescottes]]

Improvement recorded in DAMP, we are even faster than before the regression :)