Closed Bug 619737 Opened 9 years ago Closed 9 years ago

TI: Kraken audio-fft much slower with type inference

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: jandem, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: fixed-in-jaegermonkey)

Attachments

(1 file)

Some numbers:

V8: 370 ms
JM+TM: 375 ms
JM: 619 ms
JM+TI: 2870 ms

Most of the time here is spent inside FFT.prototype.forward. If I change the line "i = bufferSize/2" to "i = (bufferSize/2)|0" I get these numbers:

V8: 370 ms
JM+TM: 372 ms
JM: 607 ms
JM+TI: 324 ms

I think the problem here is that bufferSize/2 causes "i" to be marked as int32-or-double, and this makes i slower in the rest of the function. But bufferSize has value 512 so the result should fit in an int32.
Great catch.  There is this horrible glue-and-tape analysis to guess whether the result of int/int is an int or double, and it's pretty often wrong.  We just need to rip that out, always mark int/int as an int (unless maybe it's immediately passed to Math.floor or similar), and accept a few more recompilations.  Once we are interpreting some before compilation (probably post-Fx5), we'll be able to record which int/int produce doubles and avoid the recompilation entirely.
Attached patch PatchSplinter Review
What about this patch? This wins 5 seconds total on Kraken and makes JM+TI beat V8 and JM+TM on audio-beat-detection and audio-fft. SS is in the noise, V8 becomes about 20 ms faster.
Attachment #498199 - Flags: review?(bhackett1024)
Comment on attachment 498199 [details] [diff] [review]
Patch

Looks good, can add a narrower is-double inference later if we ever end up needing it.  Do you need this pushed?
Attachment #498199 - Flags: review?(bhackett1024) → review+
(In reply to comment #3)
> Do you need this pushed?

Yes, thanks in advance.
http://hg.mozilla.org/projects/jaegermonkey/rev/a2630fc3cd90
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: fixed-in-jaegermonkey
You need to log in before you can comment on or make changes to this bug.