From 71643eb9f3c3340817ea4d8a12e8b0512b30e72c Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Wed, 26 Jun 2024 12:01:46 +0800 Subject: [PATCH 01/11] WIP: MS sweep chunk --- src/policy/marksweepspace/native_ms/global.rs | 160 ++++++++++++++++-- src/util/alloc/free_list_allocator.rs | 38 +---- 2 files changed, 151 insertions(+), 47 deletions(-) diff --git a/src/policy/marksweepspace/native_ms/global.rs b/src/policy/marksweepspace/native_ms/global.rs index 06540eb3f3..5a5a47f298 100644 --- a/src/policy/marksweepspace/native_ms/global.rs +++ b/src/policy/marksweepspace/native_ms/global.rs @@ -1,17 +1,18 @@ -use std::sync::Arc; - -use atomic::Ordering; +use std::sync::{ + atomic::{AtomicUsize, Ordering}, + Arc, +}; use crate::{ policy::{marksweepspace::native_ms::*, sft::GCWorkerMutRef}, - scheduler::{GCWorkScheduler, GCWorker}, + scheduler::{GCWorkScheduler, GCWorker, WorkBucketStage}, util::{ copy::CopySemantics, heap::{BlockPageResource, PageResource}, metadata::{self, side_metadata::SideMetadataSpec, MetadataSpec}, ObjectReference, }, - vm::VMBinding, + vm::{ActivePlan, VMBinding}, }; #[cfg(feature = "is_mmtk_object")] @@ -78,6 +79,9 @@ pub struct MarkSweepSpace { /// these block lists in the space. These lists are only filled in the release phase, /// and will be moved to the abandoned lists above at the end of a GC. abandoned_in_gc: Mutex, + /// Count the number of pending release packets during the `Release` stage. + /// If all such packets are done, we can start sweeping chunks. + pending_release_packets: AtomicUsize, } unsafe impl Sync for MarkSweepSpace {} @@ -97,6 +101,15 @@ impl AbandonedBlockLists { } } + fn release_blocks(&mut self, space: &MarkSweepSpace) { + for i in 0..MI_BIN_FULL { + // Release free blocks + self.available[i].release_blocks(space); + self.consumed[i].release_blocks(space); + self.unswept[i].release_blocks(space); + } + } + fn sweep_later(&mut self, space: &MarkSweepSpace) { for i in 0..MI_BIN_FULL { // Release free blocks @@ -127,6 +140,18 @@ impl AbandonedBlockLists { } } + fn recategorize_blocks(&mut self) { + assert!(cfg!(feature = "eager_sweeping")); + for i in 0..MI_BIN_FULL { + for block in self.consumed[i].iter() { + if block.has_free_cells() { + self.consumed[i].remove(block); + self.available[i].push(block); + } + } + } + } + fn merge(&mut self, other: &mut Self) { for i in 0..MI_BIN_FULL { self.available[i].append(&mut other.available[i]); @@ -299,6 +324,7 @@ impl MarkSweepSpace { scheduler, abandoned: Mutex::new(AbandonedBlockLists::new()), abandoned_in_gc: Mutex::new(AbandonedBlockLists::new()), + pending_release_packets: AtomicUsize::new(0), } } @@ -340,16 +366,14 @@ impl MarkSweepSpace { } pub fn release(&mut self) { - if cfg!(feature = "eager_sweeping") { - // For eager sweeping, we have to sweep the lists that are abandoned to these global lists. - let mut abandoned = self.abandoned.lock().unwrap(); - abandoned.sweep(self); - } else { - // For lazy sweeping, we just move blocks from consumed to unswept. When an allocator tries - // to use them, they will sweep the block. - let mut abandoned = self.abandoned.lock().unwrap(); - abandoned.sweep_later(self); - } + let num_mutators = VM::VMActivePlan::number_of_mutators(); + // all ReleaseMutator work packets plus the ReleaseMarkSweepSpace packet + self.pending_release_packets + .store(num_mutators + 1, Ordering::SeqCst); + + let space = unsafe { &*(self as *const Self) }; + let work_packet = ReleaseMarkSweepSpace { space }; + self.scheduler.work_buckets[crate::scheduler::WorkBucketStage::Release].add(work_packet); } pub fn end_of_gc(&mut self) { @@ -419,6 +443,43 @@ impl MarkSweepSpace { pub fn get_abandoned_block_lists_in_gc(&self) -> &Mutex { &self.abandoned_in_gc } + + pub fn release_packet_done(&self) { + let old = self.pending_release_packets.fetch_sub(1, Ordering::SeqCst); + if old == 1 { + let work_packets = self.generate_sweep_tasks(); + self.scheduler.work_buckets[WorkBucketStage::Release].bulk_add(work_packets); + } + } + + fn generate_sweep_tasks(&self) -> Vec>> { + let space = unsafe { &*(self as *const Self) }; + let epilogue = Arc::new(RecategorizeBlocks { + space, + counter: AtomicUsize::new(0), + }); + let tasks = self.chunk_map.generate_tasks(|chunk| { + Box::new(SweepChunk { + space, + chunk, + epilogue: epilogue.clone(), + }) + }); + epilogue.counter.store(tasks.len(), Ordering::SeqCst); + tasks + } + + fn recategorize_blocks(&self) { + // We should now have exclusive access to them. + { + let mut abandoned = self.abandoned.try_lock().unwrap(); + abandoned.recategorize_blocks(); + } + { + let mut abandoned_in_gc = self.abandoned_in_gc.try_lock().unwrap(); + abandoned_in_gc.recategorize_blocks(); + } + } } use crate::scheduler::GCWork; @@ -454,3 +515,72 @@ impl GCWork for PrepareChunkMap { } } } + +struct ReleaseMarkSweepSpace { + space: &'static MarkSweepSpace, +} + +impl GCWork for ReleaseMarkSweepSpace { + fn do_work(&mut self, _worker: &mut GCWorker, _mmtk: &'static MMTK) { + if cfg!(feature = "eager_sweeping") { + // For eager sweeping, we have to sweep the lists that are abandoned to these global lists. + let mut abandoned = self.space.abandoned.lock().unwrap(); + abandoned.release_blocks(self.space); + + self.space.release_packet_done(); + } else { + // For lazy sweeping, we just move blocks from consumed to unswept. When an allocator tries + // to use them, they will sweep the block. + let mut abandoned = self.space.abandoned.lock().unwrap(); + abandoned.sweep_later(self.space); + } + } +} + +/// Chunk sweeping work packet. +struct SweepChunk { + space: &'static MarkSweepSpace, + chunk: Chunk, + /// A destructor invoked when all `SweepChunk` packets are finished. + epilogue: Arc>, +} + +impl GCWork for SweepChunk { + fn do_work(&mut self, worker: &mut GCWorker, mmtk: &'static MMTK) { + if self.space.chunk_map.get(self.chunk) == ChunkState::Allocated { + // number of allocated blocks. + let mut allocated_blocks = 0; + // Iterate over all allocated blocks in this chunk. + for block in self + .chunk + .iter_region::() + .filter(|block| block.get_state() != BlockState::Unallocated) + { + block.sweep::(); + // We have released unmarked blocks in `ReleaseMarkSweepSpace` and `ReleaseMutator`. + allocated_blocks += 1; + } + // Set this chunk as free if there is not live blocks. + if allocated_blocks == 0 { + self.space.chunk_map.set(self.chunk, ChunkState::Free) + } + } + self.epilogue.finish_one_work_packet(); + } +} + +struct RecategorizeBlocks { + space: &'static MarkSweepSpace, + counter: AtomicUsize, +} + +impl RecategorizeBlocks { + /// Called after a related work packet is finished. + fn finish_one_work_packet(&self) { + if 1 == self.counter.fetch_sub(1, Ordering::SeqCst) { + // We've finished releasing all the dead blocks to the BlockPageResource's thread-local queues. + // Now flush the BlockPageResource. + self.space.recategorize_blocks() + } + } +} diff --git a/src/util/alloc/free_list_allocator.rs b/src/util/alloc/free_list_allocator.rs index 2bbd10a64d..e11ee853b3 100644 --- a/src/util/alloc/free_list_allocator.rs +++ b/src/util/alloc/free_list_allocator.rs @@ -428,7 +428,6 @@ impl FreeListAllocator { const ABANDON_BLOCKS_IN_RESET: bool = true; /// Lazy sweeping. We just move all the blocks to the unswept block list. - #[cfg(not(feature = "eager_sweeping"))] fn reset(&mut self) { trace!("reset"); // consumed and available are now unswept @@ -440,7 +439,9 @@ impl FreeListAllocator { let mut sweep_later = |list: &mut BlockList| { list.release_blocks(self.space); - unswept.append(list); + if cfg!(not(feature = "eager_sweeping")) { + unswept.append(list); + } }; sweep_later(&mut self.available_blocks[bin]); @@ -448,40 +449,13 @@ impl FreeListAllocator { sweep_later(&mut self.consumed_blocks[bin]); } - if Self::ABANDON_BLOCKS_IN_RESET { + if Self::ABANDON_BLOCKS_IN_RESET || cfg!(feature = "eager_sweeping") { let mut global = self.space.get_abandoned_block_lists_in_gc().lock().unwrap(); self.abandon_blocks(&mut global); } - } - - /// Eager sweeping. We sweep all the block lists, and move them to available block lists. - #[cfg(feature = "eager_sweeping")] - fn reset(&mut self) { - debug!("reset"); - // sweep all blocks and push consumed onto available list - for bin in 0..MI_BIN_FULL { - // Sweep available blocks - self.available_blocks[bin].release_and_sweep_blocks(self.space); - self.available_blocks_stress[bin].release_and_sweep_blocks(self.space); - - // Sweep consumed blocks, and also push the blocks back to the available list. - self.consumed_blocks[bin].release_and_sweep_blocks(self.space); - if *self.context.options.precise_stress - && self.context.options.is_stress_test_gc_enabled() - { - debug_assert!(*self.context.options.precise_stress); - self.available_blocks_stress[bin].append(&mut self.consumed_blocks[bin]); - } else { - self.available_blocks[bin].append(&mut self.consumed_blocks[bin]); - } - // For eager sweeping, we should not have unswept blocks - assert!(self.unswept_blocks[bin].is_empty()); - } - - if Self::ABANDON_BLOCKS_IN_RESET { - let mut global = self.space.get_abandoned_block_lists_in_gc().lock().unwrap(); - self.abandon_blocks(&mut global); + if cfg!(feature = "eager_sweeping") { + self.space.release_packet_done(); } } From 7045cfbf6553828162afdbd2545e8d4b01e81fe1 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Wed, 26 Jun 2024 12:22:38 +0800 Subject: [PATCH 02/11] assertions and probe --- src/policy/marksweepspace/native_ms/global.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/policy/marksweepspace/native_ms/global.rs b/src/policy/marksweepspace/native_ms/global.rs index 5a5a47f298..a8cf013665 100644 --- a/src/policy/marksweepspace/native_ms/global.rs +++ b/src/policy/marksweepspace/native_ms/global.rs @@ -546,7 +546,7 @@ struct SweepChunk { } impl GCWork for SweepChunk { - fn do_work(&mut self, worker: &mut GCWorker, mmtk: &'static MMTK) { + fn do_work(&mut self, _worker: &mut GCWorker, _mmtk: &'static MMTK) { if self.space.chunk_map.get(self.chunk) == ChunkState::Allocated { // number of allocated blocks. let mut allocated_blocks = 0; @@ -556,10 +556,12 @@ impl GCWork for SweepChunk { .iter_region::() .filter(|block| block.get_state() != BlockState::Unallocated) { + assert!(block.get_state() == BlockState::Marked); block.sweep::(); // We have released unmarked blocks in `ReleaseMarkSweepSpace` and `ReleaseMutator`. allocated_blocks += 1; } + probe!(mmtk, sweep_chunk, allocated_blocks); // Set this chunk as free if there is not live blocks. if allocated_blocks == 0 { self.space.chunk_map.set(self.chunk, ChunkState::Free) From 5268e281bcc022487ce540c87cfc0be9186a01e7 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Wed, 26 Jun 2024 16:24:22 +0800 Subject: [PATCH 03/11] Minor fixes --- src/policy/immix/immixspace.rs | 1 + src/policy/marksweepspace/native_ms/global.rs | 41 +++++++------------ 2 files changed, 16 insertions(+), 26 deletions(-) diff --git a/src/policy/immix/immixspace.rs b/src/policy/immix/immixspace.rs index 3e94ee9c55..fb41b24cfe 100644 --- a/src/policy/immix/immixspace.rs +++ b/src/policy/immix/immixspace.rs @@ -937,6 +937,7 @@ impl GCWork for SweepChunk { allocated_blocks += 1; } } + probe!(mmtk, sweep_chunk, allocated_blocks); // Set this chunk as free if there is not live blocks. if allocated_blocks == 0 { self.space.chunk_map.set(self.chunk, ChunkState::Free) diff --git a/src/policy/marksweepspace/native_ms/global.rs b/src/policy/marksweepspace/native_ms/global.rs index a8cf013665..5e9ef786e2 100644 --- a/src/policy/marksweepspace/native_ms/global.rs +++ b/src/policy/marksweepspace/native_ms/global.rs @@ -123,23 +123,6 @@ impl AbandonedBlockLists { } } - fn sweep(&mut self, space: &MarkSweepSpace) { - for i in 0..MI_BIN_FULL { - self.available[i].release_and_sweep_blocks(space); - self.consumed[i].release_and_sweep_blocks(space); - self.unswept[i].release_and_sweep_blocks(space); - - // As we have swept blocks, move blocks in the unswept list to available or consumed list. - while let Some(block) = self.unswept[i].pop() { - if block.has_free_cells() { - self.available[i].push(block); - } else { - self.consumed[i].push(block); - } - } - } - } - fn recategorize_blocks(&mut self) { assert!(cfg!(feature = "eager_sweeping")); for i in 0..MI_BIN_FULL { @@ -366,11 +349,15 @@ impl MarkSweepSpace { } pub fn release(&mut self) { - let num_mutators = VM::VMActivePlan::number_of_mutators(); - // all ReleaseMutator work packets plus the ReleaseMarkSweepSpace packet - self.pending_release_packets - .store(num_mutators + 1, Ordering::SeqCst); + if cfg!(feature = "eager_sweeping") { + let num_mutators = VM::VMActivePlan::number_of_mutators(); + // all ReleaseMutator work packets plus the ReleaseMarkSweepSpace packet + self.pending_release_packets + .store(num_mutators + 1, Ordering::SeqCst); + } + // Do work in separate work packet in order not to slow down the `Release` work packet which + // blocks all `ReleaseMutator` packets. let space = unsafe { &*(self as *const Self) }; let work_packet = ReleaseMarkSweepSpace { space }; self.scheduler.work_buckets[crate::scheduler::WorkBucketStage::Release].add(work_packet); @@ -470,7 +457,6 @@ impl MarkSweepSpace { } fn recategorize_blocks(&self) { - // We should now have exclusive access to them. { let mut abandoned = self.abandoned.try_lock().unwrap(); abandoned.recategorize_blocks(); @@ -523,7 +509,8 @@ struct ReleaseMarkSweepSpace { impl GCWork for ReleaseMarkSweepSpace { fn do_work(&mut self, _worker: &mut GCWorker, _mmtk: &'static MMTK) { if cfg!(feature = "eager_sweeping") { - // For eager sweeping, we have to sweep the lists that are abandoned to these global lists. + // For eager sweeping, we release unmarked blocks, and leave marked blocks to be swept + // later in the `SweepChunk` work packet. let mut abandoned = self.space.abandoned.lock().unwrap(); abandoned.release_blocks(self.space); @@ -537,7 +524,8 @@ impl GCWork for ReleaseMarkSweepSpace { } } -/// Chunk sweeping work packet. +/// Chunk sweeping work packet. Only used by eager sweeping to sweep marked blocks after unmarked +/// blocks have been released. struct SweepChunk { space: &'static MarkSweepSpace, chunk: Chunk, @@ -556,9 +544,10 @@ impl GCWork for SweepChunk { .iter_region::() .filter(|block| block.get_state() != BlockState::Unallocated) { - assert!(block.get_state() == BlockState::Marked); - block.sweep::(); // We have released unmarked blocks in `ReleaseMarkSweepSpace` and `ReleaseMutator`. + // We shouldn't see any unmarked blocks now. + debug_assert_eq!(block.get_state(), BlockState::Marked); + block.sweep::(); allocated_blocks += 1; } probe!(mmtk, sweep_chunk, allocated_blocks); From 7638c4677e19b87d81b4525b2c2728ad61c11eeb Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Wed, 26 Jun 2024 17:13:44 +0800 Subject: [PATCH 04/11] Remove ABANDON_BLOCKS_IN_RESET --- src/util/alloc/free_list_allocator.rs | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/util/alloc/free_list_allocator.rs b/src/util/alloc/free_list_allocator.rs index e11ee853b3..0eb0a4fafb 100644 --- a/src/util/alloc/free_list_allocator.rs +++ b/src/util/alloc/free_list_allocator.rs @@ -421,12 +421,6 @@ impl FreeListAllocator { self.reset(); } - /// Do we abandon allocator local blocks in reset? - /// We should do this for GC. Otherwise, blocks will be held by each allocator, and they cannot - /// be reused by other allocators. This is measured to cause up to 100% increase of the min heap size - /// for mark sweep. - const ABANDON_BLOCKS_IN_RESET: bool = true; - /// Lazy sweeping. We just move all the blocks to the unswept block list. fn reset(&mut self) { trace!("reset"); @@ -434,11 +428,16 @@ impl FreeListAllocator { for bin in 0..MI_BIN_FULL { let unswept = self.unswept_blocks.get_mut(bin).unwrap(); - // If we abandon all the local blocks, we should have no unswept blocks here. Blocks should be either in available, or consumed. - debug_assert!(!Self::ABANDON_BLOCKS_IN_RESET || unswept.is_empty()); + // If we do eager sweeping, we should have no unswept blocks, either. + debug_assert!(!cfg!(feature = "eager_sweeping") || unswept.is_empty()); let mut sweep_later = |list: &mut BlockList| { list.release_blocks(self.space); + // Note: when sweeping eagerly, we leave the blocks in the available and consumed + // block lists. After sweeping, we will "recategorize" consumed blocks because some + // of them will become available again. + // When sweeping lazily, we move the blocks to the unswept list so that they will be + // swept by mutators. if cfg!(not(feature = "eager_sweeping")) { unswept.append(list); } @@ -449,12 +448,11 @@ impl FreeListAllocator { sweep_later(&mut self.consumed_blocks[bin]); } - if Self::ABANDON_BLOCKS_IN_RESET || cfg!(feature = "eager_sweeping") { + // Note: when sweeping eagerly, we abandon block lists immediately so that we don't need to + // go through all mutators after sweeping. + if cfg!(feature = "eager_sweeping") { let mut global = self.space.get_abandoned_block_lists_in_gc().lock().unwrap(); self.abandon_blocks(&mut global); - } - - if cfg!(feature = "eager_sweeping") { self.space.release_packet_done(); } } From 7e8b17178c4c12c39dc69dbadabac9672510cfd6 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Wed, 26 Jun 2024 17:18:36 +0800 Subject: [PATCH 05/11] Remove unnecessary assertion --- src/policy/marksweepspace/native_ms/global.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/policy/marksweepspace/native_ms/global.rs b/src/policy/marksweepspace/native_ms/global.rs index 5e9ef786e2..2e12ae7d75 100644 --- a/src/policy/marksweepspace/native_ms/global.rs +++ b/src/policy/marksweepspace/native_ms/global.rs @@ -124,7 +124,6 @@ impl AbandonedBlockLists { } fn recategorize_blocks(&mut self) { - assert!(cfg!(feature = "eager_sweeping")); for i in 0..MI_BIN_FULL { for block in self.consumed[i].iter() { if block.has_free_cells() { From e9b787f5efcd6df1aa2676a1ae803c5001acec62 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Wed, 26 Jun 2024 17:43:14 +0800 Subject: [PATCH 06/11] Minor adjustments --- src/policy/marksweepspace/native_ms/global.rs | 41 ++++++++----------- src/util/alloc/free_list_allocator.rs | 23 ++++++----- 2 files changed, 31 insertions(+), 33 deletions(-) diff --git a/src/policy/marksweepspace/native_ms/global.rs b/src/policy/marksweepspace/native_ms/global.rs index 2e12ae7d75..acb2cb677a 100644 --- a/src/policy/marksweepspace/native_ms/global.rs +++ b/src/policy/marksweepspace/native_ms/global.rs @@ -101,25 +101,27 @@ impl AbandonedBlockLists { } } - fn release_blocks(&mut self, space: &MarkSweepSpace) { - for i in 0..MI_BIN_FULL { - // Release free blocks - self.available[i].release_blocks(space); - self.consumed[i].release_blocks(space); - self.unswept[i].release_blocks(space); - } - } - fn sweep_later(&mut self, space: &MarkSweepSpace) { for i in 0..MI_BIN_FULL { // Release free blocks self.available[i].release_blocks(space); self.consumed[i].release_blocks(space); - self.unswept[i].release_blocks(space); + if cfg!(not(feature = "eager_sweeping")) { + self.unswept[i].release_blocks(space); + } else { + // If we do eager sweeping, we should have no unswept blocks. + debug_assert!(self.unswept[i].is_empty()); + } - // Move remaining blocks to unswept - self.unswept[i].append(&mut self.available[i]); - self.unswept[i].append(&mut self.consumed[i]); + // For eager sweeping, that's it. We just release unmarked blocks, and leave marked + // blocks to be swept later in the `SweepChunk` work packet. + + // For lazy sweeping, we move blocks from available and consumed to unswept. When an + // allocator tries to use them, they will sweep the block. + if cfg!(not(feature = "eager_sweeping")) { + self.unswept[i].append(&mut self.available[i]); + self.unswept[i].append(&mut self.consumed[i]); + } } } @@ -507,18 +509,11 @@ struct ReleaseMarkSweepSpace { impl GCWork for ReleaseMarkSweepSpace { fn do_work(&mut self, _worker: &mut GCWorker, _mmtk: &'static MMTK) { - if cfg!(feature = "eager_sweeping") { - // For eager sweeping, we release unmarked blocks, and leave marked blocks to be swept - // later in the `SweepChunk` work packet. - let mut abandoned = self.space.abandoned.lock().unwrap(); - abandoned.release_blocks(self.space); + let mut abandoned = self.space.abandoned.lock().unwrap(); + abandoned.sweep_later(self.space); + if cfg!(feature = "eager_sweeping") { self.space.release_packet_done(); - } else { - // For lazy sweeping, we just move blocks from consumed to unswept. When an allocator tries - // to use them, they will sweep the block. - let mut abandoned = self.space.abandoned.lock().unwrap(); - abandoned.sweep_later(self.space); } } } diff --git a/src/util/alloc/free_list_allocator.rs b/src/util/alloc/free_list_allocator.rs index 0eb0a4fafb..6b1f99aa37 100644 --- a/src/util/alloc/free_list_allocator.rs +++ b/src/util/alloc/free_list_allocator.rs @@ -424,20 +424,20 @@ impl FreeListAllocator { /// Lazy sweeping. We just move all the blocks to the unswept block list. fn reset(&mut self) { trace!("reset"); - // consumed and available are now unswept for bin in 0..MI_BIN_FULL { let unswept = self.unswept_blocks.get_mut(bin).unwrap(); - // If we do eager sweeping, we should have no unswept blocks, either. + // If we do eager sweeping, we should have no unswept blocks. debug_assert!(!cfg!(feature = "eager_sweeping") || unswept.is_empty()); let mut sweep_later = |list: &mut BlockList| { list.release_blocks(self.space); - // Note: when sweeping eagerly, we leave the blocks in the available and consumed - // block lists. After sweeping, we will "recategorize" consumed blocks because some - // of them will become available again. - // When sweeping lazily, we move the blocks to the unswept list so that they will be - // swept by mutators. + + // For eager sweeping, that's it. We just release unmarked blocks, and leave marked + // blocks to be swept later in the `SweepChunk` work packet. + + // For lazy sweeping, we move blocks from available and consumed to unswept. When + // an allocator tries to use them, they will sweep the block. if cfg!(not(feature = "eager_sweeping")) { unswept.append(list); } @@ -448,11 +448,14 @@ impl FreeListAllocator { sweep_later(&mut self.consumed_blocks[bin]); } - // Note: when sweeping eagerly, we abandon block lists immediately so that we don't need to - // go through all mutators after sweeping. - if cfg!(feature = "eager_sweeping") { + // We abandon block lists immediately. Otherwise, some mutators will hold lots of blocks + // locally and prevent other mutators to use. + { let mut global = self.space.get_abandoned_block_lists_in_gc().lock().unwrap(); self.abandon_blocks(&mut global); + } + + if cfg!(feature = "eager_sweeping") { self.space.release_packet_done(); } } From 6e88f30d90fc36271aa09f18ffea2749f2d21ad6 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Wed, 26 Jun 2024 17:54:14 +0800 Subject: [PATCH 07/11] eBPF tracing tools and docs --- tools/tracing/README.md | 3 +++ tools/tracing/timeline/capture.bt | 6 ++++++ tools/tracing/timeline/visualize.py | 6 ++++++ 3 files changed, 15 insertions(+) diff --git a/tools/tracing/README.md b/tools/tracing/README.md index c80e492d13..6ca32bacf3 100644 --- a/tools/tracing/README.md +++ b/tools/tracing/README.md @@ -30,6 +30,9 @@ Currently, the core provides the following tracepoints. - `mmtk:process_slots(num_slots: int, is_roots: bool)`: an invocation of the `process_slots` method. The first argument is the number of slots to be processed, and the second argument is whether these slots are root slots. +- `mmtk:sweep_chunk(allocated_blocks: int)`: an execution of the `SweepChunk` work packet (for + both `MarkSweepSpace` and `ImmixSpace`). `allocated_blocks` is the number of allocated blocks + in the chunk processed by the work packet. - `mmtk:bucket_opened(id: int)`: a work bucket opened. The first argument is the numerical representation of `enum WorkBucketStage`. - `mmtk:work_poll()`: a work packet is to be polled. diff --git a/tools/tracing/timeline/capture.bt b/tools/tracing/timeline/capture.bt index 5315fdb3b7..d0c9d78959 100644 --- a/tools/tracing/timeline/capture.bt +++ b/tools/tracing/timeline/capture.bt @@ -91,4 +91,10 @@ usdt:$MMTK:mmtk:plan_end_of_gc_end { } } +usdt:$MMTK:mmtk:sweep_chunk { + if (@enable_print) { + printf("sweep_chunk,meta,%d,%lu,%lu\n", tid, nsecs, arg0); + } +} + // vim: ft=bpftrace ts=4 sw=4 sts=4 et diff --git a/tools/tracing/timeline/visualize.py b/tools/tracing/timeline/visualize.py index fd9066e143..ad5c47df75 100755 --- a/tools/tracing/timeline/visualize.py +++ b/tools/tracing/timeline/visualize.py @@ -95,6 +95,12 @@ def process_log_line(self, line): current["args"]["num_slots"] = int(rest[0]) current["args"]["is_roots"] = int(rest[1]) + case "sweep_chunk": + current = self.get_current_work_packet(tid) + # eBPF may drop events. Be conservative. + if current is not None: + current["args"]["allocated_blocks"] = int(rest[0]) + if be != "meta": self.results.append(result) From 513cee8a3ac212975673a3f7aefdfd7276c7e16d Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Wed, 26 Jun 2024 17:54:31 +0800 Subject: [PATCH 08/11] SweepChunk now asserts the chunk is allocated. --- src/policy/immix/immixspace.rs | 94 +++++++++---------- src/policy/marksweepspace/native_ms/global.rs | 40 ++++---- 2 files changed, 67 insertions(+), 67 deletions(-) diff --git a/src/policy/immix/immixspace.rs b/src/policy/immix/immixspace.rs index fb41b24cfe..e56761559d 100644 --- a/src/policy/immix/immixspace.rs +++ b/src/policy/immix/immixspace.rs @@ -890,59 +890,59 @@ struct SweepChunk { impl GCWork for SweepChunk { fn do_work(&mut self, _worker: &mut GCWorker, mmtk: &'static MMTK) { + assert_eq!(self.space.chunk_map.get(self.chunk), ChunkState::Allocated); + let mut histogram = self.space.defrag.new_histogram(); - if self.space.chunk_map.get(self.chunk) == ChunkState::Allocated { - let line_mark_state = if super::BLOCK_ONLY { - None - } else { - Some(self.space.line_mark_state.load(Ordering::Acquire)) - }; - // Hints for clearing side forwarding bits. - let is_moving_gc = mmtk.get_plan().current_gc_may_move_object(); - let is_defrag_gc = self.space.defrag.in_defrag(); - // number of allocated blocks. - let mut allocated_blocks = 0; - // Iterate over all allocated blocks in this chunk. - for block in self - .chunk - .iter_region::() - .filter(|block| block.get_state() != BlockState::Unallocated) - { - // Clear side forwarding bits. - // In the beginning of the next GC, no side forwarding bits shall be set. - // In this way, we can omit clearing forwarding bits when copying object. - // See `GCWorkerCopyContext::post_copy`. - // Note, `block.sweep()` overwrites `DEFRAG_STATE_TABLE` with the number of holes, - // but we need it to know if a block is a defrag source. - // We clear forwarding bits before `block.sweep()`. - if let MetadataSpec::OnSide(side) = *VM::VMObjectModel::LOCAL_FORWARDING_BITS_SPEC { - if is_moving_gc { - let objects_may_move = if is_defrag_gc { - // If it is a defrag GC, we only clear forwarding bits for defrag sources. - block.is_defrag_source() - } else { - // Otherwise, it must be a nursery GC of StickyImmix with copying nursery. - // We don't have information about which block contains moved objects, - // so we have to clear forwarding bits for all blocks. - true - }; - if objects_may_move { - side.bzero_metadata(block.start(), Block::BYTES); - } + let line_mark_state = if super::BLOCK_ONLY { + None + } else { + Some(self.space.line_mark_state.load(Ordering::Acquire)) + }; + // Hints for clearing side forwarding bits. + let is_moving_gc = mmtk.get_plan().current_gc_may_move_object(); + let is_defrag_gc = self.space.defrag.in_defrag(); + // number of allocated blocks. + let mut allocated_blocks = 0; + // Iterate over all allocated blocks in this chunk. + for block in self + .chunk + .iter_region::() + .filter(|block| block.get_state() != BlockState::Unallocated) + { + // Clear side forwarding bits. + // In the beginning of the next GC, no side forwarding bits shall be set. + // In this way, we can omit clearing forwarding bits when copying object. + // See `GCWorkerCopyContext::post_copy`. + // Note, `block.sweep()` overwrites `DEFRAG_STATE_TABLE` with the number of holes, + // but we need it to know if a block is a defrag source. + // We clear forwarding bits before `block.sweep()`. + if let MetadataSpec::OnSide(side) = *VM::VMObjectModel::LOCAL_FORWARDING_BITS_SPEC { + if is_moving_gc { + let objects_may_move = if is_defrag_gc { + // If it is a defrag GC, we only clear forwarding bits for defrag sources. + block.is_defrag_source() + } else { + // Otherwise, it must be a nursery GC of StickyImmix with copying nursery. + // We don't have information about which block contains moved objects, + // so we have to clear forwarding bits for all blocks. + true + }; + if objects_may_move { + side.bzero_metadata(block.start(), Block::BYTES); } } - - if !block.sweep(self.space, &mut histogram, line_mark_state) { - // Block is live. Increment the allocated block count. - allocated_blocks += 1; - } } - probe!(mmtk, sweep_chunk, allocated_blocks); - // Set this chunk as free if there is not live blocks. - if allocated_blocks == 0 { - self.space.chunk_map.set(self.chunk, ChunkState::Free) + + if !block.sweep(self.space, &mut histogram, line_mark_state) { + // Block is live. Increment the allocated block count. + allocated_blocks += 1; } } + probe!(mmtk, sweep_chunk, allocated_blocks); + // Set this chunk as free if there is not live blocks. + if allocated_blocks == 0 { + self.space.chunk_map.set(self.chunk, ChunkState::Free) + } self.space.defrag.add_completed_mark_histogram(histogram); self.epilogue.finish_one_work_packet(); } diff --git a/src/policy/marksweepspace/native_ms/global.rs b/src/policy/marksweepspace/native_ms/global.rs index acb2cb677a..0b61a28a1d 100644 --- a/src/policy/marksweepspace/native_ms/global.rs +++ b/src/policy/marksweepspace/native_ms/global.rs @@ -529,26 +529,26 @@ struct SweepChunk { impl GCWork for SweepChunk { fn do_work(&mut self, _worker: &mut GCWorker, _mmtk: &'static MMTK) { - if self.space.chunk_map.get(self.chunk) == ChunkState::Allocated { - // number of allocated blocks. - let mut allocated_blocks = 0; - // Iterate over all allocated blocks in this chunk. - for block in self - .chunk - .iter_region::() - .filter(|block| block.get_state() != BlockState::Unallocated) - { - // We have released unmarked blocks in `ReleaseMarkSweepSpace` and `ReleaseMutator`. - // We shouldn't see any unmarked blocks now. - debug_assert_eq!(block.get_state(), BlockState::Marked); - block.sweep::(); - allocated_blocks += 1; - } - probe!(mmtk, sweep_chunk, allocated_blocks); - // Set this chunk as free if there is not live blocks. - if allocated_blocks == 0 { - self.space.chunk_map.set(self.chunk, ChunkState::Free) - } + assert_eq!(self.space.chunk_map.get(self.chunk), ChunkState::Allocated); + + // number of allocated blocks. + let mut allocated_blocks = 0; + // Iterate over all allocated blocks in this chunk. + for block in self + .chunk + .iter_region::() + .filter(|block| block.get_state() != BlockState::Unallocated) + { + // We have released unmarked blocks in `ReleaseMarkSweepSpace` and `ReleaseMutator`. + // We shouldn't see any unmarked blocks now. + debug_assert_eq!(block.get_state(), BlockState::Marked); + block.sweep::(); + allocated_blocks += 1; + } + probe!(mmtk, sweep_chunk, allocated_blocks); + // Set this chunk as free if there is not live blocks. + if allocated_blocks == 0 { + self.space.chunk_map.set(self.chunk, ChunkState::Free) } self.epilogue.finish_one_work_packet(); } From f1c0592e889588a3ce1e5aeff3b4a57c9d38e6b5 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Wed, 26 Jun 2024 19:51:47 +0800 Subject: [PATCH 09/11] Release lock promptly --- src/policy/marksweepspace/native_ms/global.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/policy/marksweepspace/native_ms/global.rs b/src/policy/marksweepspace/native_ms/global.rs index 0b61a28a1d..076d3ddd91 100644 --- a/src/policy/marksweepspace/native_ms/global.rs +++ b/src/policy/marksweepspace/native_ms/global.rs @@ -509,8 +509,10 @@ struct ReleaseMarkSweepSpace { impl GCWork for ReleaseMarkSweepSpace { fn do_work(&mut self, _worker: &mut GCWorker, _mmtk: &'static MMTK) { - let mut abandoned = self.space.abandoned.lock().unwrap(); - abandoned.sweep_later(self.space); + { + let mut abandoned = self.space.abandoned.lock().unwrap(); + abandoned.sweep_later(self.space); + } if cfg!(feature = "eager_sweeping") { self.space.release_packet_done(); From 2762f69171d41ac53987f71e9754a85b1eae98a6 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Thu, 27 Jun 2024 16:05:38 +0800 Subject: [PATCH 10/11] Update comment to make it more concrete. --- src/policy/marksweepspace/native_ms/global.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/policy/marksweepspace/native_ms/global.rs b/src/policy/marksweepspace/native_ms/global.rs index 076d3ddd91..7c6ba68cdd 100644 --- a/src/policy/marksweepspace/native_ms/global.rs +++ b/src/policy/marksweepspace/native_ms/global.rs @@ -79,8 +79,8 @@ pub struct MarkSweepSpace { /// these block lists in the space. These lists are only filled in the release phase, /// and will be moved to the abandoned lists above at the end of a GC. abandoned_in_gc: Mutex, - /// Count the number of pending release packets during the `Release` stage. - /// If all such packets are done, we can start sweeping chunks. + /// Count the number of pending `ReleaseMarkSweepSpace` and `ReleaseMutator` work packets during + /// the `Release` stage. pending_release_packets: AtomicUsize, } From 28febf092420092dcdfd1cc83bd1fe0c52dd3308 Mon Sep 17 00:00:00 2001 From: Kunshan Wang Date: Thu, 27 Jun 2024 19:40:20 +0800 Subject: [PATCH 11/11] Use release packet counter for lazy sweeping too --- src/plan/marksweep/global.rs | 4 - src/policy/marksweepspace/native_ms/global.rs | 77 +++++++++---------- src/util/alloc/free_list_allocator.rs | 10 +-- 3 files changed, 39 insertions(+), 52 deletions(-) diff --git a/src/plan/marksweep/global.rs b/src/plan/marksweep/global.rs index c8cc1ccd2f..b814de1f03 100644 --- a/src/plan/marksweep/global.rs +++ b/src/plan/marksweep/global.rs @@ -65,10 +65,6 @@ impl Plan for MarkSweep { self.common.release(tls, true); } - fn end_of_gc(&mut self, _tls: VMWorkerThread) { - self.ms.end_of_gc(); - } - fn collection_required(&self, space_full: bool, _space: Option>) -> bool { self.base().collection_required(self, space_full) } diff --git a/src/policy/marksweepspace/native_ms/global.rs b/src/policy/marksweepspace/native_ms/global.rs index 7c6ba68cdd..30c7629905 100644 --- a/src/policy/marksweepspace/native_ms/global.rs +++ b/src/policy/marksweepspace/native_ms/global.rs @@ -125,7 +125,7 @@ impl AbandonedBlockLists { } } - fn recategorize_blocks(&mut self) { + fn recycle_blocks(&mut self) { for i in 0..MI_BIN_FULL { for block in self.consumed[i].iter() { if block.has_free_cells() { @@ -350,12 +350,10 @@ impl MarkSweepSpace { } pub fn release(&mut self) { - if cfg!(feature = "eager_sweeping") { - let num_mutators = VM::VMActivePlan::number_of_mutators(); - // all ReleaseMutator work packets plus the ReleaseMarkSweepSpace packet - self.pending_release_packets - .store(num_mutators + 1, Ordering::SeqCst); - } + let num_mutators = VM::VMActivePlan::number_of_mutators(); + // all ReleaseMutator work packets plus the ReleaseMarkSweepSpace packet + self.pending_release_packets + .store(num_mutators + 1, Ordering::SeqCst); // Do work in separate work packet in order not to slow down the `Release` work packet which // blocks all `ReleaseMutator` packets. @@ -364,20 +362,6 @@ impl MarkSweepSpace { self.scheduler.work_buckets[crate::scheduler::WorkBucketStage::Release].add(work_packet); } - pub fn end_of_gc(&mut self) { - let from = self.abandoned_in_gc.get_mut().unwrap(); - let to = self.abandoned.get_mut().unwrap(); - to.merge(from); - - #[cfg(debug_assertions)] - from.assert_empty(); - - // BlockPageResource uses worker-local block queues to eliminate contention when releasing - // blocks, similar to how the MarkSweepSpace caches blocks in `abandoned_in_gc` before - // returning to the global pool. We flush the BlockPageResource, too. - self.pr.flush_all(); - } - /// Release a block. pub fn release_block(&self, block: Block) { self.block_clear_metadata(block); @@ -435,14 +419,21 @@ impl MarkSweepSpace { pub fn release_packet_done(&self) { let old = self.pending_release_packets.fetch_sub(1, Ordering::SeqCst); if old == 1 { - let work_packets = self.generate_sweep_tasks(); - self.scheduler.work_buckets[WorkBucketStage::Release].bulk_add(work_packets); + if cfg!(feature = "eager_sweeping") { + // When doing eager sweeping, we start sweeing now. + // After sweeping, we will recycle blocks. + let work_packets = self.generate_sweep_tasks(); + self.scheduler.work_buckets[WorkBucketStage::Release].bulk_add(work_packets); + } else { + // When doing lazy sweeping, we recycle blocks now. + self.recycle_blocks(); + } } } fn generate_sweep_tasks(&self) -> Vec>> { let space = unsafe { &*(self as *const Self) }; - let epilogue = Arc::new(RecategorizeBlocks { + let epilogue = Arc::new(RecycleBlocks { space, counter: AtomicUsize::new(0), }); @@ -457,15 +448,28 @@ impl MarkSweepSpace { tasks } - fn recategorize_blocks(&self) { + fn recycle_blocks(&self) { { let mut abandoned = self.abandoned.try_lock().unwrap(); - abandoned.recategorize_blocks(); - } - { let mut abandoned_in_gc = self.abandoned_in_gc.try_lock().unwrap(); - abandoned_in_gc.recategorize_blocks(); + + if cfg!(feature = "eager_sweeping") { + // When doing eager sweeping, previously consumed blocks may become available after + // sweeping. We recycle them. + abandoned.recycle_blocks(); + abandoned_in_gc.recycle_blocks(); + } + + abandoned.merge(&mut abandoned_in_gc); + + #[cfg(debug_assertions)] + abandoned_in_gc.assert_empty(); } + + // BlockPageResource uses worker-local block queues to eliminate contention when releasing + // blocks, similar to how the MarkSweepSpace caches blocks in `abandoned_in_gc` before + // returning to the global pool. We flush the BlockPageResource, too. + self.pr.flush_all(); } } @@ -514,9 +518,7 @@ impl GCWork for ReleaseMarkSweepSpace { abandoned.sweep_later(self.space); } - if cfg!(feature = "eager_sweeping") { - self.space.release_packet_done(); - } + self.space.release_packet_done(); } } @@ -526,7 +528,7 @@ struct SweepChunk { space: &'static MarkSweepSpace, chunk: Chunk, /// A destructor invoked when all `SweepChunk` packets are finished. - epilogue: Arc>, + epilogue: Arc>, } impl GCWork for SweepChunk { @@ -556,18 +558,15 @@ impl GCWork for SweepChunk { } } -struct RecategorizeBlocks { +struct RecycleBlocks { space: &'static MarkSweepSpace, counter: AtomicUsize, } -impl RecategorizeBlocks { - /// Called after a related work packet is finished. +impl RecycleBlocks { fn finish_one_work_packet(&self) { if 1 == self.counter.fetch_sub(1, Ordering::SeqCst) { - // We've finished releasing all the dead blocks to the BlockPageResource's thread-local queues. - // Now flush the BlockPageResource. - self.space.recategorize_blocks() + self.space.recycle_blocks() } } } diff --git a/src/util/alloc/free_list_allocator.rs b/src/util/alloc/free_list_allocator.rs index 6b1f99aa37..9f0e6119b9 100644 --- a/src/util/alloc/free_list_allocator.rs +++ b/src/util/alloc/free_list_allocator.rs @@ -418,12 +418,6 @@ impl FreeListAllocator { pub(crate) fn prepare(&mut self) {} pub(crate) fn release(&mut self) { - self.reset(); - } - - /// Lazy sweeping. We just move all the blocks to the unswept block list. - fn reset(&mut self) { - trace!("reset"); for bin in 0..MI_BIN_FULL { let unswept = self.unswept_blocks.get_mut(bin).unwrap(); @@ -455,9 +449,7 @@ impl FreeListAllocator { self.abandon_blocks(&mut global); } - if cfg!(feature = "eager_sweeping") { - self.space.release_packet_done(); - } + self.space.release_packet_done(); } fn abandon_blocks(&mut self, global: &mut AbandonedBlockLists) {