1829089 - 11.51 - 9.72% tsvgx / tsvgx + 2 more (Linux, OSX, Windows) regression on Sun April 16 2023

Status: RESOLVED WONTFIX

(Core :: Graphics, defect)

Description

[Acasandrei Beatrice (needinfo me)]

Perfherder has detected a talos performance regression from push a31f2795710eb38516f3cfa9b4f4041eb3cd3a97. 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)
12%	tsvgx	macosx1015-64-shippable-qr	e10s fission stylo webrender	117.35 -> 130.85
11%	tsvgx	windows10-64-shippable-qr	e10s fission stylo webrender	166.93 -> 186.00
11%	tsvgx	windows10-64-shippable-qr	e10s fission stylo webrender-sw	195.78 -> 216.98
10%	tsvgx	linux1804-64-shippable-qr	e10s fission stylo webrender-sw	246.19 -> 270.10

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 1821512

Comment 2

[Lee Salzman [:lsalzman]]

This is because Skia finally removed its old specialized gradient rendering code so that they could fully move to an SkRasterPipeline-based gradient pathway. This allowed them to better support color-spaces and other things. I don't think it is likely viable to backport the old gradient code, given how long ago they removed it. I can spend some time trying to do this and see if it is worthwhile or possible to fix, but the success of that is, again, unlikely, and we'll probably just have to accept this regression.

Comment 3

[Acasandrei Beatrice (needinfo me)]

(In reply to Lee Salzman [:lsalzman] from comment #2)

This is because Skia finally removed its old specialized gradient rendering code so that they could fully move to an SkRasterPipeline-based gradient pathway. This allowed them to better support color-spaces and other things. I don't think it is likely viable to backport the old gradient code, given how long ago they removed it. I can spend some time trying to do this and see if it is worthwhile or possible to fix, but the success of that is, again, unlikely, and we'll probably just have to accept this regression.

It looks like the changes were backed out for causing failures on browser_screenshots_test_full_page.js based on this comment in Bug 1821512.

Comment 4

[Timothy Nikkel (:tnikkel)]

They got relanded just after that comment and they stuck.

Comment 5

[Kelsey Gilbert [:jgilbert]]

@lsalzman: Is this WONTFIX?

Comment 6

[Lee Salzman [:lsalzman]]

(In reply to Kelsey Gilbert [:jgilbert] from comment #5)

@lsalzman: Is this WONTFIX?

Still investigating.

Comment 7

[Lee Salzman [:lsalzman]]

I tried to backport the removed/deprecated upstream code that was responsible for this particular tsvgx regression. While technically it is possible to manually adapt the removed code and apply it, it don't quite cover the majority of the regression and adds a significant amount of technical debt for us to keep managing on subsequent Skia updates.

As of now, I don't know of other major test-case in the wild that are affected by this regression. It seems to only hit one particular tsvgx subtest which is a very particular gradient micro-benchmark. For now I think it is a good idea to WONTFIX this. If it turns out this is a larger issue that is really affecting major use-cases in the wild, we can investigate bringing the aforementioned patches back to see if we can mitigate it, but with the above caveats.