899827 - Allow nursery things to be self-hosting cloned

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Terrence Cole [:terrence]]

Attachment: allow_selfhosted_clone_of_nursery_objects-v0.diff

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.

Comment 1

[Terrence Cole [:terrence]]

Attachment: support_nursery_things_in_ObjectObjectMap-v0.diff

Comment 2

[Till Schneidereit [:till]]

Comment on attachment 783424 [details] [diff] [review]
allow_selfhosted_clone_of_nursery_objects-v0.diff

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

Makes sense.

Comment 3

[Jon Coppeard (:jonco)]

Comment on attachment 783426 [details] [diff] [review]
support_nursery_things_in_ObjectObjectMap-v0.diff

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

Yes, looks good!

Comment 4

[Terrence Cole [:terrence]]

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

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/097a08472a11
https://hg.mozilla.org/mozilla-central/rev/2e7771dcfd89