lib: Convert String#indexOf usages to String#includes

[ziyunfei]

No description provided.

[rvagg]

do we have any perf data on includes() in V8?

[chrisdickinson]

It looks like it just calls indexOf internally.

[ziyunfei]

From this test, it seems that includes() is about 26% slower than indexOf() :(

[ziyunfei]

includes() can be optimized.
https://code.google.com/p/v8/issues/detail?id=3807

[cjihrig]

Haven't looked into performance, but the code LGTM, FWIW.

[bnoordhuis]

@ziyunfei You should probably assign a reviewer to that CR. yangguo or dslomov are probably good candidates, @caitp was involved with harmony-strings.js as well, IIRC.

[rvagg]

ftr, 👎 from me on this unless perf is improved. Same goes for most transitions to new V8 features in core, I'd expect them to remain unoptimised for some time just like many ES5 features (Function#bind() anyone?) are.

[chrisdickinson]

While I appreciate the intent, I'm -1 on making this sort of change. I don't see any particular benefit in bulk-switching indexOf to includes (readability, maybe? indexOf is a pretty common idiom, though, so that's not a super convincing argument.)

I'll leave this PR open until tomorrow evening to give other collaborators/TC members time to weigh in, but if no one chimes in before then I'll close it.

[jbergstroem]

fwiw, I'm with @rvagg; there's really no benefit of switching until performance at least is equal versus the benefit of readability.

[chrisdickinson]

Closing this – includes is slower than indexOf; we may revisit if it speeds up.

[littledan]

I updated String.prototype.includes in V8 to be faster using ziyunfei's patch. I think this will come out around M46, but it's also an easy cherry-pick https://codereview.chromium.org/1231673008 . Performance is much closer now. Let me know if you have any other issues with the performance of this function.

[thefourtheye]

@littledan Nice! I left a couple of comments there. Please let me know what you think about them.

[littledan]

@thefourtheye I committed the code as is. I don't think the things you pointed out would be performance issues. But thanks for taking a look.