Closed Bug 1516638 Opened 6 years ago Closed 6 years ago

2.01% JS (windows10-64) regression on push c39cd7438b60866004690e6191dec0984e996d4a (Thu Dec 20 2018)

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla66
Tracking Status
firefox-esr60 --- unaffected
firefox65 --- unaffected
firefox66 --- fixed

People

(Reporter: jmaher, Assigned: nbp)

References

Details

(Keywords: perf, regression)

We have detected an awsy regression from push:

https://hg.mozilla.org/integration/autoland/pushloghtml?changeset=c39cd7438b60866004690e6191dec0984e996d4a

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

Regressions:

  2%  JS windows10-64 opt stylo     113,510,400.80 -> 115,793,589.57


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

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 jobs in a pushlog format.

To learn more about the regressing test(s), please see: https://wiki.mozilla.org/AWSY/Tests
I know :nbp is on holiday for another few working days, so happy to hear when he gets back.  Also this is from a push 8 days ago, longer alert times and less people to work on them == delays in filing regressions.

as a note, there is a set of js benchmark improvements noted in bug 1489572, so any changes here should be balanced with that.
Flags: needinfo?(nicolas.b.pierron)
Component: General → JavaScript Engine
Product: Testing → Core
Version: Version 3 → unspecified
One of the intent of Bug 1489572 was to fix the number of chunks used to avoid a case where we spend too much time freeing space. The benchmark seems to indicate a small regression on Forced GC cases, which might be acceptable.

However, one thing we can do to avoid keeping around too much of the peak memory would be to add extra heuristics for reclaiming chunks.

Today we keep track of the peak memory of LifoAlloc, maybe we could update this mechanism to do a gradual decent of retained memory space. Ted, What do you think?
Flags: needinfo?(nicolas.b.pierron) → needinfo?(tcampbell)

I'm curious whether bug 1517758 fixed this regression or just brought separate big perf improvements by itself.

:sdetar, :nbp could you resume the discussion here? This bug has been blocked for the last 2 weeks.

Flags: needinfo?(sdetar)
Flags: needinfo?(nicolas.b.pierron)

I suspect this bug could be a duplicate of Bug 1520366, which should hopefully be reviewed next week.

Flags: needinfo?(tcampbell)
Flags: needinfo?(nicolas.b.pierron)

:nbp now that you fixed bug 1520366 can validate if this bug is a dup or not?

Flags: needinfo?(sdetar) → needinfo?(nicolas.b.pierron)

https://treeherder.mozilla.org/perf.html#/graphs?timerange=5184000&series=mozilla-central,1653965,1,4&series=mozilla-central,1653964,1,4&series=mozilla-central,1653963,1,4&series=mozilla-central,1653960,1,4&highlightedRevisions=c39cd7438b60866&highlightedRevisions=13e451a69083&selected=mozilla-central,1653960,425782,700848693,4

I do not have enough data to confirm yet that there is an improvement.

However, on the single data point that we have for the moment, the "windows10-64: JS start opt stylo After tabs open" and "windows10-64: JS start opt stylo After tabs open [+30s]" seems to have reduced a little and be almost back at the original level.

Looking again at the previous graph, the regression is gone around Bug 1517758 changes, but very little seems to have happened after Bug 1520366.

Honestly, I do not understand how Bug 1517758 could have improved these benchmarks as they are not supposed to be GC-ing in the first place. And if they were GC-ing, then removing the forced GC should have emphasized the behaviour which is now fixed by Bug 1517758.

My conclusion is that I do not understand these benchmarks enough to know what is going on.
In any case, the issue seems to have been fixed by Bug 1517758.

Flags: needinfo?(nicolas.b.pierron)
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: needinfo?(erahm)
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
Flags: needinfo?(erahm)
Assignee: nobody → nicolas.b.pierron
Target Milestone: --- → mozilla66
You need to log in before you can comment on or make changes to this bug.