Bug 15996 - Apply further sanity to match() function (for 10%!? speedup in dna-regexp?)
Summary: Apply further sanity to match() function (for 10%!? speedup in dna-regexp?)
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Eric Seidel (no email)
URL:
Keywords:
Depends on:
Blocks: 15998
  Show dependency treegraph
 
Reported: 2007-11-15 02:01 PST by Eric Seidel (no email)
Modified: 2007-11-15 16:15 PST (History)
1 user (show)

See Also:


Attachments
patch (nearly entirely search/replace) (98.08 KB, patch)
2007-11-15 02:03 PST, Eric Seidel (no email)
sam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2007-11-15 02:01:04 PST
Apply further sanity to match() function (for 10%!? speedup in dna-regexp?)

This was not intended as a speedup patch.  However SunSpider claims it's a 10% speedup to regexp-dna.  It also claims it's a regression to some other random (non-regexp  using!) tests.  I blame bad codegen.  Or the toothfairy.  I can't decide which.

TEST                   COMPARISON            FROM                 TO             DETAILS

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

** TOTAL **:           0.3% faster     4377.8ms +/- 0.1%   4365.4ms +/- 0.2%     significant

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

  3d:                  1.9% *slower*    497.6ms +/- 0.5%    507.0ms +/- 2.4%     significant
    cube:              4.6% *slower*    166.8ms +/- 0.6%    174.4ms +/- 6.9%     significant
    morph:             ??               160.4ms +/- 0.4%    160.8ms +/- 0.3%     not conclusive: might be 0.2% *slower*
    raytrace:          0.8% *slower*    170.4ms +/- 0.7%    171.8ms +/- 0.3%     significant

  access:              0.8% faster      717.4ms +/- 0.5%    711.4ms +/- 0.2%     significant
    binary-trees:      -                108.8ms +/- 1.7%    108.6ms +/- 0.6% 
    fannkuch:          1.4% faster      347.6ms +/- 0.2%    342.8ms +/- 0.2%     significant
    nbody:             -                182.0ms +/- 0.5%    181.8ms +/- 0.3% 
    nsieve:            1.0% faster       79.0ms +/- 0.0%     78.2ms +/- 0.7%     significant

  bitops:              ??               621.4ms +/- 0.4%    622.8ms +/- 1.2%     not conclusive: might be 0.2% *slower*
    3bit-bits-in-byte: -                116.4ms +/- 1.6%    115.6ms +/- 1.0% 
    bits-in-byte:      ??               153.4ms +/- 0.9%    155.6ms +/- 4.6%     not conclusive: might be 1.4% *slower*
    bitwise-and:       -                214.2ms +/- 0.3%    214.2ms +/- 0.3% 
    nsieve-bits:       -                137.4ms +/- 0.5%    137.4ms +/- 0.5% 

  controlflow:         -                155.8ms +/- 0.7%    155.4ms +/- 0.7% 
    recursive:         -                155.8ms +/- 0.7%    155.4ms +/- 0.7% 

  crypto:              -                345.0ms +/- 0.5%    343.6ms +/- 0.8% 
    aes:               -                 97.8ms +/- 0.6%     97.6ms +/- 0.7% 
    md5:               -                124.2ms +/- 0.4%    123.6ms +/- 0.9% 
    sha1:              -                123.0ms +/- 0.7%    122.4ms +/- 2.1% 

  date:                ??               368.0ms +/- 0.3%    369.2ms +/- 0.7%     not conclusive: might be 0.3% *slower*
    format-tofte:      1.5% *slower*    160.6ms +/- 0.4%    163.0ms +/- 1.3%     significant
    format-xparb:      -                207.4ms +/- 0.7%    206.2ms +/- 0.3% 

  math:                3.3% *slower*    510.8ms +/- 0.4%    527.6ms +/- 0.5%     significant
    cordic:            7.0% *slower*    213.8ms +/- 0.5%    228.8ms +/- 0.6%     significant
    partial-sums:      -                174.2ms +/- 0.3%    173.8ms +/- 0.3% 
    spectral-norm:     1.8% *slower*    122.8ms +/- 0.8%    125.0ms +/- 0.7%     significant

  regexp:              10.0% faster     296.0ms +/- 0.3%    266.4ms +/- 0.4%     significant
    dna:               10.0% faster     296.0ms +/- 0.3%    266.4ms +/- 0.4%     significant

  string:              -                865.8ms +/- 0.5%    862.0ms +/- 0.2% 
    base64:            1.3% faster      122.6ms +/- 0.9%    121.0ms +/- 0.0%     significant
    fasta:             0.4% *slower*    205.2ms +/- 0.3%    206.0ms +/- 0.4%     significant
    tagcloud:          -                197.0ms +/- 1.2%    196.2ms +/- 0.8% 
    unpack-code:       -                202.4ms +/- 0.5%    202.4ms +/- 0.3% 
    validate-input:    1.6% faster      138.6ms +/- 1.2%    136.4ms +/- 1.2%     significant
Comment 1 Eric Seidel (no email) 2007-11-15 02:03:13 PST
Created attachment 17289 [details]
patch (nearly entirely search/replace)

