reduce dupliaction between mark_sweep and mark_sweep_arena2

Author: shruti2522

oscars/src/alloc/arena2/alloc.rs

@@ -44,7 +44,16 @@ impl<T: ?Sized> ArenaHeapItem<T> {
         &mut self.value as *mut T
     }
 
-    pub(crate) fn value_mut(&mut self) -> &mut T {
+    /// Returns a raw mutable pointer to the value
+    ///
+    /// This avoids creating a `&mut self` reference, which can lead to stacked borrows
+    /// if shared references to the heap item exist
+    pub(crate) fn as_value_ptr(ptr: NonNull<Self>) -> *mut T {
+        // SAFETY: `&raw mut` computes the field address without creating a reference
+        unsafe { &raw mut (*ptr.as_ptr()).value }
+    }
+
+    fn value_mut(&mut self) -> &mut T {
         &mut self.value
     }
 }
@@ -133,6 +142,15 @@ impl<'arena> ErasedArenaPointer<'arena> {
         self.0.as_ptr()
     }
 
+    /// Extend the lifetime of this erased arena pointer to 'static
+    ///
+    /// SAFETY:
+    ///
+    /// safe because the gc collector owns the arena and keeps it alive
+    pub(crate) unsafe fn extend_lifetime(self) -> ErasedArenaPointer<'static> {
+        ErasedArenaPointer(self.0, PhantomData)
+    }
+
     /// Returns an [`ArenaPointer`] for the current [`ErasedArenaPointer`]
     ///
     /// # Safety
@@ -178,6 +196,15 @@ impl<'arena, T> ArenaPointer<'arena, T> {
     pub fn to_erased(self) -> ErasedArenaPointer<'arena> {
         self.0
     }
+
+    /// Extend the lifetime of this arena pointer to 'static
+    ///
+    /// SAFETY:
+    ///
+    /// 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)
+    }
 }
 
 const FULL_MASK: u8 = 0b0100_0000;

oscars/src/alloc/arena2/tests.rs

@@ -77,7 +77,7 @@ fn arc_drop() {
         let heap_item_mut = heap_item.as_mut();
         // Manually drop the heap item
         heap_item_mut.mark_dropped();
-        drop_in_place(heap_item_mut.value_mut());
+        drop_in_place(ArenaHeapItem::as_value_ptr(heap_item));
     };
 
     assert!(dropped.load(Ordering::SeqCst));

oscars/src/collectors/mark_sweep/cell.rs

@@ -171,6 +171,14 @@ impl<T: ?Sized> GcRefCell<T> {
         }
     }
 
+    // returns a raw pointer to the inner value or `None` if currently mutably borrowed
+    pub(crate) fn get_raw(&self) -> Option<*mut T> {
+        match self.borrow.get().borrowed() {
+            BorrowState::Writing => None,
+            _ => Some(self.cell.get()),
+        }
+    }
+
     /// Mutably borrows the wrapped value, returning an error if the value is currently borrowed.
     ///
     /// The borrow lasts until the returned `GcCellRefMut` exits scope.

oscars/src/collectors/mark_sweep/internals/gc_box.rs

@@ -46,9 +46,9 @@ impl<T: Trace + Finalize + ?Sized> WeakGcBox<T> {
     pub(crate) fn erased_inner_ptr(&self) -> NonNull<GcBox<NonTraceable>> {
         // SAFETY: `as_heap_ptr` returns a valid pointer to
         // `PoolItem` whose lifetime is tied to the pool
-        let heap_item = unsafe { self.as_heap_ptr().as_mut() };
-        // SAFETY: We just removed this value from a NonNull
-        unsafe { NonNull::new_unchecked(heap_item.as_ptr()) }
+        let heap_item: *mut PoolItem<GcBox<NonTraceable>> = self.as_heap_ptr().as_ptr();
+        // SAFETY: `PoolItem` is repr(transparent), so pointing to and returning field 0 is valid.
+        unsafe { NonNull::new_unchecked(&raw mut (*heap_item).0) }
     }
 
     pub(crate) fn as_heap_ptr(&self) -> NonNull<PoolItem<GcBox<NonTraceable>>> {

oscars/src/collectors/mark_sweep/internals/mod.rs

@@ -4,6 +4,7 @@ mod gc_header;
 mod vtable;
 
 pub(crate) use ephemeron::Ephemeron;
+pub(crate) use gc_header::{GcHeader, HeaderColor};
 pub(crate) use vtable::{DropFn, TraceFn, VTable, vtable_of};
 
 pub use self::gc_box::{GcBox, NonTraceable, WeakGcBox};

oscars/src/collectors/mark_sweep/mod.rs

@@ -171,6 +171,10 @@ impl MarkSweepGarbageCollector {
 
     // Force drops all elements in the internal tracking queues and clears
     // them without regard for reachability.
+    //
+    // 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 {

oscars/src/collectors/mark_sweep/pointers/gc.rs

@@ -44,12 +44,12 @@ impl<T: Trace> Gc<T> {
 
 impl<T: Trace + ?Sized> Gc<T> {
     pub(crate) fn as_sized_inner_ptr(&self) -> NonNull<GcBox<NonTraceable>> {
-        // SAFETY: use `addr_of_mut!` to get a raw pointer without creating
+        // SAFETY: use `&raw mut` to get a raw pointer without creating
         // a `&mut` reference, avoiding Stacked Borrows UB during GC tracing
-        let raw: *mut ArenaHeapItem<GcBox<NonTraceable>> = self.as_heap_ptr().as_ptr();
+        let raw: *mut PoolItem<GcBox<NonTraceable>> = self.as_heap_ptr().as_ptr();
         // SAFETY: `raw` is non-null because it comes from `as_heap_ptr()`
-        // `ArenaHeapItem` is `#[repr(transparent)]` so it shares the same address as field 0
-        unsafe { NonNull::new_unchecked(core::ptr::addr_of_mut!((*raw).0)) }
+        // `PoolItem` is `#[repr(transparent)]` so it shares the same address as field 0
+        unsafe { NonNull::new_unchecked(&raw mut (*raw).0) }
     }
 
     pub(crate) fn as_heap_ptr(&self) -> NonNull<PoolItem<GcBox<NonTraceable>>> {

oscars/src/collectors/mark_sweep_arena2/README.md

@@ -1,21 +1,3 @@
 # Mark sweep collector
 
-This is a basic mark-sweep collector using an underlying arena allocator.
-
-## TODO list
-
-- [x] Support weak maps
-- [x] Add Tests
-
-
-## Areas of improvement
-
-The overhead on a single allocation honestly feels a bit high. This may be worthwhile
-for now for performance gains and general API, but we should really measure and determine
-just how much overhead is being added.
-
-Currently, there is a line drawn between the allocator and the GcBox. This creates very,
-very awkward naming (ArenaPointer, ArenaHeapItem, GcBox, etc.). We may be able to combine
-the general functionality of the ArenaHeapItem, and GcBox. But also, that would then
-restrict the potential ability to switch out allocators as easily ... to be determined.
-
+This is a basic mark-sweep collector using an underlying arena2 allocator.