Closed Bug 507526 Opened 11 years ago Closed 11 years ago

use getc_unlocked in the scanner rather than getc

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: sayrer, Assigned: sayrer)

Details

Attachments

(2 files, 2 obsolete files)

** TOTAL **:           1.041x as fast    1020.1ms +/- 0.2%   979.6ms +/- 0.3%     significant

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

  3d:                  1.010x as fast     141.4ms +/- 0.8%   140.0ms +/- 0.3%     significant
    cube:              -                   40.0ms +/- 1.2%    39.4ms +/- 1.3% 
    morph:             *1.039x as slow*    28.5ms +/- 1.3%    29.6ms +/- 1.2%     significant
    raytrace:          1.027x as fast      72.9ms +/- 1.2%    71.0ms +/- 0.5%     significant

  access:              1.008x as fast     142.0ms +/- 0.5%   140.9ms +/- 0.8%     significant
    binary-trees:      1.014x as fast      43.7ms +/- 0.8%    43.1ms +/- 0.5%     significant
    fannkuch:          -                   57.9ms +/- 0.7%    57.5ms +/- 1.6% 
    nbody:             -                   26.3ms +/- 1.3%    26.2ms +/- 1.2% 
    nsieve:            -                   14.1ms +/- 1.6%    14.1ms +/- 1.6% 

  bitops:              -                   35.7ms +/- 1.6%    35.4ms +/- 1.4% 
    3bit-bits-in-byte: -                    1.7ms +/- 20.3%     1.5ms +/- 25.1% 
    bits-in-byte:      -                    8.3ms +/- 4.2%     8.3ms +/- 4.2% 
    bitwise-and:       ??                   2.2ms +/- 13.7%     2.3ms +/- 15.0%     not conclusive: might be *1.045x as slow*
    nsieve-bits:       -                   23.5ms +/- 1.6%    23.3ms +/- 1.5% 

  controlflow:         -                   33.7ms +/- 1.0%    33.4ms +/- 1.1% 
    recursive:         -                   33.7ms +/- 1.0%    33.4ms +/- 1.1% 

  crypto:              1.050x as fast      56.6ms +/- 1.7%    53.9ms +/- 2.2%     significant
    aes:               1.043x as fast      31.8ms +/- 2.3%    30.5ms +/- 3.0%     significant
    md5:               1.067x as fast      15.9ms +/- 2.6%    14.9ms +/- 1.5%     significant
    sha1:              1.047x as fast       8.9ms +/- 2.5%     8.5ms +/- 4.4%     significant

  date:                ??                 151.2ms +/- 0.4%   151.7ms +/- 0.4%     not conclusive: might be *1.003x as slow*
    format-tofte:      *1.012x as slow*    69.2ms +/- 0.7%    70.0ms +/- 0.7%     significant
    format-xparb:      -                   82.0ms +/- 0.6%    81.7ms +/- 0.4% 

  math:                -                   30.6ms +/- 2.0%    30.0ms +/- 1.9% 
    cordic:            1.056x as fast      11.3ms +/- 4.3%    10.7ms +/- 3.2%     significant
    partial-sums:      *1.008x as slow*    13.0ms +/- 0.0%    13.1ms +/- 1.7%     significant
    spectral-norm:     -                    6.3ms +/- 5.5%     6.2ms +/- 4.9% 

  regexp:              1.21x as fast       49.4ms +/- 0.7%    40.9ms +/- 0.6%     significant
    dna:               1.21x as fast       49.4ms +/- 0.7%    40.9ms +/- 0.6%     significant

  string:              1.074x as fast     379.5ms +/- 0.2%   353.4ms +/- 0.2%     significant
    base64:            -                   20.9ms +/- 1.1%    20.8ms +/- 1.4% 
    fasta:             -                   73.3ms +/- 0.7%    72.8ms +/- 0.6% 
    tagcloud:          1.131x as fast     114.6ms +/- 0.3%   101.3ms +/- 0.5%     significant
    unpack-code:       1.100x as fast     128.1ms +/- 0.4%   116.5ms +/- 0.3%     significant
    validate-input:    -                   42.6ms +/- 1.6%    42.0ms +/- 0.8%
Attached patch getc_unlocked (obsolete) — Splinter Review
Just for fun, should we try mmap?
Comment on attachment 391755 [details] [diff] [review]
getc_unlocked

Does this work with all platforms?
Attachment #391755 - Flags: review+
Attached patch _get_nolock for windows (obsolete) — Splinter Review
this works on everything except for Windows Mobile
Attachment #392173 - Attachment is patch: true
Attachment #392173 - Attachment mime type: application/octet-stream → text/plain
Attached patch works everywhereSplinter Review
Attachment #392173 - Attachment is obsolete: true
Attachment #391755 - Attachment is obsolete: true
Comment on attachment 392215 [details] [diff] [review]
works everywhere

Doesn't MSVC warn if you re#define a macro that was already defined?  If so, avoid it with #ifdef WINCE/#else/#endif.
Attachment #392215 - Flags: review+
looks like we use gcc on WinMo, but there we have it:

jsscan.cpp
e:/builds/slave/winmo-hg/build/js/src/jsscan.cpp(267) : warning C4005: 'getc_unlocked' : macro redefinition
        e:/builds/slave/winmo-hg/build/js/src/jsscan.cpp(265) : see previous definition of 'getc_unlocked'
jsscope.cpp
Always #undef before re-#define-ing?

/be
Attached patch mingw fixSplinter Review
This patch broke mingw build that doesn't support get_unlocked nor _getc_nolock. I've attached a fix that. I don't have setup to test WinCE build. Another solution would be changing that ifdef to:
#if defined(WINCE) || defined(__GNUC__)
but it's useless as gcc is used for WinCE anyway.
Attachment #394646 - Flags: review?(jorendorff)
We build WinCE with MSVC, actually.
Shouldn't we use an autoconf test or something other than a tangle of compiler and OS ifdefs?

/be
changeset b211fcf538a9 breaks build using MSVC 7.1.
http://hg.mozilla.org/mozilla-central/rev/b211fcf538a9
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
(In reply to comment #11)
> We build WinCE with MSVC, actually.

Then my patch is right although my comment was wrong (I messed the patch I've attached with the alternative one, sorry).

(In reply to comment #12)
> Shouldn't we use an autoconf test or something other than a tangle of compiler
> and OS ifdefs?

I agree, but my patch follows the original patch.
(In reply to comment #8)
> looks like we use gcc on WinMo, but there we have it:

FWIW, we use a horrible little wrapper that looks like GCC to call MSVC for WinCE builds.
Filed bug 510895 to tidy up the ifdefs.

(In reply to comment #13)
> changeset b211fcf538a9 breaks build using MSVC 7.1.

Roy, if you'd like to contribute a patch for bug 510895, it would be welcome, but even if not, please leave a comment there with details about the breakage, including complete error messages. Thanks.
(In reply to comment #17)
> Filed bug 510895 to tidy up the ifdefs.
> 
> (In reply to comment #13)
> > changeset b211fcf538a9 breaks build using MSVC 7.1.
> 
> Roy, if you'd like to contribute a patch for bug 510895, it would be welcome,
> but even if not, please leave a comment there with details about the breakage,
> including complete error messages. Thanks.

There's no _getc_nolock in MSVC 7.1.
Comment on attachment 394646 [details] [diff] [review]
mingw fix

Jacek, does the patch in bug 510895 work for you?
Attachment #394646 - Flags: review?(jorendorff)
Yes, it works great. Thank you.
You need to log in before you can comment on or make changes to this bug.