Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2124433002:
[heap] Clean up RawSweep (Closed)
|
Created:
4 years, 5 months ago by Michael Lippautz Modified:
4 years, 5 months ago Reviewers:
Hannes Payer (out of office) CC:
v8-reviews_googlegroups.com, Hannes Payer (out of office), ulan Base URL:
https://chromium.googlesource.com/v8/v8.git@master Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
Description[heap] Clean up RawSweep
- Remove unused flags (SweepingParallelism, SweepingMode)
- Make them runtime parameters rather then template parameters
- Deduce skip list rebuilding from the page itself
BUG=
Committed: https://crrev.com/187f86c589dc97fb1e794edf9d4c0e20af20389b
Cr-Commit-Position: refs/heads/master@{#37502}
Patch Set 1 #Patch Set 2 : More refactoring #Patch Set 3 : Fix crash #Patch Set 4 : Even better fix #
Total comments: 4
Patch Set 5 : Addressed comment #Messages
Total messages: 22 (10 generated)
Description was changed from ========== [heap] Clean up RawSweep BUG= ========== to ========== [heap] Clean up RawSweep - Remove unused flags - Make them runtime parameters rather then template parameters - Deduce skip list rebuilding from the page itself BUG= ==========
Description was changed from ========== [heap] Clean up RawSweep - Remove unused flags - Make them runtime parameters rather then template parameters - Deduce skip list rebuilding from the page itself BUG= ========== to ========== [heap] Clean up RawSweep - Remove unused flags (SweepingParallelism, SweepingMode) - Make them runtime parameters rather then template parameters - Deduce skip list rebuilding from the page itself BUG= ==========
mlippautz@chromium.org changed reviewers: + hpayer@chromium.org
A follow up would add another FreeSpaceTreatmentMode (madvise).
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/v2/patch-status/codereview.chromium.or...
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/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2124433002/diff/60001/src/heap/mark-compact.cc File src/heap/mark-compact.cc (right): https://codereview.chromium.org/2124433002/diff/60001/src/heap/mark-compact.c... src/heap/mark-compact.cc:3435: if (rebuild_skip_list) { Don't you want to keep skip_list != NULL? https://codereview.chromium.org/2124433002/diff/60001/src/heap/mark-compact.h File src/heap/mark-compact.h (right): https://codereview.chromium.org/2124433002/diff/60001/src/heap/mark-compact.h... src/heap/mark-compact.h:410: enum FreeSpaceTreatmentMode { IGNORE_FREE_SPACE, ZAP_FREE_SPACE }; I think nobody is calling ZAP_FREE_SPACE anymore. Can we fix that?
https://codereview.chromium.org/2124433002/diff/60001/src/heap/mark-compact.cc File src/heap/mark-compact.cc (right): https://codereview.chromium.org/2124433002/diff/60001/src/heap/mark-compact.c... src/heap/mark-compact.cc:3435: if (rebuild_skip_list) { On 2016/07/04 11:02:09, Hannes Payer (slow) wrote: > Don't you want to keep skip_list != NULL? It's part of the condition for rebuild_skip_list. https://codereview.chromium.org/2124433002/diff/60001/src/heap/mark-compact.h File src/heap/mark-compact.h (right): https://codereview.chromium.org/2124433002/diff/60001/src/heap/mark-compact.h... src/heap/mark-compact.h:410: enum FreeSpaceTreatmentMode { IGNORE_FREE_SPACE, ZAP_FREE_SPACE }; On 2016/07/04 11:02:09, Hannes Payer (slow) wrote: > I think nobody is calling ZAP_FREE_SPACE anymore. Can we fix that? Done: Used Heap::ShouldZapGarbage() as indicator whether to zap or not.
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/v2/patch-status/codereview.chromium.or...
lgtm
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
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== [heap] Clean up RawSweep - Remove unused flags (SweepingParallelism, SweepingMode) - Make them runtime parameters rather then template parameters - Deduce skip list rebuilding from the page itself BUG= ========== to ========== [heap] Clean up RawSweep - Remove unused flags (SweepingParallelism, SweepingMode) - Make them runtime parameters rather then template parameters - Deduce skip list rebuilding from the page itself BUG= ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001)
Message was sent while issue was closed.
CQ bit was unchecked.
Message was sent while issue was closed.
Description was changed from ========== [heap] Clean up RawSweep - Remove unused flags (SweepingParallelism, SweepingMode) - Make them runtime parameters rather then template parameters - Deduce skip list rebuilding from the page itself BUG= ========== to ========== [heap] Clean up RawSweep - Remove unused flags (SweepingParallelism, SweepingMode) - Make them runtime parameters rather then template parameters - Deduce skip list rebuilding from the page itself BUG= Committed: https://crrev.com/187f86c589dc97fb1e794edf9d4c0e20af20389b Cr-Commit-Position: refs/heads/master@{#37502} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/187f86c589dc97fb1e794edf9d4c0e20af20389b Cr-Commit-Position: refs/heads/master@{#37502} |
