1306008 - Investigate adding pre write barrier to JS::Heap<T>

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect, P3)

Description

[Jon Coppeard (:jonco)]

Heap<T> currently has post barriers only as I think mostly these get marked as part of the root set.  However this is not always the case, e.g. in DOMProxyHandler::GetAndClearExpandoObject there is a large comment and call to ExposeValueToActiveJS which could be avoided if we had this.  Of course there may be other places where don't do this and need to.

Comment 1

[Jon Coppeard (:jonco)]

Bug 1294747 added the ExposeValueToActiveJS in the case above.

Comment 2

[Jon Coppeard (:jonco)]

Attachment: Bug 1306008 - Give JS::Heap<T> pointer wrappers a pre-barrier r=sfink?

This adds a pre write barrier to Heap<T> so that these can be uses as non-roots in the heap without breaking our snapshot at the beginning invariant if they are written to during an incremental GC. This makes it harder to misuse and allows us to take out manual barriers in at least one place.

Comment 3

[Jon Coppeard (:jonco)]

Attachment: Bug 1306008 - Replace ObjectPtr with JS::Heap<JSObject*> r=sfink?

Heap<JSObject*> is now equivalent to ObjectPtr so we can remove the latter.

Depends on D25083

Comment 4

[Jon Coppeard (:jonco)]

Attachment: Bug 1306008 - Use new to allocate NPObjectMemberPrivate r=mccr8?

Previously NPObjectMemberPrivate was allocated with malloc which bypassed Heap<T>'s constructors.

Depends on D25084

Comment 5

[Jon Coppeard (:jonco)]

Attachment: Bug 1306008 - Remove unnecessary expose call in DOMProxyHandler r=bzbarsky?

This call is not necessary now that Heap<T> has a pre-barrier which does this automatically.

Depends on D25085

Comment 6

[Jon Coppeard (:jonco)]

I'll reply to sfink's review comments on here for visibility.

Is there a complete source of documentation on Heap<T> vs HeapPtr<T> vs GCPtr<T> vs TenuredHeap<T> vs bare T*? I never feel like I have a complete picture. It seems like the relevant dimensions include

Prebarriered?
Postbarriered?
Read barriered?
Movable?
Deletable outside of a GC?

...and I'm not sure what else. Oh, and I think the last two are only relevant for post-barriered things, since moving and deleting are only problematic if there could be pointers to them in a store buffer?

Following these patches JS::Heap<T> should be equivalent to js::HeapPtr<T>. The former is for external use and the latter is slightly more efficient (more stuff is inlined). They both have pre- and post-barriers, and are movable and deletable outside of a GC.

JS::TenuredHeap<T> is a more efficient Heap<T> for when you know you don't have a nursery pointer (it omits the post-barrier).

js::GCPtr<T> is internal and more efficient again, at the cost of not being movable or deletable outside of a GC.

I seem to frequently get into situations where I'm just not sure what to use. For example, consider PropertyDescriptor. Why does it use a bare JSObject*?

Good question. It looks like this is just a value and works in the same way as our tagged pointers - it must be used in Rooted<> if it's on the stack or HeapPtr<> if it's on the heap (if that ever happens).

Comment 7

[Jon Coppeard (:jonco)]

(In reply to Jon Coppeard (:jonco) from comment #6)
Oh, I forgot that Heap<T> does gray unmarking / expose to active JS in the comment above. I guess I'll put this all in patch for bug 1539570.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2609de72b1d1
Give JS::Heap<T> pointer wrappers a pre-barrier r=sfink
https://hg.mozilla.org/integration/mozilla-inbound/rev/4faf873bf48f
Replace ObjectPtr with JS::Heap<JSObject*> r=sfink
https://hg.mozilla.org/integration/mozilla-inbound/rev/31b14d077054
Use new to allocate NPObjectMemberPrivate r=mccr8
https://hg.mozilla.org/integration/mozilla-inbound/rev/92c43505695c
Remove unnecessary expose call in DOMProxyHandler r=bzbarsky

Comment 9

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/2609de72b1d1
https://hg.mozilla.org/mozilla-central/rev/4faf873bf48f
https://hg.mozilla.org/mozilla-central/rev/31b14d077054
https://hg.mozilla.org/mozilla-central/rev/92c43505695c