Bug 45289 – Refactoring multiline comments in the lexer

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

[Zoltan Herczeg]

Removing more labels.

[Zoltan Herczeg]

Created attachment 66713 [details]
patch

--parse-only reports a big change, but I am unsure it is really caused by the patch. Who knows...

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

** TOTAL **:                   1.063x as fast    33.6ms +/- 1.5%   31.6ms +/- 1.6%     significant

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

  jquery:                      -                  5.1ms +/- 4.4%    4.9ms +/- 4.6%
    1.3.2:                     -                  5.1ms +/- 4.4%    4.9ms +/- 4.6%

  mootools:                    1.080x as fast     5.4ms +/- 6.8%    5.0ms +/- 0.0%     significant
    1.2.2-core-nc:             1.080x as fast     5.4ms +/- 6.8%    5.0ms +/- 0.0%     significant

  prototype:                   1.067x as fast     6.4ms +/- 5.8%    6.0ms +/- 0.0%     significant
    1.6.0.3:                   1.067x as fast     6.4ms +/- 5.8%    6.0ms +/- 0.0%     significant

  concat:                      1.064x as fast    16.7ms +/- 2.1%   15.7ms +/- 2.2%     significant
    jquery-mootools-prototype: 1.064x as fast    16.7ms +/- 2.1%   15.7ms +/- 2.2%     significant

SS has no change:

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

** TOTAL **:           -                 523.1ms +/- 0.6%   521.2ms +/- 0.5%

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

[Darin Adler]

Comment on attachment 66713 [details]
patch

> +        if (UNLIKELY(m_current == '*')) {
> +            shift();
> +            if (m_current == '/') {
> +                shift();
> +                return true;
> +            }
> +            if (m_current == '*')
> +                continue;
> +        }

This ends up comparing with '*' twice, which seems like a shame given it's also slightly confusing logic. Instead of the if statement and the continue statement, we could just use while instead of if. Does that make things slower?

> @@ -1002,7 +1027,8 @@ inNumberAfterDecimalPoint:
>          m_terminator = true;
>          if (lastTokenWasRestrKeyword()) {
>              token = SEMICOLON;
> -            goto doneSemicolon;
> +            m_delimited = true;
> +            goto returnToken;
>          }
>          goto start;
>      case CharacterInvalid:

Why is this change included in the same patch? It seems unrelated and should be done based on its own merits, not as a ridealong. We can keep the doneSemicolon symbol.

[Zoltan Herczeg]

Created attachment 66863 [details]
Reworked patch

Just upload the final version of the patch.

[Zoltan Herczeg]

Landed in http://trac.webkit.org/changeset/66962.

[Zoltan Herczeg]

Created attachment 66874 [details]
removing doneSemicolon label

I still see the perf increase after apply these simple changes.

33.6ms -> 32.8ms (previous patch) -> 31.5ms (this patch)

[Zoltan Herczeg]

Landed in http://trac.webkit.org/changeset/67066

[WebKit Review Bot]

http://trac.webkit.org/changeset/67066 might have broken Leopard Intel Debug (Tests)
The following changes are on the blame list:
http://trac.webkit.org/changeset/67065
http://trac.webkit.org/changeset/67066