1530611 - 1.62 - 5.45% raptor-motionmark-htmlsuite-firefox (linux64-pgo-qr, windows10-64-pgo-qr) regression on push 9bae35dc1471ff97f0efba49083bb165453ebf35 (Mon Feb 25 2019)

Status: VERIFIED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Ionuț Goldan [:igoldan]]

Raptor has detected a Firefox performance regression from push:

https://hg.mozilla.org/integration/autoland/pushloghtml?changeset=9bae35dc1471ff97f0efba49083bb165453ebf35

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

Regressions:

5% raptor-motionmark-htmlsuite-firefox linux64-pgo-qr opt 59.44 -> 56.21
2% raptor-motionmark-htmlsuite-firefox windows10-64-pgo-qr opt 59.13 -> 58.17

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

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 Raptor jobs in a pushlog format.

To learn more about the regressing test(s) or reproducing them, please see: https://wiki.mozilla.org/Performance_sheriffing/Raptor

*** 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

[Ionuț Goldan [:igoldan]]

These are the Gecko profiles for Windows 10 Quantum Render:

before regression: https://perf-html.io/from-url/https%3A%2F%2Fqueue.taskcluster.net%2Fv1%2Ftask%2FOl0kC33lTo6TootxDXLP9Q%2Fruns%2F0%2Fartifacts%2Fpublic%2Ftest_info%2Fprofile_raptor-motionmark-htmlsuite-firefox.zip

after: https://perf-html.io/from-url/https%3A%2F%2Fqueue.taskcluster.net%2Fv1%2Ftask%2FNXA8aSsQSZuSenvdJDNMMQ%2Fruns%2F0%2Fartifacts%2Fpublic%2Ftest_info%2Fprofile_raptor-motionmark-htmlsuite-firefox.zip

Comment 2

[Dzmitry Malyshau [:kvark]]

:igoldan btw, this metric appears to be showing an improvement incorrectly:
https://treeherder.mozilla.org/perf.html#/comparesubtest?framework=10&originalProject=autoland&originalSignature=1884467&newProject=autoland&newSignature=1884467&originalRevision=b73f033efb41e21920b61fb97690fc3499d3f64b&newRevision=9bae35dc1471ff97f0efba49083bb165453ebf35

The "CSS bouncing blend circles" metric should show "53% worse" but it currently shows "53% better".

Comment 3

[Dzmitry Malyshau [:kvark]]

Looked at this today, I honestly don't see any interaction between the code and "CSS bouncing blend circles" :/

Comment 4

[Dzmitry Malyshau [:kvark]]

Attachment: Disable WR dirty regions upon reaching a coverage threshold

Comment 5

[Dzmitry Malyshau [:kvark]]

Actually, I was just looking at the wrong test. Good thing that wasn't effort wasted, since I found one thing we could do much better ;) will look at the right thing tomorrow.

Comment 6

[Dzmitry Malyshau [:kvark]]

Alright, it's fairly clear what's going on here. Currently, when we see a mix-blend mode item, we break the current picture, and it becomes a dependency of the new stacking context. If there is a list of mix-blend items, it just stretches the picture tree vertically, which makes us allocate more render tasks.

Comment 7

[Ionuț Goldan [:igoldan]]

(In reply to Dzmitry Malyshau [:kvark] from comment #2)

:igoldan btw, this metric appears to be showing an improvement incorrectly:
https://treeherder.mozilla.org/perf.html#/comparesubtest?framework=10&originalProject=autoland&originalSignature=1884467&newProject=autoland&newSignature=1884467&originalRevision=b73f033efb41e21920b61fb97690fc3499d3f64b&newRevision=9bae35dc1471ff97f0efba49083bb165453ebf35

The "CSS bouncing blend circles" metric should show "53% worse" but it currently shows "53% better".

Joel, do you know who the owner of Motionmark is?

Comment 8

[Joel Maher ( :jmaher ) (UTC -8)]

probably :gw (Glenn Watson), likewise :bbouvier could help out as well.

Comment 9

[Dzmitry Malyshau [:kvark]]

Possible solutions to the problem:

1. Revert back the code and continue using the old approach. It is slower than the new on some cases, and architecturally more hacky.
2. Track dependencies between the source and the backdrop, i.e. if the next circle only touches one circle in the backdrop, we can only move that other circle into the backdrop picture and leave the rest intact. This is problematic because the positions may dynamically change, which will make us to re-build the frame, but picture assignment is done at scene building (prior stage).
3. Optimize the current rendering by limiting the work to only the intersection area between the source and the backdrop. Current implementation works on the union of those rectangles and wastes quite a bit of pixel processing. We can generate regular blit segments for stuff outside of the intersection.

Neither of those solutions is close to a hypothetical optimum. We'll keep digging.

Comment 10

[Ionuț Goldan [:igoldan]]

(In reply to Dzmitry Malyshau [:kvark] from comment #2)

:igoldan btw, this metric appears to be showing an improvement incorrectly:
https://treeherder.mozilla.org/perf.html#/comparesubtest?framework=10&originalProject=autoland&originalSignature=1884467&newProject=autoland&newSignature=1884467&originalRevision=b73f033efb41e21920b61fb97690fc3499d3f64b&newRevision=9bae35dc1471ff97f0efba49083bb165453ebf35

The "CSS bouncing blend circles" metric should show "53% worse" but it currently shows "53% better".

:gw or :bbouvier, could one of you look over this issue? We may be interpreting this metric wrongly.

Comment 11

[Benjamin Bouvier [:bbouvier] (inactive)]

Sorry, I don't know the specifics of this benchmark.

Comment 12

[Glenn Watson [:gw]]

I think the metric that is showing is probably FPS. If that's the case then yes, this metric is being interpreted incorrectly. Is there a way to verify what the unit of the metric is that's being recorded here? (MotionMark can report metrics in various different units / styles, depending on the test configuration).

Comment 13

[Ionuț Goldan [:igoldan]]

(In reply to Glenn Watson [:gw] from comment #12)

Is there a way to verify what the unit of the metric is that's being recorded here? (MotionMark can report metrics in various different units / styles, depending on the test configuration).

From the artifact logs, I can see we're storing a "score" unit. This is how the suite looks like:

{
  "name": "CSS bouncing blend circles",
  "lowerIsBetter": true,
  "alertThreshold": 2.0,
  "replicates": [
    44.667,
    44.714,
    44.716,
    44.534,
    44.535
  ],
  "value": 44.667,
  "unit": "score"
}

Comment 14

[Ionuț Goldan [:igoldan]]

Looks like this push fixed the perf regressions. \0/

== Change summary for alert #19908 (as of Thu, 14 Mar 2019 03:08:41 GMT) ==

Improvements:

8% raptor-motionmark-htmlsuite-firefox linux64-pgo-qr opt 56.04 -> 60.74
2% raptor-motionmark-htmlsuite-firefox windows10-64-pgo-qr opt 58.02 -> 59.35

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=19908

Comment 15

[Dzmitry Malyshau [:kvark]]

Attachment: Fix tile cache clip chain in WR

Comment 16

[Phabricator Automation]

Comment on attachment 9054976 [details]
Fix tile cache clip chain in WR

Revision D25667 was moved to bug 1538711. Setting attachment 9054976 [details] to obsolete.