Speed up string scanning

Speed up string scanning

BUG=

Committed: https://crrev.com/c8f9d7ab534074cdb5aa5e851aaf91d626e131b1
Cr-Commit-Position: refs/heads/master@{#26997}

[Toon Verwaest, 2015-03-03 20:54:33]

verwaest@chromium.org changed reviewers:
+ marja@chromium.org

[Toon Verwaest, 2015-03-03 20:54:34]

PTAL

[marja, 2015-03-04 08:52:51]

How much does this give us & on which benchmark?

https://codereview.chromium.org/975043002/diff/20001/src/scanner.cc
File src/scanner.cc (right):

https://codereview.chromium.org/975043002/diff/20001/src/scanner.cc#newcode797
src/scanner.cc:797: if (c0_ > 127) {
I was briefly confused by the 127. Why so restrictive? We could have other
strings here too which could go to the fast path, not only under 127.

You could add a comment here that this is a fast path for ascii strings or sth?

https://codereview.chromium.org/975043002/diff/20001/src/scanner.h
File src/scanner.h (right):

https://codereview.chromium.org/975043002/diff/20001/src/scanner.h#newcode578
src/scanner.h:578: if (unibrow::Utf16::IsLeadSurrogate(c0_)) {
So this function is also called when c0_ is not a lead surrogate, and that's
normal, right? I was briefly confused b/c of the name.

[Toon Verwaest, 2015-03-04 12:08:53]

Addressed comment by making it kMaxAscii. That should help...
And your comment re HandleLeadSurrogate is correct.

As for benchmarks, it helps a tiny bit on tagcloud and codeload. It's not that
much, but then again the profiles are flat, so any tiny bit helps.

[marja, 2015-03-04 14:04:42]

How much is a "tiny bit"? Even though the fast path is kinda reasonable, it's..
it's a bloat for the scanner.

(Okay, not this change actually, but the previous one which requires
HandleLeadSurrogate() calls here and there... and probably we don't have tests
to make sure all the right places do remember to call it... I'm slightly
worried.)

lgtm though

[Toon Verwaest, 2015-03-04 16:30:30]

The attached perf runs indicate 1-2%.
I agree about the lead surrogate thing. However, that branch is in a very
expensive position, and does cause noticeable overhead. If someone can come up
with a less local solution, that'd be great.

[Toon Verwaest, 2015-03-04 17:35:39]

The CQ bit was checked by verwaest@chromium.org

[commit-bot: I haz the power, 2015-03-04 17:35:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/975043002/40001

[commit-bot: I haz the power, 2015-03-04 17:57:56]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-03-04 17:58:05]

Patchset 3 (id:??) landed as
https://crrev.com/c8f9d7ab534074cdb5aa5e851aaf91d626e131b1
Cr-Commit-Position: refs/heads/master@{#26997}