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

Issue 1094653002: Revert "Factor formal argument parsing into ParserBase" (Closed)

Created:
5 years, 8 months ago by wingo
Modified:
5 years, 8 months ago
Reviewers:
rossberg, marja
CC:
v8-dev, rossberg
Base URL:
https://chromium.googlesource.com/v8/v8@master
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

Revert "Factor formal argument parsing into ParserBase" Revert https://codereview.chromium.org/1078093002/ and follow-on parser patches due to a perf regression. This reverts commit 53ddccfc33f7052429e3261e15a2bbabb05760b3. This reverts commit 71d3213a3f9da3f2ade37fe22ad02d8a658172c2. This reverts commit 0f432ebb76350a69d59edc303c181c8ba1719c96. This reverts commit 1dbc43272954e8cfdf7be9a57c953a74b2a4d9da. R=marja@chromium.org Committed: https://crrev.com/37520d3e0345f209780025bb84f355802abd3ad2 Cr-Commit-Position: refs/heads/master@{#27912}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+345 lines, -577 lines) Patch
M src/messages.js View 1 chunk +1 line, -5 lines 0 comments Download
M src/parser.h View 6 chunks +8 lines, -20 lines 0 comments Download
M src/parser.cc View 6 chunks +158 lines, -172 lines 0 comments Download
M src/preparser.h View 24 chunks +109 lines, -252 lines 0 comments Download
M src/preparser.cc View 2 chunks +55 lines, -12 lines 0 comments Download
M test/cctest/test-parsing.cc View 2 chunks +14 lines, -77 lines 0 comments Download
D test/message/formal-parameters-bad-rest.js View 1 chunk +0 lines, -7 lines 0 comments Download
D test/message/formal-parameters-bad-rest.out View 1 chunk +0 lines, -4 lines 0 comments Download
D test/message/formal-parameters-strict-body.js View 1 chunk +0 lines, -5 lines 0 comments Download
D test/message/formal-parameters-strict-body.out View 1 chunk +0 lines, -4 lines 0 comments Download
D test/message/formal-parameters-trailing-comma.js View 1 chunk +0 lines, -5 lines 0 comments Download
D test/message/formal-parameters-trailing-comma.out View 1 chunk +0 lines, -4 lines 0 comments Download
D test/message/strict-formal-parameters.js View 1 chunk +0 lines, -6 lines 0 comments Download
D test/message/strict-formal-parameters.out View 1 chunk +0 lines, -4 lines 0 comments Download

Messages

Total messages: 11 (2 generated)
wingo
5 years, 8 months ago (2015-04-16 14:51:58 UTC) #1
wingo
I can confirm that this reversion set improves CodeLoad performance.
5 years, 8 months ago (2015-04-16 15:04:19 UTC) #2
wingo
On 2015/04/16 15:04:19, wingo wrote: > I can confirm that this reversion set improves CodeLoad ...
5 years, 8 months ago (2015-04-16 15:09:22 UTC) #3
wingo
On 2015/04/16 15:09:22, wingo wrote: > On 2015/04/16 15:04:19, wingo wrote: > > I can ...
5 years, 8 months ago (2015-04-16 15:25:09 UTC) #4
rossberg
LGTM (rubberstamp) Bummer.
5 years, 8 months ago (2015-04-16 16:19:35 UTC) #6
marja
... maybe it's not so bad. It's still unclear whether this caused regression or not... ...
5 years, 8 months ago (2015-04-16 16:20:54 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1094653002/1
5 years, 8 months ago (2015-04-17 09:27:32 UTC) #9
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 8 months ago (2015-04-17 09:51:26 UTC) #10
commit-bot: I haz the power
5 years, 8 months ago (2015-04-17 09:51:46 UTC) #11
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/37520d3e0345f209780025bb84f355802abd3ad2
Cr-Commit-Position: refs/heads/master@{#27912}

Powered by Google App Engine
This is Rietveld 408576698