Bug 41213 - improved lexer for JavaScriptCore
Summary: improved lexer for JavaScriptCore
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Enhancement
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-06-25 07:09 PDT by Zoltan Herczeg
Modified: 2010-06-28 14:24 PDT (History)
6 users (show)

See Also:


Attachments
patch (28.60 KB, patch)
2010-06-25 07:10 PDT, Zoltan Herczeg
sam: review-
Details | Formatted Diff | Diff
full patch (29.92 KB, patch)
2010-06-28 00:59 PDT, Zoltan Herczeg
oliver: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Zoltan Herczeg 2010-06-25 07:09:26 PDT
It is time to improve the speed of the lexer.
Comment 1 Zoltan Herczeg 2010-06-25 07:10:07 PDT
Created attachment 59765 [details]
patch
Comment 2 Oliver Hunt 2010-06-25 10:48:05 PDT
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?
Comment 3 Sam Weinig 2010-06-25 11:16:37 PDT
Comment on attachment 59765 [details]
patch

Missing ChangeLog and perf change.
Comment 4 Zoltan Herczeg 2010-06-25 11:25:16 PDT
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
Comment 5 Oliver Hunt 2010-06-25 12:54:20 PDT
(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.
Comment 6 Zoltan Herczeg 2010-06-28 00:59:28 PDT
Created attachment 59881 [details]
full patch

Sorry for the Changelog. I used to many iterations, and maintaining a Changelog is a nightmare after updates.
Comment 7 Oliver Hunt 2010-06-28 12:17:57 PDT
Comment on attachment 59881 [details]
full patch

r=me
Comment 8 Zoltan Herczeg 2010-06-28 13:23:51 PDT
thanks.

Landed in http://trac.webkit.org/changeset/62031
Closing bug.
Comment 9 WebKit Review Bot 2010-06-28 14:24:21 PDT
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