Chromium Code Reviews
|
|
Chromium Code Reviews|
Created:
4 years ago by marja Modified:
4 years ago Reviewers:
Toon Verwaest CC:
v8-reviews_googlegroups.com Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
DescriptionParser: store parameters in a ThreadedList instead of ZoneList.
ThreadedList is more memory-efficient than ZoneList. This also enables
us to use ThreadedList when making Preparser track parameters (upcoming
work).
BUG=v8:5501
Committed: https://crrev.com/b31cbbd443a91c18b8e6a5e8681425cb4fcf3ec6
Cr-Commit-Position: refs/heads/master@{#41307}
Patch Set 1 #
Total comments: 3
Patch Set 2 : parameters don't need to be in the zone #Patch Set 3 : oops #
Created: 4 years ago
Messages
Total messages: 26 (19 generated)
The CQ bit was checked by marja@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Parser: put parameters into Zone instead of storing them on the stack. Also store actual parameters in a ThreadedList instead of ZoneList. ThreadedList is more memory-efficient than ZoneList. This also enables us to use ThreadedList when making Preparser track parameters (upcoming work). BUG= ========== to ========== Parser: put parameters into Zone instead of storing them on the stack. Also store actual parameters in a ThreadedList instead of ZoneList. ThreadedList is more memory-efficient than ZoneList. This also enables us to use ThreadedList when making Preparser track parameters (upcoming work). BUG=v8:5501 ==========
marja@chromium.org changed reviewers: + verwaest@chromium.org
ptal
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
overall this looks good, just a Q https://codereview.chromium.org/2531593002/diff/1/src/parsing/parser.cc File src/parsing/parser.cc (right): https://codereview.chromium.org/2531593002/diff/1/src/parsing/parser.cc#newco... src/parsing/parser.cc:953: ParserFormalParameters* formals = Are you sure you need formals to be zone allocated? It's hard to tell from the diff here... https://codereview.chromium.org/2531593002/diff/1/src/parsing/parser.h File src/parsing/parser.h (left): https://codereview.chromium.org/2531593002/diff/1/src/parsing/parser.h#oldcod... src/parsing/parser.h:159: ZoneList<Parameter> params; Oh my... it didn't use Parameter* ... good thing your changes this :)
The CQ bit was checked by marja@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux_chromium_gn_rel on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_chromium_gn_rel/bu...)
https://codereview.chromium.org/2531593002/diff/1/src/parsing/parser.cc File src/parsing/parser.cc (right): https://codereview.chromium.org/2531593002/diff/1/src/parsing/parser.cc#newco... src/parsing/parser.cc:953: ParserFormalParameters* formals = On 2016/11/24 15:09:41, Toon Verwaest wrote: > Are you sure you need formals to be zone allocated? It's hard to tell from the > diff here... Done.
The CQ bit was checked by marja@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
Description was changed from ========== Parser: put parameters into Zone instead of storing them on the stack. Also store actual parameters in a ThreadedList instead of ZoneList. ThreadedList is more memory-efficient than ZoneList. This also enables us to use ThreadedList when making Preparser track parameters (upcoming work). BUG=v8:5501 ========== to ========== Parser: store parameters in a ThreadedList instead of ZoneList. ThreadedList is more memory-efficient than ZoneList. This also enables us to use ThreadedList when making Preparser track parameters (upcoming work). BUG=v8:5501 ==========
The CQ bit was checked by marja@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1480330516077510,
"parent_rev": "336e3ad5adbc13b385dc973b4292966f9cd7fc51", "commit_rev":
"8790daca3f527582714feb732d15a201de2d1118"}
Message was sent while issue was closed.
Description was changed from ========== Parser: store parameters in a ThreadedList instead of ZoneList. ThreadedList is more memory-efficient than ZoneList. This also enables us to use ThreadedList when making Preparser track parameters (upcoming work). BUG=v8:5501 ========== to ========== Parser: store parameters in a ThreadedList instead of ZoneList. ThreadedList is more memory-efficient than ZoneList. This also enables us to use ThreadedList when making Preparser track parameters (upcoming work). BUG=v8:5501 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Parser: store parameters in a ThreadedList instead of ZoneList. ThreadedList is more memory-efficient than ZoneList. This also enables us to use ThreadedList when making Preparser track parameters (upcoming work). BUG=v8:5501 ========== to ========== Parser: store parameters in a ThreadedList instead of ZoneList. ThreadedList is more memory-efficient than ZoneList. This also enables us to use ThreadedList when making Preparser track parameters (upcoming work). BUG=v8:5501 Committed: https://crrev.com/b31cbbd443a91c18b8e6a5e8681425cb4fcf3ec6 Cr-Commit-Position: refs/heads/master@{#41307} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/b31cbbd443a91c18b8e6a5e8681425cb4fcf3ec6 Cr-Commit-Position: refs/heads/master@{#41307} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
