579527 - JM: fast path for ValueToBoolean(int)

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[David Mandelin [:dmandelin]]

bhackett observed that we spend 29 ms in ValueToBoolean on SunSpider, and suggested fast-pathing it. I measure 97% of calls to ValueToBoolean use an int argument, so an int fast path should give a nice speedup.

JM wanted SS win: 25 ms

Comment 1

[Sean Stangl [:sstangl]]

Bug 578528 has a patch pending review that eliminates ValueToBoolean calls in the context of JSOP_STRICTEQ, which is marginally related.

Note that ValueToBoolean() has many callsites, each of which will require a separate fastpath. It is also called from stub functions (but we could potentially guard on an int and do a fast conversion before entering the stub call..).

Comment 2

[Brian Hackett [Laid off!]]

One issue with the 29ms measurement for ValueToBoolean is that it incorporates a 10ns overhead to save/restore the state and make the call (with 1.7m calls to ValueToBoolean, this works out to 17ns per call).  Because the state is synced before each branch point, there is usually nothing to save/restore at calls to ValueToBoolean so the 10ns estimate is probably way too high.

Comment 3

[David Mandelin [:dmandelin]]

(In reply to comment #1)
> Bug 578528 has a patch pending review that eliminates ValueToBoolean calls in
> the context of JSOP_STRICTEQ, which is marginally related.
> 
> Note that ValueToBoolean() has many callsites, each of which will require a
> separate fastpath. 

Can't we write a new codegen routine that does ValueToBoolean (probably with fast-pathing optional/only, in case type inference tells us something), and then just use that everywhere we now generate a call to ValueToBoolean? Or did you just mean many callsites in the generated code?

> It is also called from stub functions (but we could potentially guard on an 
> int and do a fast conversion before entering the stub call..).

I am less worried about this. I'm sure MSVC will inline calls that it can see if it helps. Someone might want to look into partial inlining to see if it helps with gcc.

Comment 4

[David Mandelin [:dmandelin]]

(In reply to comment #2)
> One issue with the 29ms measurement for ValueToBoolean is that it incorporates
> a 10ns overhead to save/restore the state and make the call (with 1.7m calls to
> ValueToBoolean, this works out to 17ns per call).  Because the state is synced
> before each branch point, there is usually nothing to save/restore at calls to
> ValueToBoolean so the 10ns estimate is probably way too high.

Thanks for pointing this out. I just ran a script that does 10M ValueToBooleans. It runs in 136 ms, and according to the profiler, 40% of the time is in ValueToBoolean. I measured 1M calls in SunSpider, so that would give an estimated win of up to 5 ms. Still not too bad, though.

Updated JM wanted SS win: 5 ms

Comment 5

[Sean Stangl [:sstangl]]

The callsite giving overhead is probably mjit::Compiler::booleanJumpScript().

Comment 6

[Jan de Mooij]

Attachment: Quick INT32 hack

Attached is a patch to optimize booleanJumpScript if the type is known to be JSVAL_TYPE_INT32. On my machine, this is at least a 5 ms. win on bitops-bits-in-byte. Also a tiny win on nsieve-bits, but that may be noise. It passes trace-tests too.

For the !isTypeKnown case, we should probably generate an INT32 check. The jumps in booleanJumpScript are a bit confusing, so I will leave this to someone else.

Anyway, the attached patch is a bit hackish, but gives a nice speedup and is very simple, so please consider using it for JM.

Comment 7

[David Anderson [:dvander] - inactive, e-mail if emergency]

Comment on attachment 460528 [details] [diff] [review]
Quick INT32 hack

Awesome, thanks! Pushed with some style nits:

http://hg.mozilla.org/users/danderson_mozilla.com/moo/rev/22d50a018124

On the graphs this was a 7ms win overall, 5ms in bits-in-byte, 1ms in format-xparb, 1ms in nsieve-bits.

Comment 8

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Jan: your patch looks to be the one that pulled JM ahead of TM conclusively on SunSpider! Thanks, and congrats. :-)

Comment 9

[Jan de Mooij [:jandem]]

@dvander: you don't think it's worthwhile to generate code to check for int values at runtime? Or would that result in asm bloat?

@Mike Shaver: thanks, that's awesome indeed :)