1467429 - displaylist_mutate / tresize / glterrain (linux64-qr) regression on push ede2ec96fe1e (Wed May 30 2018) and followups

Status: RESOLVED WONTFIX

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

Description

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

+++ This bug was initially created as a clone of Bug #1465784 +++

See bug 1465784 for the initial list of regressions. That bug (combined with bug 1466549) fixed most of the regressions. However there are still a few remaining:

displaylist_mutate linux64-qr opt e10s stylo 3,735.70 -> ~4800
tresize linux64-qr opt e10s stylo            13.48 -> ~17.50
glterrain linux64-qr opt e10s stylo          6.03 -> ~6.21

This bug is tracking these regressions.

Comment 1

[Ionuț Goldan [:igoldan]]

I'm afraid the push from https://bugzilla.mozilla.org/show_bug.cgi?id=1465784#c10 caused (at least) one new regression:

== Change summary for alert #13681 (as of Wed, 06 Jun 2018 11:33:08 GMT) ==

Regressions:

 33%  basic_compositor_video linux64-qr opt e10s stylo     2.71 -> 3.61

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

Comment 2

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

(In reply to Ionuț Goldan [:igoldan], Performance Sheriffing from comment #1)
> I'm afraid the push from
> https://bugzilla.mozilla.org/show_bug.cgi?id=1465784#c10 caused (at least)
> one new regression:

Indeed, so it did. We can use this bug to also cover that regression.

Comment 3

[Ionuț Goldan [:igoldan]]

Here are the Gecko profiles for the basic_compositor_video test:

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

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

Comment 4

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

So far I haven't been able to reproduce the basic_compositor_video regression locally. If I run the test I get ~16fps no matter what, and if I run it in xvfb-run I get better numbers but they're basically the same on the old and new code. I'm doing some try pushes to narrow down which of the two possible webrender PRs caused the regression. Also I'm not sure if the test itself is valid on webrender; I've asked mattwoodrow on IRC and am waiting for his reply.

I'll try reproducing some of the other regressions from comment 0 in my local setup.

Comment 5

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

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #2)
> (In reply to Ionuț Goldan [:igoldan], Performance Sheriffing from comment #1)
> > I'm afraid the push from
> > https://bugzilla.mozilla.org/show_bug.cgi?id=1465784#c10 caused (at least)
> > one new regression:
> 
> Indeed, so it did. We can use this bug to also cover that regression.

Also, technically, the regression was *not* from bug 1465784 comment 10, bug from bug 1466549, specifically this range:

*   5690e1bd Auto merge of #2797 - staktrace:hittest, r=nical
|\
| * 91e81d1d Always do a render after swapping in a new scene.
* b3b3d01d Auto merge of #2799 - gw3583:fix-2721, r=kvark
* 9fad33ec Invalidate drop-shadow data when local rect is animating.

(These are the two possible webrender PRs I mentioned in my previous comment)

Comment 6

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

So the regression was from servo/webrender#2797. But also mattwoodrow said that this test is basically not useful on webrender - it's meant to test the basic compositor, and if we have webrender enabled then we use webrender as the compositor, and not the basic compositor. So really we should just disable basic_compositor_video for the -qr test platforms. I'll see if I can figure out how to do that.

Comment 7

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

now that we are 100% on taskcluster, it is easier to run each talos test by itself; I am fine creating a talos-bcv test that just runs basic_compositor_video and that would not be in the list for -qr builds.

Comment 8

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

Regarding the remaining regressions here: I haven't been able to reproduce them locally, and the profiles from tryserver are not useful (partly because they don't include the relevant WR threads, and partly because profiles don't always help with tracking down ordering problems in the code). So it'll be much harder to make progress here as I'll have to do logging pushes on try and chase it down that way.

However, I should also point out that this is specific to webrender on linux64 which we're not planning on shipping for a while, so it should be ok to leave these regressions in the tree for a while. We do have explicit "don't release talos" release criteria for webrender in general which is tracked by dependencies on bug 1450997. As long we don't regress talos going from non-webrender to webrender we should be ok with regressions from one webrender-enabled push to another. I'll leave this bug open since it's useful to know which pushes did regress stuff, but it's not as urgent to work on.

Comment 9

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Closing as WONTFIX due to there being no updates in almost 2 years. All of the tests that regressed here show better performance in recent results with the exception of glterrain, which was reporting invalid results until bug 1594436 was resolved so those values are misleading.