Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(52)

Issue 1321853006: [es6] Use SubString in String{Starts,Ends}With (Closed)

Created:
5 years, 3 months ago by skomski
Modified:
5 years, 3 months ago
CC:
v8-dev
Base URL:
https://chromium.googlesource.com/v8/v8.git@master
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

[es6] Use SubString in String{Starts,Ends}With Much faster and constant than always searching the whole string ```` var allCodePoints = []; for (var i = 0; i < 65536; i++) allCodePoints[i] = i; var allCharsString = String.fromCharCode.apply(String, allCodePoints); function bench(search) { var counter = 0; print(search + " found at " + allCharsString.startsWith(search)); var start = Date.now(); while (counter++ < 5000000) { allCharsString.startsWith(search); } var end = Date.now(); print(end - start); return counter; } print("single character"); bench("\u0000"); bench("\u0050"); bench("\u1000"); ```` OLD single character found at true 374 P found at false 559 က found at false 13492 NEW single character found at true 261 P found at false 146 က found at false 146 BUG=v8:4384 LOG=N Committed: https://crrev.com/d42920ce06853d2982f52a9023ad4652f09a9277 Cr-Commit-Position: refs/heads/master@{#30599}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -2 lines) Patch
M src/string.js View 2 chunks +2 lines, -2 lines 0 comments Download

Messages

Total messages: 20 (7 generated)
skomski
5 years, 3 months ago (2015-09-03 16:13:58 UTC) #2
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1321853006/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1321853006/1
5 years, 3 months ago (2015-09-04 18:17:33 UTC) #5
Dan Ehrenberg
lgtm
5 years, 3 months ago (2015-09-04 18:18:41 UTC) #6
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: v8_presubmit on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/5566)
5 years, 3 months ago (2015-09-04 18:20:36 UTC) #8
Dan Ehrenberg
On 2015/09/04 at 18:20:36, commit-bot wrote: > Dry run: Try jobs failed on following builders: ...
5 years, 3 months ago (2015-09-04 18:27:11 UTC) #9
Dan Ehrenberg
On 2015/09/04 at 18:27:11, Dan Ehrenberg wrote: > On 2015/09/04 at 18:20:36, commit-bot wrote: > ...
5 years, 3 months ago (2015-09-04 18:29:08 UTC) #10
Jakob Kummerow
ACK. However, note that creating a substring causes an allocation, which isn't 100% ideal either. ...
5 years, 3 months ago (2015-09-04 18:37:53 UTC) #11
skomski
On 2015/09/04 18:29:08, Dan Ehrenberg wrote: > On 2015/09/04 at 18:27:11, Dan Ehrenberg wrote: > ...
5 years, 3 months ago (2015-09-04 18:39:51 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1321853006/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1321853006/1
5 years, 3 months ago (2015-09-04 21:22:02 UTC) #14
commit-bot: I haz the power
Try jobs failed on following builders: v8_presubmit on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/5573)
5 years, 3 months ago (2015-09-04 21:24:36 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1321853006/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1321853006/1
5 years, 3 months ago (2015-09-04 21:32:56 UTC) #18
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 3 months ago (2015-09-04 21:34:29 UTC) #19
commit-bot: I haz the power
5 years, 3 months ago (2015-09-04 21:34:48 UTC) #20
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/d42920ce06853d2982f52a9023ad4652f09a9277
Cr-Commit-Position: refs/heads/master@{#30599}

Powered by Google App Engine
This is Rietveld 408576698