Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1559873004:
[heap] Buffer counter updates for new space evacuation. (Closed)
|
Created:
4 years, 11 months ago by Michael Lippautz Modified:
4 years, 11 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] Buffer counter updates for new space evacuation.
* Buffer counter updates in the corresponding visitor to allow parallel
processing of pages.
* Fix a bug where we don't keep live bytes in sync when marking is already
finished.
LOG=N
BUG=524425
Committed: https://crrev.com/bfefce1e50e459a5f821b2a755802d6acaf8c85c
Cr-Commit-Position: refs/heads/master@{#33170}
Patch Set 1 #
Total comments: 3
Patch Set 2 : Move verification to VerifyMarking parts #Patch Set 3 : Fix if branch in AdjustLiveBytes and some DCHECKs #Patch Set 4 : Invariant is not holding for now #Patch Set 5 : Switch AdjustLiveBytes to checking whether we are in a HeapIterator #
Messages
Total messages: 33 (16 generated)
mlippautz@chromium.org changed reviewers: + hpayer@chromium.org
https://codereview.chromium.org/1559873004/diff/1/src/heap/mark-compact.cc File src/heap/mark-compact.cc (right): https://codereview.chromium.org/1559873004/diff/1/src/heap/mark-compact.cc#ne... src/heap/mark-compact.cc:3124: DCHECK_EQ(survivors_size, new_space_visitor.promoted_size() + In theory we could remove this check (and the DEBUG sections above) as it only makes sure that live bytes are in sync with what gets promoted/copied. For the parallel case it will likely disappear anyways.
https://codereview.chromium.org/1559873004/diff/1/src/heap/mark-compact.cc File src/heap/mark-compact.cc (right): https://codereview.chromium.org/1559873004/diff/1/src/heap/mark-compact.cc#ne... src/heap/mark-compact.cc:3124: DCHECK_EQ(survivors_size, new_space_visitor.promoted_size() + On 2016/01/04 16:43:15, Michael Lippautz wrote: > In theory we could remove this check (and the DEBUG sections above) as it only > makes sure that live bytes are in sync with what gets promoted/copied. > > For the parallel case it will likely disappear anyways. We could make that CHECK part of verify new_space. This would avoid the DEBUG ugliness.
Patchset #2 (id:20001) has been deleted
Patchset #2 (id:40001) has been deleted
https://codereview.chromium.org/1559873004/diff/1/src/heap/mark-compact.cc File src/heap/mark-compact.cc (right): https://codereview.chromium.org/1559873004/diff/1/src/heap/mark-compact.cc#ne... src/heap/mark-compact.cc:3124: DCHECK_EQ(survivors_size, new_space_visitor.promoted_size() + On 2016/01/04 17:30:45, Hannes Payer wrote: > On 2016/01/04 16:43:15, Michael Lippautz wrote: > > In theory we could remove this check (and the DEBUG sections above) as it only > > makes sure that live bytes are in sync with what gets promoted/copied. > > > > For the parallel case it will likely disappear anyways. > > We could make that CHECK part of verify new_space. This would avoid the DEBUG > ugliness. Moved the CHECK to a sub-part of VerifyMarking. (VerifyEvacuation is too late as we would like to verify marking information.)
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/1559873004/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1559873004/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/13713)
Patchset #3 (id:80001) has been deleted
PTAL
lgtm
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/1559873004/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1559873004/100001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux_dbg on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg/builds/12633)
Description was changed from ========== [heap] Buffer counter updates for new space evacuation. * Buffer counter updates in the corresponding visitor to allow parallel processing of pages. * Fix a bug where we don't keep live bytes in sync when marking is already finished. LOG=N BUG=524425 ========== to ========== [heap] Buffer counter updates for new space evacuation. * Buffer counter updates in the corresponding visitor to allow parallel processing of pages. * Fix a bug where we don't keep live bytes in sync when marking is already finished. LOG=N BUG=524425 ==========
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/1559873004/120001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1559873004/120001
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 to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1559873004/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1559873004/140001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
The CQ bit was checked by mlippautz@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1559873004/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1559873004/140001
Message was sent while issue was closed.
Description was changed from ========== [heap] Buffer counter updates for new space evacuation. * Buffer counter updates in the corresponding visitor to allow parallel processing of pages. * Fix a bug where we don't keep live bytes in sync when marking is already finished. LOG=N BUG=524425 ========== to ========== [heap] Buffer counter updates for new space evacuation. * Buffer counter updates in the corresponding visitor to allow parallel processing of pages. * Fix a bug where we don't keep live bytes in sync when marking is already finished. LOG=N BUG=524425 ==========
Message was sent while issue was closed.
Committed patchset #5 (id:140001)
Message was sent while issue was closed.
Description was changed from ========== [heap] Buffer counter updates for new space evacuation. * Buffer counter updates in the corresponding visitor to allow parallel processing of pages. * Fix a bug where we don't keep live bytes in sync when marking is already finished. LOG=N BUG=524425 ========== to ========== [heap] Buffer counter updates for new space evacuation. * Buffer counter updates in the corresponding visitor to allow parallel processing of pages. * Fix a bug where we don't keep live bytes in sync when marking is already finished. LOG=N BUG=524425 Committed: https://crrev.com/bfefce1e50e459a5f821b2a755802d6acaf8c85c Cr-Commit-Position: refs/heads/master@{#33170} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/bfefce1e50e459a5f821b2a755802d6acaf8c85c Cr-Commit-Position: refs/heads/master@{#33170} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
