Last Comment Bug 771400 - IonMonkey: Separate FreeList acquisition from getNewObject().
: IonMonkey: Separate FreeList acquisition from getNewObject().
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: x86_64 Linux
: -- normal (vote)
: ---
Assigned To: general
:
:
Mentors:
Depends on:
Blocks: IonSpeed
  Show dependency treegraph
 
Reported: 2012-07-05 18:23 PDT by Sean Stangl [:sstangl]
Modified: 2012-07-19 12:39 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (17.94 KB, patch)
2012-07-18 15:15 PDT, Sean Stangl [:sstangl]
nicolas.b.pierron: review+
Details | Diff | Splinter Review

Description Sean Stangl [:sstangl] 2012-07-05 18:23:09 PDT
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.
Comment 1 Sean Stangl [:sstangl] 2012-07-18 15:15:25 PDT
Created attachment 643617 [details] [diff] [review]
patch

Always perform initialization on the fast path. A microbenchmark of repeated calls to |new| shows a modest 3% perf improvement.
Comment 2 Nicolas B. Pierron [:nbp] 2012-07-18 17:33:25 PDT
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!
Comment 3 Sean Stangl [:sstangl] 2012-07-19 12:39:16 PDT
http://hg.mozilla.org/projects/ionmonkey/rev/8b0a7122b1aa

Note You need to log in before you can comment on or make changes to this bug.