1554650 - 2.81 - 2.95% tsvgr_opacity (linux64-shippable, linux64-shippable-qr) regression on push 21b3ac9c491755466306d68e1e198a6891c0e561

Status: RESOLVED WONTFIX

(Core :: SVG, defect)

Description

[Ionuț Goldan [:igoldan]]

Talos has detected a Firefox performance regression from push:

https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=2ad581fafc85ab939d75f7aa1a2f901f45ecb113&tochange=21b3ac9c491755466306d68e1e198a6891c0e561

As author of one of the patches included in that push, we need your help to address this regression.

Regressions:

3% tsvgr_opacity linux64-shippable opt e10s stylo 137.41 -> 141.46
3% tsvgr_opacity linux64-shippable-qr opt e10s stylo 97.67 -> 100.41

You can find links to graphs and comparison views for each of the above tests at: https://treeherder.mozilla.org/perf.html#/alerts?id=21072

On the page above you can see an alert for each affected platform as well as a link to a graph showing the history of scores for this test. There is also a link to a treeherder page showing the Talos jobs in a pushlog format.

To learn more about the regressing test(s), please see: https://wiki.mozilla.org/Performance_sheriffing/Talos/Tests

For information on reproducing and debugging the regression, either on try or locally, see: https://wiki.mozilla.org/Performance_sheriffing/Talos/Running

*** Please let us know your plans within 3 business days, or the offending patch(es) will be backed out! ***

Our wiki page outlines the common responses and expectations: https://wiki.mozilla.org/Performance_sheriffing/Talos/RegressionBugsHandling

Comment 1

[violet.bugreport]

The testcase is basically 2500 <rect> of this sort:

<rect x="0" y="0" width="12" height="12" stroke="white" opacity=".5"/>

So 2%-3% loss of performance on some certain platform is likely inevitable, given the feature being implemented here.

Without geometry property feature, we just parse the value and store it locally; with this feature, we inevitably need to pass the value of 4 more attributes to CSS engine to perform cascading, which is purely overhead in this extreme testcase.

I think it's acceptable. Actually, the old MappedAttrParser::ParseMappedAttrValue() may have a lot of room for optimization, which can improve performance for situation like this from a different perspective. But that's a separate problem, not related to Bug 1383650.

Robert, what do you think?

Comment 2

[Robert Longson [:longsonr]]

I agree.