From: Emilie Burgun Date: Sun, 10 May 2026 17:21:36 +0000 (+0200) Subject: RawBlock: switch to Cell or AtomicPtr instead of UnsafeCell X-Git-Url: https://git.sagredo.dev/?a=commitdiff_plain;h=ad614b684ced51d1d34aa7882be2c02c5f29ae1b;p=scryer-prolog.git RawBlock: switch to Cell or AtomicPtr instead of UnsafeCell This is the first step towards enabling multithreading on AtomTable. For now RawBlock will default to using Cell, which yields a byte- equivalent compiled output. Also adds an `atomic` feature, which, when enabled, will make RawBlock use an AtomicPtr instead, ensuring that it implements `Sync`. --- diff --git a/src/offset_table.rs b/src/offset_table.rs index a0194f93..f09a3583 100644 --- a/src/offset_table.rs +++ b/src/offset_table.rs @@ -12,8 +12,8 @@ use parking_lot::{Mutex, RwLock}; use crate::machine::heap::AllocError; use crate::machine::machine_indices::IndexPtr; -use crate::raw_block::RawBlock; use crate::raw_block::RawBlockTraits; +use crate::raw_block::{RawBlock, RawBlockConcurrent}; use ordered_float::OrderedFloat; @@ -93,8 +93,9 @@ impl OffsetTableImpl { // this shouldn't be able to fail let raw_block = Arc::try_unwrap(table.block.replace(RawBlock::empty_block())).unwrap(); - self.0 = - InnerOffsetTableImpl::Serial(SerialOffsetTable { block: raw_block }); + self.0 = InnerOffsetTableImpl::Serial(SerialOffsetTable { + block: raw_block.into(), + }); Ok(()) } Err(table_arc) => { @@ -130,7 +131,7 @@ struct SerialOffsetTable { #[derive(Debug)] pub struct ConcurrentOffsetTable { - block: Arcu, GlobalEpochCounterPool>, + block: Arcu, GlobalEpochCounterPool>, growth_lock: RwLock<()>, offset_locks: RwLock>>, } @@ -250,7 +251,8 @@ impl SerialOffsetTable { let serial_tbl = mem::replace(self, empty_serial_tbl); let num_tbl_entries = serial_tbl.block.used_bytes() / size_of::(); - let block = Arcu::new(serial_tbl.block, GlobalEpochCounterPool); + let raw_block: RawBlock = serial_tbl.block.into(); + let block = Arcu::new(raw_block, GlobalEpochCounterPool); let offset_locks: Vec> = (0..num_tbl_entries).map(|_| RwLock::new(())).collect(); @@ -403,6 +405,8 @@ impl F64Table { // which breaks the invariant indirection_tbl is meant to enforce. // Since this branch is never invoked, it does no harm, but that // that will eventually change. + // + // Note: may be indirectly fixed by the use of the new RawBlockConcurrency trait. { let indirection_tbl = concurrent_tbl.indirection_tbl.lock(); @@ -473,7 +477,9 @@ impl F64Table { .unwrap(); *self = Self::Serial(SerialF64Table { indirection_tbl: indirection_tbl.into_inner(), - offset_tbl: SerialOffsetTable { block: raw_block }, + offset_tbl: SerialOffsetTable { + block: raw_block.into(), + }, }); Ok(()) diff --git a/src/raw_block.rs b/src/raw_block.rs index 9f5ecc90..7adc0f03 100644 --- a/src/raw_block.rs +++ b/src/raw_block.rs @@ -1,8 +1,109 @@ use core::marker::PhantomData; use std::alloc; -use std::cell::UnsafeCell; +use std::cell::Cell; use std::ptr; +use std::sync::atomic::{AtomicPtr, Ordering}; + +trait PtrCellTrait: std::fmt::Debug { + fn new(val: *mut u8) -> Self; + + fn get(&self) -> *mut u8; + + /// Modifies the wrapped value. + fn set(&mut self, val: *mut u8); + + /// Performs an atomic compare-and-swap on the wrapped value. + /// + /// If the compare succeeded and `cb` returns `Some(new_ptr)`, stored `new_ptr` and returns `Ok(old_ptr). + /// + /// If `cb` returns `None`, returns `Err(old_ptr)`. + /// + /// May retry multiple times if the comparison fails. + fn try_update(&self, cb: impl Fn(*mut u8) -> Option<*mut u8>) -> Result<*mut u8, *mut u8>; +} + +impl PtrCellTrait for Cell<*mut u8> { + fn new(val: *mut u8) -> Self { + Cell::new(val) + } + + #[inline(always)] + fn get(&self) -> *mut u8 { + Cell::get(self) + } + + #[inline(always)] + fn set(&mut self, val: *mut u8) { + Cell::set(self, val) + } + + #[inline(always)] + fn try_update(&self, cb: impl Fn(*mut u8) -> Option<*mut u8>) -> Result<*mut u8, *mut u8> { + let val = Cell::get(self); + if let Some(new_val) = cb(val) { + Cell::set(self, new_val); + Ok(val) + } else { + Err(val) + } + } +} + +impl PtrCellTrait for AtomicPtr { + fn new(val: *mut u8) -> Self { + AtomicPtr::new(val) + } + + #[inline(always)] + fn get(&self) -> *mut u8 { + self.load(Ordering::Acquire) + } + + #[inline(always)] + fn set(&mut self, val: *mut u8) { + *self.get_mut() = val; + } + + #[inline] + fn try_update(&self, cb: impl Fn(*mut u8) -> Option<*mut u8>) -> Result<*mut u8, *mut u8> { + let mut prev = PtrCellTrait::get(self); + + while let Some(next) = cb(prev) { + match self.compare_exchange_weak(prev, next, Ordering::Relaxed, Ordering::Acquire) { + x @ Ok(_) => return x, + Err(next_prev) => prev = next_prev, + } + } + + Err(prev) + + // TODO: replace with the following once 1.95 is the msrv: + // AtomicPtr::try_update(self, Ordering::Relaxed, Ordering::Acquire, cb) + } +} + +/// Allows the choice of implementation for the mutable pointer in [`RawBlock`]. +/// Can be one of: +/// - [`RawBlockSerial`] (using a [`Cell<*mut u8>`]) +/// - [`RawBlockConcurrent`] (using a [`AtomicPtr`]) +pub(crate) trait RawBlockConcurrency { + #[allow(private_bounds)] + type PtrCell: PtrCellTrait; +} + +#[derive(Debug, Clone, Copy)] +pub struct RawBlockSerial(); +#[derive(Debug, Clone, Copy)] +pub struct RawBlockConcurrent(); + +impl RawBlockConcurrency for RawBlockSerial { + type PtrCell = Cell<*mut u8>; +} + +impl RawBlockConcurrency for RawBlockConcurrent { + type PtrCell = AtomicPtr; +} use crate::machine::heap::AllocError; @@ -13,22 +114,24 @@ pub trait RawBlockTraits { /// A block of memory with fast, lock-free appends. #[derive(Debug)] -pub struct RawBlock { +pub struct RawBlock { base: *const u8, capacity: usize, - ptr: UnsafeCell<*mut u8>, + ptr: C::PtrCell, _marker: PhantomData, + _c_marker: PhantomData, } -impl RawBlock { +impl RawBlock { #[inline] pub fn empty_block() -> Self { RawBlock { base: ptr::null(), capacity: 0, - ptr: UnsafeCell::new(ptr::null_mut()), + ptr: C::PtrCell::new(ptr::null_mut()), _marker: PhantomData, + _c_marker: PhantomData, } } @@ -51,7 +154,7 @@ impl RawBlock { } self.base = new_base; self.capacity = cap; - *self.ptr.get_mut() = self.base.cast_mut(); + self.ptr.set(self.base.cast_mut()); Ok(()) } @@ -71,7 +174,7 @@ impl RawBlock { } else { self.base = new_base; self.capacity = size * 2; - *self.ptr.get_mut() = (self.base as usize + size) as *mut _; + self.ptr.set(self.base.add(size).cast_mut()); Ok(()) } } @@ -86,7 +189,7 @@ impl RawBlock { new_block.init_at_size(self.capacity() * 2)?; let allocated = self.used_bytes(); self.base.copy_to(new_block.base.cast_mut(), allocated); - *new_block.ptr.get_mut() = new_block.base.add(allocated).cast_mut(); + new_block.ptr.set(new_block.base.add(allocated).cast_mut()); new_block.debug_check_invariants(); @@ -97,14 +200,12 @@ impl RawBlock { #[inline(always)] fn debug_check_invariants(&self) { if cfg!(debug_assertions) { - unsafe { - assert!( - *self.ptr.get() as *const _ >= self.base, - "self.ptr = {:?} < {:?} = self.base", - *self.ptr.get(), - self.base - ); - } + assert!( + self.ptr.get().cast_const() >= self.base, + "self.ptr = {:?} < {:?} = self.base", + self.ptr.get(), + self.base + ); assert!(self.used_bytes() <= self.capacity()); } @@ -117,27 +218,30 @@ impl RawBlock { #[inline] pub fn used_bytes(&self) -> usize { - // TODO: safety: UnsafeCell.get() - // TODO: safety: prove that ∀Γ: reachable, Γ |- (ptr, base): same alloc - unsafe { (*self.ptr.get()).offset_from(self.base) as usize } - } - - #[inline(always)] - unsafe fn free_bytes(&self) -> usize { - self.capacity() - self.used_bytes() + // SAFETY: + // - Invariant: `ptr` is in the same allocation as `base` + unsafe { self.ptr.get().offset_from(self.base) as usize } } pub unsafe fn alloc(&self, size: usize) -> *mut u8 { self.debug_check_invariants(); let aligned_size = size.next_multiple_of(T::align()); - if self.free_bytes() >= aligned_size { - // TODO: make this an atomic add - let ptr = *self.ptr.get(); - *self.ptr.get() = ptr.add(aligned_size) as *mut _; - ptr - } else { - ptr::null_mut() + + match self.ptr.try_update(|ptr| { + // SAFETY: + // - Invariant: `ptr` is in the same allocation as `base` + let free_bytes = unsafe { self.capacity() - ptr.offset_from(self.base) as usize }; + + if free_bytes >= aligned_size { + Some(unsafe { ptr.add(aligned_size) }) + } else { + // Not enough space: don't allocate and return a null pointer + None + } + }) { + Ok(ptr) => ptr, + Err(_) => ptr::null_mut(), } } @@ -160,9 +264,9 @@ impl RawBlock { // - Definition: self.base := alloc(self.capacity) let new_ptr = unsafe { self.base.add(new_size) }; - debug_assert!(new_ptr as usize <= (*self.ptr.get_mut()) as usize,); + debug_assert!(new_ptr as usize <= self.ptr.get() as usize,); - *self.ptr.get_mut() = new_ptr as *mut u8; + self.ptr.set(new_ptr.cast_mut()); self.debug_check_invariants(); } @@ -208,16 +312,37 @@ impl RawBlock { } } -impl Drop for RawBlock { +impl Drop for RawBlock { fn drop(&mut self) { if !self.base.is_null() { unsafe { let layout = alloc::Layout::from_size_align_unchecked(self.capacity(), T::align()); alloc::dealloc(self.base as *mut _, layout); } + } + } +} + +impl From> for RawBlock { + fn from(other: RawBlock) -> Self { + Self { + base: other.base, + capacity: other.capacity, + ptr: PtrCellTrait::new(other.ptr.get()), + _marker: PhantomData, + _c_marker: PhantomData, + } + } +} - self.base = ptr::null(); - *self.ptr.get_mut() = ptr::null_mut(); +impl From> for RawBlock { + fn from(other: RawBlock) -> Self { + Self { + base: other.base, + capacity: other.capacity, + ptr: PtrCellTrait::new(other.ptr.get()), + _marker: PhantomData, + _c_marker: PhantomData, } } }