2.23% tp5o_webext Private Bytes (windows7-32) regression on push 55e08f65ac23b380ca995319032f07121d46bf71 (Mon Sep 4 2017)

RESOLVED FIXED in Firefox 57



a year ago
a year ago


(Reporter: jmaher, Assigned: pbone)


({perf, regression, talos-regression})

57 Branch
perf, regression, talos-regression
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox55 unaffected, firefox56 unaffected, firefox57 fixed)



(1 attachment, 1 obsolete attachment)



a year ago
Talos has detected a Firefox performance regression from push:


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


  2%  tp5o_webext Private Bytes windows7-32 opt e10s     150,833,673.01 -> 154,195,420.15

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

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 → JavaScript: GC
Product: Firefox → Core
Flags: needinfo?(pbone)

Comment 1

a year ago
It's plausable that my change created this regression.

My change can affect the tuning of the JavaScript GC Nursery size which will affect this metric. I'll do some testing and see if I can create a better policy/heuristic for setting the nursery size.
Assignee: nobody → pbone
Flags: needinfo?(pbone)
Target Milestone: --- → mozilla57
Version: 53 Branch → 57 Branch
(In reply to Paul Bone [:pbone] from comment #1)
Let's revert to the original heuristic.  There are more important things to work on right now.

Comment 3

a year ago
Created attachment 8905801 [details] [diff] [review]
Bug 1397314 - Revert to original calculation for sizing the nursery

Reverting to the previous calculation for setting the nursery size seems to fix the problem.

Attachment #8905801 - Flags: review?(jcoppeard)
Comment on attachment 8905801 [details] [diff] [review]
Bug 1397314 - Revert to original calculation for sizing the nursery

Review of attachment 8905801 [details] [diff] [review]:

Great, thanks for the quick fix.

::: js/src/gc/Nursery.cpp
@@ +977,5 @@
> +    /*
> +     * Uncomment when we're ready to tune the nursery better.
> +     * bool canUsePromotionRate;
> +     * const float promotionRate = calcPromotionRate(&canUsePromotionRate);
> +     */

Please remove the commented out code.  You can add a TODO comment referencing future work if you like.
Attachment #8905801 - Flags: review?(jcoppeard) → review+

Comment 6

a year ago
Created attachment 8905868 [details] [diff] [review]
Bug 1397314 - Revert to original calculation for sizing the nursery r=jonco

Address review comments, r+ carried forward.
Attachment #8905801 - Attachment is obsolete: true
Attachment #8905868 - Flags: review+


a year ago
Keywords: checkin-needed

Comment 7

a year ago
Pushed by ryanvm@gmail.com:
Revert to original calculation for sizing the nursery. r=jonco
Keywords: checkin-needed

Comment 8

a year ago
Last Resolved: a year ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
status-firefox55: --- → unaffected
status-firefox56: --- → unaffected
status-firefox-esr52: --- → unaffected


a year ago
Duplicate of this bug: 1397622
You need to log in before you can comment on or make changes to this bug.