Closed Bug 868610 Opened 7 years ago Closed 7 years ago

Don't use arena header when cloning object literals

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla23

People

(Reporter: terrence, Assigned: terrence)

References

Details

Attachments

(1 file, 2 obsolete files)

Attached patch v0 (obsolete) — Splinter Review
This is specialized enough that I just put it inline rather than splitting it out to a new function.
Attachment #745368 - Flags: review?(wmccloskey)
Attached patch v1: facepalm (obsolete) — Splinter Review
I forgot the runtime parameter in the assertion and then didn't bother compile testing such a trivial debug-only change.
Attachment #745368 - Attachment is obsolete: true
Attachment #745368 - Flags: review?(wmccloskey)
Attachment #745399 - Flags: review?(wmccloskey)
Comment on attachment 745399 [details] [diff] [review]
v1: facepalm

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

::: js/src/jsobj.cpp
@@ +1776,5 @@
>  {
>      Rooted<TypeObject*> typeObj(cx);
>      typeObj = cx->global()->getOrCreateObjectPrototype(cx)->getNewType(cx, &ObjectClass);
> +
> +    AllocKind kind = GetGCObjectFixedSlotsKind(srcObj->numFixedSlots());

I think you want GuessObjectGCKind. Otherwise this will fail if the literal has more than 16 slots.

@@ +1777,5 @@
>      Rooted<TypeObject*> typeObj(cx);
>      typeObj = cx->global()->getOrCreateObjectPrototype(cx)->getNewType(cx, &ObjectClass);
> +
> +    AllocKind kind = GetGCObjectFixedSlotsKind(srcObj->numFixedSlots());
> +    if (CanBeFinalizedInBackground(kind, srcObj->getClass()))

I think our literals are always of class js::ObjectClass. Can you assert that and then use the background kind unconditionally?

@@ +1779,5 @@
> +
> +    AllocKind kind = GetGCObjectFixedSlotsKind(srcObj->numFixedSlots());
> +    if (CanBeFinalizedInBackground(kind, srcObj->getClass()))
> +        kind = GetBackgroundAllocKind(kind);
> +    JS_ASSERT_IF(!IsInsideNursery(cx->runtime, srcObj), kind == srcObj->tenuredGetAllocKind());

It seems more natural to use srcObj->isTenured for the test here.
Attachment #745399 - Flags: review?(wmccloskey) → review+
Attachment #745399 - Attachment is obsolete: true
Attachment #747048 - Flags: review+
Whiteboard: [checkin-needed]
https://hg.mozilla.org/mozilla-central/rev/a15fae0ee76e
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in before you can comment on or make changes to this bug.