Last Comment Bug 740212 - IonMonkey: Inline allocation for LCreateThis.
: IonMonkey: Inline allocation for LCreateThis.
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: general
:
: Jason Orendorff [:jorendorff]
Mentors:
Depends on: 732852
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-28 16:50 PDT by Sean Stangl [:sstangl]
Modified: 2012-04-02 16:39 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
x86-only patch. (9.71 KB, patch)
2012-03-28 16:50 PDT, Sean Stangl [:sstangl]
no flags Details | Diff | Splinter Review
Inline allocation. (18.95 KB, patch)
2012-03-29 15:58 PDT, Sean Stangl [:sstangl]
no flags Details | Diff | Splinter Review
Inline allocation, v2. (21.81 KB, patch)
2012-04-02 15:19 PDT, Sean Stangl [:sstangl]
dvander: review+
Details | Diff | Splinter Review

Description Sean Stangl [:sstangl] 2012-03-28 16:50:17 PDT
The attached patch translates JM's getNewObject() allocation code to IonMonkey. SS-1.0's access-binary-trees executes in ~3ms instead of ~6ms, but -- apart from DeltaBlue, which sees an improvement from 495 -> 483ms -- all other tests appear to be unaffected by inline allocation, or appear to very slightly regress.

This is a bit of a mystery.
Comment 1 Sean Stangl [:sstangl] 2012-03-28 16:50:52 PDT
Created attachment 610367 [details] [diff] [review]
x86-only patch.
Comment 2 Sean Stangl [:sstangl] 2012-03-29 15:58:16 PDT
Created attachment 610737 [details] [diff] [review]
Inline allocation.

'perf stat' shows this patch eliminating ~100,000,000 instructions from Earley-Boyer execution, but the suites are so large that allocation from Ion context appears to be but a small component of the total runtime. Further measurements show small improvements in all benchmarks.
Comment 3 David Anderson [:dvander] 2012-03-30 14:58:46 PDT
Comment on attachment 610737 [details] [diff] [review]
Inline allocation.

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

::: js/src/ion/CodeGenerator.cpp
@@ +907,5 @@
> +    pushArg(protoReg);
> +    pushArg(calleeReg);
> +
> +    if (!callVM(CreateThisInfo, lir))
> +        return false;

Looks good, but if we have a template object, this should be in the out-of-line path instead (you can just bind the inline failure to ool->entry()). We don't want to make LCreateThis as a call either, since we expect to take the fast path.

::: js/src/ion/IonMacroAssembler.cpp
@@ +381,5 @@
> +        storePtr(ImmWord(emptyObjectElements), Address(result, JSObject::offsetOfElements()));
> +    }
> +
> +    storePtr(ImmWord(templateObject->lastProperty()), Address(result, JSObject::offsetOfShape()));
> +    storePtr(ImmWord(templateObject->type()), Address(result, JSObject::offsetOfType()));

Both of these stores should use ImmGCPtr instead.
Comment 4 Sean Stangl [:sstangl] 2012-04-02 15:19:37 PDT
Created attachment 611614 [details] [diff] [review]
Inline allocation, v2.

Implements above changes. Unmarking as a call and moving to OOL appears to have made v8-deltablue run slightly more slowly (~15ms out of 485), but 'perf stat' shows a strict reduction in cycles and branches.
Comment 5 Sean Stangl [:sstangl] 2012-04-02 16:39:12 PDT
https://hg.mozilla.org/projects/ionmonkey/rev/6967695492f3

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