From 22b05c1cb98e85560b6d0696018e3b1d9a5a469c Mon Sep 17 00:00:00 2001 From: shruti2522 Date: Wed, 11 Mar 2026 22:27:18 +0000 Subject: [PATCH] feat: implement dynamic alignment for ArenaAllocator --- .github/workflows/rust.yml | 2 +- Cargo.lock | 7 + oscars/Cargo.toml | 2 + oscars/src/alloc/arena2/alloc.rs | 26 +- oscars/src/alloc/arena2/mod.rs | 75 ++++- oscars/src/alloc/arena2/tests.rs | 124 ++++++++ oscars/src/alloc/mempool3/alloc.rs | 23 ++ oscars/src/alloc/mempool3/mod.rs | 50 +++- oscars/src/alloc/mempool3/tests.rs | 61 ++++ oscars/src/collectors/mark_sweep/cell.rs | 1 + .../collectors/mark_sweep/gc_collections.rs | 29 +- .../mark_sweep/internals/ephemeron.rs | 2 +- .../collectors/mark_sweep/internals/gc_box.rs | 6 +- .../collectors/mark_sweep/internals/mod.rs | 2 +- .../collectors/mark_sweep/internals/vtable.rs | 14 + oscars/src/collectors/mark_sweep/mod.rs | 126 +++++--- oscars/src/collectors/mark_sweep/tests.rs | 279 ++++++++++++++++++ oscars/src/collectors/mark_sweep/trace.rs | 6 +- .../mark_sweep_arena2/internals/ephemeron.rs | 1 + .../mark_sweep_arena2/internals/gc_box.rs | 4 + .../src/collectors/mark_sweep_arena2/mod.rs | 9 +- oscars/src/lib.rs | 5 +- 22 files changed, 768 insertions(+), 86 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 80c6cfc..0b08ef8 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -67,7 +67,7 @@ jobs: - name: Set up Miri run: cargo +nightly miri setup - name: Run tests under Miri - run: cargo +nightly miri test -p oscars + run: cargo +nightly miri test -p oscars --all-features docs: name: Documentation diff --git a/Cargo.lock b/Cargo.lock index 3270c3f..51cd333 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -331,6 +331,7 @@ dependencies = [ "hashbrown", "oscars_derive", "rustc-hash", + "thin-vec", ] [[package]] @@ -525,6 +526,12 @@ dependencies = [ "syn", ] +[[package]] +name = "thin-vec" +version = "0.2.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "144f754d318415ac792f9d69fc87abbbfc043ce2ef041c60f16ad828f638717d" + [[package]] name = "tinytemplate" version = "1.2.1" diff --git a/oscars/Cargo.toml b/oscars/Cargo.toml index f3c81c3..26e6884 100644 --- a/oscars/Cargo.toml +++ b/oscars/Cargo.toml @@ -8,6 +8,7 @@ allocator-api2 = { version = "0.4.0", optional = true } hashbrown = "0.16.1" oscars_derive = { path = "../oscars_derive", version = "0.1.0" } rustc-hash = "2.1.1" +thin-vec = { version = "0.2", optional = true } [dev-dependencies] criterion = { version = "0.5", features = ["html_reports"] } @@ -32,3 +33,4 @@ default = ["mark_sweep"] std = [] mark_sweep = [] gc_allocator = ["dep:allocator-api2", "mark_sweep"] +thin-vec = ["dep:thin-vec", "mark_sweep"] diff --git a/oscars/src/alloc/arena2/alloc.rs b/oscars/src/alloc/arena2/alloc.rs index 3da34b5..5b5766d 100644 --- a/oscars/src/alloc/arena2/alloc.rs +++ b/oscars/src/alloc/arena2/alloc.rs @@ -203,7 +203,8 @@ impl<'arena, T> ArenaPointer<'arena, T> { /// /// safe because the gc collector owns the arena and keeps it alive pub(crate) unsafe fn extend_lifetime(self) -> ArenaPointer<'static, T> { - ArenaPointer(self.0.extend_lifetime(), PhantomData) + // SAFETY: upheld by caller + ArenaPointer(unsafe { self.0.extend_lifetime() }, PhantomData) } } @@ -341,9 +342,12 @@ impl<'arena> Arena<'arena> { value_ref: &T, ) -> Result { let size = core::mem::size_of::>(); - let alignment = core::mem::align_of_val(value_ref); + let alignment = core::mem::align_of::>(); - assert!(alignment <= self.layout.align()); + // The arena's buffer must be at least as aligned as the value we are storing. + if alignment > self.layout.align() { + return Err(ArenaAllocError::AlignmentNotPossible); + } // Safety: This is safe as `current_offset` must be less then the length // of the buffer. @@ -396,6 +400,22 @@ impl<'arena> Arena<'arena> { } result } + + /// Reset arena to its initial empty state, reusing the existing OS buffer. + /// Must only be called when `run_drop_check()` is true (all items dropped). + pub fn reset(&self) { + debug_assert!( + self.run_drop_check(), + "reset() called on an arena with live items" + ); + // Zero the buffer so stale object graphs are not observable after recycling. + // SAFETY: buffer is valid for the full layout size and was allocated with + // the same layout in try_init. + unsafe { core::ptr::write_bytes(self.buffer.as_ptr(), 0, self.layout.size()) }; + self.flags.set(ArenaState::default()); + self.last_allocation.set(core::ptr::null_mut()); + self.current_offset.set(0); + } } impl<'arena> Drop for Arena<'arena> { diff --git a/oscars/src/alloc/arena2/mod.rs b/oscars/src/alloc/arena2/mod.rs index 91b6a0a..a54cbf2 100644 --- a/oscars/src/alloc/arena2/mod.rs +++ b/oscars/src/alloc/arena2/mod.rs @@ -1,5 +1,7 @@ //! An Arena allocator that manages multiple backing arenas +use core::mem; + use rust_alloc::alloc::LayoutError; use rust_alloc::collections::LinkedList; @@ -48,11 +50,22 @@ const DEFAULT_ARENA_SIZE: usize = 4096; /// Default upper limit of 2MB (2 ^ 21) const DEFAULT_HEAP_THRESHOLD: usize = 2_097_152; +/// Minimum guaranteed alignment for every arena buffer. +const DEFAULT_MIN_ALIGNMENT: usize = 8; + +/// Maximum number of idle arenas held (4 idle pages x 4KB = 16KB of OS memory pressure buffered) +const MAX_RECYCLED_ARENAS: usize = 4; + #[derive(Debug)] pub struct ArenaAllocator<'alloc> { heap_threshold: usize, arena_size: usize, + min_alignment: usize, arenas: LinkedList>, + // empty arenas kept alive to avoid OS reallocation on the next cycle + recycled_arenas: [Option>; MAX_RECYCLED_ARENAS], + // number of idle arenas currently held + recycled_count: usize, } impl<'alloc> Default for ArenaAllocator<'alloc> { @@ -60,7 +73,10 @@ impl<'alloc> Default for ArenaAllocator<'alloc> { Self { heap_threshold: DEFAULT_HEAP_THRESHOLD, arena_size: DEFAULT_ARENA_SIZE, + min_alignment: DEFAULT_MIN_ALIGNMENT, arenas: LinkedList::default(), + recycled_arenas: core::array::from_fn(|_| None), + recycled_count: 0, } } } @@ -74,17 +90,24 @@ impl<'alloc> ArenaAllocator<'alloc> { self.heap_threshold = heap_threshold; self } + /// Override the baseline alignment for every new arena buffer. + pub fn with_min_alignment(mut self, min_alignment: usize) -> Self { + self.min_alignment = min_alignment; + self + } pub fn arenas_len(&self) -> usize { self.arenas.len() } pub fn heap_size(&self) -> usize { + // recycled arenas hold no live objects, exclude them from GC pressure self.arenas_len() * self.arena_size } pub fn is_below_threshold(&self) -> bool { - self.heap_size() <= self.heap_threshold - self.arena_size + // saturating_sub avoids underflow when heap_threshold < arena_size + self.heap_size() <= self.heap_threshold.saturating_sub(self.arena_size) } pub fn increase_threshold(&mut self) { @@ -94,13 +117,13 @@ impl<'alloc> ArenaAllocator<'alloc> { impl<'alloc> ArenaAllocator<'alloc> { pub fn try_alloc(&mut self, value: T) -> Result, ArenaAllocError> { + // Determine the minimum alignment this type requires. + let required_alignment = mem::align_of::>(); + let active = match self.get_active_arena() { Some(arena) => arena, None => { - // TODO: don't hard code alignment - // - // TODO: also, we need a min-alignment - self.initialize_new_arena()?; + self.initialize_new_arena(required_alignment)?; self.get_active_arena().expect("must exist, we just set it") } }; @@ -108,8 +131,12 @@ impl<'alloc> ArenaAllocator<'alloc> { match active.get_allocation_data(&value) { // SAFETY: TODO Ok(data) => unsafe { Ok(active.alloc_unchecked::(value, data)) }, - Err(ArenaAllocError::OutOfMemory) => { - self.initialize_new_arena()?; + // The active arena is either full or was created with an alignment + // that is too small for this type. Either way, close it and spin up + // a fresh arena that satisfies the alignment requirement. + Err(ArenaAllocError::OutOfMemory | ArenaAllocError::AlignmentNotPossible) => { + active.close(); + self.initialize_new_arena(required_alignment)?; let new_active = self.get_active_arena().expect("must exist"); new_active.try_alloc(value) } @@ -127,8 +154,28 @@ impl<'alloc> ArenaAllocator<'alloc> { .transpose() } - pub fn initialize_new_arena(&mut self) -> Result<(), ArenaAllocError> { - let new_arena = Arena::try_init(self.arena_size, 16)?; + /// Initialize a fresh arena, attempting to reuse a recycled one first. + pub fn initialize_new_arena( + &mut self, + required_alignment: usize, + ) -> Result<(), ArenaAllocError> { + let alignment = self.min_alignment.max(required_alignment); + + // Check the recycle list first to avoid an OS allocation. + if self.recycled_count > 0 { + self.recycled_count -= 1; + if let Some(recycled) = self.recycled_arenas[self.recycled_count].take() { + // arena.reset() was already called when it was parked. + // Only reuse if its original alignment satisfies the current requirement, + // otherwise drop it and fall through to a fresh OS allocation. + if recycled.layout.align() >= alignment { + self.arenas.push_front(recycled); + return Ok(()); + } + } + } + + let new_arena = Arena::try_init(self.arena_size, alignment)?; self.arenas.push_front(new_arena); Ok(()) } @@ -138,8 +185,14 @@ impl<'alloc> ArenaAllocator<'alloc> { } pub fn drop_dead_arenas(&mut self) { - for dead_arenas in self.arenas.extract_if(|a| a.run_drop_check()) { - drop(dead_arenas) + for arena in self.arenas.extract_if(|a| a.run_drop_check()) { + if self.recycled_count < MAX_RECYCLED_ARENAS { + //reset in place and park in the reserve. + arena.reset(); + self.recycled_arenas[self.recycled_count] = Some(arena); + self.recycled_count += 1; + } + // else: arena drops here, returning memory to the OS } } diff --git a/oscars/src/alloc/arena2/tests.rs b/oscars/src/alloc/arena2/tests.rs index 48179fa..bc142b2 100644 --- a/oscars/src/alloc/arena2/tests.rs +++ b/oscars/src/alloc/arena2/tests.rs @@ -89,6 +89,71 @@ fn arc_drop() { assert_eq!(allocator.arenas_len(), 0); } +#[test] +fn recycled_arena_avoids_realloc() { + let mut allocator = ArenaAllocator::default().with_arena_size(512); + + let mut ptrs = Vec::new(); + for i in 0..16 { + ptrs.push(allocator.try_alloc(i).unwrap().as_ptr()); + } + assert_eq!(allocator.arenas_len(), 1); + // heap_size counts only live arenas, so capture it while one is active. + let heap_while_live = allocator.heap_size(); + assert_eq!(heap_while_live, 512); + + for mut ptr in ptrs { + unsafe { ptr.as_mut().mark_dropped() }; + } + allocator.drop_dead_arenas(); + + // After recycling, the arena is parked, no live arenas, so heap_size is 0. + assert_eq!(allocator.arenas_len(), 0); + assert_eq!(allocator.heap_size(), 0); + // recycled_count == 1 proves the arena was parked in the recycle slot, not freed to the OS. + assert_eq!(allocator.recycled_count, 1); + + // Allocate again, must reuse the recycled arena without growing OS footprint. + // heap_size returns to the same value as when a live arena was present. + for i in 16..32 { + let _ = allocator.try_alloc(i).unwrap(); + } + assert_eq!(allocator.arenas_len(), 1); + assert_eq!(allocator.heap_size(), heap_while_live); + // recycled_count == 0 proves the recycled slot was consumed rather than a new OS allocation. + assert_eq!(allocator.recycled_count, 0); +} + +#[test] +fn max_recycled_cap_respected() { + let mut allocator = ArenaAllocator::default().with_arena_size(128); + + let mut ptrs_per_arena: Vec>>> = Vec::new(); + + for _ in 0..5 { + let mut ptrs = Vec::new(); + let target_len = allocator.arenas_len() + 1; + while allocator.arenas_len() < target_len { + ptrs.push(allocator.try_alloc(0u64).unwrap().as_ptr()); + } + ptrs_per_arena.push(ptrs); + } + assert_eq!(allocator.arenas_len(), 5); + + for ptrs in ptrs_per_arena { + for mut ptr in ptrs { + unsafe { ptr.as_mut().mark_dropped() }; + } + } + + allocator.drop_dead_arenas(); + + assert_eq!(allocator.arenas_len(), 0); + assert_eq!(allocator.heap_size(), 0); + // The recycled list holds exactly max_recycled pages. + assert_eq!(allocator.recycled_count, 4); +} + // === test for TaggedPtr::as_ptr === // // `TaggedPtr::as_ptr` must use `addr & !MASK` to unconditionally clear the high @@ -119,3 +184,62 @@ fn as_ptr_clears_not_flips_tag_bit() { ptr_a.as_mut().mark_dropped(); } } + +// === test for Dynamic Alignment === // + +#[test] +fn test_over_aligned_type() { + #[repr(C, align(512))] + struct HighlyAligned { + _data: [u8; 128], + } + + let mut allocator = ArenaAllocator::default().with_arena_size(4096); + let ptr = allocator + .try_alloc(HighlyAligned { _data: [0; 128] }) + .unwrap(); + + let addr = ptr.as_ptr().as_ptr() as usize; + assert_eq!(addr % 512, 0); + assert_eq!(allocator.arenas_len(), 1); +} + +#[test] +fn test_alignment_upgrade_after_small_alloc() { + #[repr(C, align(512))] + struct BigAlign([u8; 16]); + + let mut allocator = ArenaAllocator::default().with_arena_size(4096); + + // force the first arena to use 8-byte alignment + let _small = allocator.try_alloc(0u8).unwrap(); + assert_eq!(allocator.arenas_len(), 1); + + let ptr = allocator.try_alloc(BigAlign([0; 16])).unwrap(); + + let addr = ptr.as_ptr().as_ptr() as usize; + assert_eq!(addr % 512, 0); + assert_eq!(allocator.arenas_len(), 2); +} + +#[test] +fn test_alignment_upgrade_on_full_arena() { + #[repr(C, align(512))] + struct BigAlign([u8; 16]); + + let mut allocator = ArenaAllocator::default().with_arena_size(4096); + + // fill the first arena + let mut count = 0usize; + while allocator.arenas_len() < 2 { + let _ = allocator.try_alloc(0u64).unwrap(); + count += 1; + assert!(count < 1024); + } + + let ptr = allocator.try_alloc(BigAlign([0; 16])).unwrap(); + + let addr = ptr.as_ptr().as_ptr() as usize; + assert_eq!(addr % 512, 0); + assert_eq!(allocator.arenas_len(), 3); +} diff --git a/oscars/src/alloc/mempool3/alloc.rs b/oscars/src/alloc/mempool3/alloc.rs index d47c269..f12fa78 100644 --- a/oscars/src/alloc/mempool3/alloc.rs +++ b/oscars/src/alloc/mempool3/alloc.rs @@ -277,6 +277,29 @@ impl SlotPool { pub fn run_drop_check(&self) -> bool { self.live.get() == 0 } + + /// Reset this pool to the empty state it had after `try_init`, reusing the + /// existing OS buffer. Must only be called when `run_drop_check()` is true. + /// + /// After `reset()` the pool is ready for `alloc_slot` without any further + /// OS interaction, avoiding a round trip through the global allocator. + pub fn reset(&self) { + debug_assert_eq!( + self.live.get(), + 0, + "reset() called on a non-empty SlotPool (live = {})", + self.live.get() + ); + // Clear the bitmap so all slots become free again. + // + // SAFETY: buffer is valid for at least `bitmap_bytes` and was + // originally zero initialised in try_init with the same length. + unsafe { + core::ptr::write_bytes(self.buffer.as_ptr(), 0, self.bitmap_bytes); + } + self.bump.set(0); + self.free_list.set(None); + } } impl Drop for SlotPool { diff --git a/oscars/src/alloc/mempool3/mod.rs b/oscars/src/alloc/mempool3/mod.rs index dec54aa..1263e77 100644 --- a/oscars/src/alloc/mempool3/mod.rs +++ b/oscars/src/alloc/mempool3/mod.rs @@ -56,6 +56,10 @@ pub struct PoolAllocator<'alloc> { pub(crate) free_cache: Cell, // per size class cached index of the last pool used by alloc_slot pub(crate) alloc_cache: [Cell; 12], + // empty slot pools kept alive to avoid OS reallocation on the next cycle + pub(crate) recycled_pools: Vec, + // maximum number of idle pages held across all size classes + pub(crate) max_recycled: usize, _marker: core::marker::PhantomData<&'alloc ()>, } @@ -82,6 +86,9 @@ impl<'alloc> Default for PoolAllocator<'alloc> { Cell::new(usize::MAX), Cell::new(usize::MAX), ], + recycled_pools: Vec::new(), + // one idle page per size class keeps memory pressure manageable + max_recycled: SIZE_CLASSES.len(), _marker: core::marker::PhantomData, } } @@ -155,6 +162,29 @@ impl<'alloc> PoolAllocator<'alloc> { } // need a new pool for this size class + // try the recycle list first + // to avoid a round trip through the OS allocator + if let Some(pos) = self + .recycled_pools + .iter() + .rposition(|p| p.slot_size == slot_size) + { + let pool = self.recycled_pools.swap_remove(pos); + // pool.reset() was already called in drop_empty_pools when it was parked + let slot_ptr = pool.alloc_slot().ok_or(PoolAllocError::OutOfMemory)?; + let insert_idx = self.slot_pools.len(); + self.slot_pools.push(pool); + self.alloc_cache[sc_idx].set(insert_idx); + + // SAFETY: slot_ptr was successfully allocated for this size class + return unsafe { + let dst = slot_ptr.as_ptr() as *mut PoolItem; + dst.write(PoolItem(value)); + Ok(PoolPointer::from_raw(NonNull::new_unchecked(dst))) + }; + } + + // Recycle list had no match, allocate a fresh page from the OS. let total = self.page_size.max(slot_size * 4); let new_pool = SlotPool::try_init(slot_size, total, 16)?; self.current_heap_size += new_pool.layout.size(); @@ -267,16 +297,22 @@ impl<'alloc> PoolAllocator<'alloc> { false } - /// drop empty slot pools and bump pages + /// Reclaim slot pool pages that became empty after a GC sweep. + /// + /// Empty pages are parked in a recycle list (up to `max_recycled`) + /// to avoid global allocator round trips on the next allocation. pub fn drop_empty_pools(&mut self) { - self.slot_pools.retain(|p| { - if p.run_drop_check() { - self.current_heap_size = self.current_heap_size.saturating_sub(p.layout.size()); - false + // Drain fully empty slot pools into the recycle list. + for pool in self.slot_pools.extract_if(.., |p| p.run_drop_check()) { + if self.recycled_pools.len() < self.max_recycled { + pool.reset(); + self.recycled_pools.push(pool); } else { - true + self.current_heap_size = self.current_heap_size.saturating_sub(pool.layout.size()); } - }); + } + + // Bump pages have no size class affinity so we always free them. self.bump_pages.retain(|p| { if p.run_drop_check() { self.current_heap_size = self.current_heap_size.saturating_sub(p.layout.size()); diff --git a/oscars/src/alloc/mempool3/tests.rs b/oscars/src/alloc/mempool3/tests.rs index b6b43b5..1991dcc 100644 --- a/oscars/src/alloc/mempool3/tests.rs +++ b/oscars/src/alloc/mempool3/tests.rs @@ -173,3 +173,64 @@ fn slot_count_tight_capacity() { assert_eq!(bitmap_bytes, 8); assert_eq!(slot_count, 7); } + +/// Verify that recycled empty slot pools are reused on the next `try_alloc` +/// without allocating new OS memory, the heap_size should be unchanged. +#[test] +fn recycled_pool_avoids_realloc() { + let mut allocator = PoolAllocator::default().with_page_size(4096); + + let ptrs: Vec<_> = (0u64..16) + .map(|i| allocator.try_alloc(i).unwrap().as_ptr()) + .collect(); + assert_eq!(allocator.slot_pools.len(), 1); + let heap_after_first_alloc = allocator.current_heap_size; + + for ptr in ptrs { + allocator.free_slot(ptr.cast::()); + } + allocator.drop_empty_pools(); + + assert_eq!(allocator.slot_pools.len(), 0); + assert_eq!(allocator.recycled_pools.len(), 1); + assert_eq!(allocator.current_heap_size, heap_after_first_alloc); + + let heap_before_second_alloc = allocator.current_heap_size; + for i in 16u64..32 { + let _ = allocator.try_alloc(i).unwrap(); + } + + assert_eq!(allocator.slot_pools.len(), 1); + assert_eq!(allocator.recycled_pools.len(), 0); + assert_eq!(allocator.current_heap_size, heap_before_second_alloc); +} + +/// Verify that when more pools become empty than `max_recycled` allows, +/// the overflow is freed to the OS. +#[test] +fn max_recycled_cap_respected() { + let mut allocator = PoolAllocator::default().with_page_size(32); + allocator.max_recycled = 0; + + let p1 = allocator.try_alloc(1u64).unwrap().as_ptr(); + let px = allocator.try_alloc(2u64).unwrap().as_ptr(); + let py = allocator.try_alloc(3u64).unwrap().as_ptr(); + assert_eq!(allocator.slot_pools.len(), 1); + + let p2 = allocator.try_alloc(4u64).unwrap().as_ptr(); + assert_eq!(allocator.slot_pools.len(), 2); + + let heap_before = allocator.current_heap_size; + + allocator.free_slot(p1.cast::()); + allocator.free_slot(px.cast::()); + allocator.free_slot(py.cast::()); + allocator.free_slot(p2.cast::()); + + allocator.max_recycled = 1; + allocator.drop_empty_pools(); + + assert_eq!(allocator.slot_pools.len(), 0); + assert_eq!(allocator.recycled_pools.len(), 1); + assert!(allocator.current_heap_size < heap_before); +} diff --git a/oscars/src/collectors/mark_sweep/cell.rs b/oscars/src/collectors/mark_sweep/cell.rs index d14e4fb..ef4c10a 100644 --- a/oscars/src/collectors/mark_sweep/cell.rs +++ b/oscars/src/collectors/mark_sweep/cell.rs @@ -172,6 +172,7 @@ impl GcRefCell { } // returns a raw pointer to the inner value or `None` if currently mutably borrowed + #[allow(dead_code)] pub(crate) fn get_raw(&self) -> Option<*mut T> { match self.borrow.get().borrowed() { BorrowState::Writing => None, diff --git a/oscars/src/collectors/mark_sweep/gc_collections.rs b/oscars/src/collectors/mark_sweep/gc_collections.rs index 87c6e9f..15ae612 100644 --- a/oscars/src/collectors/mark_sweep/gc_collections.rs +++ b/oscars/src/collectors/mark_sweep/gc_collections.rs @@ -202,7 +202,7 @@ mod tests { #[test] fn gc_alloc_vec_basic() { - let collector = &MarkSweepGarbageCollector::default(); + let collector = &mut MarkSweepGarbageCollector::default(); let vec = GcAllocVec::new_in(collector); let gc_vec = Gc::new_in(GcRefCell::new(vec), collector); @@ -214,16 +214,19 @@ mod tests { assert_eq!(gc_vec.borrow()[0], 1); assert_eq!(gc_vec.borrow()[1], 2); assert_eq!(gc_vec.borrow()[2], 3); + + drop(gc_vec); + collector.collect(); } #[test] fn gc_alloc_vec_survives_collection() { - let collector = &mut MarkSweepGarbageCollector::default() + let collector = MarkSweepGarbageCollector::default() .with_page_size(256) .with_heap_threshold(512); - let vec = GcAllocVec::with_capacity(100, collector); - let gc_vec = Gc::new_in(GcRefCell::new(vec), collector); + let vec = GcAllocVec::with_capacity(100, &collector); + let gc_vec = Gc::new_in(GcRefCell::new(vec), &collector); for i in 0..100u64 { gc_vec.borrow_mut().push(i); @@ -233,15 +236,23 @@ mod tests { assert_eq!(gc_vec.borrow().len(), 100); assert_eq!(gc_vec.borrow()[50], 50); + + // Drop the handle and run a normal collection cycle so cleanup happens + // through regular sweep logic instead of collector-drop teardown. + drop(gc_vec); + collector.collect(); } #[test] fn gc_alloc_box_basic() { - let collector = &MarkSweepGarbageCollector::default(); + let collector = &mut MarkSweepGarbageCollector::default(); let boxed = GcAllocBox::new_in(42u64, collector); let gc_box = Gc::new_in(GcRefCell::new(boxed), collector); assert_eq!(**gc_box.borrow(), 42); + + drop(gc_box); + collector.collect(); } #[test] @@ -255,11 +266,14 @@ mod tests { assert_eq!(gc_box.borrow().len(), 5); assert_eq!(gc_box.borrow()[2], 3); + + drop(gc_box); + collector.collect(); } #[test] fn gc_alloc_vec_with_gc_pointers() { - let collector = &MarkSweepGarbageCollector::default(); + let collector = &mut MarkSweepGarbageCollector::default(); let vec = GcAllocVec::new_in(collector); let gc_vec = Gc::new_in(GcRefCell::new(vec), collector); @@ -278,5 +292,8 @@ mod tests { assert_eq!(gc_vec.borrow().len(), 2); assert_eq!(*gc_vec.borrow()[0].borrow(), 100); assert_eq!(*gc_vec.borrow()[1].borrow(), 200); + + drop(gc_vec); + collector.collect(); } } diff --git a/oscars/src/collectors/mark_sweep/internals/ephemeron.rs b/oscars/src/collectors/mark_sweep/internals/ephemeron.rs index 85b22cd..91f513f 100644 --- a/oscars/src/collectors/mark_sweep/internals/ephemeron.rs +++ b/oscars/src/collectors/mark_sweep/internals/ephemeron.rs @@ -142,7 +142,7 @@ pub(crate) const fn vtable_of() -> &'sta }, finalize_fn: |this| unsafe { let ephemeron = this.cast::>>().as_ref().value(); - Finalize::finalize(ephemeron); + Trace::run_finalizer(ephemeron); }, _key_type_id: TypeId::of::(), _key_size: size_of::>(), diff --git a/oscars/src/collectors/mark_sweep/internals/gc_box.rs b/oscars/src/collectors/mark_sweep/internals/gc_box.rs index edf0d34..42a8ac2 100644 --- a/oscars/src/collectors/mark_sweep/internals/gc_box.rs +++ b/oscars/src/collectors/mark_sweep/internals/gc_box.rs @@ -6,7 +6,7 @@ use crate::collectors::mark_sweep::Finalize; use crate::collectors::mark_sweep::internals::gc_header::{GcHeader, HeaderColor}; use crate::collectors::mark_sweep::{Trace, TraceColor}; -use super::{DropFn, TraceFn, VTable, vtable_of}; +use super::{DropFn, FinalizeFn, TraceFn, VTable, vtable_of}; pub struct NonTraceable(()); @@ -166,6 +166,10 @@ impl GcBox { self.vtable.drop_fn() } + pub(crate) fn finalize_fn(&self) -> FinalizeFn { + self.vtable.finalize_fn() + } + pub(crate) fn size(&self) -> usize { self.vtable.size() } diff --git a/oscars/src/collectors/mark_sweep/internals/mod.rs b/oscars/src/collectors/mark_sweep/internals/mod.rs index 63f896b..e87ffd1 100644 --- a/oscars/src/collectors/mark_sweep/internals/mod.rs +++ b/oscars/src/collectors/mark_sweep/internals/mod.rs @@ -5,6 +5,6 @@ mod vtable; pub(crate) use ephemeron::Ephemeron; pub(crate) use gc_header::{GcHeader, HeaderColor}; -pub(crate) use vtable::{DropFn, TraceFn, VTable, vtable_of}; +pub(crate) use vtable::{DropFn, FinalizeFn, TraceFn, VTable, vtable_of}; pub use self::gc_box::{GcBox, NonTraceable, WeakGcBox}; diff --git a/oscars/src/collectors/mark_sweep/internals/vtable.rs b/oscars/src/collectors/mark_sweep/internals/vtable.rs index b8eb643..93404a1 100644 --- a/oscars/src/collectors/mark_sweep/internals/vtable.rs +++ b/oscars/src/collectors/mark_sweep/internals/vtable.rs @@ -27,12 +27,20 @@ pub(crate) const fn vtable_of() -> &'static VTable { // SAFETY: The caller must ensure the erased pointer is not dropped or deallocated. unsafe { core::ptr::drop_in_place(this.as_mut()) }; } + + // SAFETY: The caller must ensure that the passed erased pointer is `GcBox`. + unsafe fn finalize_fn(this: GcErasedPointer) { + // SAFETY: The caller must ensure that the passed erased pointer is `GcBox`. + let value = unsafe { this.cast::>>().as_ref().value() }; + Trace::run_finalizer(value); + } } impl HasVTable for T { const VTABLE: &'static VTable = &VTable { trace_fn: T::trace_fn, drop_fn: T::drop_fn, + finalize_fn: T::finalize_fn, type_id: TypeId::of::(), size: size_of::>(), }; @@ -43,11 +51,13 @@ pub(crate) const fn vtable_of() -> &'static VTable { pub(crate) type TraceFn = unsafe fn(this: GcErasedPointer, color: TraceColor); pub(crate) type DropFn = unsafe fn(this: GcErasedPointer); +pub(crate) type FinalizeFn = unsafe fn(this: GcErasedPointer); #[derive(Debug)] pub(crate) struct VTable { trace_fn: TraceFn, drop_fn: DropFn, + finalize_fn: FinalizeFn, type_id: TypeId, size: usize, } @@ -61,6 +71,10 @@ impl VTable { self.drop_fn } + pub(crate) fn finalize_fn(&self) -> FinalizeFn { + self.finalize_fn + } + pub(crate) const fn type_id(&self) -> TypeId { self.type_id } diff --git a/oscars/src/collectors/mark_sweep/mod.rs b/oscars/src/collectors/mark_sweep/mod.rs index 9bef614..f47c0b7 100644 --- a/oscars/src/collectors/mark_sweep/mod.rs +++ b/oscars/src/collectors/mark_sweep/mod.rs @@ -114,21 +114,21 @@ impl Drop for MarkSweepGarbageCollector { } } - // Reclaim all collector-owned weak maps. - // Single-threaded, so this is safe. - for &map_ptr in self.weak_maps.borrow().iter() { - unsafe { - let _ = rust_alloc::boxed::Box::from_raw(map_ptr.as_ptr()); - } - } - // SAFETY: // `Gc` pointers act as if they live forever (`'static`). - // if the GC drops while they exist, we leak the memory to prevent a UAF - if self.pools_len() > 0 - && (!self.root_queue.borrow().is_empty() - || !self.pending_root_queue.borrow().is_empty()) - { + // if the GC drops while rooted values still exist, we leak memory to prevent UAF. + let has_rooted_values = self + .root_queue + .borrow() + .iter() + .any(|node| unsafe { node.as_ref().value().is_rooted() }) + || self + .pending_root_queue + .borrow() + .iter() + .any(|node| unsafe { node.as_ref().value().is_rooted() }); + + if self.pools_len() > 0 && has_rooted_values { // Unrooted items are NOT swept here so they intentionally leak // instead of triggering a Use-After-Free. // The underlying arena pools WILL be dropped (and OS memory reclaimed) @@ -137,6 +137,7 @@ impl Drop for MarkSweepGarbageCollector { // No rooted items are alive. Sweep and clean up the remaining // cycles and loose allocations before the allocator natively drops. self.sweep_all_queues(); + self.reclaim_dead_weak_maps(); } } } @@ -163,52 +164,92 @@ impl MarkSweepGarbageCollector { // memory so we can still inspect the trace color on ephemerons; // use sweep_color since alive objects were marked with it. self.sweep_trace_color(sweep_color); - - // finally tell the allocator to reclaim raw OS memory - // from arenas that are completely empty now - self.allocator.borrow_mut().drop_empty_pools(); } - // Force drops all elements in the internal tracking queues and clears - // them without regard for reachability. + // Force-collect all tracked items in collector teardown. + // + // Phases: + // 1. finalize everything + // 2. drop + free everything + // + // Since this runs only during collector drop (not a normal collection + // cycle), we don't need reachability marking here. // // NOTE: This intentionally differs from arena2's sweep_all_queues. // arena3 uses`free_slot` calls to reclaim memory. // arena2 uses a bitmap (`mark_dropped`) and reclaims automatically fn sweep_all_queues(&self) { let ephemerons = core::mem::take(&mut *self.ephemeron_queue.borrow_mut()); - for ephemeron in ephemerons { + let roots = core::mem::take(&mut *self.root_queue.borrow_mut()); + let pending_e = core::mem::take(&mut *self.pending_ephemeron_queue.borrow_mut()); + let pending_r = core::mem::take(&mut *self.pending_root_queue.borrow_mut()); + + // Phase 1: finalize everything while all allocations are still alive. + for node in roots.iter().chain(pending_r.iter()).copied() { + let node_ref = unsafe { node.as_ref() }; + let gc_box = node_ref.value(); + unsafe { gc_box.finalize_fn()(node) }; + } + + for ephemeron in ephemerons.iter().chain(pending_e.iter()).copied() { let ephemeron_ref = unsafe { ephemeron.as_ref() }; - unsafe { ephemeron_ref.value().drop_fn()(ephemeron) }; - self.allocator - .borrow_mut() - .free_slot(ephemeron.cast::()); + let vtable = ephemeron_ref.value(); + unsafe { vtable.finalize_fn()(ephemeron) }; } - let roots = core::mem::take(&mut *self.root_queue.borrow_mut()); + // Phase 2: drop and free all tracked values. for node in roots { let node_ref = unsafe { node.as_ref() }; - unsafe { node_ref.value().drop_fn()(node) }; + let gc_box = node_ref.value(); + unsafe { gc_box.drop_fn()(node) }; self.allocator.borrow_mut().free_slot(node.cast::()); } - let pending_e = core::mem::take(&mut *self.pending_ephemeron_queue.borrow_mut()); - for ephemeron in pending_e { + for node in pending_r { + let node_ref = unsafe { node.as_ref() }; + let gc_box = node_ref.value(); + unsafe { gc_box.drop_fn()(node) }; + self.allocator.borrow_mut().free_slot(node.cast::()); + } + + for ephemeron in ephemerons { let ephemeron_ref = unsafe { ephemeron.as_ref() }; - unsafe { ephemeron_ref.value().drop_fn()(ephemeron) }; + let vtable = ephemeron_ref.value(); + unsafe { vtable.drop_fn()(ephemeron) }; self.allocator .borrow_mut() .free_slot(ephemeron.cast::()); } - let pending_r = core::mem::take(&mut *self.pending_root_queue.borrow_mut()); - for node in pending_r { - let node_ref = unsafe { node.as_ref() }; - unsafe { node_ref.value().drop_fn()(node) }; - self.allocator.borrow_mut().free_slot(node.cast::()); + for ephemeron in pending_e { + let ephemeron_ref = unsafe { ephemeron.as_ref() }; + let vtable = ephemeron_ref.value(); + unsafe { vtable.drop_fn()(ephemeron) }; + self.allocator + .borrow_mut() + .free_slot(ephemeron.cast::()); } } + fn reclaim_dead_weak_maps(&self) { + // During collector teardown, reclaim only maps that have already been + // marked dead by `WeakMap::drop`. + self.weak_maps.borrow_mut().retain(|&map_ptr| { + // SAFETY: the pointer is valid as long as it's in this list. + let map = unsafe { map_ptr.as_ref() }; + let is_map_alive = map.is_alive(); + if !is_map_alive { + // SAFETY: `map_ptr` came from `rust_alloc::boxed::Box::into_raw` when + // the weak map was registered, so it must be reclaimed with + // `rust_alloc::boxed::Box::from_raw` (not allocator-api2 `Box`). + unsafe { + let _ = rust_alloc::boxed::Box::from_raw(map_ptr.as_ptr()); + } + } + is_map_alive + }); + } + // Extracts and sweeps items that are considered dead (different trace color). fn sweep_trace_color(&self, sweep_color: TraceColor) { // We use retain and manually drop deleted maps to satisfy Miri's @@ -216,17 +257,21 @@ impl MarkSweepGarbageCollector { self.weak_maps.borrow_mut().retain(|&map_ptr| { // SAFETY: the pointer is valid as long as it's in this list. let map = unsafe { map_ptr.as_ref() }; - if map.is_alive() { + let is_map_alive = map.is_alive(); + if is_map_alive { // We need mut access to prune. unsafe { (&mut *map_ptr.as_ptr()).prune_dead_entries(sweep_color) }; - true } else { // WeakMap was dropped, reclaim the inner allocation. + // + // SAFETY: `map_ptr` came from `rust_alloc::boxed::Box::into_raw` when + // the weak map was registered, so it must be reclaimed with + // `rust_alloc::boxed::Box::from_raw` (not allocator-api2 `Box`). unsafe { let _ = rust_alloc::boxed::Box::from_raw(map_ptr.as_ptr()); } - false } + is_map_alive }); self.run_sweep_phase(); @@ -235,8 +280,9 @@ impl MarkSweepGarbageCollector { let new_color = sweep_color.flip(); self.trace_color.set(new_color); - // NOTE: It would actually be interesting to reuse the pools that are empty rather - // than drop the page and reallocate when a new page is needed ... TBD + // Reclaim OS memory from pool pages that became fully empty during the sweep above. + // Empty pool pages are parked in a recycle list rather than immediately freed to the OS, + // allowing the next try_alloc to pull from that list and avoid OS allocation thrashing. self.allocator.borrow_mut().drop_empty_pools(); // Drain pending queues while `is_collecting` is still true so that any @@ -294,7 +340,7 @@ impl MarkSweepGarbageCollector { // Check if the value is not reachable, i.e. dead. if !gc_box.is_reachable(color) { // Finalize the dead item - gc_box.finalize(); + unsafe { gc_box.finalize_fn()(*node) }; // Recheck if the value is now rooted again after finalization. if gc_box.is_rooted() { unsafe { gc_box.trace_fn()(*node, color) }; diff --git a/oscars/src/collectors/mark_sweep/tests.rs b/oscars/src/collectors/mark_sweep/tests.rs index 31e89d0..100feb6 100644 --- a/oscars/src/collectors/mark_sweep/tests.rs +++ b/oscars/src/collectors/mark_sweep/tests.rs @@ -642,3 +642,282 @@ mod gc_edge_cases { collector.collect(); } } + +#[cfg(feature = "thin-vec")] +mod thin_vec_trace { + use thin_vec::ThinVec; + + use crate::collectors::mark_sweep::MarkSweepGarbageCollector; + use crate::collectors::mark_sweep::cell::GcRefCell; + use crate::collectors::mark_sweep::pointers::Gc; + use crate::{Finalize, Trace}; + + /// `ThinVec>` keeps inner `Gc` values alive across collections. + #[test] + fn thin_vec_keeps_inner_gc_alive() { + let collector = &mut MarkSweepGarbageCollector::default() + .with_page_size(4096) + .with_heap_threshold(8_192); + + let a = Gc::new_in(GcRefCell::new(1u64), collector); + let b = Gc::new_in(GcRefCell::new(2u64), collector); + let c = Gc::new_in(GcRefCell::new(3u64), collector); + + let mut vec: ThinVec>> = ThinVec::new(); + vec.push(a.clone()); + vec.push(b.clone()); + vec.push(c.clone()); + + let container = Gc::new_in(vec, collector); + + collector.collect(); + + assert_eq!(*a.borrow(), 1u64, "a was incorrectly swept"); + assert_eq!(*b.borrow(), 2u64, "b was incorrectly swept"); + assert_eq!(*c.borrow(), 3u64, "c was incorrectly swept"); + + drop(container); + drop(a); + drop(b); + drop(c); + collector.collect(); + } + + /// An empty `ThinVec` does not cause panics during tracing. + #[test] + fn thin_vec_empty_trace() { + let collector = &mut MarkSweepGarbageCollector::default() + .with_page_size(256) + .with_heap_threshold(512); + + let empty: ThinVec> = ThinVec::new(); + let gc = Gc::new_in(empty, collector); + + collector.collect(); + + drop(gc); + collector.collect(); + } + + /// `ThinVec` can be embedded inside a derived `Trace` struct. + #[test] + fn thin_vec_in_derived_trace_struct() { + #[derive(Finalize, Trace)] + struct AstNode { + value: u64, + children: ThinVec>, + } + + let collector = &mut MarkSweepGarbageCollector::default() + .with_page_size(4096) + .with_heap_threshold(8_192); + + let leaf = Gc::new_in( + AstNode { + value: 42, + children: ThinVec::new(), + }, + collector, + ); + + let mut root_children = ThinVec::new(); + root_children.push(leaf.clone()); + + let root = Gc::new_in( + AstNode { + value: 0, + children: root_children, + }, + collector, + ); + + collector.collect(); + + assert_eq!(root.value, 0); + assert_eq!(leaf.value, 42); + + drop(root); + drop(leaf); + collector.collect(); + } +} + +#[test] +fn collector_drop_runs_destructors_for_live_gc_values() { + use core::cell::Cell; + use rust_alloc::rc::Rc; + + struct DropSpy { + drops: Rc>, + } + + impl Drop for DropSpy { + fn drop(&mut self) { + self.drops.set(self.drops.get() + 1); + } + } + + impl Finalize for DropSpy {} + + // SAFETY: `DropSpy` has no traceable children. + unsafe impl Trace for DropSpy { + crate::empty_trace!(); + } + + let drops = Rc::new(Cell::new(0)); + { + let collector = &mut MarkSweepGarbageCollector::default() + .with_page_size(128) + .with_heap_threshold(256); + + let _gc = Gc::new_in( + DropSpy { + drops: Rc::clone(&drops), + }, + collector, + ); + + assert_eq!(drops.get(), 0); + } + + assert_eq!(drops.get(), 1, "collector drop should run value destructor"); +} + +#[test] +fn collector_drop_runs_ephemeron_value_destructors_for_live_values() { + use core::cell::Cell; + use rust_alloc::rc::Rc; + + struct DropSpy { + drops: Rc>, + } + + impl Drop for DropSpy { + fn drop(&mut self) { + self.drops.set(self.drops.get() + 1); + } + } + + impl Finalize for DropSpy {} + + // SAFETY: `DropSpy` has no traceable children. + unsafe impl Trace for DropSpy { + crate::empty_trace!(); + } + + let drops = Rc::new(Cell::new(0)); + { + let collector = &mut MarkSweepGarbageCollector::default() + .with_page_size(128) + .with_heap_threshold(256); + + let mut map = WeakMap::new(collector); + let key = Gc::new_in(1u64, collector); + + map.insert( + &key, + DropSpy { + drops: Rc::clone(&drops), + }, + collector, + ); + + assert_eq!(drops.get(), 0); + } + + assert_eq!( + drops.get(), + 1, + "collector drop should run ephemeron value destructor" + ); +} + +#[test] +fn collector_drop_runs_finalizers_for_live_gc_values() { + use core::cell::Cell; + use rust_alloc::rc::Rc; + + struct FinalizeSpy { + finalized: Rc>, + } + + impl Finalize for FinalizeSpy { + fn finalize(&self) { + self.finalized.set(self.finalized.get() + 1); + } + } + + // SAFETY: `FinalizeSpy` has no traceable children. + unsafe impl Trace for FinalizeSpy { + crate::empty_trace!(); + } + + let finalized = Rc::new(Cell::new(0)); + { + let collector = &mut MarkSweepGarbageCollector::default() + .with_page_size(128) + .with_heap_threshold(256); + + let _gc = Gc::new_in( + FinalizeSpy { + finalized: Rc::clone(&finalized), + }, + collector, + ); + + assert_eq!(finalized.get(), 0); + } + + assert_eq!( + finalized.get(), + 1, + "collector drop should run value finalizer" + ); +} + +#[test] +fn collector_drop_runs_ephemeron_value_finalizers_for_live_values() { + use core::cell::Cell; + use rust_alloc::rc::Rc; + + struct FinalizeSpy { + finalized: Rc>, + } + + impl Finalize for FinalizeSpy { + fn finalize(&self) { + self.finalized.set(self.finalized.get() + 1); + } + } + + // SAFETY: `FinalizeSpy` has no traceable children. + unsafe impl Trace for FinalizeSpy { + crate::empty_trace!(); + } + + let finalized = Rc::new(Cell::new(0)); + { + let collector = &mut MarkSweepGarbageCollector::default() + .with_page_size(128) + .with_heap_threshold(256); + + let mut map = WeakMap::new(collector); + let key = Gc::new_in(1u64, collector); + + map.insert( + &key, + FinalizeSpy { + finalized: Rc::clone(&finalized), + }, + collector, + ); + + assert_eq!(finalized.get(), 0); + } + + assert_eq!( + finalized.get(), + 1, + "collector drop should run ephemeron value finalizer" + ); +} diff --git a/oscars/src/collectors/mark_sweep/trace.rs b/oscars/src/collectors/mark_sweep/trace.rs index b5ba3b7..6544118 100644 --- a/oscars/src/collectors/mark_sweep/trace.rs +++ b/oscars/src/collectors/mark_sweep/trace.rs @@ -311,14 +311,11 @@ unsafe impl Trace }); } -// TODO: Needed for boa_engine - -/* #[cfg(feature = "thin-vec")] impl Finalize for thin_vec::ThinVec {} #[cfg(feature = "thin-vec")] -// SAFETY: All the inner elements of the `Vec` are correctly marked. +// SAFETY: All the inner elements of the `ThinVec` are correctly marked. unsafe impl Trace for thin_vec::ThinVec { custom_trace!(this, mark, { for e in this { @@ -326,7 +323,6 @@ unsafe impl Trace for thin_vec::ThinVec { } }); } -*/ impl Finalize for Option {} // SAFETY: The inner value of the `Option` is correctly marked. diff --git a/oscars/src/collectors/mark_sweep_arena2/internals/ephemeron.rs b/oscars/src/collectors/mark_sweep_arena2/internals/ephemeron.rs index 67c93a2..16c6937 100644 --- a/oscars/src/collectors/mark_sweep_arena2/internals/ephemeron.rs +++ b/oscars/src/collectors/mark_sweep_arena2/internals/ephemeron.rs @@ -32,6 +32,7 @@ impl Ephemeron { } } + #[allow(dead_code)] // TODO: figure out what to do with this pub fn key(&self) -> &K { self.key.value() } diff --git a/oscars/src/collectors/mark_sweep_arena2/internals/gc_box.rs b/oscars/src/collectors/mark_sweep_arena2/internals/gc_box.rs index d7e83cd..78b7c5f 100644 --- a/oscars/src/collectors/mark_sweep_arena2/internals/gc_box.rs +++ b/oscars/src/collectors/mark_sweep_arena2/internals/gc_box.rs @@ -69,12 +69,14 @@ impl WeakGcBox { } impl WeakGcBox { + #[allow(dead_code)] // TODO: could be used for safe upgrading of weak refs pub(crate) fn inner_ptr(&self) -> crate::alloc::arena2::ArenaPointer<'static, GcBox> { // SAFETY: This pointer started out as a `GcBox`, so it's safe to cast // it back, the `PhantomData` guarantees that the type `T` is still correct unsafe { self.inner_ptr.to_typed_arena_pointer::>() } } + #[allow(dead_code)] // TODO: could be used to safely resolve weak refs pub fn value(&self) -> &T { self.inner_ptr().as_inner_ref().value() } @@ -138,6 +140,7 @@ impl GcBox { } } + #[allow(dead_code)] // TODO: could be used for debugging root leaks pub fn roots(&self) -> u16 { self.header.roots() } @@ -154,6 +157,7 @@ impl GcBox { self.header.is_rooted() } + #[allow(dead_code)] // TODO: figure out what to do with this pub fn mark(&self) { self.header.mark(HeaderColor::Grey); } diff --git a/oscars/src/collectors/mark_sweep_arena2/mod.rs b/oscars/src/collectors/mark_sweep_arena2/mod.rs index dcd9b6c..3678aed 100644 --- a/oscars/src/collectors/mark_sweep_arena2/mod.rs +++ b/oscars/src/collectors/mark_sweep_arena2/mod.rs @@ -169,10 +169,6 @@ impl MarkSweepGarbageCollector { // memory so we can still inspect the trace color on ephemerons; // use sweep_color since alive objects were marked with it. self.sweep_trace_color(sweep_color); - - // finally tell the allocator to reclaim raw OS memory - // from arenas that are completely empty now - self.allocator.borrow_mut().drop_dead_arenas(); } // Force drops all elements in the internal tracking queues and clears @@ -245,8 +241,9 @@ impl MarkSweepGarbageCollector { let new_color = sweep_color.flip(); self.trace_color.set(new_color); - // NOTE: It would actually be interesting to reuse the arenas that are dead rather - // than drop the page and reallocate when a new page is needed ... TBD + // Reclaim OS memory from arenas that became fully empty during the sweep above. + // Empty arenas are parked in a recycle list rather than immediately freed to the OS, + // allowing the next try_alloc to pull from that list and avoid OS allocation thrashing. self.allocator.borrow_mut().drop_dead_arenas(); // Drain pending queues while `is_collecting` is still true so that any diff --git a/oscars/src/lib.rs b/oscars/src/lib.rs index a52fffc..01b19d3 100644 --- a/oscars/src/lib.rs +++ b/oscars/src/lib.rs @@ -3,15 +3,12 @@ //! Things that may be contained here in: allocators, garbage collectors, memory //! management primitives. -#![no_std] +#![cfg_attr(not(any(test, feature = "std")), no_std)] extern crate self as oscars; extern crate alloc as rust_alloc; -#[cfg(feature = "std")] -extern crate std; - #[cfg(feature = "mark_sweep")] pub use crate::collectors::mark_sweep::*; #[cfg(feature = "mark_sweep")]