1640844 - Investigate nursery performance in the ARES-6 benchmark

Status: RESOLVED FIXED

(Core :: JavaScript: GC, task)

Description

[Jon Coppeard (:jonco)]

Following bug 1637896 I took a profile of the first few seconds of the benchmark:

https://profiler.firefox.com/public/34fd941df4a993bd8e4aaf9e578a9092b00abf6f/calltree/?globalTrackOrder=0-1&localTrackOrderByPid=87469-1-2-0~87472-0-1~&thread=2&v=4

This shows a period with extremely frequent minor GCs. It's likely we could improve our score on this benchmark by doing some tuning here.

Comment 1

[Jon Coppeard (:jonco)]

Attachment: Bug 1640844 - Part 1: Improve nursery size calculation to reduce size changes r=sfink

When running benchmarks the nursery size changes frequently, often at every collection.

This patch uses a moving average of the calculated growth factor. This takes into account information from previous collections as well when calculating the nursery size and producees much more stable results.

Comment 2

[Jon Coppeard (:jonco)]

Attachment: Bug 1640844 - Part 5: Take account of how much time we spend collecting when calculating nursery size r?sfink

This adds a goal of using less then 1% of total time collecthing the nursery, which has the effect of increasing the nursery size where a lot of nursery garbage is created but little is tenured.

In local testing this is a win on ARES6 and octane, but perfherder paints a more ambiguous picture. I'd like to land this anyway and we can back it out if it causes problems.

Depends on D80002

Comment 3

[Jon Coppeard (:jonco)]

I'll land the patches separately so we can see the cause of any regressions.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2d6665fa9b43
Improve nursery size calculation to reduce size changes r=sfink

Comment 5

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/2d6665fa9b43

Comment 6

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c269b2b6cdd2
Backed out changeset 2d6665fa9b43 for causing regressions.

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backout merged: https://hg.mozilla.org/mozilla-central/rev/c269b2b6cdd2

Comment 8

[Jon Coppeard (:jonco)]

Attachment: Bug 1640844 - Part 2: Don't use smoothed growth factor for initial collection r?sfink

This only uses the smoothed growth factor if it contains data from previous collections.

This should fix the awsy regressions from when this landed before. Fast ramp up is important in startup to avoid growing the tenured heap which can lead to larger overall memory usage.

Depends on D80002

Comment 9

[Jon Coppeard (:jonco)]

I think I've fixed the memory regression so I'll try relanding this.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/26ee57e3a7b0
Part 1: Improve nursery size calculation to reduce size changes r=sfink
https://hg.mozilla.org/integration/autoland/rev/425a08e7e999
Part 2: Don't use smoothed growth factor for initial collection r=sfink

Comment 11

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/26ee57e3a7b0
https://hg.mozilla.org/mozilla-central/rev/425a08e7e999

Comment 12

[Jon Coppeard (:jonco)]

Attachment: Bug 1640844 - Part 3: Minimize nursery size in shrinking GCs r?sfink

Runtimes that are not currently running any JS may tie up signficant memory in their nurseries. This patch shrinks 'idle' nurseries to their minimum size (default 256KB). Idleness is determined by no cells being allocated in them for 2 seconds (I'm open to discussions about the exact limit). This means that even if more nursery memory is used temporarily, we release it again in a timely manner.

Comment 13

[Jon Coppeard (:jonco)]

Attachment: Bug 1640844 - Part 4: Only used smoothed growth factor for recent collections r?sfink

It only makes sense to use the smoothing if the nursery has been collected recently. If not it's unlikely to correlate with recent usage. This change makes us ramp up more quickly after a period of inactivity, as we did before the earlier patches in this series landed.

Depends on D82969

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5d0c327a36a8
Part 3: Minimize nursery size in shrinking GCs r=sfink
https://hg.mozilla.org/integration/autoland/rev/7df94b4f2b9f
Part 4: Only used smoothed growth factor for recent collections r=sfink

Comment 15

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/5d0c327a36a8
https://hg.mozilla.org/mozilla-central/rev/7df94b4f2b9f

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e692dc0153e2
Part 5: Take account of how much time we spend collecting when calculating nursery size r=sfink

Comment 17

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/e692dc0153e2

Comment 18

[Florin Strugariu [:Bebe]]

== Change summary for alert #26525 (as of Thu, 16 Jul 2020 07:20:26 GMT) ==

Improvements:

5% JS windows7-32-shippable opt tp6 154,233,295.40 -> 146,304,383.68
4% JS linux1804-64-shippable-qr opt tp6 197,936,476.58 -> 189,533,440.23
4% JS windows7-32-shippable opt tp6 153,609,995.70 -> 147,327,726.87
4% JS windows10-64-shippable opt tp6 196,955,925.59 -> 189,425,260.74
4% JS macosx1014-64-shippable opt tp6 198,991,964.39 -> 191,527,408.73
4% JS windows10-64-shippable-qr opt tp6 198,916,803.67 -> 191,566,881.77
4% JS windows10-64-shippable-qr opt 106,325,912.80 -> 102,603,602.22
3% JS linux1804-64-shippable opt 103,653,366.40 -> 100,151,233.52
3% JS linux1804-64-shippable-qr opt 103,000,295.89 -> 99,934,971.58
3% JS macosx1014-64-shippable opt 104,240,325.02 -> 101,140,014.01
3% JS windows7-32-shippable opt 75,643,511.28 -> 73,676,777.48

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=26525

Comment 19

[Jon Coppeard (:jonco)]

This didn't have exactly the outcome I expected but I'm not planning on making any more changes in this bug.