Closed
Bug 740212
Opened 13 years ago
Closed 13 years ago
IonMonkey: Inline allocation for LCreateThis.
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
People
(Reporter: sstangl, Unassigned)
References
Details
Attachments
(1 file, 2 obsolete files)
21.81 KB,
patch
|
dvander
:
review+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Comment 1•13 years ago
|
||
Reporter | ||
Comment 2•13 years ago
|
||
'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.
Attachment #610367 -
Attachment is obsolete: true
Attachment #610737 -
Flags: review?(dvander)
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.
Attachment #610737 -
Flags: review?(dvander)
Reporter | ||
Comment 4•13 years ago
|
||
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.
Attachment #610737 -
Attachment is obsolete: true
Attachment #611614 -
Flags: review?(dvander)
Updated•13 years ago
|
Attachment #611614 -
Flags: review?(dvander) → review+
Reporter | ||
Comment 5•13 years ago
|
||
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•