Closed Bug 1357345 Opened 7 years ago Closed 7 years ago

4.18 - 4.31% sessionrestore / sessionrestore_no_auto_restore (windows7-32) regression on push 9bd131795d81 (Sat Apr 15 2017)

Categories

(Add-on SDK Graveyard :: General, defect)

Unspecified
Windows
defect
Not set
normal

Tracking

(Performance Impact:none)

RESOLVED WONTFIX
Performance Impact none

People

(Reporter: igoldan, Unassigned)

References

Details

(4 keywords)

Talos has detected a Firefox performance regression from push b9a1e8ade106a8bce340b1717dea4870543536b9. As author of one of the patches included in that push, we need your help to address this regression.

Regressions:

  4%  sessionrestore_no_auto_restore windows7-32 pgo e10s     722.6 -> 753.75
  4%  sessionrestore windows7-32 pgo e10s                     690.21 -> 719.08


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

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
Component: Untriaged → General
Keywords: addon-compat
Product: Firefox → Add-on SDK
(In reply to igoldan from comment #1)
> :kmag This is a better view over the data points:
> 
> https://treeherder.mozilla.org/perf.html#/graphs?series=%5Bmozilla-inbound,
> 639a8626a20b70264e8800e4d23f770a898408cc,1,1%5D&series=%5Bmozilla-inbound,
> 196b82960327035de720500e1a5f9f0154cf97ad,1,1%5D&series=%5Bmozilla-inbound,
> c22155824871783bb5d8f514b3a01b4c885034c0,1,1%5D&series=%5Bmozilla-inbound,
> e9484a55bdf595b7b86aea582bb9016cdd1ba956,1,1%5D&zoom=1492150342815.264,
> 1492258847828.816,610.4089219330855,983.6431226765799&selected=%5Bmozilla-
> inbound,639a8626a20b70264e8800e4d23f770a898408cc,193067,92274672,1%5D

It's not clear to me that this shows a regression. The numbers for that push look consistent with the ones for the pushes from several hours earlier.
I agree with you :kmag- I did a few extra retriggers to help pinpoint the exact cause.
ok, this is 2 revisions earlier from bug 1356569 and revision:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=8bffc18d9f251372ba69d5dc49591251258089ee&tochange=9bd131795d81977c1d21c9c4aff06b3f4c69956e

:florian- it appears your changes listed above ^ are related to these performance regressions.  What is odd is these appear to primarily be pgo regressions, not opt.  So that could be a random artifact of the pgo process.  If you could take a quick look through your code and see if anything related to session restore would be affected, that would help decide what to do with this regression.
Assignee: kmaglione+bmo → nobody
Blocks: 1356569
No longer blocks: 1314861
Flags: needinfo?(florian)
Summary: 4.18 - 4.31% sessionrestore / sessionrestore_no_auto_restore (windows7-32) regression on push b9a1e8ade106a8bce340b1717dea4870543536b9 (Sat Apr 15 2017) → 4.18 - 4.31% sessionrestore / sessionrestore_no_auto_restore (windows7-32) regression on push 9bd131795d81 (Sat Apr 15 2017)
(In reply to Joel Maher ( :jmaher) from comment #4)
> ok, this is 2 revisions earlier from bug 1356569 and revision:
> https://hg.mozilla.org/integration/mozilla-inbound/
> pushloghtml?fromchange=8bffc18d9f251372ba69d5dc49591251258089ee&tochange=9bd1
> 31795d81977c1d21c9c4aff06b3f4c69956e
> 
> :florian- it appears your changes listed above ^ are related to these
> performance regressions.  What is odd is these appear to primarily be pgo
> regressions, not opt.  So that could be a random artifact of the pgo
> process.  If you could take a quick look through your code and see if
> anything related to session restore would be affected, that would help
> decide what to do with this regression.

The changes are just automated cleanup, removing unused trailing parameters. The patch touches some session store code, but well... it's a tree-wide cleanup that touches 1000+ files.

You say that you see a regression only with pgo, could it be that somehow pgo optimizes for the cases where all parameters are provided?

Do you have more information about what sessionrestore_no_auto_restore and sessionrestore measure exactly? Do we have profiles of where the time is being spent?
Flags: needinfo?(florian)
:yoric, can you help :florian understand the sessionrestore tests?  We do not have profiles for them, this has been sort of broken lately as we switched from spsProfiler to geckoProfiler.
Flags: needinfo?(dteller)
Both tests measure time from process start until all tabs are opened. In `sessionrestore_no_auto_restore`, there is no session to restore, while in `sessionrestore`, there is one.
Flags: needinfo?(dteller)
We should track Talos sessionrestore regressions for Quantum Flow.
Whiteboard: [qf]
Talos regressions should have their own workflow. This is not inherently a Quantum Flow bug.
Whiteboard: [qf] → [qf-]
this bug is still open, :florian, can you look at this?  I
Flags: needinfo?(florian)
(In reply to Joel Maher ( :jmaher) from comment #10)
> this bug is still open, :florian, can you look at this?  I

It doesn't seem actionable. PGO-only, no profile, a giant patch that landed more than a month ago and can't easily be backed out for testing...
Flags: needinfo?(florian)
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WONTFIX
Performance Impact: --- → -
Whiteboard: [qf-]
You need to log in before you can comment on or make changes to this bug.