Closed Bug 1401682 Opened 2 years ago Closed 2 years ago

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

Categories

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

57 Branch
defect

Tracking

()

VERIFIED FIXED
Firefox 58
Tracking Status
firefox-esr52 --- unaffected
firefox55 --- unaffected
firefox56 --- unaffected
firefox57 --- fixed
firefox58 --- fixed

People

(Reporter: jmaher, Assigned: Mardak)

References

Details

(Keywords: perf, regression, talos-regression)

Attachments

(1 file)

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?
Flags: needinfo?(edilee)
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)
Component: Untriaged → Activity Streams: Newtab
Assignee: nobody → edilee
Flags: needinfo?(khudson)
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
Priority: -- → P2
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 khudson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/66f49bef247d
Adjust activity stream fade in timing to avoid tart regression. r=k88hudson
https://hg.mozilla.org/mozilla-central/rev/66f49bef247d
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
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
Blocks: 1406120
Component: Activity Streams: Newtab → New Tab Page
You need to log in before you can comment on or make changes to this bug.