Closed
Bug 1161077
Opened 9 years ago
Closed 9 years ago
Allocate unboxed arrays in the nursery
Categories
(Core :: JavaScript: GC, defect)
Core
JavaScript: GC
Tracking
()
RESOLVED
FIXED
mozilla40
Tracking | Status | |
---|---|---|
firefox40 | --- | fixed |
People
(Reporter: bhackett1024, Assigned: bhackett1024)
References
Details
Attachments
(2 files)
28.88 KB,
patch
|
terrence
:
review+
|
Details | Diff | Splinter Review |
41.77 KB,
patch
|
terrence
:
review+
|
Details | Diff | Splinter Review |
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).
Assignee | ||
Comment 1•9 years ago
|
||
This patch renames Nursery::hugeSlots and related bits to mallocedBuffers.
Assignee: nobody → bhackett1024
Attachment #8600946 -
Flags: review?(terrence)
Assignee | ||
Comment 2•9 years ago
|
||
The rest of the changes.
Attachment #8600947 -
Flags: review?(terrence)
Comment 3•9 years ago
|
||
> 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 4•9 years ago
|
||
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 5•9 years ago
|
||
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+
Comment 7•9 years ago
|
||
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: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Assignee | ||
Comment 9•9 years ago
|
||
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.
Description
•