1787526 - Implement semispace nursery collection

Status: RESOLVED FIXED

(Core :: JavaScript: GC, enhancement, P2)

Description

[Steve Fink [:sfink] [:s:]]

The goal is to reduce unnecessary tenuring, when we tenure short-lived but recent allocations because we hit the end of the nursery. These appear to be fairly frequent in the matrix-react benchmark, for example.

Comment 1

[Steve Fink [:sfink] [:s:]]

Current design: allocate a large contiguous region of virtual memory space, roughly double the desired size of the nursery. Use the second half as the nursery space initially. When collecting, leave some portion in the nursery (call it K bytes). Back up the start of the nursery-allocatable region by K and run the mutator again to fill it up. Collect all but the K most recently allocated bytes, including the K-sized chunk left over from the previous iteration. Keep backing up the nursery start until hitting the beginning of the virtual memory area, then jump back to using the second half and repeat. In pictures, using 1/3 the size of the initial nursery-allocatable space as K:

[______] initial empty nursery
[___XXX] allocate into 2nd half
[_____X] collect all but last 1/3 of allocated space
[__XXXX] allocate into region just before preserved space
[____X_] collect all but most recent K sized chunk
[_XXXX_] allocate
[___X__] collect
[XXXX__] allocate
[__X___] collect
[__XXXX] now allocating into 2nd half again
[_____X] repeat

There are many details, but that's the basic idea.

Comment 2

[Steve Fink [:sfink] [:s:]]

Attachment: Bug 1787526 - Create a preserved region in the nursery and do not collect it in non-evicting nursery collections

Comment 3

[Steve Fink [:sfink] [:s:]]

Attachment: Bug 1787526 - Add PRESERVED_NURSERY as a getMarkInfo() result

Comment 4

[Steve Fink [:sfink] [:s:]]

Attachment: Bug 1787526 - Implement removeRange for OrderedHash{Set,Map}

Comment 5

[Steve Fink [:sfink] [:s:]]

Attachment: Bug 1787526 - Implement OrderedHashTable::MutableRange::removeFront().

Comment 6

[Steve Fink [:sfink] [:s:]]

Attachment: Bug 1787526 - Allow truncating a LifoAlloc to the position of an Enum.

Comment 7

[Steve Fink [:sfink] [:s:]]

Attachment: Bug 1787526 - Implement OrderedHash{Map,Set}::sweepNurseryRanges to discard (selected) ranges allocated in the nursery.

Comment 8

[Steve Fink [:sfink] [:s:]]

Attachment: Bug 1787526 - Small IsWriteableAddress cleanup.

Comment 9

[Steve Fink [:sfink] [:s:]]

Attachment: Bug 1787526 - Plumb nursery param through *::sweepAfterMinorGC - CONSIDER NOT DOING THIS? It could be an accessor on the tracer or something.

Comment 10

[Steve Fink [:sfink] [:s:]]

Attachment: Bug 1787526 - Update checks for isAboutToBeFinalizedDuringMinorGC to accommodate preserved region.

Comment 11

[Steve Fink [:sfink] [:s:]]

Attachment: Bug 1787526 - Fix up *::sweepAfterMinorGC

Comment 12

[Jon Coppeard (:jonco)]

I spent a while looking into a partial nursery collection because I originally thought this would work out better than a semispace nursery.

This was based on the assumption that most live data would be at the end of the nursery. The idea was that we could collect everything up to some tail region, which we would skip until the next collection which would collect everything up to the new tail. This turned out not to be as good as I had thought because we would have to promote everything referenced from the tail region even if it was unreachable, unless we also traced through this region. Also, tracking the tail region within a nursery made up of discontiguous chunks proved complex and error prone.

Instead I made the decision to implement a standard semispace nursery.

Comment 13

[Jon Coppeard (:jonco)]

Attachment: Bug 1787526 - Part 1: Add configuration for semispace nursery r?sfink

Add a GC parameter and pref for semispace nursery which is disabled by default.

Enable it for the shell rootanalysis job to get get some test coverage.

Comment 14

[Jon Coppeard (:jonco)]

Attachment: Bug 1787526 - Part 2: Disconnect nursery capacity and max chunk count r?sfink

Store these separately because the number of chunks will be per semispace and
the capacity will be the total for both semispaces when semispace collection is
enabled.

Depends on D196431

Comment 15

[Jon Coppeard (:jonco)]

Attachment: Bug 1787526 - Part 3: Add a nursery space abstraction r?sfink

This moves a bunch of nursery data fields to a new Space struct. We will add a
second one of these to support semispace collection.

Depends on D196432

Comment 16

[Jon Coppeard (:jonco)]

Attachment: Bug 1787526 - Part 4: Add a from space r?sfink

Depends on D196433

Comment 17

[Jon Coppeard (:jonco)]

Attachment: Bug 1787526 - Part 5: Allocate chunks to both nursery spaces if semispace collection is enabled r?sfink

Depends on D196434

Comment 18

[Jon Coppeard (:jonco)]

