From: Mark Thom Date: Sat, 21 Jun 2025 06:19:32 +0000 (-0700) Subject: synchronize offset table growth with the borrowing of offset pointers X-Git-Tag: v0.10.0~35^2~9 X-Git-Url: https://git.sagredo.dev/?a=commitdiff_plain;h=2844b5a9582958e782d6c89a1abd1073b116806b;p=scryer-prolog.git synchronize offset table growth with the borrowing of offset pointers --- diff --git a/src/machine/compile.rs b/src/machine/compile.rs index 0ce7b5b8..fcd2364f 100644 --- a/src/machine/compile.rs +++ b/src/machine/compile.rs @@ -1337,6 +1337,8 @@ impl<'a, LS: LoadState<'a>> Loader<'a, LS> { settings.is_dynamic(), ); + drop(index_ptr); + let index_ptr = if settings.is_dynamic() { IndexPtr::dynamic_index(code_ptr) } else { @@ -2229,12 +2231,11 @@ impl<'a, LS: LoadState<'a>> Loader<'a, LS> { if let Some(filename) = self.listing_src_file_name() { if let Some(ref mut module) = self.wam_prelude.indices.modules.get_mut(&filename) { - let code_idx = LS::machine_st(&mut self.payload) + let index_ptr = *LS::machine_st(&mut self.payload) .arena .code_index_tbl .lookup_mut(offset.into()); - let index_ptr = *code_idx; let offset = *module.code_dir.entry(key).or_insert(offset); set_code_index::( diff --git a/src/machine/dispatch.rs b/src/machine/dispatch.rs index 20d60c94..59816f17 100644 --- a/src/machine/dispatch.rs +++ b/src/machine/dispatch.rs @@ -2696,9 +2696,9 @@ impl Machine { } } &Instruction::CallNamed(arity, name, idx) => { - let idx = self.machine_st.arena.code_index_tbl.lookup(idx.into()); + let idx = *self.machine_st.arena.code_index_tbl.lookup(idx.into()); - try_or_throw!(self.machine_st, self.try_call(name, arity, *idx)); + try_or_throw!(self.machine_st, self.try_call(name, arity, idx)); if self.machine_st.fail { self.machine_st.backtrack(); @@ -2707,9 +2707,9 @@ impl Machine { } } &Instruction::ExecuteNamed(arity, name, idx) => { - let idx = self.machine_st.arena.code_index_tbl.lookup(idx.into()); + let idx = *self.machine_st.arena.code_index_tbl.lookup(idx.into()); - try_or_throw!(self.machine_st, self.try_execute(name, arity, *idx)); + try_or_throw!(self.machine_st, self.try_execute(name, arity, idx)); if self.machine_st.fail { self.machine_st.backtrack(); @@ -2718,18 +2718,18 @@ impl Machine { } } &Instruction::DefaultCallNamed(arity, name, idx) => { - let idx = self.machine_st.arena.code_index_tbl.lookup(idx.into()); + let idx = *self.machine_st.arena.code_index_tbl.lookup(idx.into()); - try_or_throw!(self.machine_st, self.try_call(name, arity, *idx)); + try_or_throw!(self.machine_st, self.try_call(name, arity, idx)); if self.machine_st.fail { self.machine_st.backtrack(); } } &Instruction::DefaultExecuteNamed(arity, name, idx) => { - let idx = self.machine_st.arena.code_index_tbl.lookup(idx.into()); + let idx = *self.machine_st.arena.code_index_tbl.lookup(idx.into()); - try_or_throw!(self.machine_st, self.try_execute(name, arity, *idx)); + try_or_throw!(self.machine_st, self.try_execute(name, arity, idx)); if self.machine_st.fail { self.machine_st.backtrack(); diff --git a/src/machine/load_state.rs b/src/machine/load_state.rs index 123c7be5..931ee273 100644 --- a/src/machine/load_state.rs +++ b/src/machine/load_state.rs @@ -48,6 +48,7 @@ pub(super) fn set_code_index<'a, LS: LoadState<'a>>( } }; + drop(code_idx_ptr); payload.retraction_info.push_record(record); } @@ -507,6 +508,7 @@ impl<'a, LS: LoadState<'a>> Loader<'a, LS> { let code_ptr = code_index_tbl.lookup((*code_index).into()); if !code_ptr.is_undefined() && !code_ptr.is_dynamic_undefined() { + drop(code_ptr); let old_index_ptr = code_index.replace(code_index_tbl, IndexPtr::undefined()); self.payload.retraction_info.push_record( @@ -559,6 +561,9 @@ impl<'a, LS: LoadState<'a>> Loader<'a, LS> { let target_code_ptr = code_index_tbl.lookup(target_code_idx.into()); if module_code_ptr == target_code_ptr { + drop(module_code_ptr); + drop(target_code_ptr); + let old_index_ptr = target_code_idx .replace(code_index_tbl, IndexPtr::undefined()); payload diff --git a/src/machine/loader.rs b/src/machine/loader.rs index cfec0bf6..2a65a1f9 100644 --- a/src/machine/loader.rs +++ b/src/machine/loader.rs @@ -1228,6 +1228,8 @@ impl<'a, LS: LoadState<'a>> Loader<'a, LS> { .lookup_mut(offset.into()); if code_idx_ptr.is_undefined() { + drop(code_idx_ptr); + set_code_index::( &mut self.payload, &compilation_target, @@ -2159,6 +2161,7 @@ impl Machine { .remove_predicate_skeleton(&compilation_target, &key); let offset = loader.get_or_insert_code_index(key, compilation_target); + let mut code_idx = loader .payload .machine_st @@ -2168,6 +2171,8 @@ impl Machine { code_idx.set(IndexPtr::undefined()); + drop(code_idx); + loader.payload.compilation_target = clause_clause_compilation_target; while let Some(target_pos) = clause_clause_target_poses.pop() { diff --git a/src/machine/mod.rs b/src/machine/mod.rs index fb336248..e9e91f89 100644 --- a/src/machine/mod.rs +++ b/src/machine/mod.rs @@ -253,7 +253,7 @@ impl Machine { ) -> std::process::ExitCode { if let Some(module) = self.indices.modules.get(&module_name) { if let Some(code_idx) = module.code_dir.get(&key) { - let index_ptr = self.machine_st.arena.code_index_tbl.lookup(code_idx.into()); + let index_ptr = *self.machine_st.arena.code_index_tbl.lookup(code_idx.into()); let p = index_ptr.local().unwrap(); // Leave a halting choice point to backtrack to in case the predicate fails or throws. diff --git a/src/machine/unify.rs b/src/machine/unify.rs index 6d3bc3ad..252b0f48 100644 --- a/src/machine/unify.rs +++ b/src/machine/unify.rs @@ -289,10 +289,10 @@ pub(crate) trait Unifier: DerefMut { (HeapCellValueTag::F64Offset, f2) => { let machine_st = self.deref_mut(); - let f1 = machine_st.arena.f64_tbl.lookup(f1); - let f2 = machine_st.arena.f64_tbl.lookup(f2.into()); + let f1 = *machine_st.arena.f64_tbl.lookup(f1); + let f2 = *machine_st.arena.f64_tbl.lookup(f2.into()); - self.fail = **f1 != **f2; + self.fail = *f1 != *f2; } _ => { self.fail = true; diff --git a/src/offset_table.rs b/src/offset_table.rs index a384cb84..f7d9efdf 100644 --- a/src/offset_table.rs +++ b/src/offset_table.rs @@ -1,7 +1,7 @@ use std::cell::UnsafeCell; use std::hash::{Hash, Hasher}; use std::ops::{Deref, DerefMut}; -use std::sync::{Arc, Mutex}; +use std::sync::{Arc, RwLock, RwLockReadGuard}; use std::{fmt, mem, ptr}; use arcu::atomic::Arcu; @@ -48,11 +48,60 @@ impl RawBlockTraits for IndexPtr { #[derive(Debug)] pub struct OffsetTableImpl(InnerOffsetTableImpl); +impl From>> for OffsetTableImpl { + #[inline] + fn from(value: Arc>) -> Self { + OffsetTableImpl(InnerOffsetTableImpl::Concurrent(value)) + } +} + impl OffsetTableImpl { #[inline(always)] pub fn new() -> Self { Self(InnerOffsetTableImpl::Serial(SerialOffsetTable::new())) } + + #[must_use = "the returned concurrent table must be absorbed into the owned OffsetTable"] + pub fn single_to_concurrent(&mut self) -> Arc> { + match &mut self.0 { + InnerOffsetTableImpl::Serial(serial_tbl) => { + let empty_serial_tbl = SerialOffsetTable { + block: RawBlock::empty_block(), + }; + + let serial_tbl = mem::replace(serial_tbl, empty_serial_tbl); + let block = Arcu::new(serial_tbl.block, GlobalEpochCounterPool); + + let growth_lock = RwLock::new(()); + let concurrent_tbl = Arc::new(ConcurrentOffsetTable { block, growth_lock }); + + self.0 = InnerOffsetTableImpl::Concurrent(concurrent_tbl.clone()); + + concurrent_tbl + } + InnerOffsetTableImpl::Concurrent(concurrent_tbl) => concurrent_tbl.clone(), + } + } + + #[must_use = "the transition to a single-threaded offset table may fail if the concurrent table is held from multiple places"] + pub fn concurrent_to_single(&mut self) -> Result<(), ()> { + match &mut self.0 { + InnerOffsetTableImpl::Serial(_serial_tbl) => Ok(()), + InnerOffsetTableImpl::Concurrent(concurrent_tbl) => { + let lock_guard = concurrent_tbl.growth_lock.write().unwrap(); + let raw_block = concurrent_tbl.block.replace(RawBlock::empty_block()); + + match Arc::try_unwrap(raw_block) { + Ok(block) => { + drop(lock_guard); + self.0 = InnerOffsetTableImpl::Serial(SerialOffsetTable { block }); + Ok(()) + } + Err(_) => Err(()), + } + } + } + } } impl Default for OffsetTableImpl { @@ -61,6 +110,17 @@ impl Default for OffsetTableImpl { } } +#[derive(Debug)] +struct SerialOffsetTable { + block: RawBlock, +} + +#[derive(Debug)] +pub struct ConcurrentOffsetTable { + block: Arcu, GlobalEpochCounterPool>, + growth_lock: RwLock<()>, +} + #[derive(Debug)] enum InnerOffsetTableImpl { Serial(SerialOffsetTable), @@ -80,9 +140,13 @@ impl InnerOffsetTableImpl { #[inline(always)] fn lookup<'a>(&'a self, offset: usize) -> TablePtr<'a, T> { match self { - Self::Concurrent(concurrent_tbl) => { - TablePtr(InnerTablePtr::Concurrent(concurrent_tbl.lookup(offset))) - } + Self::Concurrent(concurrent_tbl) => TablePtr({ + let (rcu_ref, guard_lock) = concurrent_tbl.lookup(offset); + InnerTablePtr::Concurrent { + rcu_ref, + guard_lock, + } + }), Self::Serial(serial_tbl) => unsafe { TablePtr(InnerTablePtr::Serial(serial_tbl.lookup(offset))) }, @@ -92,12 +156,18 @@ impl InnerOffsetTableImpl { #[inline(always)] fn lookup_mut<'a>(&'a mut self, offset: usize) -> TablePtrMut<'a, T> { match self { - Self::Concurrent(concurrent_tbl) => TablePtrMut(InnerTablePtrMut::Concurrent( - concurrent_tbl.lookup_mut(offset), - )), - Self::Serial(serial_tbl) => unsafe { - TablePtrMut(InnerTablePtrMut::Serial(serial_tbl.lookup_mut(offset))) - }, + InnerOffsetTableImpl::Concurrent(concurrent_tbl) => TablePtrMut({ + let (rcu_ref, guard_lock) = concurrent_tbl.lookup_mut(offset); + InnerTablePtrMut::Concurrent { + rcu_ref, + guard_lock, + } + }), + InnerOffsetTableImpl::Serial(serial_tbl) => { + TablePtrMut(InnerTablePtrMut::Serial(unsafe { + serial_tbl.lookup_mut(offset) + })) + } } } } @@ -142,11 +212,6 @@ impl OffsetTable for OffsetTableImpl { } } -#[derive(Debug)] -struct SerialOffsetTable { - block: RawBlock, -} - impl SerialOffsetTable { #[inline] fn new() -> Self { @@ -184,16 +249,10 @@ impl SerialOffsetTable { } } -#[derive(Debug)] -pub struct ConcurrentOffsetTable { - block: Arcu, GlobalEpochCounterPool>, - update: Mutex<()>, -} - impl ConcurrentOffsetTable { #[allow(clippy::missing_safety_doc)] unsafe fn build_with(&self, value: T) -> usize { - let update_guard = self.update.lock(); + let update_guard = self.growth_lock.write().unwrap(); // we don't have an index table for lookups as AtomTable does so // just get the epoch after we take the upgrade lock @@ -224,16 +283,25 @@ impl ConcurrentOffsetTable { } #[inline] - fn lookup(&self, offset: usize) -> RcuRef, T> { - RcuRef::try_map(self.block.read(), |raw_block| unsafe { + fn lookup<'a>(&'a self, offset: usize) -> (RcuRef, T>, RwLockReadGuard<'a, ()>) { + let growth_lock_guard = self.growth_lock.read().unwrap(); + + let rcu_ref = RcuRef::try_map(self.block.read(), |raw_block| unsafe { raw_block.base.add(offset).cast::().as_ref() }) - .expect("The offset should result in a non-null pointer") + .expect("The offset should result in a non-null pointer"); + + (rcu_ref, growth_lock_guard) } #[inline] - fn lookup_mut(&self, offset: usize) -> RcuRef, UnsafeCell> { - RcuRef::try_map(self.block.read(), |raw_block| unsafe { + fn lookup_mut<'a>( + &'a self, + offset: usize, + ) -> (RcuRef, UnsafeCell>, RwLockReadGuard<'a, ()>) { + let growth_lock_guard = self.growth_lock.read().unwrap(); + + let rcu_ref = RcuRef::try_map(self.block.read(), |raw_block| unsafe { raw_block .base .add(offset) @@ -241,7 +309,9 @@ impl ConcurrentOffsetTable { .cast::>() .as_ref() }) - .expect("The offset should result in a non-null pointer") + .expect("The offset should result in a non-null pointer"); + + (rcu_ref, growth_lock_guard) } } @@ -293,7 +363,11 @@ pub struct TablePtr<'a, T: RawBlockTraits>(InnerTablePtr<'a, T>); #[derive(Debug)] enum InnerTablePtr<'a, T: RawBlockTraits> { - Concurrent(RcuRef, T>), + Concurrent { + rcu_ref: RcuRef, T>, + #[allow(dead_code)] + guard_lock: RwLockReadGuard<'a, ()>, + }, Serial(&'a T), } @@ -336,7 +410,7 @@ impl Deref for TablePtr<'_, T> { #[inline] fn deref(&self) -> &Self::Target { match &self.0 { - InnerTablePtr::Concurrent(rcu_ref) => rcu_ref, + InnerTablePtr::Concurrent { rcu_ref, .. } => rcu_ref, InnerTablePtr::Serial(ref_mut) => ref_mut, } } @@ -366,7 +440,11 @@ pub struct TablePtrMut<'a, T: RawBlockTraits>(InnerTablePtrMut<'a, T>); #[derive(Debug)] enum InnerTablePtrMut<'a, T: RawBlockTraits> { - Concurrent(RcuRef, UnsafeCell>), + Concurrent { + rcu_ref: RcuRef, UnsafeCell>, + #[allow(dead_code)] + guard_lock: RwLockReadGuard<'a, ()>, + }, Serial(&'a mut T), } @@ -409,7 +487,9 @@ impl Deref for TablePtrMut<'_, T> { #[inline] fn deref(&self) -> &Self::Target { match &self.0 { - InnerTablePtrMut::Concurrent(rcu_ref) => unsafe { rcu_ref.get().as_ref().unwrap() }, + InnerTablePtrMut::Concurrent { rcu_ref, .. } => unsafe { + rcu_ref.get().as_ref().unwrap() + }, InnerTablePtrMut::Serial(ref_mut) => ref_mut, } } @@ -419,7 +499,7 @@ impl DerefMut for TablePtrMut<'_, T> { #[inline] fn deref_mut(&mut self) -> &mut Self::Target { match &mut self.0 { - InnerTablePtrMut::Concurrent(rcu_ref) => unsafe { + InnerTablePtrMut::Concurrent { rcu_ref, .. } => unsafe { &mut *rcu_ref.get().as_mut().unwrap() }, InnerTablePtrMut::Serial(ref_mut) => ref_mut, @@ -431,7 +511,7 @@ impl TablePtrMut<'_, IndexPtr> { #[inline] pub fn set(&mut self, val: IndexPtr) { match &mut self.0 { - InnerTablePtrMut::Concurrent(rcu_ref) => unsafe { + InnerTablePtrMut::Concurrent { rcu_ref, .. } => unsafe { *rcu_ref.get() = val; }, InnerTablePtrMut::Serial(ref_mut) => { @@ -443,7 +523,7 @@ impl TablePtrMut<'_, IndexPtr> { #[inline] pub fn replace(&mut self, val: IndexPtr) -> IndexPtr { match &mut self.0 { - InnerTablePtrMut::Concurrent(rcu_ref) => unsafe { rcu_ref.get().replace(val) }, + InnerTablePtrMut::Concurrent { rcu_ref, .. } => unsafe { rcu_ref.get().replace(val) }, InnerTablePtrMut::Serial(ref_mut) => mem::replace(*ref_mut, val), } } diff --git a/src/raw_block.rs b/src/raw_block.rs index 3c39c77d..02bea9f0 100644 --- a/src/raw_block.rs +++ b/src/raw_block.rs @@ -19,7 +19,7 @@ pub struct RawBlock { impl RawBlock { #[inline] - fn empty_block() -> Self { + pub fn empty_block() -> Self { RawBlock { base: ptr::null(), top: ptr::null(),