Avoid copying strings when parsing CSS colors on the slow path.

Avoid copying strings when parsing CSS colors on the slow path.

parseHexColor was always calling value().toString() forcing a string
copy of every hex color inside the parser. We can instead change
Color::parseHexColor to take a StringView and pass value() directly
avoiding this string copy.

CSSTokenizer::consumeName() also has a fast path which avoids making
string copies when the token value doesn't contain escapes. This path
didn't work when the input to the parser was a value that's not
followed by a semi-colon (ex. color = "#ccc" in JS) because
peekWithoutReplacement would return NUL when we accessed the first out
of range position, which then aborted the loop, and made us go down the
string copy path usually used for escapes.

This patch makes parsing colors on the slow path 2x faster:

ex.
for (var i = 0; i < 50000; ++i) {
    span.style.color = " #ddd"; // note the space before the #.

The color parsing fast path still seems about 2x faster than this
optimized path, but this patch gets us much closer to not needing the
color parsing fast path and makes whitespace in your colors less
terrible for performance.

The patch will also help CSS parsing in general by avoiding string
copies for every hex color.

BUG=605792
Committed: https://crrev.com/0d4987c045a9cbc5033295f0aee38a2456f039bf
Cr-Commit-Position: refs/heads/master@{#402097}

[esprehn, 2016-06-24 23:58:35]

Description was changed from

==========
Avoid copying strings when parsing CSS colors on the slow path.

parseHexColor was always calling value().toString() forcing a string
copy of every hex color inside the parser. We can instead change
Color::parseHexColor to take a StringView and pass value() directly
avoiding this string copy.

CSSTokenizer::consumeName() also has a fast path which avoids making
string copies when the token value doesn't contain escapes. This path
didn't work when the input to the parser was a value that's not
followed by a semi-colon (ex. color = "#ccc" in JS) because
peekWithoutReplacement would return NUL when we accessed the first out
of range position, which then aborted the loop, and made us go down the
string copy path usually used for escapes.

This patch makes parsing colors on the slow path 2x faster:

ex.
for (var i = 0; i < 50000; ++i) {
    span.style.color = " #ddd"; // note the space before the #.

The color parsing fast path still seems about 2x faster than this
optimized path, but this patch gets us much closer to not needing the
color parsing fast path and makes whitespace in your colors less
terrible for performance.

BUG=605792
==========

to

==========
Avoid copying strings when parsing CSS colors on the slow path.

parseHexColor was always calling value().toString() forcing a string
copy of every hex color inside the parser. We can instead change
Color::parseHexColor to take a StringView and pass value() directly
avoiding this string copy.

CSSTokenizer::consumeName() also has a fast path which avoids making
string copies when the token value doesn't contain escapes. This path
didn't work when the input to the parser was a value that's not
followed by a semi-colon (ex. color = "#ccc" in JS) because
peekWithoutReplacement would return NUL when we accessed the first out
of range position, which then aborted the loop, and made us go down the
string copy path usually used for escapes.

This patch makes parsing colors on the slow path 2x faster:

ex.
for (var i = 0; i < 50000; ++i) {
    span.style.color = " #ddd"; // note the space before the #.

The color parsing fast path still seems about 2x faster than this
optimized path, but this patch gets us much closer to not needing the
color parsing fast path and makes whitespace in your colors less
terrible for performance.

The patch will also help CSS parsing in general by avoiding string
copies for every hex color.

BUG=605792
==========

[esprehn, 2016-06-25 01:57:58]

The CQ bit was checked by esprehn@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-25 01:58:13]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-25 03:20:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-25 03:20:09]

Dry run: This issue passed the CQ dry run.

[esprehn, 2016-06-25 05:55:29]

esprehn@chromium.org changed reviewers:
+ meade@chromium.org, timloh@chromium.org

[Timothy Loh, 2016-06-27 00:14:25]

lgtm

https://codereview.chromium.org/2100573002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/css/parser/CSSTokenizer.cpp (right):

https://codereview.chromium.org/2100573002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/css/parser/CSSTokenizer.cpp:675: if (cc == '\0'
&& m_input.offset() + size < m_input.length())
good find!

[esprehn, 2016-06-27 00:59:36]

The CQ bit was checked by esprehn@chromium.org

[commit-bot: I haz the power, 2016-06-27 00:59:41]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-27 02:49:58]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-06-27 02:52:23]

Description was changed from

==========
Avoid copying strings when parsing CSS colors on the slow path.

parseHexColor was always calling value().toString() forcing a string
copy of every hex color inside the parser. We can instead change
Color::parseHexColor to take a StringView and pass value() directly
avoiding this string copy.

CSSTokenizer::consumeName() also has a fast path which avoids making
string copies when the token value doesn't contain escapes. This path
didn't work when the input to the parser was a value that's not
followed by a semi-colon (ex. color = "#ccc" in JS) because
peekWithoutReplacement would return NUL when we accessed the first out
of range position, which then aborted the loop, and made us go down the
string copy path usually used for escapes.

This patch makes parsing colors on the slow path 2x faster:

ex.
for (var i = 0; i < 50000; ++i) {
    span.style.color = " #ddd"; // note the space before the #.

The color parsing fast path still seems about 2x faster than this
optimized path, but this patch gets us much closer to not needing the
color parsing fast path and makes whitespace in your colors less
terrible for performance.

The patch will also help CSS parsing in general by avoiding string
copies for every hex color.

BUG=605792
==========

to

==========
Avoid copying strings when parsing CSS colors on the slow path.

parseHexColor was always calling value().toString() forcing a string
copy of every hex color inside the parser. We can instead change
Color::parseHexColor to take a StringView and pass value() directly
avoiding this string copy.

CSSTokenizer::consumeName() also has a fast path which avoids making
string copies when the token value doesn't contain escapes. This path
didn't work when the input to the parser was a value that's not
followed by a semi-colon (ex. color = "#ccc" in JS) because
peekWithoutReplacement would return NUL when we accessed the first out
of range position, which then aborted the loop, and made us go down the
string copy path usually used for escapes.

This patch makes parsing colors on the slow path 2x faster:

ex.
for (var i = 0; i < 50000; ++i) {
    span.style.color = " #ddd"; // note the space before the #.

The color parsing fast path still seems about 2x faster than this
optimized path, but this patch gets us much closer to not needing the
color parsing fast path and makes whitespace in your colors less
terrible for performance.

The patch will also help CSS parsing in general by avoiding string
copies for every hex color.

BUG=605792

Committed: https://crrev.com/0d4987c045a9cbc5033295f0aee38a2456f039bf
Cr-Commit-Position: refs/heads/master@{#402097}
==========

[commit-bot: I haz the power, 2016-06-27 02:52:24]

Patchset 2 (id:??) landed as
https://crrev.com/0d4987c045a9cbc5033295f0aee38a2456f039bf
Cr-Commit-Position: refs/heads/master@{#402097}