1062416 - 3-5% Linux/Win TART regression on fx-team (v.34) August 22 from rev d76f7a57af61

Status: RESOLVED WONTFIX

(Firefox :: New Tab Page, defect)

Description

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

here is a graph:
http://graphs.mozilla.org/graph.html#tests=[[293,132,35],[293,132,31],[293,132,33]]&sel=1407164407434,1409756407434&displayrange=30&datatype=running

I did some retriggers for linux,linux64,win8:
https://tbpl.mozilla.org/?tree=Fx-Team&fromchange=e03842f0980f&tochange=a66207560239&jobname=fx-team%20talos%20svgr

This seems to be all related to directory tiles, what I want to know is if this is expected on these 3 platforms or not?

Comment 1

[Ed Lee :Mardak]

The relevant bugs from that push:
https://hg.mozilla.org/integration/fx-team/pushloghtml?changeset=d76f7a57af61

Bug 1042876 Bug 1053530 Bug 1042214 Bug 1057404

I would guess either Bug 1053530 or Bug 1042214 but neither should have a significant impact on performance.

A guess is Bug 1053530 added extra xul:panel to the newtab page for increased complexity but that should be hidden by preloading. Or Bug 1042214 for adding extra processing logic each about:newtab open that only runs after it's visible.

Comment 2

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

should we investigate this?  it sounds like this might not be expected.  One way to tell would be back each of those out and see if it helps at all.  Do you know if we can just back those specific patches out, or are there too many dependencies?

Comment 3

[Avi Halachmi (:avih)]

