Closed
Bug 837359
Opened 11 years ago
Closed 11 years ago
BaselineCompiler: Add optimized stub for JSOP_THIS null/undef => "global this" case
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: djvj, Unassigned)
References
(Blocks 1 open bug)
Details
Attachments
(1 file, 1 obsolete file)
775 bytes,
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
when JSOP_THIS encounters a null or undefined, it computes the |this| object from the currenet global. The computed |this| object should be stable for that global. This means that when the incoming this is null or undefined, the resulting this object can be cached and returned via an optimized stub.
Reporter | ||
Comment 1•11 years ago
|
||
Attachment #709350 -
Flags: review?(jdemooij)
Comment 2•11 years ago
|
||
Is the difference measurable? JM (at least without TI) stubs JSOP_THIS, it would be nice if we could do the same..
Comment 3•11 years ago
|
||
(In reply to Jan de Mooij [:jandem] from comment #2) > Is the difference measurable? JM (at least without TI) stubs JSOP_THIS, it > would be nice if we could do the same.. To be clear, it calls a stub if the value is a primitive.
Reporter | ||
Comment 4•11 years ago
|
||
When I run with just baseline (disabling Ion), it seems to have about a 0.1/16 ms (or 1/160) difference in 3d-cube, but it's noisy so I can't say for sure. I saw it hitting about 5000 thousand times in 3d-cube before Ion got entered. The implementation was simple and would take about a half hour or so, so I went ahead and did it. To be honest I was expecting a bigger impact but apparently not. It might help 3d-cube a bit to have Ion do the same thing. It handles a particular call to |CreateP| in 3d-cube which is called in non-constructing form, but CreateP uses this. The presence of |this| in CreateP without a definitely-typed object for thisval makes ion not compile CreateP, which means the engine ends up doing an Ion=>JM invoke for all the calls to CreateP from within Ion. Eliminating this and allowing CreateP to be inlined into the caller would probably be a definite win. However, given that we don't expect it to be too common for callers to call functions in non-constructing form if the function uses |this|, it's probably not a huge priority.
Comment 5•11 years ago
|
||
Comment on attachment 709350 [details] [diff] [review] Patch Review of attachment 709350 [details] [diff] [review]: ----------------------------------------------------------------- I think we should replace the IC with a direct VM call if |this| is not an object. The slowest part of the IC is probably recovering the script + pc, if we use a VM call we can just pass these to the stub. (The arguments checks should give us type information about |this|)
Attachment #709350 -
Flags: review?(jdemooij)
Reporter | ||
Comment 6•11 years ago
|
||
Well, the fallback stub doesn't strictly need to get the script or pc to call BoxNonStrictThis. If we're not doing any other optimizations, we can just keep the fallback stub as is and remove the script/pc retrieval from it.
Reporter | ||
Comment 7•11 years ago
|
||
Attachment #709350 -
Attachment is obsolete: true
Attachment #709758 -
Flags: review?(jdemooij)
Comment 8•11 years ago
|
||
Comment on attachment 709758 [details] [diff] [review] Remove unnecessary script/pc access from JSOP_THIS fallback stub. Review of attachment 709758 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/ion/BaselineIC.cpp @@ +1066,5 @@ > > static bool > DoThisFallback(JSContext *cx, ICThis_Fallback *stub, HandleValue thisv, MutableHandleValue ret) > { > + FallbackICSpew(cx, stub, "NewArray"); Nit: "This"
Attachment #709758 -
Flags: review?(jdemooij) → review+
Reporter | ||
Comment 9•11 years ago
|
||
https://hg.mozilla.org/projects/ionmonkey/rev/4b013896bfd5
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•