1228439 - 2.5% regression on tp5o on MacOS X 10.10 e10s on Nov 23 (v.45) from push cae5c087063d

Status: RESOLVED WONTFIX

(Core :: Graphics, defect)

Description

[William Lachance (:wlach)]

It looks like bug 1221840 caused a performance regression on the tp5o benchmark on macos x 10.10:

I did a bunch of retriggers in the area, you can see it on the graph here: https://treeherder.allizom.org/perf.html#/graphs?series=[mozilla-inbound,3ec2958352e2cb98df247808db574209f4ab5eb8,1]&highlightedRevisions=cae5c087063d&zoom=1448263720671.233,1448348730698.6301,228.6956344825634,272.1738953521286

Here's the compare view showing the summary:

https://treeherder.mozilla.org/perf.html#/compare?originalProject=mozilla-inbound&originalRevision=16e98ad8c97b&newProject=mozilla-inbound&newRevision=cae5c087063d&filterTest=tp5o%20opt%20e10s&filterPlatform=osx&showOnlyImportant=0&showUnreliablePlatforms=1

You can see a detailed breakdown of which tests regressed the most here:

https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=mozilla-inbound&originalRevision=16e98ad8c97b&newProject=mozilla-inbound&newRevision=cae5c087063d&originalSignature=3ec2958352e2cb98df247808db574209f4ab5eb8&newSignature=3ec2958352e2cb98df247808db574209f4ab5eb8

mchang: I don't think there's any need to back this out before Monday (this is a smaller regression on a less important platform), but please look into this sooner than later and let us know what your plan is. I assume you know how to diagnose Talos regressions.

Comment 1

[Mason Chang [Inactive] [:mchang]]

I tried backing out the patch and pushing to try again bit by bit. I got 3 try results:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a44fd5898448
https://treeherder.mozilla.org/#/jobs?repo=try&revision=741fce7ae5aa
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7080ea5cb93f

None showed any regressions and I can't reproduce this locally.

Comment 2

[William Lachance (:wlach)]

(In reply to Mason Chang [:mchang] from comment #1)
> I tried backing out the patch and pushing to try again bit by bit. I got 3
> try results:
> 
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=a44fd5898448
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=741fce7ae5aa
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=7080ea5cb93f
> 
> None showed any regressions and I can't reproduce this locally.

Hmm, what are you using as a baseline? You should compare each of these against the baseline mozilla-central (or whatever) revision that you're using.

Comment 3

[Mason Chang [Inactive] [:mchang]]

(In reply to William Lachance (:wlach) from comment #2)
> (In reply to Mason Chang [:mchang] from comment #1)
> > I tried backing out the patch and pushing to try again bit by bit. I got 3
> > try results:
> > 
> > https://treeherder.mozilla.org/#/jobs?repo=try&revision=a44fd5898448
> > https://treeherder.mozilla.org/#/jobs?repo=try&revision=741fce7ae5aa
> > https://treeherder.mozilla.org/#/jobs?repo=try&revision=7080ea5cb93f
> > 
> > None showed any regressions and I can't reproduce this locally.
> 
> Hmm, what are you using as a baseline? You should compare each of these
> against the baseline mozilla-central (or whatever) revision that you're
> using.

I was comparing it against the inbound baseline in from comment 0. I also updated my m-c to the commited revision from comment 0.

Comment 4

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

one difference here, the regression seems to be e10s only, these try pushes are non-e10s.  Luckily we have a way to add jobs to a push!!!  I have added jobs and some retriggers- we can examine the results when they come in.

If we cannot determine this after the additional jobs, then I would assume we close it as wontfix.

Comment 5

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

it looks like all 3 try pushes show no regression compared to the baseline which they were pushed from.  Each try push builds upon each other, Is this not repeatable?  Maybe the thing to verify that is to push to try with the latest tip, then push to try with a backout of the full patch and compare those.

Comment 6

[Mason Chang [Inactive] [:mchang]]

(In reply to Joel Maher (:jmaher) from comment #5)
> it looks like all 3 try pushes show no regression compared to the baseline
> which they were pushed from.  Each try push builds upon each other, Is this
> not repeatable?  Maybe the thing to verify that is to push to try with the
> latest tip, then push to try with a backout of the full patch and compare
> those.

Good idea.

Base commit with backout cae5c087063d, base commit 319be5e7ce30:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=bdf6c5263e1d

Re-add CG part 1:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9623b60b5dcb

Re-add CG Part 2 (includes part 1):
https://treeherder.mozilla.org/#/jobs?repo=try&revision=61ba5ed8c00d

Re-add CG changes Part 3: (includes 1 and 2):
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3ed1df65189a

Comment 7

[Mason Chang [Inactive] [:mchang]]

Base with backout vs part 1: No Change
https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=bdf6c5263e1d&newProject=try&newRevision=9623b60b5dcb

Base with backout vs part 2: No Change
https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=bdf6c5263e1d&newProject=try&newRevision=61ba5ed8c00d

Base with backout vs part 3: 0.9% Change
https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=bdf6c5263e1d&newProject=try&newRevision=3ed1df65189a

This is confusing. The patches were deletions from the patch in bug 1221840. I was expecting that I would see a regression from part 1 and we'd fall back to the baseline. Instead, after I deleted the 3 parts that are specific to CG, the regression showed up.

Comment 8

[William Lachance (:wlach)]

Is it possible that patches that have landed since your change have changed the performance characteristics here?

At this point, I wonder if it might be worth doing some extra try pushes just to validate that we can reproduce the original regression.

Comment 9

[Mason Chang [Inactive] [:mchang]]

Here's a push of the base m-c, 319be5e7ce30, which includes the original bug, and is the base of the commits in comment 7.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5c3e4ff59ab3

Comment 10

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

hmm, this is the only one showing a slight regression:
https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=bdf6c5263e1d&newProject=try&newRevision=3ed1df65189a&filter=tp5o%20opt%20e10s&showOnlyImportant=0

otherwise these patches are making no changes.  I retriggered a bit more on the last push from comment 9

Comment 11

[Mason Chang [Inactive] [:mchang]]

This is confusing. The base commit from comment 9 compared to the last push in comment 7, which includes the full patch:

https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=5c3e4ff59ab3&newProject=try&newRevision=3ed1df65189a

It's showing an improvement with the patch compared to the backout? Maybe something else was added on top but we improved it?

Comment 12

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

it is hard to tell.  Have we hit the point of too much time spent on trying to figure this out?

Comment 13

[Mason Chang [Inactive] [:mchang]]

(In reply to Joel Maher (:jmaher) from comment #12)
> it is hard to tell.  Have we hit the point of too much time spent on trying
> to figure this out?

I think so. I'm not even sure we could back out if we wanted to, since the patch fixes youtube on windows. Can we resolve this as a WONTFIX?

Comment 14

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

sounds like a plan.