1800133 - 16.22 - 12.09% displaylist_mutate / displaylist_mutate + 4 more (Linux, OSX, Windows) regression on Sun November 6 2022

Status: REOPENED

(Core :: Graphics: WebRender, defect, P2)

Description

[Andrej (:aglavic)]

Perfherder has detected a talos performance regression from push 2d53a4acaad28c95bc1a19d05b379bf25f850be0. 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)
16%	displaylist_mutate	macosx1015-64-shippable-qr	e10s fission stylo webrender	1,662.83 -> 1,932.55
16%	displaylist_mutate	windows10-64-shippable-qr	e10s fission stylo webrender-sw	2,254.13 -> 2,619.72
15%	displaylist_mutate	windows10-64-shippable-qr	e10s fission stylo webrender-sw	2,263.13 -> 2,591.70
14%	displaylist_mutate	macosx1015-64-shippable-qr	e10s fission stylo webrender-sw	1,676.69 -> 1,917.99
13%	displaylist_mutate	linux1804-64-shippable-qr	e10s fission stylo webrender	2,329.06 -> 2,627.95
12%	displaylist_mutate	linux1804-64-shippable-qr	e10s fission stylo webrender-sw	2,124.57 -> 2,381.36

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 offending 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.

For more information on performance sheriffing please see our FAQ.

Comment 1

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

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

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1800133 - Make displaylist tests runnable outside of talos. r=#gfx-reviewers

No behavior change, but sometimes it's easier to do this for local
testing / profiling.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Seems most of the regression comes from the mutate_inactive, which basically haves thousands of transformed leafs.

So we're doing some extra work there eagerly, which otherwise we wouldn't need to do, given the stacking context is empty and we're not going to use the reused item... Let me think how to best handle this.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1800133 - Difference in appUnitsPerDevPixel only matter when there's a clip chain. r=#gfx-reviewers

We might get unnecessary cache misses otherwise.

Depends on D161884

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1800133 - Minor perf improvements to DefineClipChain. r=#gfx-reviewers

This removes a redundant hashmap lookup.

Depends on D161888

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1800133 - GetScrollLayer always returns a space id. r=#gfx-reviewers

This is just clean-up.

Depends on D161889

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1800133 - Optimize out empty sublists a bit better. r=#gfx-reviewers

This avoids unnecessarily starting clip lists.

Depends on D161890

Comment 9

[Timothy Nikkel (:tnikkel)]

I pushed these patches to try, unfortunately they don't seem to improve the displaylist mutate number.

Comment 10

[Emilio Cobos Álvarez (:emilio)]

Yeah, I know, they improve the number locally fwiw (and I can see no regression locally), which is rather annoying :)

I have other ideas but I ran out of time yesterday, still these should be general improvements to the code.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7adfd2b5de60
Minor perf improvements to DefineClipChain. r=gfx-reviewers,nical
https://hg.mozilla.org/integration/autoland/rev/68945e78986f
GetScrollLayer always returns a space id. r=gfx-reviewers,nical
https://hg.mozilla.org/integration/autoland/rev/95df1c60245c
Optimize out empty sublists a bit better. r=tnikkel

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/11140ee10035
Make displaylist tests runnable outside of talos. r=gfx-reviewers,perftest-reviewers,tnikkel,nical,sparky

Comment 13

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/7adfd2b5de60
https://hg.mozilla.org/mozilla-central/rev/68945e78986f
https://hg.mozilla.org/mozilla-central/rev/95df1c60245c
https://hg.mozilla.org/mozilla-central/rev/11140ee10035

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a7d99169878d
Difference in appUnitsPerDevPixel only matter when there's a clip chain. r=gfx-reviewers,gw

Comment 15

[Iulian Moraru]

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

Comment 16

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

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

Comment 17

[Jim Mathies [:jimm]]

Emilio, is this something we'll try to uplift to 108? If not, please update the tracking flags. Thanks!

Comment 18

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

The leave-open keyword is there and there is no activity for 6 months.
:emilio, maybe it's time to close this bug?
For more information, please visit BugBot documentation.

Comment 19

[Glenn Watson [:gw]]

Emilio, should we close this WONTFIX?

Comment 20

[Emilio Cobos Álvarez (:emilio)]

I think this was probably fixed and forgot to remove the leave open

Comment 21

[Timothy Nikkel (:tnikkel)]

IIRC the patches in here were meant to fix the regression but they didn't move the needle, so the regression still exists.

Comment 22

[Emilio Cobos Álvarez (:emilio)]

I see... I realistically won't have time to address this short term... I don't have many other optimisation ideas in this code other than rejiggerimg webrender or gecko to not have this mismatch in how clips / scroll IDs work. Do you know if something along those lines is on the the roadmap?

Comment 23

[Timothy Nikkel (:tnikkel)]

Understandable.

There is a lot of display list work planned yes.