Shark data for SunSpider revealed that 1.7% of execution time under JaegerMonkey was spent within js_ValueToBoolean(). I therefore counted the # of occurrences of types passed into stubs::ValueToBoolean(), with the following results: int32 936 493 boolean 741 477 string 23 323 object 4 312 null/undef 3 541 double 1 Similarly, I also counted the same data for js_ValueToBoolean() when using only the interpreter: int32 936 493 string 23 331 object 5 205 null/undef 3 099 double 1 boolean 0 Based on the above frequencies, I reordered the type checks within js_ValueToBoolean() for a cheap win (~3ms locally on SunSpider; no test slowed), and inserted a fast-path for boolean for JSOP_ADD and JSOP_OR, which were responsible for the majority of calls to ValueToBoolean(). For the v8 benchmark, stubs::ValueToBoolean() is called with the following type frequencies in the context of JSOP_ADD and JSOP_OR: boolean 3 197 366 int32 16 766 null/undef 8 385 object 8 string 0 double 0 The fast-path for boolean eliminates all 3.2 million calls and much of the syncing work, for the cost of two branches in the fast path and three in the ool path. In most tests, this gets faster; in raytracer, the OOL path for conversion is more frequently taken, resulting in ~30ms slowdown locally. In total with the patch, v8 becomes ~100ms faster: about 2%.
Created attachment 454943 [details] [diff] [review] Reorder js_ValueToBoolean() checking order for a small win.
Attachment #454943 - Flags: review?(dvander)
Created attachment 454945 [details] [diff] [review] Fast-path boolean for JSOP_AND, JSOP_OR.
Attachment #454945 - Flags: review?(dvander)
Depending on FrameState changes to eliminate frame.forgetEverything() call, which should give a much nicer perf score.
Depends on: 574930
Attachment #454943 - Flags: review?(dvander) → review+
http://hg.mozilla.org/users/danderson_mozilla.com/moo/rev/568850e1f3a1 Pushed the patch that was actually r+'d. That explains the surprise when I pushed the other patch! This bug is left open since everything can be dramatically improved if forgetEverything() goes away.
Attachment #454945 - Flags: review?(dvander) → review+
> This bug is left open since everything can be dramatically improved if > forgetEverything() goes away. I'm gonna close this out to keep things simple. Feel free to file a followup bug on forgetEverything if it's separate from planned register allocation work.
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Just a ping... did forgetEverything() go away?
It did not -- see http://hg.mozilla.org/users/danderson_mozilla.com/moo/file/e0988eae6c08/js/src/methodjit/FastOps.cpp#l749 . frame.forgetEverything() must currently be called on both sides of a JS jump, since the FrameState (tracking our stack and registers) in both locations must be equivalent. The easiest way to make them equivalent is to remove all known data and reload from memory. FrameState changes are tracked in bug 574930. Chris Leary and I did a bit of work on a possible solution to FrameState branching/merging, but it's heavily incomplete: https://hg.mozilla.org/users/sean.stangl_gmail.com/jitpaths/ .
You need to log in before you can comment on or make changes to this bug.