1401682 - 2.29 - 6.38% tart (linux64, windows7-32) regression on push 2ecda1649fe43924e185445db870d7fd7e27bb0b (Wed Sep 20 2017)

Status: VERIFIED FIXED

(Firefox :: New Tab Page, defect, P2)

Description

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

Talos has detected a Firefox performance regression from push:

https://hg.mozilla.org/integration/autoland/pushloghtml?changeset=2ecda1649fe43924e185445db870d7fd7e27bb0b

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

Regressions:

  6%  tart summary linux64 pgo e10s     3.75 -> 3.99
  5%  tart summary linux64 opt e10s     4.12 -> 4.34
  2%  tart summary windows7-32 pgo e10s 4.28 -> 4.38


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

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/Buildbot/Talos/Tests

For information on reproducing and debugging the regression, either on try or locally, see: https://wiki.mozilla.org/Buildbot/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/Buildbot/Talos/RegressionBugsHandling

Comment 1

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

:Mardak, can you take a look at this regression?

Comment 2

[Ed Lee :Mardak]

That export did contain the fading. And tart does open new tabs fast enough to not be preloaded (but is prerendered). I would think animating fade could compete with the tab animating time.

Comment 3

[Ed Lee :Mardak]

Looks like it's the transition:

https://treeherder.mozilla.org/perf.html#/compare?originalProject=mozilla-central&originalRevision=319a34bea9e4&newProject=try&newRevision=36ad8c34596843afc175587bc2612338e51e1377&framework=1&filter=tart%2032%20opt&showOnlyImportant=0

I backed out the one line `-  transition: opacity 0.1s ease-in-out; }`

win32 opt tart improves by 2%.

Comment 4

[Ed Lee :Mardak]

After playing around with various transition timings. Here's the results for various duration / delay in ms for linux64 opt:

100/ 10  1.91% regress
100/ 50  2.05% regress
100/100 -2.23%
100/110 -3.57%
 50/115 -3.98%
 75/115 -3.98%
100/115 -4.49%
100/120 -4.72%
 50/125 -5.09%
 75/125 -4.90%
100/125 -4.86%
100/130 -5.09%
100/150 -5.13%
100/250 -5.27%
100/500 -4.49%
100/1.s -5.23%
100/2.s -4.90%
100/5.s -5.09%

These are for 5 runs, so there's some noise but basically anything around 5% ± 0.2% improvement seems to indicate a full restoration of tart avoiding the regression for this bug.

Probably good to note that the `Linux x64 QuantumRender opt` were practically unchanged for any of those values, so we should be able to revert the fix here with QuantumRender enabled.

Checking with UX on acceptable transitions.

Comment 5

[Ed Lee :Mardak]

Attachment: Bug 1401682 - Adjust activity stream fade in timing to avoid tart regression.

Review commit: https://reviewboard.mozilla.org/r/182396/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/182396/

Comment 6

[Ed Lee :Mardak]

k88hudson, if this is good, we can land this on autoland and separately backport the fix with more comments, etc.

Comment 7

[Ed Lee :Mardak]

Alternatively, we can just fix this as part of a usual export. https://github.com/mozilla/activity-stream/pull/3571/files

Comment 8

[Kate Hudson :k88hudson]

Comment on attachment 8910940 [details]
Bug 1401682 - Adjust activity stream fade in timing to avoid tart regression.

https://reviewboard.mozilla.org/r/182396/#review187938

Nice!

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/66f49bef247d
Adjust activity stream fade in timing to avoid tart regression. r=k88hudson

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/66f49bef247d

Comment 11

[Ed Lee :Mardak]

Just to confirm, this did improve tart 6%:
https://treeherder.mozilla.org/perf.html#/compare?originalProject=autoland&originalRevision=754a0d9a44ea&newProject=autoland&newRevision=66f49bef247d&framework=1

Maybe no alert for the improvement because bug 1397918 had a measured ~40% tart regression:
https://treeherder.mozilla.org/perf.html#/alerts?id=9621

Comment 12

[Ed Lee :Mardak]

Comment on attachment 8910940 [details]
Bug 1401682 - Adjust activity stream fade in timing to avoid tart regression.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1401418 export that added the fade
[User impact if declined]: Slower tab opening as measured by talos tart
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: Not really
[Why is the change risky/not risky?]: 1 line css change adjusting times
[String changes made/needed]: None

Comment 13

[mdew]

Could be pushed to FF57?

Comment 14

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8910940 [details]
Bug 1401682 - Adjust activity stream fade in timing to avoid tart regression.

Fix a trivial perf regression, taking it.
Should be in 57b3

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/0922e331d03d

Comment 16

[Ionuț Goldan [:igoldan]]

(In reply to Ed Lee :Mardak from comment #11)
> Maybe no alert for the improvement because bug 1397918 had a measured ~40%
> tart regression:
> https://treeherder.mozilla.org/perf.html#/alerts?id=9621

Actually, we did receive improvements notifications. They look very good:

== Change summary for alert #9649 (as of September 22 2017 14:22 UTC) ==

Improvements:

  6%  tart summary linux64 opt e10s     5.99 -> 5.62
  2%  tart summary windows7-32 pgo e10s 4.32 -> 4.22

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