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

RESOLVED FIXED in Firefox 57

Status

()

Core
JavaScript: GC
RESOLVED FIXED
5 months ago
4 months ago

People

(Reporter: jmaher, Assigned: pbone)

Tracking

(Blocks: 1 bug, {perf, regression, talos-regression})

57 Branch
mozilla57
perf, regression, talos-regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

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

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

5 months ago
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

Updated

5 months ago
Component: Untriaged → JavaScript: GC
Product: Firefox → Core

Updated

5 months ago
Flags: needinfo?(pbone)
(Assignee)

Comment 1

5 months 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
Status: NEW → ASSIGNED
Flags: needinfo?(pbone)
Target Milestone: --- → mozilla57
Version: 53 Branch → 57 Branch

Comment 2

5 months ago
(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.
(Assignee)

Comment 3

5 months 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.

https://treeherder.mozilla.org/perf.html#/compare?originalProject=mozilla-central&newProject=try&newRevision=0ceb4bcd04c6b8c1b6b6c0eef1960f8aece2fe0d&framework=1&filter=tp5o&showOnlyImportant=0&selectedTimeRange=172800
Attachment #8905801 - Flags: review?(jcoppeard)

Comment 5

5 months ago
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+
(Assignee)

Comment 6

5 months 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+
(Assignee)

Updated

5 months ago
Keywords: checkin-needed

Comment 7

5 months ago
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

Comment 8

5 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/7ac52205995f
Status: ASSIGNED → RESOLVED
Last Resolved: 5 months ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
status-firefox55: --- → unaffected
status-firefox56: --- → unaffected
status-firefox-esr52: --- → unaffected
(Assignee)

Updated

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