Closed Bug 784188 Opened 7 years ago Closed 7 years ago

IonMonkey: use BACKGROUND finalize kind template CallObject

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: luke, Assigned: luke)

Details

(Whiteboard: [ion:p1])

Attachments

(1 file)

CallObject is background-finalizable so IonBuilder::createCallObject needs a
  kind = gc::GetBackgroundAllocKind(kind)
for the template object.

This seems to be the cause of us using out-of-line paths for CallObject creation.  A preliminary fix improves our v8 score from ~1600 to ~1700 on my machine.

For one thing, it would be good to understand why exactly this is this mistake caused this behavior.  For another, we probably should audit the other places template objects are created.  Perhaps IonBuilder::createCallObject can just call CallObject::create?
Setting p1 since this is an awkward bug to have.
Whiteboard: [ion:p1]
Attached patch fixSplinter Review
This patch refactors CallObject creation a bit so that IonBuilder::createCallObject doesn't duplicate any of the shape/type/kind logic.

I also noticed that there is an unused 'global' parameter passed to NewCallObject so I removed that also.
Assignee: general → luke
Status: NEW → ASSIGNED
Attachment #656177 - Flags: review?(dvander)
I gave a quick scan for the other uses of AllocKind and the rest seem to go through more standard new-object paths that call GetBackgroundAllocKind as a matter of course.
Comment on attachment 656177 [details] [diff] [review]
fix

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

Nice, thanks for fixing this.

::: js/src/vm/ScopeObject.cpp
@@ +165,5 @@
> +        return NULL;
> +
> +    HeapSlot *slots;
> +    if (!PreallocateObjectDynamicSlots(cx, shape, &slots))
> +        return NULL;

existing bug, but need a cx->free_ or something here to avoid leaking |slots|
Attachment #656177 - Flags: review?(dvander) → review+
https://hg.mozilla.org/projects/ionmonkey/rev/630296b1c46d
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.