511737 - TM: fast path for writing a double into an array

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Andreas Gal :gal]

4ms speedup on SS. Its a 2% speedup or so for about a third of the test cases. I renew my call for more profiling.

Comment 1

[Andreas Gal :gal]

Attachment: patch

Comment 2

[Robert Sayre]

:) nice work

Comment 3

[Nicholas Nethercote [inactive]]

(In reply to comment #0)
> 4ms speedup on SS. 

I don't trust SS's results enough to present them as definitively as that.  I have regularly seen changes that should not affect performance at all ~5ms variation, presumably due to slightly different code layouts causing different interactions with caches, branch predictors, etc.  I don't trust any optimisation until I've seen it give a speed-up on at least two configurations (eg. I 'hg pull -u' and the speed-up is still present).

And even on a single configuration, using --runs=100 on my Mac I often see variations of 2--3ms from run to run.

Comment 4

[Andreas Gal :gal]

Yes, our benchmarks are pretty lame. Yes, we discuss that in every other bug. The patch removes a bunch of spilling between two function calls in LIR and allows GCC to inline the box code into the array set path. Except if those instructions are all free (along with the memory access hat happens), I am pretty confident that this is N cycles faster than before. Can we agree at least on that?

Comment 5

[Nicholas Nethercote [inactive]]

(In reply to comment #4)
> Can we agree at least on that?

Sure :)

Comment 6

[Robert Sayre]

should be easy to write a microbenchmark to prove out this optimization, if we want to do that.

Comment 7

[David Mandelin [:dmandelin]]

(In reply to comment #6)
> should be easy to write a microbenchmark to prove out this optimization, if we
> want to do that.

I was about to suggest that. Or taking the SS benchmark that is most improved by this patch and increasing the run length by 10-100x.

Comment 8

[Andreas Gal :gal]

test case:

var a = [];
for (var i = 0; i < 10000000; ++i)
    a[1] = 1.5;

without:

real	0m0.522s
user	0m0.392s
sys	0m0.127s

with:

real	0m0.477s
user	0m0.346s
sys	0m0.126s

Comment 9

[Andreas Gal :gal]

I added a missing INT_FITS_INTO_JSVAL. Pushing with that.

Comment 10

[Andreas Gal :gal]

http://hg.mozilla.org/tracemonkey/rev/21b21088c0ef

Comment 11

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/21b21088c0ef

Comment 12

[Robert Sayre]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/6f2279d24162