Attachment: Bug 1787526 - Part 6: Switch between nursery spaces on collection r?sfink

This pushes a bunch of methods into the Space struct doesn't have much effect
beyond alterating between the semispaces.

Depends on D196435

Comment 19

[Jon Coppeard (:jonco)]

Attachment: Bug 1787526 - Part 7: Implement semispace nursery r?sfink

Implement semispace nursery by promoting some cells to a second nursery
generation. Since we've swapped semispaces and are evicting from-space to
to-sapce, promotion works like normal allocation. In the same way, swapping
store buffers makes post barriers also work like normal.

While tracing store buffers, we may need to re-add the store buffer entry. The
TenuringTrace has a flag that is set if any allocation is made into the next
generation which is used to check whether this is necessary.

Whether to tenure a particular nursery cell is based on its position in the
nursery. Cells that were promoted in the last collection are tenured.

Sometimes we tenure everything and totally empty the nursery. This is decided
based on the nursery collection reason. The GC uses the EVICT_NURSERY reason
to make sure everything is tenured during major GC.

Depends on D196438

Comment 20

[Jon Coppeard (:jonco)]

Attachment: Bug 1787526 - Part 8: Rename references to nursery collection 'tenuring' things to 'promotion' r?sfink

Since we won't always be tenuring things this renames use of the term
during nursery collection to 'promotion' instead.

I haven't yet renamed TenuringTracer. Naming suggestions welcome.

Depends on D196440

Comment 21

[Jon Coppeard (:jonco)]

Attachment: Bug 1787526 - Part 9: Move malloced buffer set into Nursery::Space r?sfink

Depends on D196441

Comment 22

[Jon Coppeard (:jonco)]

Attachment: Bug 1787526 - Part 10: Move wasm trailer block data into Nursery::Space r?sfink

Depends on D196442

Comment 23

[Jon Coppeard (:jonco)]

Attachment: Bug 1787526 - Part 11: Add ChunkKind to the chunk header and use it to optimise Nursery::inCollectedRegion r?sfink

Depends on D196443

Comment 24

[Jon Coppeard (:jonco)]

Attachment: Bug 1787526 - Part 12: Fix TypedArrayObject move hook now we can't assume the destination object is tenured r?sfink

Depends on D196444

Comment 25

[Jon Coppeard (:jonco)]

Attachment: Bug 1787526 - Part 13: Update JS Map/Set object handling of nursery objects r?sfink

Without a doubt this was the hardest part to get working.

Depends on D196445

Comment 26

[Jon Coppeard (:jonco)]

Attachment: Bug 1787526 - Part 14: Sweep nursery entries in NurseryAwareHashMap r?sfink

This sweeps the nursery entries in the NurseryAwareHashMap after minor GC
rather than just clearing them.

Depends on D196446

Comment 27

[Jon Coppeard (:jonco)]

Attachment: Bug 1787526 - Part 15: Make pretenuring work with a semispace nursery r?sfink

We now have to track numbers of objects promoted to the second generation so we
can check this against the number tenured.

This removes a small optimsation I made a while ago where we assume we an
allocation site is not in the linked list if it has a nursery allocation count of
zero. That no longrer holds because it can be in the list if the second
generation count is non-zero.

This changes some pretenuring parameters. I found this important at the time,
but I'm going to go back and check it's still necessary.

Depends on D196447

Comment 28

[Jon Coppeard (:jonco)]

Attachment: Bug 1787526 - Part 16: Fix handling of string to atom cache r?sfink

The atom cache is cleared after minor GC. Arguably we should sweep it instead
to leave strings that were promoted to the second nursery generation. I don't
know how important this is.

Depends on D196448

Comment 29

[Jon Coppeard (:jonco)]

Attachment: Bug 1787526 - Part 17: Fix sweeping array buffer inner view objects r?sfink

Depends on D196449

Comment 30

[Jon Coppeard (:jonco)]

Attachment: Bug 1787526 - Part 18: Browser integration for semispace nursery r?sfink

This filters CycleCollectedRuntime::mNurseryObjects so that only objects that
have died are finalized, and objects promoted to the second nursery generation
remain. Also a post barrier is added when updating a wrapper cache object.

I'll ask Andrew for review on this too when it gets closer to landing.

Depends on D196450

Comment 31

[Jon Coppeard (:jonco)]

Attachment: Bug 1787526 - Part 19: Fix freeing LIFO chunks for semispace nursery collection r?sfink

We use a second list so that blocks to be freed go for two nursery collections
before being freed, unless we're evicting the whole nursery.

I had to suffle things around at the start of GC so that we always start a
background free for the blocks queued by purgeRuntime().

Depends on D196451

Comment 32

[Jon Coppeard (:jonco)]

Attachment: Bug 1787526 - Part 20: Disable semispace nursery collection in some tests where it causes a problem r?sfink

Depends on D196452

Comment 33

[Jon Coppeard (:jonco)]

Attachment: Bug 1787526 - Part 21: Add JS API test for semispace nursery collection r?sfink

