Bug 44104 – Refactor number parsing in the lexer

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Zoltan Herczeg]

Remove that uncountable number of gotos there.

[Zoltan Herczeg]

Created attachment 64578 [details]
patch

[Zoltan Herczeg]

SS has no change (forgot to mention in the ChangeLog)

TEST                           COMPARISON            FROM                 TO             DETAILS

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

** TOTAL **:                   -                 33.3ms +/- 2.0%   32.7ms +/- 1.1%

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

  jquery:                      -                  5.2ms +/- 5.8%    4.9ms +/- 4.6%
    1.3.2:                     -                  5.2ms +/- 5.8%    4.9ms +/- 4.6%

  mootools:                    -                  5.6ms +/- 6.6%    5.5ms +/- 6.8%
    1.2.2-core-nc:             -                  5.6ms +/- 6.6%    5.5ms +/- 6.8%

  prototype:                   -                  6.2ms +/- 4.9%    6.2ms +/- 4.9%
    1.6.0.3:                   -                  6.2ms +/- 4.9%    6.2ms +/- 4.9%

  concat:                      -                 16.3ms +/- 2.1%   16.1ms +/- 1.4%
    jquery-mootools-prototype: -                 16.3ms +/- 2.1%   16.1ms +/- 1.4%

[Zoltan Herczeg]

Some thoughts:

There is still one got in the code, and I have no idea how to remove that.

Octal numbers seems unsupported in the newest ecmascript:

Past editions of ECMAScript have included additional syntax and semantics for specifying octal literals and octal escape sequences. These have been removed from this edition of ECMAScript.

And octal parsing looks strange to me:
01.1 is invalid, while 09.1 is a valid number (if an octal number followed by '8' or '9' it is considered as a decimal number.)

Although there are regression tests for the current parsing, thus I follow it right now.

[Darin Adler]

Comment on attachment 64578 [details]
patch

> +        record8(m_current);

Seems a shame we have to do this just in case of the overflow. It would be nice to find a way to skip this in the fast path.

> +    int maximumDigits = !m_buffer8.size() ? 9 : -1;

This is a tricky line of code and needs a comment.

[Zoltan Herczeg]

Created attachment 64817 [details]
patch 2

[Zoltan Herczeg]

> Seems a shame we have to do this just in case of the overflow. It would be nice to find a way to skip this in the fast path.

The way is found, but the introduced complexity is not necessary worth the efforts.

Now the parsing is slightly faster:

TEST                           COMPARISON            FROM                 TO             DETAILS

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

** TOTAL **:                   1.028x as fast    33.3ms +/- 2.0%   32.4ms +/- 1.1%     significant

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

  jquery:                      -                  5.2ms +/- 5.8%    5.0ms +/- 0.0%
    1.3.2:                     -                  5.2ms +/- 5.8%    5.0ms +/- 0.0%

  mootools:                    -                  5.6ms +/- 6.6%    5.3ms +/- 6.5%
    1.2.2-core-nc:             -                  5.6ms +/- 6.6%    5.3ms +/- 6.5%

  prototype:                   -                  6.2ms +/- 4.9%    6.1ms +/- 3.7%
    1.6.0.3:                   -                  6.2ms +/- 4.9%    6.1ms +/- 3.7%

  concat:                      -                 16.3ms +/- 2.1%   16.0ms +/- 0.0%
    jquery-mootools-prototype: -                 16.3ms +/- 2.1%   16.0ms +/- 0.0%

[Darin Adler]

Comment on attachment 64817 [details]
patch 2

I don’t have time to review right now. I think the extra complexity is worth it. The fact that it caused a measurable speedup is exciting. I think we can make additional speedups building on this.

[Zoltan Herczeg]

Landed in http://trac.webkit.org/changeset/66375
Closing bug.