Optimize String.prototype.includes

Optimize String.prototype.includes

This patch removes the MathMax call from String.prototype.includes
in order to improve performance. With some quick and dirty benchmarking,
(test case courtesy of the node folks) a sizable performance gain is visible:

d8> function testIndexOf() { var stringArray = [ 'hello', 'world', '123', 'abc' ]; return stringArray.some(function(val, idx, arr) { return val.indexOf('world') !== -1 })}
d8> function testIncludes() { var stringArray = [ 'hello', 'world', '123', 'abc' ]; return stringArray.some(function(val, idx, arr) { return val.includes('world') })}
d8> function testTime(fn) { var before = Date.now(); fn(); return Date.now() - before; }
d8> testTime(function() { for (var i = 0; i < 10000000; i++) { testIncludes() } })
2244
d8> testTime(function() { for (var i = 0; i < 10000000; i++) { testIndexOf() } })
2212

Compare that to before the test, when the performance difference was much larger:

d8> testTime(function() { for (var i = 0; i < 10000000; i++) { testIndexOf() } })
2223
d8> testTime(function() { for (var i = 0; i < 10000000; i++) { testIncludes() } })
2650

In my runs, performance of both functions drifts up and down, but running them in quick
succession back and forth shows a roughly consistent delta of about this magnitude.

String.prototype.includes is still slightly (maybe 5%) slower than String.prototype.indexOf,
but the effect is significantly reduced.

R=adamk
BUG=v8:3807
LOG=Y

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

[Dan Ehrenberg, 2015-07-14 22:24:50]

This patch is basically a rebase of https://codereview.chromium.org/832713009 .

[adamk, 2015-07-14 23:24:55]

Can you re-run these benchmarks with the s/$toInteger/TO_INTEGER/ change and
update the CL description if it actually changes the results?

https://codereview.chromium.org/1231673008/diff/1/src/string.js
File src/string.js (right):

https://codereview.chromium.org/1231673008/diff/1/src/string.js#newcode1040
src/string.js:1040: var searchString = TO_STRING_INLINE(searchString);
No need for "var" here, you're re-assigning a parameter now.

https://codereview.chromium.org/1231673008/diff/1/src/string.js#newcode1044
src/string.js:1044: pos = $toInteger(pos);
TO_INTEGER here for more performance

[Dan Ehrenberg, 2015-07-14 23:45:48]

https://codereview.chromium.org/1231673008/diff/1/src/string.js
File src/string.js (right):

https://codereview.chromium.org/1231673008/diff/1/src/string.js#newcode1040
src/string.js:1040: var searchString = TO_STRING_INLINE(searchString);
On 2015/07/14 23:24:55, adamk wrote:
> No need for "var" here, you're re-assigning a parameter now.

Done.

https://codereview.chromium.org/1231673008/diff/1/src/string.js#newcode1044
src/string.js:1044: pos = $toInteger(pos);
On 2015/07/14 23:24:55, adamk wrote:
> TO_INTEGER here for more performance

This doesn't affect my benchmark because %_ArgumentsLength() == 1, but I made
the change for consistency.

[adamk, 2015-07-14 23:48:28]

lgtm

[Dan Ehrenberg, 2015-07-15 00:01:52]

The CQ bit was checked by littledan@chromium.org

[commit-bot: I haz the power, 2015-07-15 00:02:07]

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

[commit-bot: I haz the power, 2015-07-15 01:01:46]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-07-15 01:02:06]

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

[thefourtheye_, 2015-07-15 01:26:00]

sakthiprofessional@gmail.com changed reviewers:
+ sakthiprofessional@gmail.com

[thefourtheye_, 2015-07-15 01:26:00]

https://codereview.chromium.org/1231673008/diff/20001/src/string.js
File src/string.js (right):

https://codereview.chromium.org/1231673008/diff/20001/src/string.js#newcode1040
src/string.js:1040: searchString = TO_STRING_INLINE(searchString);
Isn't this a DEOPT? Using the argument variable in both sides of an assignment
expression?

https://codereview.chromium.org/1231673008/diff/20001/src/string.js#newcode1050
src/string.js:1050: var searchStringLength = searchString.length;
Very minor optimization, but can we move this above and return true if it is
zero?