Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1470253002:
[heap] Refactor evacuation for young and old gen into visitors. (Closed)
|
Created:
5 years ago by Michael Lippautz Modified:
5 years ago Reviewers:
Hannes Payer (out of office) CC:
v8-reviews_googlegroups.com Base URL:
https://chromium.googlesource.com/v8/v8.git@master Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
Description[heap] Refactor evacuation for young and old gen into visitors.
Create a visitor for evacuating objects for young and old generation. This is
the first step of preparing a task to process, both, newspace and oldspace
pages in parallel.
BUG=chromium:524425
LOG=N
Committed: https://crrev.com/138d9bae5d7014e0d205634a49b5eac3697744c8
Cr-Commit-Position: refs/heads/master@{#32349}
Patch Set 1 #
Total comments: 2
Patch Set 2 : Addressed comment; Added code comments and DCHECK #
Total comments: 2
Patch Set 3 : Addressed comment #Patch Set 4 : Rebase #Patch Set 5 : don't check for WAS_SWEPT, as we never sweep newspace pages #Messages
Total messages: 24 (10 generated)
Description was changed from ========== [heap] Refactor evacuation for young and old gen into visitors. BUG= ========== to ========== [heap] Refactor evacuation for young and old gen into visitors. Create a visitor for evacuating objects for young and old generation. This is the first step of preparing a task to process newspace and oldspace in parallel. BUG= ==========
Description was changed from ========== [heap] Refactor evacuation for young and old gen into visitors. Create a visitor for evacuating objects for young and old generation. This is the first step of preparing a task to process newspace and oldspace in parallel. BUG= ========== to ========== [heap] Refactor evacuation for young and old gen into visitors. Create a visitor for evacuating objects for young and old generation. This is the first step of preparing a task to process newspace and oldspace in parallel. BUG=chromium:524425 LOG=N ==========
PTAL Nothing concurrent happening for newspace as of now.
mlippautz@chromium.org changed reviewers: + hpayer@chromium.org
https://codereview.chromium.org/1470253002/diff/1/src/heap/mark-compact.cc File src/heap/mark-compact.cc (right): https://codereview.chromium.org/1470253002/diff/1/src/heap/mark-compact.cc#ne... src/heap/mark-compact.cc:1644: HeapObjectVisitor* visitor, If this method returns a bool indicating if all objects were iterated successfully, then you would not need the aborted state.
https://codereview.chromium.org/1470253002/diff/1/src/heap/mark-compact.cc File src/heap/mark-compact.cc (right): https://codereview.chromium.org/1470253002/diff/1/src/heap/mark-compact.cc#ne... src/heap/mark-compact.cc:1644: HeapObjectVisitor* visitor, On 2015/11/24 16:07:35, Hannes Payer wrote: > If this method returns a bool indicating if all objects were iterated > successfully, then you would not need the aborted state. Done. Also added a comment in the .h file and a further DCHECK that checks that marking information is still present.
lgtm https://codereview.chromium.org/1470253002/diff/20001/src/heap/mark-compact.cc File src/heap/mark-compact.cc (right): https://codereview.chromium.org/1470253002/diff/20001/src/heap/mark-compact.c... src/heap/mark-compact.cc:3151: IterateLiveObjectsOnPage(p, &new_space_visitor, kClearMarkbits); This one should always return true. Can we DCHECK that?
https://codereview.chromium.org/1470253002/diff/20001/src/heap/mark-compact.cc File src/heap/mark-compact.cc (right): https://codereview.chromium.org/1470253002/diff/20001/src/heap/mark-compact.c... src/heap/mark-compact.cc:3151: IterateLiveObjectsOnPage(p, &new_space_visitor, kClearMarkbits); On 2015/11/26 09:57:32, Hannes Payer wrote: > This one should always return true. Can we DCHECK that? Done.
The CQ bit was checked by mlippautz@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1470253002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1470253002/60001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_mac_rel on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_mac_rel/builds/12383)
The CQ bit was checked by mlippautz@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1470253002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1470253002/80001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by mlippautz@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from hpayer@chromium.org Link to the patchset: https://codereview.chromium.org/1470253002/#ps80001 (title: "don't check for WAS_SWEPT, as we never sweep newspace pages")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1470253002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1470253002/80001
Message was sent while issue was closed.
Committed patchset #5 (id:80001)
Message was sent while issue was closed.
Description was changed from ========== [heap] Refactor evacuation for young and old gen into visitors. Create a visitor for evacuating objects for young and old generation. This is the first step of preparing a task to process, both, newspace and oldspace pages in parallel. BUG=chromium:524425 LOG=N ========== to ========== [heap] Refactor evacuation for young and old gen into visitors. Create a visitor for evacuating objects for young and old generation. This is the first step of preparing a task to process, both, newspace and oldspace pages in parallel. BUG=chromium:524425 LOG=N Committed: https://crrev.com/138d9bae5d7014e0d205634a49b5eac3697744c8 Cr-Commit-Position: refs/heads/master@{#32349} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/138d9bae5d7014e0d205634a49b5eac3697744c8 Cr-Commit-Position: refs/heads/master@{#32349}
Message was sent while issue was closed.
A revert of this CL (patchset #5 id:80001) has been created in https://codereview.chromium.org/1483393002/ by hpayer@chromium.org. The reason for reverting is: Still investigating bad canary.. |
