Closed Bug 1401682 Opened 2 years ago Closed 2 years ago
.29 - 6 .38% tart (linux64, windows7-32) regression on push 2ecda1649fe43924e185445db870d7fd7e27bb0b (Wed Sep 20 2017)
59 bytes, text/x-review-board-request
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
:Mardak, can you take a look at this regression?
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.
Flags: needinfo?(edilee) → needinfo?(khudson)
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%.
Assignee: nobody → edilee
Version: 53 Branch → 57 Branch
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.
k88hudson, if this is good, we can land this on autoland and separately backport the fix with more comments, etc.
Alternatively, we can just fix this as part of a usual export. https://github.com/mozilla/activity-stream/pull/3571/files
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!
Attachment #8910940 - Flags: review?(khudson) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/66f49bef247d Adjust activity stream fade in timing to avoid tart regression. r=k88hudson
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
Status: RESOLVED → VERIFIED
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
Attachment #8910940 - Flags: approval-mozilla-beta?
Could be pushed to FF57?
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
Attachment #8910940 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(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
Component: Activity Streams: Newtab → New Tab Page
You need to log in before you can comment on or make changes to this bug.