Closed Bug 1161077 Opened 5 years ago Closed 5 years ago

Allocate unboxed arrays in the nursery

Categories

(Core :: JavaScript: GC, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox40 --- fixed

People

(Reporter: bhackett, Assigned: bhackett)

References

Details

Attachments

(2 files)

Unboxed arrays have finalizers, which prevents them from being allocated in the nursery.  There is a JSCLASS_FINALIZE_FROM_NURSERY flag which would allow this, but all the unboxed arrays in the nursery would be accumulated in a list and would still have their finalizer called when the nursery is swept, which is pretty inefficient.  It would be better if there was a way in which objects could have a finalizer but not require that finalizer to run when they are nursery allocated.

The upcoming patches add a mechanism for this.  The methods used for allocating slots/elements arrays for native objects are generalized so that other objects can allocate other types of buffers.  The same ownership rules apply to these new buffers:

- If an object is in the tenured heap, the buffers are malloc'ed and the object owns them and is responsible for destroying them during finalization.

- If an object is in the nursery, the buffers are either also in the nursery or they are malloc'ed and the nursery manages them, freeing or handing them off as necessary at the next nursery collection.

JSCLASS_FINALIZE_FROM_NURSERY is removed (I think it used to be used in the browser, but no longer is) and replaced with a JSCLASS_SKIP_NURSERY_FINALIZE flag.  Objects with this flag can have finalizers, but those finalizers can't do anything except for free buffers associated that were allocated with a new AllocateObjectBuffer/ReallocateObjectBuffer API (which is also used by native objects).
Attached patch rename hugeSlotsSplinter Review
This patch renames Nursery::hugeSlots and related bits to mallocedBuffers.
Assignee: nobody → bhackett1024
Attachment #8600946 - Flags: review?(terrence)
Attached patch patchSplinter Review
The rest of the changes.
Attachment #8600947 - Flags: review?(terrence)
> JSCLASS_FINALIZE_FROM_NURSERY is removed (I think it used to be used in the browser, but no longer is) 

Patches to use that flag in the browser are in bug 1120016, which hasn't landed.
Comment on attachment 8600946 [details] [diff] [review]
rename hugeSlots

Review of attachment 8600946 [details] [diff] [review]:
-----------------------------------------------------------------

Great! I've been meaning to write exactly this patch for ages.
Attachment #8600946 - Flags: review?(terrence) → review+
Comment on attachment 8600947 [details] [diff] [review]
patch

Review of attachment 8600947 [details] [diff] [review]:
-----------------------------------------------------------------

(In reply to Andrew McCreight (awayish through 5/5) [:mccr8] from comment #3)
> > JSCLASS_FINALIZE_FROM_NURSERY is removed (I think it used to be used in the browser, but no longer is) 
> 
> Patches to use that flag in the browser are in bug 1120016, which hasn't
> landed.

And unfortunately won't, at least in that form. As Brian discovered, the finalization mechanism is just too slow. Jon came up with a great way to do the cleanup we need using only barriers, so we have a path forward for allocating events in the nursery, although there are still other blockers there. Removing the finalization mechanism is something that needs to happen and I'm happy to do it here.
Attachment #8600947 - Flags: review?(terrence) → review+
Apparently this patch landed on mozilla-central but didn't got marked as such.
https://hg.mozilla.org/mozilla-central/rev/b2781b5c0f12
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
The second push above removes the JSAPI test that the first push somehow missed.
You need to log in before you can comment on or make changes to this bug.