1503794 - 3.1 - 9.83% displaylist_mutate (linux64-qr, windows10-64-qr) regression on push 68aa8f295bd5bd25ad05b003972cedb977e5a570 (Wed Oct 31 2018)

Status: VERIFIED FIXED

(Core :: Graphics: WebRender, defect, P3)

Description

[Ionuț Goldan [:igoldan]]

Talos has detected a Firefox performance regression from push:

https://hg.mozilla.org/integration/autoland/pushloghtml?changeset=68aa8f295bd5bd25ad05b003972cedb977e5a570

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

Regressions:

 10%  displaylist_mutate windows10-64-qr opt e10s stylo     5,069.52 -> 5,567.95
  3%  displaylist_mutate linux64-qr opt e10s stylo          5,064.31 -> 5,221.44


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

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

[Ionuț Goldan [:igoldan]]

:kats can you also mention which of the following bugs is more related to these regressions?

bug 1503528
bug 1502585
bug 1503442

Thanks!

Comment 2

[Ionuț Goldan [:igoldan]]

I'll post the Gecko profiles soon.

Comment 3

[Ionuț Goldan [:igoldan]]

Here are the Gecko profiles for displaylist_mutate on Windows 10 QR:

before: https://perf-html.io/from-url/https%3A%2F%2Fqueue.taskcluster.net%2Fv1%2Ftask%2FDsbX9YyxTXy3mHaO764NuA%2Fruns%2F0%2Fartifacts%2Fpublic%2Ftest_info%2Fprofile_displaylist_mutate.zip

after: https://perf-html.io/from-url/https%3A%2F%2Fqueue.taskcluster.net%2Fv1%2Ftask%2Fb3YP590kTVuKPOeansO2Mg%2Fruns%2F0%2Fartifacts%2Fpublic%2Ftest_info%2Fprofile_displaylist_mutate.zip


And for displaylist_mutate on Linux 64bit QR:

before: https://perf-html.io/from-url/https%3A%2F%2Fqueue.taskcluster.net%2Fv1%2Ftask%2FfeVY_sIoTfqQz9uuCJyyuA%2Fruns%2F0%2Fartifacts%2Fpublic%2Ftest_info%2Fprofile_displaylist_mutate.zip

after: https://perf-html.io/from-url/https%3A%2F%2Fqueue.taskcluster.net%2Fv1%2Ftask%2FZ-0hrRXPQ9qmv73HzCODdg%2Fruns%2F0%2Fartifacts%2Fpublic%2Ftest_info%2Fprofile_displaylist_mutate.zip

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Based on the try pushes in bug 1503442, it is a regression from servo/webrender#3244 (i.e. bug 1503442).

Comment 5

[Glenn Watson [:gw]]

I tried to look at the profiles, but they seem to only contain profile information for the content process. I expect this regression is probably in the GPU process inside WR code. Is it possible to get a profile with threads from the GPU process? Or is there a way to show that in the existing profiles? (I don't know the profiler very well).

I'm not particularly surprised by this regression. That patch adds a significant amount of extra calculation work, by pre-calculating the bounding rects of primitives and building them into clusters during scene building. However, the follow up work that actually takes advantage of this is not yet implemented (the follow up work will make use of these clusters to remove the need for most per-primitive bounding rect and visibility calculations during frame building).

Because this is a fairly large amount of code changes, I'm landing it in incremental parts.

So, ideally we'd take this hit for now, and follow up on it when the rest of the picture caching work lands (I could take this bug and comment on it when that arrives, if that suits).

Does that seem reasonable?

Comment 6

[Ionuț Goldan [:igoldan]]

(In reply to Glenn Watson [:gw] from comment #5)
> So, ideally we'd take this hit for now, and follow up on it when the rest of
> the picture caching work lands (I could take this bug and comment on it when
> that arrives, if that suits).
> 
> Does that seem reasonable?

Yes, sounds good and I agree. Will be waiting for the next related bugs.

Comment 7

[Ionuț Goldan [:igoldan]]

(In reply to Ionuț Goldan [:igoldan], Performance Sheriffing from comment #6)
> (In reply to Glenn Watson [:gw] from comment #5)
> > So, ideally we'd take this hit for now, and follow up on it when the rest of
> > the picture caching work lands (I could take this bug and comment on it when
> > that arrives, if that suits).
> > 
> > Does that seem reasonable?
> 
> Yes, sounds good and I agree. Will be waiting for the next related bugs.

:gw have you filed any bugs for this matter?

Comment 8

[Glenn Watson [:gw]]

Not specifically for this regression - https://bugzilla.mozilla.org/show_bug.cgi?id=1494775 is the metabug for the picture caching work which we expect to resolve the regressions here, when complete. If / when I am able to land incremental patches I'll reference them here too.

Comment 9

[Glenn Watson [:gw]]

I think this can be marked fixed and closed now?

Comment 10

[Ionuț Goldan [:igoldan]]

(In reply to Glenn Watson [:gw] from comment #9)
> I think this can be marked fixed and closed now?

It looks like so. But please confirm whether bug 1511042 is the one that won back the performance.

Comment 11

[Kartikaya Gupta (email:[e-mail])]

It's not bug 1511042 that won back the performance from this regression, but it's also not easy to point at a single improvement and say "it was this change that won back the perf from this particular regression". If I had to pick one based on the graph I'd say bug 1509302, since that was the one that brought the displaylist_mutate perf back up to better than the original levels.