We could compute |this| eagerly iff the front-end determines that |this| is used in the method body. For JM we'd flag a bit on the script so that the methodjit compiler emits an eager |this| computation on activation and stores the value in the frame, as in bug 556277, eliding the check emitted by mjit::Compiler::jsop_this() and permitting us to inline further references to the |this| object. I'm not sure whether this front-end analysis will avoid the issue with XPC_WN_JSOp_ThisObject in bug 556277, but we should think about how to get around that because the benchmark wins are tantalizing ( https://bugzilla.mozilla.org/show_bug.cgi?id=556277#c1 ).

Found XPC_WN_JSOp_ThisObject in the code base and I think I understand the issue, and why it doesn't apply. In bug 556227 we had to make it interpreter invariant that |this| was computed for the current frame, so calls to C++ code were also having their thisOp invoked. Contrastingly, in the JM scheme we can eagerly compute |this| in a prologue for a single method which is understood to contain multiple uses of |this| (therefore likely to pay off). In the proposal I made above, we're not changing the calling convention to say that |this| will always be available -- theoretically CSEing the computation of |this| and hoisting it to the top of the method so you know it's available in the body. I'm still reading through http://hg.mozilla.org/tracemonkey/rev/52be13ea0488 pondering what the positive implications are for doing the calculation from the caller instead of the callee.

WIP patch. As Jason pointed out, keeping the eager this invariant makes everything simpler. Now need to update jsop_callname in mjit::Compiler via NameOp to also maintain the same invariant. Jason also landed a patch on TM recently to cache the JSObjects created for global classes -- I think he said that would avoid the window object creation overhead problem from the referenced bug.

No win on my desktop for either sunspider or v8, but the number of JSOP_THIS slowcalls dropped to zero. Now hoisting computation into header without front-end analysis to see what that does.

When I said no win, I meant fairly epic win. 1.1s on v8 on my macbook -- was accidentally running with "-j -m", and something weird was happening where the tracer was activating. I guess we can conclude we eliminated ~100 cycles of overhead for each of the 33e6 slow calls. I figure computed-this was an unpredictable branch and that there's spill/fill overhead and loss of I$ locality for the slow call -- not sure if that quite adds up. I kind of want to count the number of |this| computations and see if they were reduced in number somehow that I'm not thinking of. In any case, I'm off for a victory burrito. Outstanding work: - make check shows embedder API is getting a null function value in js_Construct hook invocation. - tracer support for the new invariant needs to be ported.

Comment on attachment 454755 [details] [diff] [review] Eager-this ported to js::Value API with JM support. Awesome. > /* Primitive |this| should not be passed to slow natives. */ >- JSObject *thisp = &fp->thisv.asObject(); >+ JSObject *thisp = fun ? fp->getThisObject(cx) : NULL; Just a note that we need to check this with Jason for the jsapi-test failure. > if OPTIONS.show_cmd: >- print(cmd) >+ print(subprocess.list2cmdline(cmd)) Remember not to check this in - separate bug.

http://hg.mozilla.org/users/danderson_mozilla.com/moo/rev/e30feb436f0e From callgrind it appears that we really were saving about a hundred instructions per slowcall, which microbenchmarks support. Unhelpfully, js::ComputeThisFromArgv wasn't getting inlined into the slow call body, so that's a nice thing to force inline in the follow up patch. In the new scheme this is what happens, step by step, for the interested: function Duck() { this.hitCount = 0; } Duck.prototype.punch() { this.hitCount++; } (function() { var d = new Duck(); for (var i = 0; i < 33e6; i++) d.punch(); })(); - First setting of this: ExecuteScript => Execute => pushes the primordial frame. |this| is the result of calling the thisObject hook on the scope chain object. The scope chain object and the result are the global passed in from the shell. - Second setting of |this| is a NullTag() given during the anonymous function's invocation. This indicates that |this| should be resolved on-demand by the callee as a global. - Third setting of |this| is under the |new| slowcall, which generates a new object to be used as |this| and clobbers the null value that the bytecode stream places on the stack. This is our boxing duck. - Fourth setting of |this| is under the slowcall. The bytecode sequence looks thusly: [jaeger] JSOps 0 00021: 10 getlocal 0 [jaeger] JSOps 1 00024: 10 callprop "punch" [jaeger] JSOps 2 00027: 10 call 0 And that call (27) sets the newly pushed frame's JSStackFrame::thisv to to the getlocal result, which callprop guarantees isObject as a postcondition. - All subsequent calls to |punch| perform the same getlocal-result-to-pushed-frame-thisv movement under a fastcall. Next up: a patch ensuring that |this| is computed in the prologue of methods that the front-end determines to require it. This approach turns JSOP_THIS into a single move instruction -- there is no longer a slow call guard on the NullTag() value, because thisv is never null in a methods that needs it.

Hoist computation of eager this in the prologue by flagging JSOP_THIS usage in the parser.

Comment on attachment 455054 [details] [diff] [review] Eager this computation in prologue. No perf change on my machine. Maybe we should investigate eliminating the type load. In jsop_this(), if !script->strictModeCode, I *think* we should be able to predict thisv as always being JSVAL_TAG_OBJECT.

Stolen from jorendorff's patch in the referenced bug.

(In reply to comment #5) > > /* Primitive |this| should not be passed to slow natives. */ > >- JSObject *thisp = &fp->thisv.asObject(); > >+ JSObject *thisp = fun ? fp->getThisObject(cx) : NULL; > > Just a note that we need to check this with Jason for the jsapi-test failure. ...What failure?

(In reply to comment #10) > ...What failure? There was some jsapi-test that was failing because of an initialization value of |this|... sorry I don't remember any more details than that, it seems to have disappeared. Maybe dvander remembers?

It was a trace test where .call (I think) had changed from a slow to a fast native, and this changeset disabled it asserting: http://hg.mozilla.org/users/danderson_mozilla.com/moo/rev/39d35f51ea51

Global frames should get the right non-null |this| eagerly, no? /be

Comment on attachment 462311 [details] [diff] [review] JM eager this tracer changes. This patch needs rebasing. Also, it should live in a separate bug. I'm going to move it to bug 587809, as it fixes that bug.

Okay, I'm going to mark this as fixed since we broke out the other bug for the tracer changes.