Closed Bug 514267 Opened 10 years ago Closed 10 years ago

TM: Fast path for js_StringToInt32 if string length is 1

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
Tracking Status
status1.9.2 --- beta1-fixed

People

(Reporter: wagnerg, Assigned: wagnerg)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file, 2 obsolete files)

The SS benchmark suite only uses this function with single characters.
The idea is to generate a fast check at the beginning for single characters.

SS benchmark suite:
24567 calls of js_StringToInt32 with str->length()==1 out of 24567 calls
Attached patch version 1 (obsolete) — Splinter Review
TEST                   COMPARISON            FROM                 TO             DETAILS

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

** TOTAL **:           1.007x as fast    782.1ms +/- 0.2%   776.3ms +/- 0.2%     significant

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

  3d:                  -                 119.3ms +/- 0.6%   118.9ms +/- 0.4% 
    cube:              -                  33.2ms +/- 0.9%    33.0ms +/- 0.0% 
    morph:             ??                 24.5ms +/- 1.5%    24.6ms +/- 1.5%     not conclusive: might be *1.004x as slow*
    raytrace:          -                  61.6ms +/- 0.6%    61.3ms +/- 0.6% 

  access:              1.018x as fast    117.8ms +/- 0.4%   115.7ms +/- 0.4%     significant
    binary-trees:      *1.012x as slow*   32.6ms +/- 1.1%    33.0ms +/- 0.0%     significant
    fannkuch:          1.044x as fast     52.5ms +/- 0.7%    50.3ms +/- 0.7%     significant
    nbody:             -                  21.4ms +/- 1.7%    21.3ms +/- 1.6% 
    nsieve:            -                  11.3ms +/- 3.1%    11.1ms +/- 2.0% 

  bitops:              *1.041x as slow*   31.8ms +/- 2.1%    33.1ms +/- 1.2%     significant
    3bit-bits-in-byte: ??                  1.2ms +/- 25.1%     1.5ms +/- 25.1%     not conclusive: might be *1.25x as slow*
    bits-in-byte:      -                   8.9ms +/- 2.5%     8.9ms +/- 2.5% 
    bitwise-and:       *1.050x as slow*    2.0ms +/- 0.0%     2.1ms +/- 10.8%     significant
    nsieve-bits:       *1.046x as slow*   19.7ms +/- 1.8%    20.6ms +/- 1.8%     significant

  controlflow:         1.018x as fast     28.4ms +/- 1.3%    27.9ms +/- 0.8%     significant
    recursive:         1.018x as fast     28.4ms +/- 1.3%    27.9ms +/- 0.8%     significant

  crypto:              ??                 47.3ms +/- 1.2%    47.7ms +/- 2.0%     not conclusive: might be *1.008x as slow*
    aes:               ??                 27.2ms +/- 2.7%    27.4ms +/- 2.8%     not conclusive: might be *1.007x as slow*
    md5:               ??                 12.8ms +/- 2.4%    13.0ms +/- 2.6%     not conclusive: might be *1.016x as slow*
    sha1:              -                   7.3ms +/- 4.7%     7.3ms +/- 4.7% 

  date:                1.004x as fast    114.2ms +/- 0.3%   113.7ms +/- 0.3%     significant
    format-tofte:      -                  57.7ms +/- 0.6%    57.7ms +/- 0.6% 
    format-xparb:      1.009x as fast     56.5ms +/- 0.7%    56.0ms +/- 0.0%     significant

  math:                -                  25.9ms +/- 2.4%    25.9ms +/- 2.4% 
    cordic:            -                   8.6ms +/- 4.3%     8.6ms +/- 4.3% 
    partial-sums:      ??                 11.9ms +/- 1.9%    12.0ms +/- 0.0%     not conclusive: might be *1.008x as slow*
    spectral-norm:     -                   5.4ms +/- 6.8%     5.3ms +/- 6.5% 

  regexp:              -                  39.8ms +/- 0.8%    39.6ms +/- 0.9% 
    dna:               -                  39.8ms +/- 0.8%    39.6ms +/- 0.9% 

  string:              1.015x as fast    257.6ms +/- 0.5%   253.8ms +/- 0.2%     significant
    base64:            1.114x as fast     12.7ms +/- 2.7%    11.4ms +/- 3.2%     significant
    fasta:             1.013x as fast     54.8ms +/- 0.5%    54.1ms +/- 0.7%     significant
    tagcloud:          1.012x as fast     82.2ms +/- 0.9%    81.2ms +/- 0.4%     significant
    unpack-code:       -                  82.6ms +/- 0.6%    82.0ms +/- 0.4% 
    validate-input:    -                  25.3ms +/- 1.4%    25.1ms +/- 0.9%
Attached patch shorter version (obsolete) — Splinter Review
Attachment #398214 - Attachment is obsolete: true
Attachment #398707 - Flags: review?(brendan)
Comment on attachment 398707 [details] [diff] [review]
shorter version

>diff -r 66235f4c4eea js/src/jsbuiltins.cpp
>--- a/js/src/jsbuiltins.cpp	Fri Sep 04 15:58:10 2009 +1000
>+++ b/js/src/jsbuiltins.cpp	Fri Sep 04 10:42:42 2009 -0700
>@@ -186,16 +186,23 @@ JS_DEFINE_CALLINFO_2(extern, DOUBLE, js_
> 
> int32 FASTCALL
> js_StringToInt32(JSContext* cx, JSString* str)
> {
>     const jschar* bp;
>     const jschar* end;
>     const jschar* ep;
>     jsdouble d;
>+    
>+    if (str->length() == 1) {
>+        jschar c = str->chars()[0];
>+        if ((c >= '0') && (c <= '9'))

Don't over-parenthesize. Also some tests like this use number-line order:

    if ('0' <= c && c <= '9')

but the alternative style is variable on the left of the relational. These greps are approximate, but the tie is striking:

$ grep '[a-z] >= .*&& .*<' *cpp | wc -l
      38
brendan-eichs-computer-2:src brendaneich$ grep '[^a-z] <= [a-z].*&& .*<' *cpp | wc -l
      38

If you ask me, I'll say "use number line order."

r=me with at least the over-paren nit addressed.

/be
Attachment #398707 - Flags: review?(brendan) → review+
Attachment #398707 - Attachment is obsolete: true
Great -- just need a volunteer to land the latest patch (with the hg option to attribute it to you, which I never remember). Thanks,

/be
Keywords: checkin-needed
http://hg.mozilla.org/mozilla-central/rev/c216d3357c5c
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Assignee: general → wagnerg
You need to log in before you can comment on or make changes to this bug.