Closed Bug 771400 Opened 12 years ago Closed 12 years ago

IonMonkey: Separate FreeList acquisition from getNewObject().

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: sstangl, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

getNewObject() is infallible except for freelist slot acquisition: if no slot exists, we call js_CreateThisForFunctionWithProto().

Instead, we could call a VM function that merely instantiates a new FreeList for our AllocKind, possibly causing a GC. Then object creation could reuse the inline path, which is significantly faster.

Likely only a small perf bump due to the relative rarity of the failure case.
Attached patch patchSplinter Review
Always perform initialization on the fast path. A microbenchmark of repeated calls to |new| shows a modest 3% perf improvement.
Attachment #643617 - Flags: review?(nicolas.b.pierron)
Comment on attachment 643617 [details] [diff] [review]
patch

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

Apply nit about the oolCallVM and remove the OutOfLineNewGCThing class.
And r+.

::: js/src/ion/CodeGenerator.cpp
@@ +1459,5 @@
> +    JS_ASSERT(!lir->isCall());
> +    saveLive(lir);
> +
> +    pushArg(ImmWord(thingSize));
> +    pushArg(ImmWord((int)allocKind));

Use Imm32 and not ImmWord, because ImmWord are marked as relocatable except that these values are not pointers.

@@ +1512,2 @@
>  
> +    OutOfLineNewGCThing *ool = new OutOfLineNewGCThing(lir, allocKind, objReg);

Use oolCallVM, you will love it:

    typedef JSObject *(*pf)(JSContext *cx, gc::AllocKind allocKind, size_t thingSize);
    static const VMFunction NewGCThingInfo = FunctionInfo<pf>(js::ion::NewGCThing);

    size_t thingSize = gc::Arena::thingSize(allocKind);

    OutOfLineCode *ool = oolCallVM(NewGCThingInfo, lir,
                                  (ArgList(), Imm32((int)allocKind), Imm32(thingSize)),
                                  StoreRegisterTo(objReg));

::: js/src/ion/IonMacroAssembler.cpp
@@ +419,4 @@
>  
> +        int elementsOffset = JSObject::offsetOfFixedElements();
> +
> +        addPtr(Imm32(elementsOffset), obj);

This is not that important, but this is the kind of case where compiler and I usually prefer « lea ».

@@ +419,5 @@
>  
> +        int elementsOffset = JSObject::offsetOfFixedElements();
> +
> +        addPtr(Imm32(elementsOffset), obj);
> +        storePtr(obj, Address(obj, -elementsOffset + JSObject::offsetOfElements()));

oh, that hurts!
Attachment #643617 - Flags: review?(nicolas.b.pierron) → review+
http://hg.mozilla.org/projects/ionmonkey/rev/8b0a7122b1aa
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.