Closed Bug 1397314 Opened 5 years ago Closed 5 years ago

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

Categories

(Core :: JavaScript: GC, defect)

57 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox-esr52 --- unaffected
firefox55 --- unaffected
firefox56 --- unaffected
firefox57 --- fixed

People

(Reporter: jmaher, Assigned: pbone)

References

Details

(Keywords: perf, regression, talos-regression)

Attachments

(1 file, 1 obsolete file)

Talos has detected a Firefox performance regression from push:

https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=55e08f65ac23b380ca995319032f07121d46bf71

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

Regressions:

  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)
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
Status: NEW → ASSIGNED
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 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+
Address review comments, r+ carried forward.
Attachment #8905801 - Attachment is obsolete: true
Attachment #8905868 - Flags: review+
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7ac52205995f
Revert to original calculation for sizing the nursery. r=jonco
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/7ac52205995f
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Duplicate of this bug: 1397622
You need to log in before you can comment on or make changes to this bug.