Summary: | Refactoring multiline comments in the lexer | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Zoltan Herczeg <zherczeg> | ||||||||
Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | abarth, darin, eric, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | PC | ||||||||||
OS: | OS X 10.5 | ||||||||||
Attachments: |
|
Description
Zoltan Herczeg
2010-09-07 04:47:04 PDT
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%
=============================================================================
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. Created attachment 66863 [details]
Reworked patch
Just upload the final version of the patch.
Landed in http://trac.webkit.org/changeset/66962. 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)
Landed in http://trac.webkit.org/changeset/67066 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 |