1914554 - Regression on SP3-Charts-observable-plot around 21Aug2024

Status: NEW

(Core :: SVG, defect)

Description

[Mayank Bansal]

SP3-Charts-observable-plot

Range: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=0f3caf3ee705aa5bd086626b1bc2e4c0a907410d&tochange=bf098c42b0a02f385576aa6b2751c86f5cefdec4
Suspect : Bug 878346

Comment 1

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

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

:emilio, since you are the author of the regressor, bug 878346, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Hmm, there are other potentially significant SVG changes as well (bug 1914182 in particular). Would be good to have a proper bisect? But sure happy to look into it if it turns out to be due to my patch.

Alex, could you confirm whether it was my patch or bug 1914182 (or something else JS related like bug 1913950)?

Comment 3

[Mayank Bansal]

This is https://treeherder.mozilla.org/perfherder/alerts?id=1758

Comment 4

[Acasandrei Beatrice (needinfo me)]

This bug was created before sheriffs finished the investigation on Alert summary #1758 for identifying the right culprit. I will update the bug with the right information once I have it. Thanks :emilio for your input.

Comment 5

[Acasandrei Beatrice (needinfo me)]

The graphs from Alert summary #1802 are clear now and show that Bug 878346 has caused the following browsertime regression (from push 246ed0e1a9345f7c55759d1a4e96cdfc89b8ba91).

Regressions:

Ratio	Test	Platform	Options	Absolute values (old vs new)	Performance Profiles
6%	speedometer3 Charts-observable-plot/Stacked by 6/Async	windows11-64-nightlyasrelease-qr	fission webrender	3.59 -> 3.81	Before/After
6%	speedometer3 Charts-observable-plot/Dotted/Async	windows11-64-nightlyasrelease-qr	fission webrender	4.55 -> 4.82	Before/After
5%	speedometer3 Charts-observable-plot/Stacked by 6/Async	windows11-64-shippable-qr	fission webrender	3.62 -> 3.82	Before/After
5%	speedometer3 Charts-observable-plot/Stacked by 6/Async	macosx1400-64-shippable-qr	fission webrender	2.33 -> 2.45	Before/After
5%	speedometer3 Charts-observable-plot/Stacked by 20/Async	macosx1400-64-shippable-qr	fission webrender	3.21 -> 3.38	Before/After
5%	speedometer3 Charts-observable-plot/Stacked by 20/Async	macosx1400-64-shippable-qr	fission webrender	3.24 -> 3.40	Before/After
4%	speedometer3 Charts-observable-plot/Stacked by 20/Async	windows11-64-nightlyasrelease-qr	fission webrender	5.07 -> 5.28	Before/After
4%	speedometer3 Charts-observable-plot/Dotted/Async	macosx1400-64-shippable-qr	fission webrender	2.62 -> 2.72	Before/After
4%	speedometer3 Charts-observable-plot/Dotted/total	windows11-64-nightlyasrelease-qr	fission webrender	11.71 -> 12.14	Before/After
4%	motionmark-1-3	macosx1400-64-shippable-qr	fission webrender	1,962.72 -> 1,892.01
3%	speedometer3 Charts-observable-plot/Stacked by 20/Async	windows11-64-shippable-qr	fission webrender	5.09 -> 5.26	Before/After
3%	speedometer3 Charts-observable-plot/Dotted/Async	windows11-64-shippable-qr	fission webrender	4.75 -> 4.87	Before/After
2%	speedometer3 Charts-observable-plot/Dotted/total	windows11-64-shippable-qr	fission webrender	12.08 -> 12.36

As author of one of the patches included in that push, we need your help to address this regression.
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 1802

For more information on performance sheriffing please see our FAQ.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Wrong emilio :)

Comment 7

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

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

Comment 8

[Emilio Cobos Álvarez (:emilio)]

I'm very confused at the motionmark numbers from here.

Are the subtests numbers wrong (I regressed instead of improved them)? Or is the result wrong?

Comment 9

[Acasandrei Beatrice (needinfo me)]

It looks like there is only a regression for score and an improvement for Suits. I believe the remaining results do not satisfy the criteria required for the application to classify them as regressions or improvements.

Comment 10

[Mike Conley (:mconley) (:⚙️)]

Attachment: One of the regression graphs

Hey emilio,

I'm the Engineering REO for 131, which this regression affects. It seems at first glance from comment 5 that this is a regression across a number of Speedometer 3 subtests, and they haven't really improved since this first landed (see attachment).

Presuming this is a real regression (which it looks like it is?) are you able to give me a sense of how important the regressing bug is? For example, should we be considering a backout for 131?

Comment 11

[Mike Conley (:mconley) (:⚙️)]

Hey emilio,

I'm the Engineering REO for 131, which this regression affects. It seems at first glance from comment 5 that this is a regression across a number of Speedometer 3 subtests, and they haven't really improved since this first landed:

Comment 12

[Robert Longson [:longsonr]]

bug 878346 is an important SVG 2 correctness fix that has been a problem for some time since Chrome implemented it a while ago.

Here are some people running into this...

https://stackoverflow.com/questions/69074733/transitioning-transform-attribute-of-a-group-element-not-working-in-firefox
https://stackoverflow.com/questions/20361746/firefox-svg-transform-not-working
https://stackoverflow.com/questions/78761132/svg-component-rendering-inconsistently-across-browsers

I imagine I could find others too. We can continue to try to improve performance but we really shouldn't back this out.

Comment 13

[Frank Doty [:fdoty]]

Removing perf impact flag as that is typically used for user facing performance issues. While SP3 regression can arguably be described as user facing, it is more so an issue that the SP3 team should investigate as it is not degrading usability of the browser. This will still be tracked, just not triaged as user facing.

Comment 14

[Jonathan Kew [:jfkthame]]

Given the relatively limited scope (it doesn't seem to have affected all SVG content, only specific cases), I'm calling this an S3, though there does appear to be a genuine regression that we should try to mitigate if possible.