This patch depends on bug 15995, and in fact, the SunSpider numbers above include that patch, so it's possible my other whitespace changes caused the 10% speedup... somehow I wouldn't be surprised.
Comment 2 Eric Seidel (no email) 2007-11-15 02:12:55 PST
Binary size information.

Before:
1551620

After bug 15995:
1551620

After this bug:
1547524

Not sure why this change makes the binary *smaller*, but it does.
Comment 3 Eric Seidel (no email) 2007-11-15 02:14:31 PST
I guess the binary size change could be related to fewer gotos and the error reporting code no longer being inlined.
Comment 4 Eric Seidel (no email) 2007-11-15 02:19:05 PST
My latest sunspider numbers show this patch as a 0.7% speedup.  Again, I don't think this is necessarily related to the patch itself, but rather some codegen side effect.  Binary size being one possibility.

I still think this is worth reviewing and landing.

TEST                   COMPARISON            FROM                 TO             DETAILS

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

** TOTAL **:           0.7% faster     4379.8ms +/- 0.2%   4347.4ms +/- 0.2%     significant

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

  3d:                  -                498.6ms +/- 0.4%    498.2ms +/- 0.4% 
    cube:              -                167.2ms +/- 0.8%    166.8ms +/- 1.1% 
    morph:             -                160.8ms +/- 0.6%    160.4ms +/- 0.4% 
    raytrace:          ??               170.6ms +/- 0.4%    171.0ms +/- 0.0%     not conclusive: might be 0.2% *slower*

  access:              0.7% faster      716.4ms +/- 0.2%    711.4ms +/- 0.2%     significant
    binary-trees:      0.4% *slower*    108.0ms +/- 0.0%    108.4ms +/- 0.6%     significant
    fannkuch:          1.5% faster      347.8ms +/- 0.2%    342.6ms +/- 0.2%     significant
    nbody:             ??               181.8ms +/- 0.3%    182.0ms +/- 0.0%     not conclusive: might be 0.1% *slower*
    nsieve:            -                 78.8ms +/- 0.7%     78.4ms +/- 0.9% 

  bitops:              0.2% faster      621.8ms +/- 0.2%    620.4ms +/- 0.2%     significant
    3bit-bits-in-byte: -                115.8ms +/- 0.5%    115.4ms +/- 0.6% 
    bits-in-byte:      -                153.4ms +/- 0.4%    153.4ms +/- 0.4% 
    bitwise-and:       -                214.8ms +/- 0.3%    214.6ms +/- 0.3% 
    nsieve-bits:       0.6% faster      137.8ms +/- 0.4%    137.0ms +/- 0.0%     significant

  controlflow:         -                156.0ms +/- 1.0%    155.6ms +/- 0.4% 
    recursive:         -                156.0ms +/- 1.0%    155.6ms +/- 0.4% 

  crypto:              0.9% faster      344.8ms +/- 0.3%    341.8ms +/- 0.3%     significant
    aes:               -                 97.6ms +/- 0.7%     97.4ms +/- 0.7% 
    md5:               0.8% faster      124.2ms +/- 0.4%    123.2ms +/- 0.5%     significant
    sha1:              1.5% faster      123.0ms +/- 0.7%    121.2ms +/- 1.1%     significant

  date:                -                368.8ms +/- 0.8%    367.4ms +/- 0.5% 
    format-tofte:      -                161.8ms +/- 1.5%    161.2ms +/- 0.6% 
    format-xparb:      -                207.0ms +/- 0.9%    206.2ms +/- 0.7% 

  math:                3.2% *slower*    511.4ms +/- 0.5%    527.6ms +/- 0.8%     significant
    cordic:            6.5% *slower*    215.0ms +/- 0.8%    229.0ms +/- 1.2%     significant
    partial-sums:      -                173.8ms +/- 0.6%    173.8ms +/- 0.6% 
    spectral-norm:     1.8% *slower*    122.6ms +/- 0.6%    124.8ms +/- 1.9%     significant

  regexp:              10.3% faster     296.6ms +/- 0.2%    266.0ms +/- 0.0%     significant
    dna:               10.3% faster     296.6ms +/- 0.2%    266.0ms +/- 0.0%     significant

  string:              0.7% faster      865.4ms +/- 0.5%    859.0ms +/- 0.3%     significant
    base64:            2.1% faster      122.8ms +/- 1.1%    120.2ms +/- 0.5%     significant
    fasta:             ??               205.2ms +/- 0.7%    206.4ms +/- 0.3%     not conclusive: might be 0.6% *slower*
    tagcloud:          1.2% faster      196.8ms +/- 0.8%    194.4ms +/- 1.1%     significant
    unpack-code:       0.2% faster      202.0ms +/- 0.0%    201.6ms +/- 0.6%     significant
    validate-input:    1.6% faster      138.6ms +/- 0.5%    136.4ms +/- 1.4%     significant

Comment 5 Eric Seidel (no email) 2007-11-15 02:41:37 PST
It looks like darin noticed similar SunSpider craziness relating to regexp in bug 11231.
Comment 6 Sam Weinig 2007-11-15 11:23:07 PST
Comment on attachment 17289 [details]
patch (nearly entirely search/replace)

Needs a changelog.

r=me.
Comment 7 Eric Seidel (no email) 2007-11-15 16:15:02 PST
Landed.