Also disable semispace nusery collection in a few JS API tests where it causes them to fail.

Depends on D196453

Comment 34

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/754b631234b1
Part 1: Add configuration for semispace nursery r=sfink
https://hg.mozilla.org/integration/autoland/rev/916e8b1c1f1f
Part 2: Disconnect nursery capacity and max chunk count r=sfink
https://hg.mozilla.org/integration/autoland/rev/ac216a48df3a
Part 3: Add a nursery space abstraction r=sfink
https://hg.mozilla.org/integration/autoland/rev/3bb94de73aeb
Part 4: Add a from space r=sfink
https://hg.mozilla.org/integration/autoland/rev/6f5d85e44b88
Part 5: Allocate chunks to both nursery spaces if semispace collection is enabled r=sfink
https://hg.mozilla.org/integration/autoland/rev/859314ce1f64
Part 6: Switch between nursery spaces on collection r=sfink
https://hg.mozilla.org/integration/autoland/rev/855def3146ee
Part 7: Implement semispace nursery r=sfink
https://hg.mozilla.org/integration/autoland/rev/784ef6379bd0
Part 8: Rename references to nursery collection 'tenuring' things to 'promotion' r=sfink
https://hg.mozilla.org/integration/autoland/rev/81f2b46b7289
Part 9: Move malloced buffer set into Nursery::Space r=sfink
https://hg.mozilla.org/integration/autoland/rev/ea0235576102
Part 10: Move wasm trailer block data into Nursery::Space r=sfink
https://hg.mozilla.org/integration/autoland/rev/921ab63cd1eb
Part 11: Add ChunkKind to the chunk header and use it to optimise Nursery::inCollectedRegion r=sfink
https://hg.mozilla.org/integration/autoland/rev/6e7744490c9f
Part 12: Fix TypedArrayObject move hook now we can't assume the destination object is tenured r=sfink
https://hg.mozilla.org/integration/autoland/rev/55650abb3bd2
Part 13: Update JS Map/Set object handling of nursery objects r=sfink
https://hg.mozilla.org/integration/autoland/rev/21128c6c3b71
Part 14: Sweep nursery entries in NurseryAwareHashMap r=sfink
https://hg.mozilla.org/integration/autoland/rev/96c952c35012
Part 15: Make pretenuring work with a semispace nursery r=sfink
https://hg.mozilla.org/integration/autoland/rev/912a737ad481
Part 16: Fix handling of string to atom cache r=sfink
https://hg.mozilla.org/integration/autoland/rev/e6b185287249
Part 17: Fix sweeping array buffer inner view objects r=sfink
https://hg.mozilla.org/integration/autoland/rev/88b7d0819d7a
Part 18: Browser integration for semispace nursery r=sfink
https://hg.mozilla.org/integration/autoland/rev/5e4831d9d689
Part 19: Fix freeing LIFO chunks for semispace nursery collection r=sfink
https://hg.mozilla.org/integration/autoland/rev/c2a7dd9ea790
Part 20: Disable semispace nursery collection in some tests where it causes a problem r=sfink
https://hg.mozilla.org/integration/autoland/rev/59783e60bc55
Part 21: Add JS API test for semispace nursery collection r=sfink
https://hg.mozilla.org/integration/autoland/rev/c02cbc98a3b0
apply code formatting via Lando

Comment 35

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/754b631234b1
https://hg.mozilla.org/mozilla-central/rev/916e8b1c1f1f
https://hg.mozilla.org/mozilla-central/rev/ac216a48df3a
https://hg.mozilla.org/mozilla-central/rev/3bb94de73aeb
https://hg.mozilla.org/mozilla-central/rev/6f5d85e44b88
https://hg.mozilla.org/mozilla-central/rev/859314ce1f64
https://hg.mozilla.org/mozilla-central/rev/855def3146ee
https://hg.mozilla.org/mozilla-central/rev/784ef6379bd0
https://hg.mozilla.org/mozilla-central/rev/81f2b46b7289
https://hg.mozilla.org/mozilla-central/rev/ea0235576102
https://hg.mozilla.org/mozilla-central/rev/921ab63cd1eb
https://hg.mozilla.org/mozilla-central/rev/6e7744490c9f
https://hg.mozilla.org/mozilla-central/rev/55650abb3bd2
https://hg.mozilla.org/mozilla-central/rev/21128c6c3b71
https://hg.mozilla.org/mozilla-central/rev/96c952c35012
https://hg.mozilla.org/mozilla-central/rev/912a737ad481
https://hg.mozilla.org/mozilla-central/rev/e6b185287249
https://hg.mozilla.org/mozilla-central/rev/88b7d0819d7a
https://hg.mozilla.org/mozilla-central/rev/5e4831d9d689
https://hg.mozilla.org/mozilla-central/rev/c2a7dd9ea790
https://hg.mozilla.org/mozilla-central/rev/59783e60bc55
https://hg.mozilla.org/mozilla-central/rev/c02cbc98a3b0