Bug 41213 – improved lexer for JavaScriptCore

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

[Zoltan Herczeg]

It is time to improve the speed of the lexer.

[Zoltan Herczeg]

Created attachment 59765 [details]
patch

[Oliver Hunt]

Comment on attachment 59765 [details]
patch

This looks sane but i'm in a bus so can't really review properly -- what's the perf win from this?

[Sam Weinig]

Comment on attachment 59765 [details]
patch

Missing ChangeLog and perf change.

[Zoltan Herczeg]

Performance gain is minimal. I plan to refactor the other parts of lexer (want to remove the majority of the gotos), just not everything in one step, except if you prefer the one big change.

TEST                           COMPARISON            FROM                 TO             DETAILS

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

** TOTAL **:                   -                 34.0ms +/- 2.6%   33.2ms +/- 2.0%

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

  jquery:                      ??                 5.1ms +/- 4.4%    5.3ms +/- 6.5%     not conclusive: might be *1.039x as slow*
    1.3.2:                     ??                 5.1ms +/- 4.4%    5.3ms +/- 6.5%     not conclusive: might be *1.039x as slow*

  mootools:                    ??                 5.3ms +/- 6.5%    5.4ms +/- 6.8%     not conclusive: might be *1.019x as slow*
    1.2.2-core-nc:             ??                 5.3ms +/- 6.5%    5.4ms +/- 6.8%     not conclusive: might be *1.019x as slow*

  prototype:                   -                  6.4ms +/- 5.8%    6.1ms +/- 3.7%
    1.6.0.3:                   -                  6.4ms +/- 5.8%    6.1ms +/- 3.7%

  concat:                      1.049x as fast    17.2ms +/- 3.3%   16.4ms +/- 2.3%     significant
    jquery-mootools-prototype: 1.049x as fast    17.2ms +/- 3.3%   16.4ms +/- 2.3%     significant

[Oliver Hunt]

(In reply to comment #4)
> Performance gain is minimal. I plan to refactor the other parts of lexer (want to remove the majority of the gotos), just not everything in one step, except if you prefer the one big change.
Nope, i completely agree with this approach.

Like Sam said missing a changelog == badness, otherwise this looks good.

[Zoltan Herczeg]

Created attachment 59881 [details]
full patch

Sorry for the Changelog. I used to many iterations, and maintaining a Changelog is a nightmare after updates.

[Oliver Hunt]

Comment on attachment 59881 [details]
full patch

r=me

[Zoltan Herczeg]

thanks.

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

[WebKit Review Bot]

http://trac.webkit.org/changeset/62031 might have broken GTK Linux 32-bit Debug
The following changes are on the blame list:
http://trac.webkit.org/changeset/62032
http://trac.webkit.org/changeset/62033
http://trac.webkit.org/changeset/62034
http://trac.webkit.org/changeset/62031