Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1577853007:
[heap] Parallel newspace evacuation, semispace copy, and compaction \o/ (Closed)
|
Created:
4 years, 11 months ago by Michael Lippautz Modified:
4 years, 11 months ago 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] Parallel newspace evacuation, semispace copy, and compaction \o/
All parallelism can be turned off using --predictable, or --noparallel-compaction.
This patch completely parallelizes
- semispace copy: from space -> to space (within newspace)
- newspace evacuation: newspace -> oldspace
- oldspace compaction: oldspace -> oldspace
Previously newspace has been handled sequentially (semispace copy, newspace
evacuation) before compacting oldspace in parallel. However, on a high level
there are no dependencies between those two actions, hence we parallelize them
altogether. We base the number of evacuation tasks on the overall set of
to-be-processed pages (newspace + oldspace compaction pages).
Some low-level details:
- The hard cap on number of tasks has been lifted
- We cache store buffer entries locally before merging them back into the global
StoreBuffer in a finalization phase.
- We cache AllocationSite operations locally before merging them back into the
global pretenuring storage in a finalization phase.
- AllocationSite might be compacted while they would be needed for newspace
evacuation. To mitigate any problems we defer checking allocation sites for
newspace till merging locally buffered data.
CQ_EXTRA_TRYBOTS=tryserver.v8:v8_linux_arm64_gc_stress_dbg,v8_linux_gc_stress_dbg,v8_mac_gc_stress_dbg,v8_linux64_asan_rel,v8_linux64_tsan_rel,v8_mac64_asan_rel
BUG=chromium:524425
LOG=N
R=hpayer@chromium.org, ulan@chromium.org
Committed: https://crrev.com/8f0fd8c0370ae8c5aab56491b879d7e30c329062
Cr-Commit-Position: refs/heads/master@{#33523}
Patch Set 1 : #Patch Set 2 : Fix case where AlloctionSites are compacted before using them for newspace pages #Patch Set 3 : StoreBuffer compaction uses local hashmap instead of counter on page #Patch Set 4 : Add additional check for scan on scavenge page in store buffer and remove unneeded synchronization #Patch Set 5 : Fix compilation and NewSpace.top() handling for FindMemento #Patch Set 6 : Add scalable store buffer based on local buffering #Patch Set 7 : Various non-functional changes #
Total comments: 15
Patch Set 8 : Rebase (2 changes factored out) and addressed comments #
Total comments: 8
Patch Set 9 : Addressed comments #Patch Set 10 : Refactoring #
Total comments: 16
Patch Set 11 : Addressed comments #Patch Set 12 : Rebase + fix compilation #
Total comments: 21
Patch Set 13 : Addressed comments #Patch Set 14 : Addressed comment #Patch Set 15 : Rebase and fix compilation #Patch Set 16 : Disable concurrent sweeping for test expecting # of pages released #
Messages
Total messages: 75 (45 generated)
Description was changed from ========== [heap] Parallel evacuation, semispace copy, and compaction \o/ BUG= ========== to ========== [heap] Parallel newspace evacuation, semispace copy, and compaction \o/ BUG= ==========
Patchset #1 (id:1) has been deleted
Description was changed from ========== [heap] Parallel newspace evacuation, semispace copy, and compaction \o/ BUG= ========== to ========== [heap] Parallel newspace evacuation, semispace copy, and compaction \o/ <TODO> BUG= ==========
Patchset #2 (id:40001) has been deleted
Description was changed from ========== [heap] Parallel newspace evacuation, semispace copy, and compaction \o/ <TODO> BUG= ========== to ========== [heap] Parallel newspace evacuation, semispace copy, and compaction \o/ <TODO> BUG=chromium:524425 LOG=N ==========
Patchset #4 (id:100001) has been deleted
Patchset #6 (id:160001) has been deleted
Patchset #7 (id:200001) has been deleted
mlippautz@chromium.org changed reviewers: + hpayer@chromium.org, ulan@chromium.org
Please take a look! The following changes are in principle dependencies that could independently land before this one: - Removing store_buffer_count on MemoryChunk and replace it with a HashMap at the call cite. - Move to synchronized counters in Heap. - The change how AllocationSite_s are looked up through mementos. - Number of parallel tasks. Let me know if you feel that something should land independently. Performance wise this reduces the actual evacuation part to an avg of 1.8ms from 4.71ms (just for new space) in v8.infinite_scroll https://codereview.chromium.org/1577853007/diff/220001/src/heap/heap-inl.h File src/heap/heap-inl.h (right): https://codereview.chromium.org/1577853007/diff/220001/src/heap/heap-inl.h#ne... src/heap/heap-inl.h:471: AllocationMemento* Heap::FindAllocationMemento(HeapObject* object) { Splitting FindAllocationMemento was needed because we cannot safely access NewSpace.top() in a concurrent world. New space is used concurrently to allocated LABs, so we would need synchronization to safely read top(). (Furthermore the check is only needed when coming from the runtime.) https://codereview.chromium.org/1577853007/diff/220001/src/heap/heap-inl.h#ne... src/heap/heap-inl.h:543: // Entering cached feedback is used in the parallel case. We are not allowed We cannot touch the allocation site (hence GetAllocationSiteUnchecked), because the site might have been evacuated itself in another task. We can safely perform all necessary checks while merging the local info. https://codereview.chromium.org/1577853007/diff/220001/src/heap/mark-compact.cc File src/heap/mark-compact.cc (left): https://codereview.chromium.org/1577853007/diff/220001/src/heap/mark-compact.... src/heap/mark-compact.cc:3131: const int kMaxCompactionTasks = 8; I removed the hard limit here. We are still capped by #pages and #cores-1, so this should be fine. https://codereview.chromium.org/1577853007/diff/220001/src/heap/spaces.cc File src/heap/spaces.cc (right): https://codereview.chromium.org/1577853007/diff/220001/src/heap/spaces.cc#new... src/heap/spaces.cc:2854: if (!is_local()) { I cannot think of a reasonable heuristic to do some sweeping right now so I disabled it for evacuation. We could do something based on live_bytes, e.g., if you cannot sweep at least 1/10 of {size_in_bytes} on a page, do not sweep it. This would limit the amount of sweeping done. https://codereview.chromium.org/1577853007/diff/220001/src/heap/spaces.h File src/heap/spaces.h (right): https://codereview.chromium.org/1577853007/diff/220001/src/heap/spaces.h#newc... src/heap/spaces.h:2995: class CompactionSpaceCollection : public Malloced { This class has become a bit of a mess (contains inputs and output), so I would like to split it into - EvacuationBackends (containing the CompactionSpace_s) - EvacuationResult (containing book keeping, pretenuring feedback, and store buffer) in a refactoring CL. https://codereview.chromium.org/1577853007/diff/220001/src/heap/spaces.h#newc... src/heap/spaces.h:3029: static const int kInitialLocalPretenuringFeedbackCapacity = 256; = the length of the previously used global scratchpad. https://codereview.chromium.org/1577853007/diff/220001/src/heap/store-buffer.h File src/heap/store-buffer.h (right): https://codereview.chromium.org/1577853007/diff/220001/src/heap/store-buffer.... src/heap/store-buffer.h:238: static const int kBufferSize = 16 * KB; Profiled using v8.infinite_scroll.
Description was changed from ========== [heap] Parallel newspace evacuation, semispace copy, and compaction \o/ <TODO> BUG=chromium:524425 LOG=N ========== to ========== [heap] Parallel newspace evacuation, semispace copy, and compaction \o/ This patch completely parallelizes - semispace copy: from space -> to space (within newspace) - newspace evacuation: newspace -> oldspace - oldspace compaction: oldspace -> oldspace Previously newspace has been handled sequentially (semispace copy, newspace evacuation) before compacting oldspace in parallel. However, on a high level there are no dependencies between those two actions, hence we parallelize them altogether. We base the number of evacuation tasks on the overall set of to-be-processed pages (newspace + oldspace compaction pages). Low-level details: - The hard cap on number of tasks has been reduced - We cache store buffer entries locally before merging them back into the global StoreBuffer in a finalization phase. - We cache AllocationSite operations locally before merging them back into the global pretenuring storage in a finalization phase. - AllocationSite might be compacted while they would be needed for newspace evacuation. To mitigate any problems we defer checking allocation sites for newspace till merging locally buffered data. BUG=chromium:524425 LOG=N ==========
Description was changed from ========== [heap] Parallel newspace evacuation, semispace copy, and compaction \o/ This patch completely parallelizes - semispace copy: from space -> to space (within newspace) - newspace evacuation: newspace -> oldspace - oldspace compaction: oldspace -> oldspace Previously newspace has been handled sequentially (semispace copy, newspace evacuation) before compacting oldspace in parallel. However, on a high level there are no dependencies between those two actions, hence we parallelize them altogether. We base the number of evacuation tasks on the overall set of to-be-processed pages (newspace + oldspace compaction pages). Low-level details: - The hard cap on number of tasks has been reduced - We cache store buffer entries locally before merging them back into the global StoreBuffer in a finalization phase. - We cache AllocationSite operations locally before merging them back into the global pretenuring storage in a finalization phase. - AllocationSite might be compacted while they would be needed for newspace evacuation. To mitigate any problems we defer checking allocation sites for newspace till merging locally buffered data. BUG=chromium:524425 LOG=N ========== to ========== [heap] Parallel newspace evacuation, semispace copy, and compaction \o/ This patch completely parallelizes - semispace copy: from space -> to space (within newspace) - newspace evacuation: newspace -> oldspace - oldspace compaction: oldspace -> oldspace Previously newspace has been handled sequentially (semispace copy, newspace evacuation) before compacting oldspace in parallel. However, on a high level there are no dependencies between those two actions, hence we parallelize them altogether. We base the number of evacuation tasks on the overall set of to-be-processed pages (newspace + oldspace compaction pages). Some low-level details: - The hard cap on number of tasks has been reduced - We cache store buffer entries locally before merging them back into the global StoreBuffer in a finalization phase. - We cache AllocationSite operations locally before merging them back into the global pretenuring storage in a finalization phase. - AllocationSite might be compacted while they would be needed for newspace evacuation. To mitigate any problems we defer checking allocation sites for newspace till merging locally buffered data. BUG=chromium:524425 LOG=N ==========
Description was changed from ========== [heap] Parallel newspace evacuation, semispace copy, and compaction \o/ This patch completely parallelizes - semispace copy: from space -> to space (within newspace) - newspace evacuation: newspace -> oldspace - oldspace compaction: oldspace -> oldspace Previously newspace has been handled sequentially (semispace copy, newspace evacuation) before compacting oldspace in parallel. However, on a high level there are no dependencies between those two actions, hence we parallelize them altogether. We base the number of evacuation tasks on the overall set of to-be-processed pages (newspace + oldspace compaction pages). Some low-level details: - The hard cap on number of tasks has been reduced - We cache store buffer entries locally before merging them back into the global StoreBuffer in a finalization phase. - We cache AllocationSite operations locally before merging them back into the global pretenuring storage in a finalization phase. - AllocationSite might be compacted while they would be needed for newspace evacuation. To mitigate any problems we defer checking allocation sites for newspace till merging locally buffered data. BUG=chromium:524425 LOG=N ========== to ========== [heap] Parallel newspace evacuation, semispace copy, and compaction \o/ This patch completely parallelizes - semispace copy: from space -> to space (within newspace) - newspace evacuation: newspace -> oldspace - oldspace compaction: oldspace -> oldspace Previously newspace has been handled sequentially (semispace copy, newspace evacuation) before compacting oldspace in parallel. However, on a high level there are no dependencies between those two actions, hence we parallelize them altogether. We base the number of evacuation tasks on the overall set of to-be-processed pages (newspace + oldspace compaction pages). Some low-level details: - The hard cap on number of tasks has been reduced - We cache store buffer entries locally before merging them back into the global StoreBuffer in a finalization phase. - We cache AllocationSite operations locally before merging them back into the global pretenuring storage in a finalization phase. - AllocationSite might be compacted while they would be needed for newspace evacuation. To mitigate any problems we defer checking allocation sites for newspace till merging locally buffered data. - Use atomics for counter updates in Heap. BUG=chromium:524425 LOG=N ==========
I looked at everything except allocation site changes. Looks good. Few comments: https://codereview.chromium.org/1577853007/diff/220001/src/heap/mark-compact.cc File src/heap/mark-compact.cc (right): https://codereview.chromium.org/1577853007/diff/220001/src/heap/mark-compact.... src/heap/mark-compact.cc:3080: evacuation_candidates_.Add(it.next()); As discussed offline, it would be cleaner to have two separate lists for old and new space. So that we have an invariant the evacuation_candidates_ contains only old space evacuation candidates. https://codereview.chromium.org/1577853007/diff/220001/src/heap/spaces.cc File src/heap/spaces.cc (right): https://codereview.chromium.org/1577853007/diff/220001/src/heap/spaces.cc#new... src/heap/spaces.cc:2844: if (collector->sweeping_in_progress() /*&& !is_local() */) { Debug left over? https://codereview.chromium.org/1577853007/diff/220001/src/heap/spaces.cc#new... src/heap/spaces.cc:2854: if (!is_local()) { On 2016/01/14 19:51:55, Michael Lippautz wrote: > I cannot think of a reasonable heuristic to do some sweeping right now so I > disabled it for evacuation. > > We could do something based on live_bytes, e.g., if you cannot sweep at least > 1/10 of {size_in_bytes} on a page, do not sweep it. This would limit the amount > of sweeping done. As discussed offline, it might help to sweep unconditionally but set the upper limit of the number of pages to sweep. https://codereview.chromium.org/1577853007/diff/220001/src/heap/store-buffer.cc File src/heap/store-buffer.cc (right): https://codereview.chromium.org/1577853007/diff/220001/src/heap/store-buffer.... src/heap/store-buffer.cc:198: static_cast<uint32_t>(reinterpret_cast<uintptr_t>(containing_chunk))); Shifting out the lower zero bits of the address will reduce collisions because the hashmap implementation just looks at the lower bits of the hash.
Patchset #8 (id:240001) has been deleted
Patchset #8 (id:260001) has been deleted
PTAL. Addressed all comments and rebased as 2 trivial changes landed independently: - https://codereview.chromium.org/1585843010/ - https://codereview.chromium.org/1593583002/ I will experiment now with the heuristic for helping sweeping during compaction. https://codereview.chromium.org/1577853007/diff/220001/src/heap/mark-compact.cc File src/heap/mark-compact.cc (right): https://codereview.chromium.org/1577853007/diff/220001/src/heap/mark-compact.... src/heap/mark-compact.cc:3080: evacuation_candidates_.Add(it.next()); On 2016/01/15 10:44:43, ulan wrote: > As discussed offline, it would be cleaner to have two separate lists for old and > new space. So that we have an invariant the evacuation_candidates_ contains only > old space evacuation candidates. Done. https://codereview.chromium.org/1577853007/diff/220001/src/heap/spaces.cc File src/heap/spaces.cc (right): https://codereview.chromium.org/1577853007/diff/220001/src/heap/spaces.cc#new... src/heap/spaces.cc:2844: if (collector->sweeping_in_progress() /*&& !is_local() */) { On 2016/01/15 10:44:43, ulan wrote: > Debug left over? Done. https://codereview.chromium.org/1577853007/diff/220001/src/heap/spaces.cc#new... src/heap/spaces.cc:2854: if (!is_local()) { On 2016/01/15 10:44:43, ulan wrote: > On 2016/01/14 19:51:55, Michael Lippautz wrote: > > I cannot think of a reasonable heuristic to do some sweeping right now so I > > disabled it for evacuation. > > > > We could do something based on live_bytes, e.g., if you cannot sweep at least > > 1/10 of {size_in_bytes} on a page, do not sweep it. This would limit the > amount > > of sweeping done. > > As discussed offline, it might help to sweep unconditionally but set the upper > limit of the number of pages to sweep. Done. Set a limit to 10 pages. Will investigate further. https://codereview.chromium.org/1577853007/diff/220001/src/heap/store-buffer.cc File src/heap/store-buffer.cc (right): https://codereview.chromium.org/1577853007/diff/220001/src/heap/store-buffer.... src/heap/store-buffer.cc:198: static_cast<uint32_t>(reinterpret_cast<uintptr_t>(containing_chunk))); On 2016/01/15 10:44:43, ulan wrote: > Shifting out the lower zero bits of the address will reduce collisions because > the hashmap implementation just looks at the lower bits of the hash. Done in https://codereview.chromium.org/1593583002/
Forgot to mention: We discussed offline that an Evacuator, that bundles all evacuation logic, could make sense as a refactoring.
Description was changed from ========== [heap] Parallel newspace evacuation, semispace copy, and compaction \o/ This patch completely parallelizes - semispace copy: from space -> to space (within newspace) - newspace evacuation: newspace -> oldspace - oldspace compaction: oldspace -> oldspace Previously newspace has been handled sequentially (semispace copy, newspace evacuation) before compacting oldspace in parallel. However, on a high level there are no dependencies between those two actions, hence we parallelize them altogether. We base the number of evacuation tasks on the overall set of to-be-processed pages (newspace + oldspace compaction pages). Some low-level details: - The hard cap on number of tasks has been reduced - We cache store buffer entries locally before merging them back into the global StoreBuffer in a finalization phase. - We cache AllocationSite operations locally before merging them back into the global pretenuring storage in a finalization phase. - AllocationSite might be compacted while they would be needed for newspace evacuation. To mitigate any problems we defer checking allocation sites for newspace till merging locally buffered data. - Use atomics for counter updates in Heap. BUG=chromium:524425 LOG=N ========== to ========== [heap] Parallel newspace evacuation, semispace copy, and compaction \o/ This patch completely parallelizes - semispace copy: from space -> to space (within newspace) - newspace evacuation: newspace -> oldspace - oldspace compaction: oldspace -> oldspace Previously newspace has been handled sequentially (semispace copy, newspace evacuation) before compacting oldspace in parallel. However, on a high level there are no dependencies between those two actions, hence we parallelize them altogether. We base the number of evacuation tasks on the overall set of to-be-processed pages (newspace + oldspace compaction pages). Some low-level details: - The hard cap on number of tasks has been reduced - We cache store buffer entries locally before merging them back into the global StoreBuffer in a finalization phase. - We cache AllocationSite operations locally before merging them back into the global pretenuring storage in a finalization phase. - AllocationSite might be compacted while they would be needed for newspace evacuation. To mitigate any problems we defer checking allocation sites for newspace till merging locally buffered data. BUG=chromium:524425 LOG=N ==========
Description was changed from ========== [heap] Parallel newspace evacuation, semispace copy, and compaction \o/ This patch completely parallelizes - semispace copy: from space -> to space (within newspace) - newspace evacuation: newspace -> oldspace - oldspace compaction: oldspace -> oldspace Previously newspace has been handled sequentially (semispace copy, newspace evacuation) before compacting oldspace in parallel. However, on a high level there are no dependencies between those two actions, hence we parallelize them altogether. We base the number of evacuation tasks on the overall set of to-be-processed pages (newspace + oldspace compaction pages). Some low-level details: - The hard cap on number of tasks has been reduced - We cache store buffer entries locally before merging them back into the global StoreBuffer in a finalization phase. - We cache AllocationSite operations locally before merging them back into the global pretenuring storage in a finalization phase. - AllocationSite might be compacted while they would be needed for newspace evacuation. To mitigate any problems we defer checking allocation sites for newspace till merging locally buffered data. BUG=chromium:524425 LOG=N ========== to ========== [heap] Parallel newspace evacuation, semispace copy, and compaction \o/ This patch completely parallelizes - semispace copy: from space -> to space (within newspace) - newspace evacuation: newspace -> oldspace - oldspace compaction: oldspace -> oldspace Previously newspace has been handled sequentially (semispace copy, newspace evacuation) before compacting oldspace in parallel. However, on a high level there are no dependencies between those two actions, hence we parallelize them altogether. We base the number of evacuation tasks on the overall set of to-be-processed pages (newspace + oldspace compaction pages). Some low-level details: - The hard cap on number of tasks has been reduced - We cache store buffer entries locally before merging them back into the global StoreBuffer in a finalization phase. - We cache AllocationSite operations locally before merging them back into the global pretenuring storage in a finalization phase. - AllocationSite might be compacted while they would be needed for newspace evacuation. To mitigate any problems we defer checking allocation sites for newspace till merging locally buffered data. BUG=chromium:524425 LOG=N R=hpayer@chromium.org, ulan@chromium.org ==========
Description was changed from ========== [heap] Parallel newspace evacuation, semispace copy, and compaction \o/ This patch completely parallelizes - semispace copy: from space -> to space (within newspace) - newspace evacuation: newspace -> oldspace - oldspace compaction: oldspace -> oldspace Previously newspace has been handled sequentially (semispace copy, newspace evacuation) before compacting oldspace in parallel. However, on a high level there are no dependencies between those two actions, hence we parallelize them altogether. We base the number of evacuation tasks on the overall set of to-be-processed pages (newspace + oldspace compaction pages). Some low-level details: - The hard cap on number of tasks has been reduced - We cache store buffer entries locally before merging them back into the global StoreBuffer in a finalization phase. - We cache AllocationSite operations locally before merging them back into the global pretenuring storage in a finalization phase. - AllocationSite might be compacted while they would be needed for newspace evacuation. To mitigate any problems we defer checking allocation sites for newspace till merging locally buffered data. BUG=chromium:524425 LOG=N R=hpayer@chromium.org, ulan@chromium.org ========== to ========== [heap] Parallel newspace evacuation, semispace copy, and compaction \o/ All parallelism can be turned of using --predictable, or --noparallel-compaction. This patch completely parallelizes - semispace copy: from space -> to space (within newspace) - newspace evacuation: newspace -> oldspace - oldspace compaction: oldspace -> oldspace Previously newspace has been handled sequentially (semispace copy, newspace evacuation) before compacting oldspace in parallel. However, on a high level there are no dependencies between those two actions, hence we parallelize them altogether. We base the number of evacuation tasks on the overall set of to-be-processed pages (newspace + oldspace compaction pages). Some low-level details: - The hard cap on number of tasks has been reduced - We cache store buffer entries locally before merging them back into the global StoreBuffer in a finalization phase. - We cache AllocationSite operations locally before merging them back into the global pretenuring storage in a finalization phase. - AllocationSite might be compacted while they would be needed for newspace evacuation. To mitigate any problems we defer checking allocation sites for newspace till merging locally buffered data. BUG=chromium:524425 LOG=N R=hpayer@chromium.org, ulan@chromium.org ==========
Patchset #8 (id:280001) has been deleted
Description was changed from ========== [heap] Parallel newspace evacuation, semispace copy, and compaction \o/ All parallelism can be turned of using --predictable, or --noparallel-compaction. This patch completely parallelizes - semispace copy: from space -> to space (within newspace) - newspace evacuation: newspace -> oldspace - oldspace compaction: oldspace -> oldspace Previously newspace has been handled sequentially (semispace copy, newspace evacuation) before compacting oldspace in parallel. However, on a high level there are no dependencies between those two actions, hence we parallelize them altogether. We base the number of evacuation tasks on the overall set of to-be-processed pages (newspace + oldspace compaction pages). Some low-level details: - The hard cap on number of tasks has been reduced - We cache store buffer entries locally before merging them back into the global StoreBuffer in a finalization phase. - We cache AllocationSite operations locally before merging them back into the global pretenuring storage in a finalization phase. - AllocationSite might be compacted while they would be needed for newspace evacuation. To mitigate any problems we defer checking allocation sites for newspace till merging locally buffered data. BUG=chromium:524425 LOG=N R=hpayer@chromium.org, ulan@chromium.org ========== to ========== [heap] Parallel newspace evacuation, semispace copy, and compaction \o/ All parallelism can be turned of using --predictable, or --noparallel-compaction. This patch completely parallelizes - semispace copy: from space -> to space (within newspace) - newspace evacuation: newspace -> oldspace - oldspace compaction: oldspace -> oldspace Previously newspace has been handled sequentially (semispace copy, newspace evacuation) before compacting oldspace in parallel. However, on a high level there are no dependencies between those two actions, hence we parallelize them altogether. We base the number of evacuation tasks on the overall set of to-be-processed pages (newspace + oldspace compaction pages). Some low-level details: - The hard cap on number of tasks has been lifted - We cache store buffer entries locally before merging them back into the global StoreBuffer in a finalization phase. - We cache AllocationSite operations locally before merging them back into the global pretenuring storage in a finalization phase. - AllocationSite might be compacted while they would be needed for newspace evacuation. To mitigate any problems we defer checking allocation sites for newspace till merging locally buffered data. BUG=chromium:524425 LOG=N R=hpayer@chromium.org, ulan@chromium.org ==========
Patchset #8 (id:300001) has been deleted
Patchset #8 (id:320001) has been deleted
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/1577853007/340001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1577853007/340001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== [heap] Parallel newspace evacuation, semispace copy, and compaction \o/ All parallelism can be turned of using --predictable, or --noparallel-compaction. This patch completely parallelizes - semispace copy: from space -> to space (within newspace) - newspace evacuation: newspace -> oldspace - oldspace compaction: oldspace -> oldspace Previously newspace has been handled sequentially (semispace copy, newspace evacuation) before compacting oldspace in parallel. However, on a high level there are no dependencies between those two actions, hence we parallelize them altogether. We base the number of evacuation tasks on the overall set of to-be-processed pages (newspace + oldspace compaction pages). Some low-level details: - The hard cap on number of tasks has been lifted - We cache store buffer entries locally before merging them back into the global StoreBuffer in a finalization phase. - We cache AllocationSite operations locally before merging them back into the global pretenuring storage in a finalization phase. - AllocationSite might be compacted while they would be needed for newspace evacuation. To mitigate any problems we defer checking allocation sites for newspace till merging locally buffered data. BUG=chromium:524425 LOG=N R=hpayer@chromium.org, ulan@chromium.org ========== to ========== [heap] Parallel newspace evacuation, semispace copy, and compaction \o/ All parallelism can be turned off using --predictable, or --noparallel-compaction. This patch completely parallelizes - semispace copy: from space -> to space (within newspace) - newspace evacuation: newspace -> oldspace - oldspace compaction: oldspace -> oldspace Previously newspace has been handled sequentially (semispace copy, newspace evacuation) before compacting oldspace in parallel. However, on a high level there are no dependencies between those two actions, hence we parallelize them altogether. We base the number of evacuation tasks on the overall set of to-be-processed pages (newspace + oldspace compaction pages). Some low-level details: - The hard cap on number of tasks has been lifted - We cache store buffer entries locally before merging them back into the global StoreBuffer in a finalization phase. - We cache AllocationSite operations locally before merging them back into the global pretenuring storage in a finalization phase. - AllocationSite might be compacted while they would be needed for newspace evacuation. To mitigate any problems we defer checking allocation sites for newspace till merging locally buffered data. BUG=chromium:524425 LOG=N R=hpayer@chromium.org, ulan@chromium.org ==========
First round of comments. https://codereview.chromium.org/1577853007/diff/340001/src/heap/heap-inl.h File src/heap/heap-inl.h (right): https://codereview.chromium.org/1577853007/diff/340001/src/heap/heap-inl.h#ne... src/heap/heap-inl.h:471: AllocationMemento* Heap::FindAllocationMemento(HeapObject* object) { As discussed offline: There was a reason why there is just version of that function: to not forget which one is the right one to use. Let's templatize this function based on it's caller, GC or Runtime, and perform the corresponding checks. https://codereview.chromium.org/1577853007/diff/340001/src/heap/heap-inl.h#ne... src/heap/heap-inl.h:545: // till actually merging the data. We could add a similar DCHECK as above: low-level allocation site map check || or is forwarding pointer && high level map check of forwarded object. WDYT? https://codereview.chromium.org/1577853007/diff/340001/src/heap/heap.h File src/heap/heap.h (right): https://codereview.chromium.org/1577853007/diff/340001/src/heap/heap.h#newcod... src/heap/heap.h:2750: class TimedScope { That sounds like a general util class. https://codereview.chromium.org/1577853007/diff/340001/src/heap/mark-compact.cc File src/heap/mark-compact.cc (right): https://codereview.chromium.org/1577853007/diff/340001/src/heap/mark-compact.... src/heap/mark-compact.cc:3793: int max_pages) { We should optimize for sweeping as many pages as possible on the sweeper threads. 1) It should just sweep pages that have many dead objects, i.e. low live bytes. Maybe use a separate data structure that holds pointers to pages in increase live bytes order. 2) I like max_pages, maybe just 1 is good enough? Let's get some data and then optimize for latency. The change should be done in a separate CL to figure out if this regresses OOM.
Patchset #10 (id:380001) has been deleted
PTAL This update includes the refactoring that I was talking about. It properly encapsulates all local state and divides the actual evacuation in 2 phases (local, global). The reason why this was actually necessary (or at least preferable) now is that we cannot update the counters atomically (because of the scavenger), requiring us to change where we actually allocate our visitors. https://codereview.chromium.org/1577853007/diff/340001/src/heap/heap-inl.h File src/heap/heap-inl.h (right): https://codereview.chromium.org/1577853007/diff/340001/src/heap/heap-inl.h#ne... src/heap/heap-inl.h:471: AllocationMemento* Heap::FindAllocationMemento(HeapObject* object) { On 2016/01/18 11:46:33, Hannes Payer wrote: > As discussed offline: > There was a reason why there is just version of that function: to not forget > which one is the right one to use. Let's templatize this function based on it's > caller, GC or Runtime, and perform the corresponding checks. Done. https://codereview.chromium.org/1577853007/diff/340001/src/heap/heap-inl.h#ne... src/heap/heap-inl.h:545: // till actually merging the data. On 2016/01/18 11:46:33, Hannes Payer wrote: > We could add a similar DCHECK as above: > low-level allocation site map check || or is forwarding pointer && high level > map check of forwarded object. > WDYT? I would like to avoid touching the address here. The complete check is done during merging in Heap::MergeAllocationSitePretenuringFeedback. https://codereview.chromium.org/1577853007/diff/340001/src/heap/heap.h File src/heap/heap.h (right): https://codereview.chromium.org/1577853007/diff/340001/src/heap/heap.h#newcod... src/heap/heap.h:2750: class TimedScope { On 2016/01/18 11:46:33, Hannes Payer wrote: > That sounds like a general util class. Done. https://codereview.chromium.org/1577853007/diff/340001/src/heap/mark-compact.cc File src/heap/mark-compact.cc (right): https://codereview.chromium.org/1577853007/diff/340001/src/heap/mark-compact.... src/heap/mark-compact.cc:3793: int max_pages) { On 2016/01/18 11:46:34, Hannes Payer wrote: > We should optimize for sweeping as many pages as possible on the sweeper > threads. > 1) It should just sweep pages that have many dead objects, i.e. low live bytes. > Maybe use a separate data structure that holds pointers to pages in increase > live bytes order. > 2) I like max_pages, maybe just 1 is good enough? > Let's get some data and then optimize for latency. The change should be done in > a separate CL to figure out if this regresses OOM. As discussed offline, this will go in separately: See https://codereview.chromium.org/1596343004/ For reference: (1): Yes, using a variable sized array sorted by live bytes should do it. (2): For latency, yes. For memory, we have to see.
Description was changed from ========== [heap] Parallel newspace evacuation, semispace copy, and compaction \o/ All parallelism can be turned off using --predictable, or --noparallel-compaction. This patch completely parallelizes - semispace copy: from space -> to space (within newspace) - newspace evacuation: newspace -> oldspace - oldspace compaction: oldspace -> oldspace Previously newspace has been handled sequentially (semispace copy, newspace evacuation) before compacting oldspace in parallel. However, on a high level there are no dependencies between those two actions, hence we parallelize them altogether. We base the number of evacuation tasks on the overall set of to-be-processed pages (newspace + oldspace compaction pages). Some low-level details: - The hard cap on number of tasks has been lifted - We cache store buffer entries locally before merging them back into the global StoreBuffer in a finalization phase. - We cache AllocationSite operations locally before merging them back into the global pretenuring storage in a finalization phase. - AllocationSite might be compacted while they would be needed for newspace evacuation. To mitigate any problems we defer checking allocation sites for newspace till merging locally buffered data. BUG=chromium:524425 LOG=N R=hpayer@chromium.org, ulan@chromium.org ========== to ========== [heap] Parallel newspace evacuation, semispace copy, and compaction \o/ All parallelism can be turned off using --predictable, or --noparallel-compaction. This patch completely parallelizes - semispace copy: from space -> to space (within newspace) - newspace evacuation: newspace -> oldspace - oldspace compaction: oldspace -> oldspace Previously newspace has been handled sequentially (semispace copy, newspace evacuation) before compacting oldspace in parallel. However, on a high level there are no dependencies between those two actions, hence we parallelize them altogether. We base the number of evacuation tasks on the overall set of to-be-processed pages (newspace + oldspace compaction pages). Some low-level details: - The hard cap on number of tasks has been lifted - We cache store buffer entries locally before merging them back into the global StoreBuffer in a finalization phase. - We cache AllocationSite operations locally before merging them back into the global pretenuring storage in a finalization phase. - AllocationSite might be compacted while they would be needed for newspace evacuation. To mitigate any problems we defer checking allocation sites for newspace till merging locally buffered data. CQ_EXTRA_TRYBOTS=tryserver.v8:v8_linux_arm64_gc_stress_dbg,v8_linux_gc_stress_dbg,v8_mac_gc_stress_dbg,v8_linux64_asan_rel,v8_linux64_tsan_rel,v8_mac64_asan_rel BUG=chromium:524425 LOG=N R=hpayer@chromium.org, ulan@chromium.org ==========
https://codereview.chromium.org/1577853007/diff/400001/src/heap/heap-inl.h File src/heap/heap-inl.h (right): https://codereview.chromium.org/1577853007/diff/400001/src/heap/heap-inl.h#ne... src/heap/heap-inl.h:471: template <int find_memento_mode> s/int find_memento_mode/Heap:FindMementoMode mode/ https://codereview.chromium.org/1577853007/diff/400001/src/heap/heap-inl.h#ne... src/heap/heap-inl.h:497: UNREACHABLE(); default should be the last case https://codereview.chromium.org/1577853007/diff/400001/src/heap/heap-inl.h#ne... src/heap/heap-inl.h:548: key, static_cast<uint32_t>(bit_cast<uintptr_t>(key) >> 3)); Same as before. https://codereview.chromium.org/1577853007/diff/400001/src/heap/heap.cc File src/heap/heap.cc (right): https://codereview.chromium.org/1577853007/diff/400001/src/heap/heap.cc#newco... src/heap/heap.cc:534: site, static_cast<uint32_t>(bit_cast<uintptr_t>(site) >> 3)); move that magic here into a function. 3 should be a constant in that function. https://codereview.chromium.org/1577853007/diff/400001/src/heap/heap.h File src/heap/heap.h (right): https://codereview.chromium.org/1577853007/diff/400001/src/heap/heap.h#newcod... src/heap/heap.h:641: enum FindMementoMode { kForRuntime, kForParallelEvacuation }; kForGC would be enough detail. https://codereview.chromium.org/1577853007/diff/400001/src/heap/heap.h#newcod... src/heap/heap.h:932: template <int find_memento_mode> s/int find_memento_mode/Heap:FindMementoMode mode/ https://codereview.chromium.org/1577853007/diff/400001/src/heap/mark-compact.cc File src/heap/mark-compact.cc (right): https://codereview.chromium.org/1577853007/diff/400001/src/heap/mark-compact.... src/heap/mark-compact.cc:1728: private: The private section is not very useful anymore. https://codereview.chromium.org/1577853007/diff/400001/src/heap/mark-compact.h File src/heap/mark-compact.h (right): https://codereview.chromium.org/1577853007/diff/400001/src/heap/mark-compact.... src/heap/mark-compact.h:426: int max_pages = 0); Please remove the sweeper change from the CL. This makes the review complicated.
Patchset #11 (id:420001) has been deleted
Patchset #11 (id:440001) has been deleted
Patchset #11 (id:460001) has been deleted
Addressed comments. The sweeper change is a dependency that needs to land before, as otherwise we might end up sweeping the whole heap during compaction. https://codereview.chromium.org/1577853007/diff/400001/src/heap/heap-inl.h File src/heap/heap-inl.h (right): https://codereview.chromium.org/1577853007/diff/400001/src/heap/heap-inl.h#ne... src/heap/heap-inl.h:471: template <int find_memento_mode> On 2016/01/20 13:19:39, Hannes Payer wrote: > s/int find_memento_mode/Heap:FindMementoMode mode/ Done. https://codereview.chromium.org/1577853007/diff/400001/src/heap/heap-inl.h#ne... src/heap/heap-inl.h:497: UNREACHABLE(); On 2016/01/20 13:19:39, Hannes Payer wrote: > default should be the last case Done. https://codereview.chromium.org/1577853007/diff/400001/src/heap/heap-inl.h#ne... src/heap/heap-inl.h:548: key, static_cast<uint32_t>(bit_cast<uintptr_t>(key) >> 3)); On 2016/01/20 13:19:39, Hannes Payer wrote: > Same as before. Done. https://codereview.chromium.org/1577853007/diff/400001/src/heap/heap.cc File src/heap/heap.cc (right): https://codereview.chromium.org/1577853007/diff/400001/src/heap/heap.cc#newco... src/heap/heap.cc:534: site, static_cast<uint32_t>(bit_cast<uintptr_t>(site) >> 3)); On 2016/01/20 13:19:39, Hannes Payer wrote: > move that magic here into a function. 3 should be a constant in that function. Done: Heap::ObjectHash https://codereview.chromium.org/1577853007/diff/400001/src/heap/heap.h File src/heap/heap.h (right): https://codereview.chromium.org/1577853007/diff/400001/src/heap/heap.h#newcod... src/heap/heap.h:641: enum FindMementoMode { kForRuntime, kForParallelEvacuation }; On 2016/01/20 13:19:39, Hannes Payer wrote: > kForGC would be enough detail. Done. https://codereview.chromium.org/1577853007/diff/400001/src/heap/heap.h#newcod... src/heap/heap.h:932: template <int find_memento_mode> On 2016/01/20 13:19:39, Hannes Payer wrote: > s/int find_memento_mode/Heap:FindMementoMode mode/ Done. https://codereview.chromium.org/1577853007/diff/400001/src/heap/mark-compact.cc File src/heap/mark-compact.cc (right): https://codereview.chromium.org/1577853007/diff/400001/src/heap/mark-compact.... src/heap/mark-compact.cc:1728: private: On 2016/01/20 13:19:39, Hannes Payer wrote: > The private section is not very useful anymore. Done. https://codereview.chromium.org/1577853007/diff/400001/src/heap/mark-compact.h File src/heap/mark-compact.h (right): https://codereview.chromium.org/1577853007/diff/400001/src/heap/mark-compact.... src/heap/mark-compact.h:426: int max_pages = 0); On 2016/01/20 13:19:39, Hannes Payer wrote: > Please remove the sweeper change from the CL. This makes the review complicated. Done.
Just nits. LGTM. Awesome! https://codereview.chromium.org/1577853007/diff/500001/src/heap/heap-inl.h File src/heap/heap-inl.h (right): https://codereview.chromium.org/1577853007/diff/500001/src/heap/heap-inl.h#ne... src/heap/heap-inl.h:520: uint32_t Heap::ObjectHash(Address address) { What about moving this function into globals.h? It does not really belong to heap. https://codereview.chromium.org/1577853007/diff/500001/src/heap/mark-compact.cc File src/heap/mark-compact.cc (right): https://codereview.chromium.org/1577853007/diff/500001/src/heap/mark-compact.... src/heap/mark-compact.cc:1593: local_store_buffer_); Symmetrically to the slots buffer we should only pass in the store buffer when moving to an old space. https://codereview.chromium.org/1577853007/diff/500001/src/heap/mark-compact.... src/heap/mark-compact.cc:3161: success = collector_->VisitLiveObjects(p, visitor, kClearMarkbits); DCHECK that new space abortion cannot be aborted. https://codereview.chromium.org/1577853007/diff/500001/src/heap/mark-compact.... src/heap/mark-compact.cc:3173: // There could be popular pages in the list of evacuation candidates not? https://codereview.chromium.org/1577853007/diff/500001/src/heap/mark-compact.... src/heap/mark-compact.cc:3178: return !aborted; Just use success. https://codereview.chromium.org/1577853007/diff/500001/src/heap/mark-compact.... src/heap/mark-compact.cc:3232: ->heap() Let's keep a reference to the heap since we are already passing it in. Then we do not have to call isolate() and go over the isolate. https://codereview.chromium.org/1577853007/diff/500001/src/heap/mark-compact.... src/heap/mark-compact.cc:3258: const int cores = call it available_cores https://codereview.chromium.org/1577853007/diff/500001/src/heap/mark-compact.... src/heap/mark-compact.cc:3268: const int tasks_capped_cores = Min(cores, tasks_capped_pages); just return Min(cores, tasks_capped_pages); https://codereview.chromium.org/1577853007/diff/500001/src/utils-inl.h File src/utils-inl.h (right): https://codereview.chromium.org/1577853007/diff/500001/src/utils-inl.h#newcode1 src/utils-inl.h:1: // Copyright 2015 the V8 project authors. All rights reserved. It is 2016.
https://codereview.chromium.org/1577853007/diff/500001/src/heap/heap-inl.h File src/heap/heap-inl.h (right): https://codereview.chromium.org/1577853007/diff/500001/src/heap/heap-inl.h#ne... src/heap/heap-inl.h:520: uint32_t Heap::ObjectHash(Address address) { On 2016/01/25 16:04:13, Hannes Payer wrote: > What about moving this function into globals.h? It does not really belong to > heap. Done. https://codereview.chromium.org/1577853007/diff/500001/src/heap/mark-compact.cc File src/heap/mark-compact.cc (right): https://codereview.chromium.org/1577853007/diff/500001/src/heap/mark-compact.... src/heap/mark-compact.cc:1593: local_store_buffer_); On 2016/01/25 16:04:13, Hannes Payer wrote: > Symmetrically to the slots buffer we should only pass in the store buffer when > moving to an old space. Done. https://codereview.chromium.org/1577853007/diff/500001/src/heap/mark-compact.... src/heap/mark-compact.cc:3161: success = collector_->VisitLiveObjects(p, visitor, kClearMarkbits); On 2016/01/25 16:04:13, Hannes Payer wrote: > DCHECK that new space abortion cannot be aborted. The caller DCHECKs appropriately that we never abort for newspace pages. (EvacuateSinglePage currently does not care whether it's a newspace or oldspace page.) https://codereview.chromium.org/1577853007/diff/500001/src/heap/mark-compact.... src/heap/mark-compact.cc:3173: // There could be popular pages in the list of evacuation candidates On 2016/01/25 16:04:13, Hannes Payer wrote: > not? Done. https://codereview.chromium.org/1577853007/diff/500001/src/heap/mark-compact.... src/heap/mark-compact.cc:3178: return !aborted; On 2016/01/25 16:04:13, Hannes Payer wrote: > Just use success. Can't do that because in theory we could abort an old space page in between processing many old space pages. This could happen when the sweeper catches up. Furthermore we need to go over all pages (cannot break out) and mark them appropriately to ensure proper finalization later on. https://codereview.chromium.org/1577853007/diff/500001/src/heap/mark-compact.... src/heap/mark-compact.cc:3232: ->heap() On 2016/01/25 16:04:13, Hannes Payer wrote: > Let's keep a reference to the heap since we are already passing it in. Then we > do not have to call isolate() and go over the isolate. Done. https://codereview.chromium.org/1577853007/diff/500001/src/heap/mark-compact.... src/heap/mark-compact.cc:3258: const int cores = On 2016/01/25 16:04:13, Hannes Payer wrote: > call it available_cores Done. https://codereview.chromium.org/1577853007/diff/500001/src/heap/mark-compact.... src/heap/mark-compact.cc:3268: const int tasks_capped_cores = Min(cores, tasks_capped_pages); On 2016/01/25 16:04:13, Hannes Payer wrote: > just return Min(cores, tasks_capped_pages); Done. https://codereview.chromium.org/1577853007/diff/500001/src/utils-inl.h File src/utils-inl.h (right): https://codereview.chromium.org/1577853007/diff/500001/src/utils-inl.h#newcode1 src/utils-inl.h:1: // Copyright 2015 the V8 project authors. All rights reserved. On 2016/01/25 16:04:13, Hannes Payer wrote: > It is 2016. 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/1577853007/520001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1577853007/520001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux_arm64_gc_stress_dbg on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_gc_stress_db...) v8_linux_gcc_compile_rel on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...) v8_linux_mips64el_compile_rel on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mips64el_compile_r...)
https://codereview.chromium.org/1577853007/diff/500001/src/heap/mark-compact.cc File src/heap/mark-compact.cc (right): https://codereview.chromium.org/1577853007/diff/500001/src/heap/mark-compact.... src/heap/mark-compact.cc:3161: success = collector_->VisitLiveObjects(p, visitor, kClearMarkbits); On 2016/01/25 16:26:14, Michael Lippautz wrote: > On 2016/01/25 16:04:13, Hannes Payer wrote: > > DCHECK that new space abortion cannot be aborted. > > The caller DCHECKs appropriately that we never abort for newspace pages. > > (EvacuateSinglePage currently does not care whether it's a newspace or oldspace > page.) Acknowledged. https://codereview.chromium.org/1577853007/diff/500001/src/heap/mark-compact.... src/heap/mark-compact.cc:3178: return !aborted; On 2016/01/25 16:26:15, Michael Lippautz wrote: > On 2016/01/25 16:04:13, Hannes Payer wrote: > > Just use success. > > Can't do that because in theory we could abort an old space page in between > processing many old space pages. This could happen when the sweeper catches up. > Furthermore we need to go over all pages (cannot break out) and mark them > appropriately to ensure proper finalization later on. As discussed offline: it should be possible.
https://codereview.chromium.org/1577853007/diff/500001/src/heap/mark-compact.cc File src/heap/mark-compact.cc (right): https://codereview.chromium.org/1577853007/diff/500001/src/heap/mark-compact.... src/heap/mark-compact.cc:3178: return !aborted; On 2016/01/25 17:07:23, Hannes Payer wrote: > On 2016/01/25 16:26:15, Michael Lippautz wrote: > > On 2016/01/25 16:04:13, Hannes Payer wrote: > > > Just use success. > > > > Can't do that because in theory we could abort an old space page in between > > processing many old space pages. This could happen when the sweeper catches > up. > > Furthermore we need to go over all pages (cannot break out) and mark them > > appropriately to ensure proper finalization later on. > > As discussed offline: it should be possible. 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/1577853007/560001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1577853007/560001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux_gc_stress_dbg on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gc_stress_dbg/buil...)
Patchset #15 (id:560001) has been deleted
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/1577853007/580001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1577853007/580001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux64_asan_rel on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel/builds/...)
Patchset #15 (id:580001) has been deleted
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/1577853007/600001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1577853007/600001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux64_avx2_rel on tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel/builds/...)
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/1577853007/620001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1577853007/620001
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
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/1577853007/#ps620001 (title: "Disable concurrent sweeping for test expecting # of pages released")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1577853007/620001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1577853007/620001
Message was sent while issue was closed.
Description was changed from ========== [heap] Parallel newspace evacuation, semispace copy, and compaction \o/ All parallelism can be turned off using --predictable, or --noparallel-compaction. This patch completely parallelizes - semispace copy: from space -> to space (within newspace) - newspace evacuation: newspace -> oldspace - oldspace compaction: oldspace -> oldspace Previously newspace has been handled sequentially (semispace copy, newspace evacuation) before compacting oldspace in parallel. However, on a high level there are no dependencies between those two actions, hence we parallelize them altogether. We base the number of evacuation tasks on the overall set of to-be-processed pages (newspace + oldspace compaction pages). Some low-level details: - The hard cap on number of tasks has been lifted - We cache store buffer entries locally before merging them back into the global StoreBuffer in a finalization phase. - We cache AllocationSite operations locally before merging them back into the global pretenuring storage in a finalization phase. - AllocationSite might be compacted while they would be needed for newspace evacuation. To mitigate any problems we defer checking allocation sites for newspace till merging locally buffered data. CQ_EXTRA_TRYBOTS=tryserver.v8:v8_linux_arm64_gc_stress_dbg,v8_linux_gc_stress_dbg,v8_mac_gc_stress_dbg,v8_linux64_asan_rel,v8_linux64_tsan_rel,v8_mac64_asan_rel BUG=chromium:524425 LOG=N R=hpayer@chromium.org, ulan@chromium.org ========== to ========== [heap] Parallel newspace evacuation, semispace copy, and compaction \o/ All parallelism can be turned off using --predictable, or --noparallel-compaction. This patch completely parallelizes - semispace copy: from space -> to space (within newspace) - newspace evacuation: newspace -> oldspace - oldspace compaction: oldspace -> oldspace Previously newspace has been handled sequentially (semispace copy, newspace evacuation) before compacting oldspace in parallel. However, on a high level there are no dependencies between those two actions, hence we parallelize them altogether. We base the number of evacuation tasks on the overall set of to-be-processed pages (newspace + oldspace compaction pages). Some low-level details: - The hard cap on number of tasks has been lifted - We cache store buffer entries locally before merging them back into the global StoreBuffer in a finalization phase. - We cache AllocationSite operations locally before merging them back into the global pretenuring storage in a finalization phase. - AllocationSite might be compacted while they would be needed for newspace evacuation. To mitigate any problems we defer checking allocation sites for newspace till merging locally buffered data. CQ_EXTRA_TRYBOTS=tryserver.v8:v8_linux_arm64_gc_stress_dbg,v8_linux_gc_stress_dbg,v8_mac_gc_stress_dbg,v8_linux64_asan_rel,v8_linux64_tsan_rel,v8_mac64_asan_rel BUG=chromium:524425 LOG=N R=hpayer@chromium.org, ulan@chromium.org ==========
Message was sent while issue was closed.
Committed patchset #16 (id:620001)
Message was sent while issue was closed.
Description was changed from ========== [heap] Parallel newspace evacuation, semispace copy, and compaction \o/ All parallelism can be turned off using --predictable, or --noparallel-compaction. This patch completely parallelizes - semispace copy: from space -> to space (within newspace) - newspace evacuation: newspace -> oldspace - oldspace compaction: oldspace -> oldspace Previously newspace has been handled sequentially (semispace copy, newspace evacuation) before compacting oldspace in parallel. However, on a high level there are no dependencies between those two actions, hence we parallelize them altogether. We base the number of evacuation tasks on the overall set of to-be-processed pages (newspace + oldspace compaction pages). Some low-level details: - The hard cap on number of tasks has been lifted - We cache store buffer entries locally before merging them back into the global StoreBuffer in a finalization phase. - We cache AllocationSite operations locally before merging them back into the global pretenuring storage in a finalization phase. - AllocationSite might be compacted while they would be needed for newspace evacuation. To mitigate any problems we defer checking allocation sites for newspace till merging locally buffered data. CQ_EXTRA_TRYBOTS=tryserver.v8:v8_linux_arm64_gc_stress_dbg,v8_linux_gc_stress_dbg,v8_mac_gc_stress_dbg,v8_linux64_asan_rel,v8_linux64_tsan_rel,v8_mac64_asan_rel BUG=chromium:524425 LOG=N R=hpayer@chromium.org, ulan@chromium.org ========== to ========== [heap] Parallel newspace evacuation, semispace copy, and compaction \o/ All parallelism can be turned off using --predictable, or --noparallel-compaction. This patch completely parallelizes - semispace copy: from space -> to space (within newspace) - newspace evacuation: newspace -> oldspace - oldspace compaction: oldspace -> oldspace Previously newspace has been handled sequentially (semispace copy, newspace evacuation) before compacting oldspace in parallel. However, on a high level there are no dependencies between those two actions, hence we parallelize them altogether. We base the number of evacuation tasks on the overall set of to-be-processed pages (newspace + oldspace compaction pages). Some low-level details: - The hard cap on number of tasks has been lifted - We cache store buffer entries locally before merging them back into the global StoreBuffer in a finalization phase. - We cache AllocationSite operations locally before merging them back into the global pretenuring storage in a finalization phase. - AllocationSite might be compacted while they would be needed for newspace evacuation. To mitigate any problems we defer checking allocation sites for newspace till merging locally buffered data. CQ_EXTRA_TRYBOTS=tryserver.v8:v8_linux_arm64_gc_stress_dbg,v8_linux_gc_stress_dbg,v8_mac_gc_stress_dbg,v8_linux64_asan_rel,v8_linux64_tsan_rel,v8_mac64_asan_rel BUG=chromium:524425 LOG=N R=hpayer@chromium.org, ulan@chromium.org Committed: https://crrev.com/8f0fd8c0370ae8c5aab56491b879d7e30c329062 Cr-Commit-Position: refs/heads/master@{#33523} ==========
Message was sent while issue was closed.
Patchset 16 (id:??) landed as https://crrev.com/8f0fd8c0370ae8c5aab56491b879d7e30c329062 Cr-Commit-Position: refs/heads/master@{#33523}
Message was sent while issue was closed.
https://build.chromium.org/p/chromium.webrtc/builders/Mac%20Tester/builds/496... # # Fatal error in ../../v8/src/heap/heap.cc, line 575 # Check failed: found_count > 0 (0 vs. 0). # ==== C stack trace =============================== 1: V8_Fatal 2: v8::internal::Heap::ProcessPretenuringFeedback() 3: v8::internal::Heap::PerformGarbageCollection(v8::internal::GarbageCollector, v8::GCCallbackFlags) 4: v8::internal::Heap::CollectGarbage(v8::internal::GarbageCollector, char const*, char const*, v8::GCCallbackFlags) 5: v8::internal::Heap::HandleGCRequest() 6: v8::internal::StackGuard::HandleInterrupts() 7: v8::internal::Runtime_StackGuard(int, v8::internal::Object**, v8::internal::Isolate*) Fails on all webrtc chromium bots. Guess it's from this CL?
Message was sent while issue was closed.
A revert of this CL (patchset #16 id:620001) has been created in https://codereview.chromium.org/1643473002/ by machenbach@chromium.org. The reason for reverting is: [Sheriff] Leads to crashes on all webrtc chromium testers, e.g.: https://build.chromium.org/p/chromium.webrtc/builders/Mac%20Tester/builds/49664. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
