Remove that uncountable number of gotos there.
Created attachment 64578 [details] patch
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%
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.
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.
Created attachment 64817 [details] patch 2
> 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%
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.
Landed in http://trac.webkit.org/changeset/66375 Closing bug.