(In reply to Ed Lee :Mardak from comment #1)
> ...
> A guess is Bug 1053530 added extra xul:panel to the newtab page for
> increased complexity but that should be hidden by preloading. Or Bug 1042214
> for adding extra processing logic each about:newtab open that only runs
> after it's visible.

Both sound relevant. Can we do a try push with those two backed out and check if the hypothesis is correct?

Comment 4

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

I have backed out 1053530:
https://tbpl.mozilla.org/?tree=Try&rev=eb04c31a43ce

bug 1042214 had a lot of conflicts, I was not certain of the editing I was doing vs messing things up.

lets wait a couple hours and see what the results show!

Comment 5

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

in this try push: https://tbpl.mozilla.org/?tree=Try&rev=eb04c31a43ce, you can see that linux32 and win8 show a slight win over what is currently posting on graph server.

linux32:
graph server regression aug 25: +0.2
graph server current range: 5.75->5.9
try push range: 5.68-5.71 (1 high value, added a few more retriggers)

win8:
graph server regression aug 25: +0.2
graph server current range: 7.6->7.9
try push range: 7.64->7.75

So unless we have a cumulative effect with this specific patch, I would not say that the patch from bug 1042214 is the problem.

Comment 6

[Ed Lee :Mardak]

Thanks for running that test with backing out bug 1053530. I'll try backing out bug 1042214 to see how that behaves.

Comment 7

[Ed Lee :Mardak]

I tried pushing various "backouts" of bug 1042214 as it doesn't cleanly backout without much modification.

http://graphs.mozilla.org/graph.html#tests=[[293,113,33]]&sel=1410407855021.7517,1410411040730.908,5.5968083830091455,5.894830044289606&displayrange=7&datatype=running

https://tbpl.mozilla.org/?tree=Try&rev=87f4a6c11329
https://tbpl.mozilla.org/?tree=Try&rev=221a44e429fa
https://tbpl.mozilla.org/?tree=Try&rev=3940ed793271

I'm not entirely sure how to interpret those results? How should I compare the performance?

Comment 8

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

not much of a difference (i am not sure what your base push was).  

linux range on try push 221a44e429fa: 5.66-5.8
win8 range on try push 87f4a6c11329: 7.63-7.78

We had a regression Sept 11th which needs to be defined, but your push appears to be based on sept 10th tree. which means the target ranges are the same as comment 5.

Comment 9

[Lawrence Mandel [:lmandel] (use needinfo)]

Joel - Is this still an issue on 34?
Ed - If this is an issue, what's the next step to fix the regression?

Comment 10

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

this is still an issue on v.34.  In fact the regression has sustained for linux32/64 on v.35, but other fixes have fixed win8!

here is a graph to help highlight it:
http://graphs.mozilla.org/graph.html#tests=%5B%5B293,132,33%5D,%5B293,132,35%5D,%5B293,132,31%5D,%5B293,52,35%5D,%5B293,52,31%5D,%5B293,52,33%5D%5D&sel=1405247486875,1413023486875&displayrange=90&datatype=running

Comment 11

[Ed Lee :Mardak]

There's active work in bug 1059558 (also tracking34) to improve new tab animation performance, and if bug 1042214 was on the hook, it could potentially be from the uninterruptible reflows, but that was recently fixed by bug 1071635, and I don't see much improvement in the performance graphs.

Comment 12

[Lawrence Mandel [:lmandel] (use needinfo)]

Ed - What's the plan for this bug? Are we going to wait for a fix for bug 1059558, which I don't think is likely to come in 34?

Comment 13

[Ed Lee :Mardak]

I tried running tart for each of the commits of the original push to see if it was the other commits not originally suggested. But I'm not sure if I did something wrong because they don't show up in graphs.mozilla.org try non-pgo win5.1 ?

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=87470887284b
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=6ae570db32e5
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=a2a3baa965a8
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=efef0d7051e2
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=8dbf77f51aaa

Comment 14

[Ed Lee :Mardak]

Ah, I can just grab the numbers directly from the raw output. In order of applying each commit from before and the 4 commits. I'll retrigger to get more runs:

Bug 1056976 tart: 6.20
Bug 1057404 tart: 6.28
Bug 1042214 tart: 6.10
Bug 1053530 tart: 6.19
Bug 1042876 tart: 6.25

Comment 15

[Ed Lee :Mardak]

11 svgr runs for each commit sorted by score:

Bug 1056976 6.09 6.20 6.20 6.22 6.23  6.25  6.26 6.26 6.31 6.32 6.35
Bug 1057404 6.16 6.17 6.18 6.19 6.20  6.25  6.28 6.29 6.29 6.29 6.32
Bug 1042214 6.10 6.13 6.14 6.15 6.19  6.20  6.20 6.22 6.28 6.28 6.32
Bug 1053530 6.17 6.17 6.19 6.19 6.19  6.20  6.23 6.28 6.32 6.35 6.38
Bug 1042876 6.17 6.17 6.18 6.21 6.25  6.25  6.25 6.32 6.34 6.36 6.37

jmaher, is there a usual way to compare?

Going by the median/6th result of 11 runs, there doesn't seem to be much change from the baseline (first line) to each of the additional tiles related commits. There's a .05 change, but the spread seems to be consistently around ±.1

Comment 16

[Ed Lee :Mardak]

The numbers from comment 15 were for win5.1/xp. Below is win6.1/7:

Bug 1056976 8.48 8.48 8.51 8.54 8.55  8.60  8.65 8.68 8.69 8.69 8.69
Bug 1057404 8.38 8.39 8.42 8.47 8.48  8.59  8.60 8.62 8.66 8.69 8.73
Bug 1042214 8.11 8.49 8.54 8.58 8.60  8.61  8.73 8.91 8.92 8.92 8.97
Bug 1053530 8.48 8.50 8.58 8.59 8.66  8.73  8.79 8.81 8.90 8.92 9.00
Bug 1042876 8.46 8.56 8.61 8.65 8.65  8.66  8.66 8.80 8.81 8.89 9.03

Changes in the median is within .13 with the spread usually ±.3

Comment 17

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

Ed- thanks for doing this.  That is great to see these numbers.

Unfortunately, those results don't really show anything useful; this might be a problem of stuff caught in noise or if you are comparing with updated patches other things affecting the end result.

A slight chance here is that 2 patches working together are causing the regression instead of a solo patch.  But for now we might be stuck without any options.

Comment 18

[Ed Lee :Mardak]

I generated those numbers by reverting to before the commits then applying the tiles related changes one commit at a time (so the last set of numbers has all tiles related commits applied).

The larger newtab performance work is happening in bug 1059558 with initial approaches needing some bake time on nightly.