Closed Bug 899827 Opened 7 years ago Closed 7 years ago

Allow nursery things to be self-hosting cloned

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla25

People

(Reporter: terrence, Assigned: terrence)

References

Details

Attachments

(2 files)

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)
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 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+
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.
Duplicate of this bug: 900395
https://hg.mozilla.org/mozilla-central/rev/097a08472a11
https://hg.mozilla.org/mozilla-central/rev/2e7771dcfd89
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.