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

Issue 1243053005: Revert "In RegExp, lastIndex is read with ToLength, not ToInteger" (Closed)

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

Description

Revert "In RegExp, lastIndex is read with ToLength, not ToInteger" $toLength is slow, causing a 3.8%-8% regression in the Octane RegExp benchmark. Reverting this patch brings it back up. To make this change, we'll need a faster implementation fo $toLength. BUG=chromium:513160 LOG=Y R=adamk Committed: https://crrev.com/477d651c6a978bdf34954048a235895c62dab0ac Cr-Commit-Position: refs/heads/master@{#29830}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+9 lines, -11 lines) Patch
M src/regexp.js View 2 chunks +6 lines, -6 lines 0 comments Download
M test/mozilla/mozilla.status View 1 chunk +0 lines, -2 lines 0 comments Download
M test/test262-es6/test262-es6.status View 1 chunk +3 lines, -0 lines 0 comments Download
M test/test262/test262.status View 1 chunk +0 lines, -3 lines 0 comments Download

Messages

Total messages: 12 (3 generated)
Dan Ehrenberg
5 years, 5 months ago (2015-07-24 00:20:39 UTC) #1
adamk
lgtm
5 years, 5 months ago (2015-07-24 00:32:24 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1243053005/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1243053005/1
5 years, 5 months ago (2015-07-24 03:54:42 UTC) #4
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 5 months ago (2015-07-24 06:21:11 UTC) #5
commit-bot: I haz the power
Patchset 1 (id:??) landed as https://crrev.com/477d651c6a978bdf34954048a235895c62dab0ac Cr-Commit-Position: refs/heads/master@{#29830}
5 years, 5 months ago (2015-07-24 06:21:37 UTC) #6
Michael Achenbach
A revert of this CL (patchset #1 id:1) has been created in https://codereview.chromium.org/1254723005/ by machenbach@chromium.org. ...
5 years, 5 months ago (2015-07-24 08:25:43 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1243053005/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1243053005/1
5 years, 4 months ago (2015-07-27 08:37:28 UTC) #9
commit-bot: I haz the power
Try jobs failed on following builders: v8_linux_rel on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel/builds/7995) v8_mac_rel on tryserver.v8 (JOB_FAILED, ...
5 years, 4 months ago (2015-07-27 08:38:10 UTC) #11
Yang
5 years, 4 months ago (2015-07-27 14:08:21 UTC) #12
On 2015/07/27 08:38:10, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   v8_linux_rel on tryserver.v8 (JOB_FAILED,
> http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel/builds/7995)
>   v8_mac_rel on tryserver.v8 (JOB_FAILED,
> http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel/builds/8104)
>   v8_win_nosnap_shared_compile_rel on tryserver.v8 (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_compil...)

Replaced by https://codereview.chromium.org/1256193002/

Powered by Google App Engine
This is Rietveld 408576698