1126467 - Implement a WeakHeap pointer type capable of handling weak references to nursery things

Status: NEW

(Core :: JavaScript: GC, enhancement)

Description

[Terrence Cole [:terrence]]

The eventual goal is to replace the ad-hoc weakness of WrapperCache pointers with something that is more structured. As an initial target, we want to make it possible for weak things to be allocated in the nursery (with a finalizer). Eventually, we'd like the full GC to handle these references automatically so that we don't even need a finalizer.

Comment 1

[Terrence Cole [:terrence]]

I think this needs to happen in roughly 3 parts:

First, add a new weak store buffer updated by the new WeakHeap type that causes the minor collection to automatically null out or update the reference in the minor GC. This is a bit weird because it gives us fully automatic weak behavior for minor GC, but we will still rely on the finalizer and a magic reserved slots during the major GC.

Second, add constructor/destructor calls to WeakHeap to register the weak root in a new container. In general this would be /way/ too much new storage since we need an entry for every wrappercache in the system. However, there are a couple saving graces here: (1) we no longer need the reserved slot for wrapper cached things, so we may actually gain space on 32bit, (2) we no longer need a finalizer for wrapper cache objects, so our foreground, non-incremental sweeping will be tremendously faster, (3) the HoldJSObject table can be the same table, so that goes away as well, and most importantly (4) the implementation from the browser side will be much simpler. Specifically, WeakHeap will be magic enough to get weak-pointer-with-optional-strongness behavior with no further effort from gecko: just declare as WeakHeap and let the GC handle the rest.

Third, wrapper cache entries no longer have a finalizer, so we get nursery allocation for free. This means that we no longer need to allow finalizable things in the nursery and can rip out all the supporting infra I just added. On the flip side, wrapper cache entries are nursery allocated by default now so we'll almost certainly need to add some new API to ensure that !isProbablyShortLived() class objects get allocated in Tenured. We'll want to profile here and ensure that support to move things around via webidl lands with the series so that we can fixup as needed.

This is a high-level overview and I'm sure there are some nasty landmines hidden in the details. However, my gut feeling is that the wrapper cache is an important enough workload to merit this sort of attention. In particular, moving to a fully automatic scheme like this and removing the spooky action at a distance between the pointer and the instance's finalizer will set us up nicely to fully incrementalize the sweeping of these edges.

Comment 2

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

(In reply to Terrence Cole [:terrence] from comment #1)
> Third, wrapper cache entries no longer have a finalizer, so we get nursery
> allocation for free. This means that we no longer need to allow finalizable
> things in the nursery and can rip out all the supporting infra I just added.

Though we might want to look into whether we have other finalized things cluttering up the tenured heap first, in case we want to use that supporting infra for something else.

Comment 3

[Terrence Cole [:terrence]]

Attachment: impl_weakheap_pointer-v0.diff

This is the first major piece.

Note that there is a bit of a downside with this patch as both the storebuffer and the finalizers are nulling the pointer. We still need the finalizer for the tenured heap, however, and I don't want to make the stronger assertion that anything we put into the nursery is a WrapperCache -- we should have the option of using the nursery for non-weak finalizable things too.

From a-typically long minorGC:
finalized 513 of 514 things
MinorGC:           CC_WAITING   2.7%    3    440      6      1      9      0      0      0     66      0     33      0      1     40      2      0     18     44      0     12    206      0      0

That translates to:
fixing up 514 weak list entries: 18us
running the 513 finalizers:      44us

Comment 4

[Terrence Cole [:terrence]]

Comment on attachment 8556792 [details] [diff] [review]
impl_weakheap_pointer-v0.diff

You know a new GC idea is working when there are intermittent crashes. \o/

It seems we can HoldJSObject on an event and I need to figure out what the best way to remap those entries is when they move.

Comment 5

[Andrew McCreight [:mccr8]]

HoldJSObject is called on the C++ object, not the JS object, so which entries are you talking about?

Comment 6

[Andrew McCreight [:mccr8]]

Oh, I see, you are tracking these objects from the JS object somehow?  I don't see where that is happening in the patch, at least just looking at the browser part of it.

Comment 7

[Terrence Cole [:terrence]]

(In reply to Andrew McCreight [:mccr8] from comment #6)
> Oh, I see, you are tracking these objects from the JS object somehow?  I
> don't see where that is happening in the patch, at least just looking at the
> browser part of it.

This happens automatically in barriers in WeakHeap.

(In reply to Andrew McCreight [:mccr8] from comment #5)
> HoldJSObject is called on the C++ object, not the JS object, so which
> entries are you talking about?

Yes, but this eventually does call PreserveWrapper. Somehow I need to let the barrier mechanism know when an edge has been made strong so that we can mark instead of discarding. The long-term plan obviously has this capability, so I think the right approach is to just do it and skip the intermediate states. I'm not surprised, given the amount of complexity here, and making the intermediate states performance neutral would have been a pain anyway.