Bug 15972 – Add BinaryOperatorNode class to clean up the Node tree

• Status: RESOLVED
• Resolution: LATER
• Component: JavaScriptCore

[Eric Seidel (no email)]

Add BinaryOperatorNode class to clean up the Node tree

[Eric Seidel (no email)]

Created attachment 17241 [details]
cleanup patch

[Geoffrey Garen]

Comment on attachment 17241 [details]
cleanup patch

operatorAsString should be KJS_FAST_CALL.

Looks good. r=me if the SunSpider gods do not disapprove.

[Eric Seidel (no email)]

Created attachment 17248 [details]
improved patch, SunSpider reports wild results

SunSpider reports absolutely wild results from this:

TEST                   COMPARISON            FROM                 TO             DETAILS

=============================================================================

** TOTAL **:           2.0% *slower*   4372.2ms +/- 0.2%   4460.8ms +/- 0.3%     significant

=============================================================================

  3d:                  1.4% *slower*    494.2ms +/- 0.3%    501.2ms +/- 1.0%     significant
    cube:              ??               166.4ms +/- 0.7%    168.2ms +/- 3.2%     not conclusive: might be 1.1% *slower*
    morph:             ??               160.0ms +/- 0.5%    160.4ms +/- 0.4%     not conclusive: might be 0.3% *slower*
    raytrace:          2.9% *slower*    167.8ms +/- 0.3%    172.6ms +/- 0.6%     significant

  access:              -                713.4ms +/- 0.2%    712.8ms +/- 0.5% 
    binary-trees:      1.3% *slower*    108.8ms +/- 0.5%    110.2ms +/- 0.9%     significant
    fannkuch:          1.0% faster      344.8ms +/- 0.2%    341.4ms +/- 0.6%     significant
    nbody:             ??               181.6ms +/- 0.4%    182.4ms +/- 0.4%     not conclusive: might be 0.4% *slower*
    nsieve:            ??                78.2ms +/- 0.7%     78.8ms +/- 0.7%     not conclusive: might be 0.8% *slower*

  bitops:              ??               618.0ms +/- 0.1%    618.8ms +/- 0.2%     not conclusive: might be 0.1% *slower*
    3bit-bits-in-byte: 1.9% *slower*    114.2ms +/- 0.9%    116.4ms +/- 0.6%     significant
    bits-in-byte:      0.7% *slower*    152.6ms +/- 0.4%    153.6ms +/- 0.4%     significant
    bitwise-and:       -                214.4ms +/- 0.3%    214.4ms +/- 0.3% 
    nsieve-bits:       1.8% faster      136.8ms +/- 0.4%    134.4ms +/- 0.5%     significant

  controlflow:         ??               154.6ms +/- 0.4%    155.2ms +/- 0.4%     not conclusive: might be 0.4% *slower*
    recursive:         ??               154.6ms +/- 0.4%    155.2ms +/- 0.4%     not conclusive: might be 0.4% *slower*

  crypto:              -                343.8ms +/- 0.7%    343.6ms +/- 0.2% 
    aes:               0.8% faster       97.8ms +/- 0.6%     97.0ms +/- 0.0%     significant
    md5:               0.5% *slower*    123.4ms +/- 0.6%    124.0ms +/- 0.0%     significant
    sha1:              -                122.6ms +/- 1.4%    122.6ms +/- 0.6% 

  date:                1.9% *slower*    364.2ms +/- 0.2%    371.0ms +/- 0.2%     significant
    format-tofte:      1.4% *slower*    160.8ms +/- 0.3%    163.0ms +/- 0.5%     significant
    format-xparb:      2.3% *slower*    203.4ms +/- 0.3%    208.0ms +/- 0.4%     significant

  math:                -                524.4ms +/- 0.4%    522.6ms +/- 0.6% 
    cordic:            1.0% faster      229.4ms +/- 0.5%    227.0ms +/- 1.4%     significant
    partial-sums:      -                173.0ms +/- 0.5%    173.0ms +/- 0.5% 
    spectral-norm:     ??               122.0ms +/- 0.7%    122.6ms +/- 0.6%     not conclusive: might be 0.5% *slower*

  regexp:              23.4% *slower*   295.4ms +/- 0.2%    364.4ms +/- 1.9%     significant
    dna:               23.4% *slower*   295.4ms +/- 0.2%    364.4ms +/- 1.9%     significant

  string:              0.8% *slower*    864.2ms +/- 0.3%    871.2ms +/- 0.3%     significant
    base64:            1.8% *slower*    120.0ms +/- 0.7%    122.2ms +/- 0.5%     significant
    fasta:             ??               205.6ms +/- 0.5%    206.4ms +/- 0.3%     not conclusive: might be 0.4% *slower*
    tagcloud:          1.0% *slower*    197.0ms +/- 0.4%    199.0ms +/- 0.4%     significant
    unpack-code:       1.8% *slower*    202.0ms +/- 0.4%    205.6ms +/- 0.5%     significant
    validate-input:    1.1% faster      139.6ms +/- 1.0%    138.0ms +/- 0.6%     significant

[Eric Seidel (no email)]

Looking at the time samples for --shark --tests dna, there is very very little difference.  Certainly nothing that could account for 23%.  I guess the next step is to try an cache-miss session and see if there is a difference there.

[Eric Seidel (no email)]

I added --shark-cache (bug 15983) to check for level 2 cache misses, and also didn't see a difference.  I'm really sad about this mystery code-gen regression. :(  Especially since this kinda blocks other nice work like bug 15973.

[Eric Seidel (no email)]

This mystery regression may not be there next time we try.  We'll just keep this patch around until we understand why it regresses. :)

[Eric Seidel (no email)]

Huzzah!  I updated this patch to TOT, and now it's still showing slowness. but it's showing slowness in areas that it actually could have changed (like math-heavy tests).  Investigating.

[Eric Seidel (no email)]

I've given up on this patch.  Closing.