Closed
Bug 899827
Opened 11 years ago
Closed 11 years ago
Allow nursery things to be self-hosting cloned
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla25
People
(Reporter: terrence, Assigned: terrence)
References
Details
Attachments
(2 files)
(deleted),
patch
|
till
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
jonco
:
review+
|
Details | Diff | Splinter Review |
The Intl API adds many sub-objects to the interface it exposes. Since these are created by a script with no special handling, e.g. Object.create, we allocate many, if not all of these into the Nursery. When the intl jsapi-tests access the Intl object, the tree of objects containing nursery things gets cloned. The self-hosting clone method CloneObject currently relies on Cell::tenuredGetAllocKind to get the alloc kind for the new objects and fails on non-tenured objects. This currently makes any build with both --enable-gcgenerational and --enable-intl-api crash immediately.
However, CloneObject does not actually use the thing size when copying properties: it uses getProperty/setProperty on each property. It would works if we just made the thing size equal to 2 (to allow JSFunction) in all cases. A more satisfying approach is what I have attached: use the "right" thing kind for tenured objects and use something reasonable for nursery objects.
Additionally, when we later hit a GC in the middle of Clone, some of these objects are in the AutoObjectObjectHashMap clone uses. This map does not yes support moving, so we hit the guarding assertion. The second patch in the series fixes this marker.
Attachment #783424 -
Flags: review?(till)
Attachment #783424 -
Flags: feedback?(wmccloskey)
Assignee | ||
Comment 1•11 years ago
|
||
Attachment #783426 -
Flags: review?(jcoppeard)
Comment 2•11 years ago
|
||
Comment on attachment 783424 [details] [diff] [review]
allow_selfhosted_clone_of_nursery_objects-v0.diff
Review of attachment 783424 [details] [diff] [review]:
-----------------------------------------------------------------
Makes sense.
Attachment #783424 -
Flags: review?(till) → review+
Comment 3•11 years ago
|
||
Comment on attachment 783426 [details] [diff] [review]
support_nursery_things_in_ObjectObjectMap-v0.diff
Review of attachment 783426 [details] [diff] [review]:
-----------------------------------------------------------------
Yes, looks good!
Attachment #783426 -
Flags: review?(jcoppeard) → review+
Assignee | ||
Comment 4•11 years ago
|
||
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/097a08472a11
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/2e7771dcfd89
Bill, I went ahead and pushed because this is making TBPL orange on the first jsapi test and we're not getting any real feedback right now. If you see a better way to do this, I'll happily push a follow-up.
Comment 6•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/097a08472a11
https://hg.mozilla.org/mozilla-central/rev/2e7771dcfd89
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
Attachment #783424 -
Flags: feedback?(wmccloskey)
You need to log in
before you can comment on or make changes to this bug.
Description
•