Closed Bug 1530611 Opened 8 months ago Closed 7 months ago

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

Categories

(Core :: Graphics: WebRender, defect)

Unspecified
All
defect
Not set

Tracking

()

VERIFIED FIXED
mozilla67
Tracking Status
firefox-esr60 --- unaffected
firefox65 --- unaffected
firefox66 --- unaffected
firefox67 --- fixed

People

(Reporter: igoldan, Assigned: kvark)

References

(Blocks 2 open bugs)

Details

(Keywords: perf, regression)

Attachments

(2 obsolete files)

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

Component: General → Graphics: WebRender
Product: Testing → Core
Flags: needinfo?(dmalyshau)
Assignee: nobody → dmalyshau
Status: NEW → ASSIGNED
Flags: needinfo?(dmalyshau)
Flags: needinfo?(igoldan)

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

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.

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.

(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?

Flags: needinfo?(igoldan) → needinfo?(jmaher)

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

Flags: needinfo?(jmaher)
Attachment #9050179 - Attachment description: [wip] Disable WR dirty regions upon reaching a coverage threshold → Disable WR dirty regions upon reaching a coverage threshold

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.

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

Flags: needinfo?(gwatson)
Flags: needinfo?(bbouvier)

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

Flags: needinfo?(bbouvier)

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

Flags: needinfo?(gwatson)

(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"
}

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

Status: ASSIGNED → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
Target Milestone: --- → mozilla67
Attachment #9050179 - Attachment is obsolete: true
Attached file Fix tile cache clip chain in WR (obsolete) —
Attachment #9050179 - Attachment is obsolete: false

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.

Attachment #9054976 - Attachment is obsolete: true
Attachment #9050179 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.