From 21a5e21027585a0feee33da2cdf6795f47453718 Mon Sep 17 00:00:00 2001 From: Techcable Date: Mon, 10 Aug 2020 14:16:11 -0700 Subject: [PATCH] [simple] Add a "simple" single-threaded context implementation This is much simpler, because it assumes there is only one active context. There is no need to lock because only a single active thread can use the garbage collector at a time. The most important outcome of this change is that it starts to seperate the implementation of contexts from the mark/sweep collector internals. Eventually I'd like to seperate out the context implementation for indpendent use. Ideally it would be shared alongside the generational collector (#14). There is no real performance change (for the single threaded benchmark): Standard "sync" contexts: binary_trees 21 46.5 sec, 385 MB New "simple" contexts: binary_trees 21 45.4 sec, 385 MB --- libs/simple/Cargo.toml | 9 +- libs/simple/examples/binary_trees.rs | 2 +- libs/simple/src/context/mod.rs | 242 +++++++++++++ libs/simple/src/context/simple.rs | 209 +++++++++++ .../src/{context.rs => context/sync.rs} | 340 +++++------------- libs/simple/src/handles.rs | 5 + libs/simple/src/lib.rs | 104 +++--- libs/simple/src/utils.rs | 1 + 8 files changed, 622 insertions(+), 290 deletions(-) create mode 100644 libs/simple/src/context/mod.rs create mode 100644 libs/simple/src/context/simple.rs rename libs/simple/src/{context.rs => context/sync.rs} (72%) diff --git a/libs/simple/Cargo.toml b/libs/simple/Cargo.toml index 5b7bb34..1b0b02d 100644 --- a/libs/simple/Cargo.toml +++ b/libs/simple/Cargo.toml @@ -12,8 +12,9 @@ edition = "2018" zerogc = { path = "../..", version = "0.1.0" } once_cell = { version = "1.4.0", optional = true } # Concurrency +# TODO: Make this optional for the single-threaded implementation parking_lot = { version = "0.10", features = ["nightly"] } -crossbeam = "0.7" +crossbeam = { version = "0.7" } # Logging slog = "2.5" # [Optional] Serde support @@ -22,6 +23,7 @@ serde = { version = "1", optional = true } [features] default = [ "small-object-arenas", # Without this, allocating small objects is slow + "sync", # Thread-safety by default ] # Use very fast dedicated arenas for small objects. # This makes allocation much faster @@ -37,6 +39,11 @@ small-object-arenas = ["once_cell"] # This risks stack overflow at a possible performance gain # See commit 9a9634d68a4933d implicit-grey-stack = [] +# Allow multiple threads to access the garbage collector +# by creating a seperate context for each. +# +# This can increase overhead by requiring communication between threads. +sync = [] [dev-dependencies] # Used for examples :) diff --git a/libs/simple/examples/binary_trees.rs b/libs/simple/examples/binary_trees.rs index 337238a..e7269c9 100644 --- a/libs/simple/examples/binary_trees.rs +++ b/libs/simple/examples/binary_trees.rs @@ -58,7 +58,7 @@ fn main() { o!("bench" => file!()) ); let collector = SimpleCollector::with_logger(logger); - let mut gc = collector.create_context(); + let mut gc = collector.into_context(); { let depth = max_depth + 1; let tree = bottom_up_tree(&gc, depth); diff --git a/libs/simple/src/context/mod.rs b/libs/simple/src/context/mod.rs new file mode 100644 index 0000000..cc91510 --- /dev/null +++ b/libs/simple/src/context/mod.rs @@ -0,0 +1,242 @@ +//! The implementation of [::zerogc::CollectorContext] that is +//! shared among both thread-safe and thread-unsafe code. + +#[cfg(feature = "sync")] +mod sync; +#[cfg(not(feature = "sync"))] +mod simple; +#[cfg(feature = "sync")] +pub use self::sync::*; +#[cfg(not(feature = "sync"))] +pub use self::simple::*; + +use zerogc::prelude::*; +use super::{SimpleCollector, RawSimpleCollector, DynTrace}; +use std::mem::ManuallyDrop; +use std::ptr::NonNull; + + +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +pub enum ContextState { + /// The context is active. + /// + /// Its contents are potentially being mutated, + /// so the `shadow_stack` doesn't necessarily + /// reflect the actual set of thread roots. + /// + /// New objects could be allocated that are not + /// actually being tracked in the `shadow_stack`. + Active, + /// The context is waiting at a safepoint + /// for a collection to complete. + /// + /// The mutating thread is blocked for the + /// duration of the safepoint (until collection completes). + /// + /// Therefore, its `shadow_stack` is guarenteed to reflect + /// the actual set of thread roots. + SafePoint { + /// The id of the collection we are waiting for + collection_id: u64 + }, + /// The context is frozen. + /// Allocation or mutation can't happen + /// but the mutator thread isn't actually blocked. + /// + /// Unlike a safepoint, this is explicitly unfrozen at the + /// user's discretion. + /// + /// Because no allocation or mutation can happen, + /// its shadow_stack stack is guarenteed to + /// accurately reflect the roots of the context. + #[cfg_attr(not(feature = "sync"), allow(unused))] // TODO: Implement frozen for simple contexts? + Frozen, +} +impl ContextState { + #[cfg_attr(not(feature = "sync"), allow(unused))] // TODO: Implement frozen for simple contexts? + fn is_frozen(&self) -> bool { + matches!(*self, ContextState::Frozen) + } +} + +/* + * These form a stack of contexts, + * which all share owns a pointer to the RawContext, + * The raw context is implicitly bound to a single thread + * and manages the state of all the contexts. + * + * https://llvm.org/docs/GarbageCollection.html#the-shadow-stack-gc + * Essentially these objects maintain a shadow stack + * + * The pointer to the RawContext must be Arc, since the + * collector maintains a weak reference to it. + * I use double indirection with a `Rc` because I want + * `recurse_context` to avoid the cost of atomic operations. + * + * SimpleCollectorContexts mirror the application stack. + * They can be stack allocated inside `recurse_context`. + * All we would need to do is internally track ownership of the original + * context. The sub-collector in `recurse_context` is very clearly + * restricted to the lifetime of the closure + * which is a subset of the parent's lifetime. + * + * We still couldn't be Send, since we use interior mutablity + * inside of RawContext that is not thread-safe. + */ +pub struct SimpleCollectorContext { + raw: *mut RawContext, + /// Whether we are the root context + /// + /// Only the root actually owns the `Arc` + /// and is responsible for dropping it + root: bool +} +impl SimpleCollectorContext { + #[cfg(not(feature = "sync"))] + pub(crate) unsafe fn from_collector(collector: &SimpleCollector) -> Self { + SimpleCollectorContext { + raw: Box::into_raw(ManuallyDrop::into_inner( + RawContext::from_collector(collector.0.clone()) + )), + root: true // We are the exclusive owner + } + } + #[cfg(feature = "sync")] + pub(crate) unsafe fn register_root(collector: &SimpleCollector) -> Self { + SimpleCollectorContext { + raw: Box::into_raw(ManuallyDrop::into_inner( + RawContext::register_new(&collector.0) + )), + root: true, // We are responsible for unregistering + } + } + #[inline] + pub(crate) fn collector(&self) -> &RawSimpleCollector { + unsafe { &(*self.raw).collector } + } + #[inline(always)] + unsafe fn with_shadow_stack( + &self, value: *mut &mut T, func: impl FnOnce() -> R + ) -> R { + let old_link = (*(*self.raw).shadow_stack.get()).last; + let new_link = ShadowStackLink { + element: NonNull::new_unchecked( + std::mem::transmute::< + *mut dyn DynTrace, + *mut (dyn DynTrace + 'static) + >(value as *mut dyn DynTrace) + ), + prev: old_link + }; + (*(*self.raw).shadow_stack.get()).last = &new_link; + let result = func(); + debug_assert_eq!( + (*(*self.raw).shadow_stack.get()).last, + &new_link + ); + (*(*self.raw).shadow_stack.get()).last = new_link.prev; + result + } + #[cold] + unsafe fn trigger_basic_safepoint(&self, element: &mut &mut T) { + self.with_shadow_stack(element, || { + (*self.raw).trigger_safepoint(); + }) + } +} +impl Drop for SimpleCollectorContext { + #[inline] + fn drop(&mut self) { + if self.root { + unsafe { + self.collector().free_context(self.raw); + } + } + } +} +unsafe impl GcContext for SimpleCollectorContext { + type System = SimpleCollector; + + #[inline] + unsafe fn basic_safepoint(&mut self, value: &mut &mut T) { + debug_assert_eq!((*self.raw).state.get(), ContextState::Active); + if (*self.raw).collector.should_collect() { + self.trigger_basic_safepoint(value); + } + debug_assert_eq!((*self.raw).state.get(), ContextState::Active); + } + + unsafe fn freeze(&mut self) { + (*self.raw).collector.manager.freeze_context(&*self.raw); + } + + unsafe fn unfreeze(&mut self) { + (*self.raw).collector.manager.unfreeze_context(&*self.raw); + } + + #[inline] + unsafe fn recurse_context(&self, value: &mut &mut T, func: F) -> R + where T: Trace, F: for<'gc> FnOnce(&'gc mut Self, &'gc mut T) -> R { + debug_assert_eq!((*self.raw).state.get(), ContextState::Active); + self.with_shadow_stack(value, || { + let mut sub_context = ManuallyDrop::new(SimpleCollectorContext { + /* + * safe to copy because we wont drop it + * Lifetime is guarenteed to be restricted to + * the closure. + */ + raw: self.raw, + root: false /* don't drop our pointer!!! */ + }); + let result = func(&mut *sub_context, value); + debug_assert!(!sub_context.root); + // No need to run drop code on context..... + std::mem::forget(sub_context); + debug_assert_eq!((*self.raw).state.get(), ContextState::Active); + result + }) + } +} + +/// It's not safe for a context to be sent across threads. +/// +/// We use (thread-unsafe) interior mutability to maintain the +/// shadow stack. Since we could potentially be cloned via `safepoint_recurse!`, +/// implementing `Send` would allow another thread to obtain a +/// reference to our internal `&RefCell`. Further mutation/access +/// would be undefined..... +impl !Send for SimpleCollectorContext {} + +// +// Root tracking +// + +#[repr(C)] +#[derive(Debug)] +pub(crate) struct ShadowStackLink { + pub element: NonNull, + /// The previous link in the chain, + /// or NULL if there isn't any + pub prev: *const ShadowStackLink +} + +#[derive(Clone, Debug)] +pub struct ShadowStack { + /// The last element in the shadow stack, + /// or NULL if it's empty + pub(crate) last: *const ShadowStackLink +} +impl ShadowStack { + unsafe fn as_vec(&self) -> Vec<*mut dyn DynTrace> { + let mut result: Vec<_> = self.reverse_iter().collect(); + result.reverse(); + result + } + #[inline] + pub(crate) unsafe fn reverse_iter(&self) -> impl Iterator + '_ { + std::iter::successors( + self.last.as_ref(), + |link| link.prev.as_ref() + ).map(|link| link.element.as_ptr()) + } +} diff --git a/libs/simple/src/context/simple.rs b/libs/simple/src/context/simple.rs new file mode 100644 index 0000000..c9a934b --- /dev/null +++ b/libs/simple/src/context/simple.rs @@ -0,0 +1,209 @@ +//! A simpler implementation of [::zerogc::CollectorContext] +//! that doesn't support multiple threads/contexts. +//! +//! In exchange, there is no locking :) + +use std::sync::{Arc}; +use std::cell::{Cell, UnsafeCell, RefCell}; + +use slog::{Logger, FnValue, trace, o}; + +use crate::{RawSimpleCollector}; +use std::mem::ManuallyDrop; +use std::fmt::{self, Debug, Formatter}; +use crate::context::{ShadowStack, ContextState}; + +/// Manages coordination of garbage collections +/// +/// This is factored out of the main code mainly due to +/// differences from single-threaded collection +pub(crate) struct CollectionManager { + /// Access to the internal state + state: RefCell, + /// Whether a collection is currently in progress + /// + /// Used for debugging only + collecting: Cell, + /// Sanity check to ensure there's only one context + has_existing_context: Cell, +} +impl CollectionManager { + pub(crate) fn new() -> Self { + CollectionManager { + state: RefCell::new(CollectorState::new()), + collecting: Cell::new(false), + has_existing_context: Cell::new(false) + } + } + #[inline] + pub fn is_collecting(&self) -> bool { + self.collecting.get() + } + #[inline] + pub fn should_trigger_collection(&self) -> bool { + /* + * Unlike the sync context manager, we can assume + * there is only a single thread. + * Therefore we don't need to check for other threads + * having a collection in progress when we're at a safepoint. + * + * If we were having a collection, control flow is already + * taken over by the collector ;) + */ + false + } + pub(super) unsafe fn freeze_context(&self, context: &RawContext) { + debug_assert_eq!(context.state.get(), ContextState::Active); + unimplemented!("Freezing single-threaded contexts") + } + pub(super) unsafe fn unfreeze_context(&self, _context: &RawContext) { + // We can't freeze, so we sure can't unfreeze + unreachable!("Can't unfreeze a single-threaded context") + } +} +pub struct RawContext { + /// Since we're the only context, we should (logically) + /// be the only owner. + /// + /// This is still an Arc for easier use alongside the + /// thread-safe implementation + pub(crate) collector: Arc, + // NOTE: We are Send, not Sync + pub(super) shadow_stack: UnsafeCell, + // TODO: Does the collector access this async? + pub(super) state: Cell, + logger: Logger +} +impl Debug for RawContext { + fn fmt(&self, f: &mut Formatter) -> fmt::Result { + f.debug_struct("RawContext") + .field( + "collector", + &format_args!("{:p}", &self.collector) + ) + .field( + "shadow_stacks", + // We're assuming this is valid.... + unsafe { &*self.shadow_stack.get() } + ) + .field("state", &self.state.get()) + .finish() + } +} +impl RawContext { + pub(crate) unsafe fn from_collector(collector: Arc) -> ManuallyDrop> { + assert!( + !collector.manager.has_existing_context + .replace(true), + "Already created a context for the collector!" + ); + let logger = collector.logger.new(o!()); + let context = ManuallyDrop::new(Box::new(RawContext { + logger: logger.clone(), collector, + shadow_stack: UnsafeCell::new(ShadowStack { + last: std::ptr::null_mut(), + }), + state: Cell::new(ContextState::Active) + })); + trace!( + logger, "Initializing context"; + "ptr" => format_args!("{:p}", &**context), + ); + context + } + /// Trigger a safepoint for this context. + /// + /// This implicitly attempts a collection, + /// potentially blocking until completion.. + /// + /// Undefined behavior if mutated during collection + #[cold] + #[inline(never)] + pub unsafe fn trigger_safepoint(&self) { + /* + * Begin a collection. + * + * Since we are the only collector we don't have + * to worry about awaiting other threads stopping + * at a safepoint. + * This simplifies the implementation considerably. + */ + assert!(!self.collector.collecting.get()); + self.collector.collecting.set(true); + let collection_id = self.collector.state.borrow_mut() + .next_pending_id(); + trace!( + self.logger, + "Creating collector"; + "id" => collection_id, + "ctx_ptr" => format_args!("{:?}", self) + ); + let shadow_stack = &*self.shadow_stack.get(); + let ptr = self as *const RawContext as *mut RawContext; + // Change our state to mark we are now waiting at a safepoint + assert_eq!(self.state.replace(ContextState::SafePoint { + collection_id, + }), ContextState::Active); + trace!( + self.logger, "Beginning collection"; + "ptr" => ?ptr, + "shadow_stack" => FnValue(|_| format!("{:?}", shadow_stack.as_vec())), + "state" => ?self.state, + "collection_id" => collection_id, + "original_size" => self.collector.heap.allocator.allocated_size(), + ); + self.collector.perform_raw_collection(&[ptr]); + assert_eq!( + self.state.replace(ContextState::Active), + ContextState::SafePoint { collection_id } + ); + assert!(self.collector.collecting.replace(false)); + } + /// Borrow a reference to the shadow stack, + /// assuming this context is valid (not active). + /// + /// A context is valid if it is either frozen + /// or paused at a safepont. + pub unsafe fn assume_valid_shadow_stack(&self) -> &ShadowStack { + match self.state.get() { + ContextState::Active => unreachable!("active context: {:?}", self), + ContextState::SafePoint { .. } | ContextState::Frozen { .. } => {}, + } + &*self.shadow_stack.get() + } +} + +// Pending collections + +/// Keeps track of a pending collection (if any) +/// +/// This must be held under a write lock for a collection to happen. +/// This must be held under a read lock to prevent collections. +pub struct CollectorState { + next_pending_id: u64 +} +impl CollectorState { + pub fn new() -> Self { + CollectorState { + next_pending_id: 0 + } + } + fn next_pending_id(&mut self) -> u64 { + let id = self.next_pending_id; + self.next_pending_id = id.checked_add(1) + .expect("Overflow"); + id + } +} +/* + * Implementing methods to control collector state. + * + * Because we're defined in the same crate + * we can use an inhernent impl instead of an extension trait. + */ +impl RawSimpleCollector { + #[inline] + pub(crate) unsafe fn free_context(&self, _raw: *mut RawContext) { + // No extra work to do - automatic Drop handles everything + } +} diff --git a/libs/simple/src/context.rs b/libs/simple/src/context/sync.rs similarity index 72% rename from libs/simple/src/context.rs rename to libs/simple/src/context/sync.rs index 7494015..5164065 100644 --- a/libs/simple/src/context.rs +++ b/libs/simple/src/context/sync.rs @@ -1,57 +1,105 @@ use std::sync::{Arc}; use std::cell::{Cell, UnsafeCell}; -use std::sync::atomic::{Ordering}; +use std::sync::atomic::{Ordering, AtomicBool}; use slog::{Logger, FnValue, trace, Drain, o}; -use zerogc::{GcContext, Trace}; -use crate::{SimpleCollector, RawSimpleCollector, DynTrace}; +use crate::{RawSimpleCollector}; use crate::utils::ThreadId; use std::mem::ManuallyDrop; use std::fmt::{Debug, Formatter}; use std::{fmt, mem}; use std::collections::HashSet; -use parking_lot::{Mutex, RwLockReadGuard, RwLockUpgradableReadGuard, RwLockWriteGuard}; -use std::ptr::NonNull; +use parking_lot::{Mutex, RwLockReadGuard, RwLockUpgradableReadGuard, RwLockWriteGuard, Condvar, RwLock}; -#[derive(Copy, Clone, Debug, Eq, PartialEq)] -enum ContextState { - /// The context is active. +use super::{ShadowStack}; +use crate::context::ContextState; + +/// Manages coordination of garbage collections +/// +/// This is factored out of the main code mainly due to +/// differences from single-threaded collection +pub(crate) struct CollectionManager { + /// Lock on the internal state + state: RwLock, + /// Simple flag on whether we're currently collecting /// - /// Its contents are potentially being mutated, - /// so the `shadow_stack` doesn't necessarily - /// reflect the actual set of thread roots. + /// This should be true whenever `self.state.pending` is `Some`. + /// However if you don't hold the lock you may not always + /// see a fully consistent state. + collecting: AtomicBool, + /// The condition variable for all contexts to be valid /// - /// New objects could be allocated that are not - /// actually being tracked in the `shadow_stack`. - Active, - /// The context is waiting at a safepoint - /// for a collection to complete. + /// In order to be valid, a context must be either frozen + /// or paused at a safepoint. /// - /// The mutating thread is blocked for the - /// duration of the safepoint (until collection completes). + /// After a collection is marked as pending, threads must wait until + /// all contexts are valid before the actual work can begin. + valid_contexts_wait: Condvar, + /// Wait until a garbage collection is over. /// - /// Therefore, its `shadow_stack` is guarenteed to reflect - /// the actual set of thread roots. - SafePoint { - /// The id of the collection we are waiting for - collection_id: u64 - }, - /// The context is frozen. - /// Allocation or mutation can't happen - /// but the mutator thread isn't actually blocked. + /// This must be separate from `valid_contexts_wait`. + /// A garbage collection can only begin once all contexts are valid, + /// so this condition logically depends on `valid_contexts_wait`. + collection_wait: Condvar, + /// The mutex used alongside `valid_contexts_wait` /// - /// Unlike a safepoint, this is explicitly unfrozen at the - /// user's discretion. + /// This doesn't actually protect any data. It's just + /// used because [parking_lot::RwLock] doesn't support condition vars. + /// This is the [officially suggested solution](https://github.com/Amanieu/parking_lot/issues/165) + // TODO: Should we replace this with `known_collections`? + valid_contexts_lock: Mutex<()>, + /// The mutex used alongside `collection_wait` /// - /// Because no allocation or mutation can happen, - /// its shadow_stack stack is guarenteed to - /// accurately reflect the roots of the context. - Frozen, + /// Like `collection_wait`, his doesn't actually protect any data. + collection_wait_lock: Mutex<()>, } -impl ContextState { - fn is_frozen(&self) -> bool { - matches!(*self, ContextState::Frozen) +impl CollectionManager { + pub(crate) fn new() -> Self { + CollectionManager { + state: RwLock::new(CollectorState::new()), + valid_contexts_wait: Condvar::new(), + collection_wait: Condvar::new(), + valid_contexts_lock: Mutex::new(()), + collection_wait_lock: Mutex::new(()), + collecting: AtomicBool::new(false), + } + } + #[inline] + pub fn should_trigger_collection(&self) -> bool { + /* + * Use relaxed ordering. Eventually consistency is correct + * enough for our use cases. It may delay some threads reaching + * a safepoint for a little bit, but it avoids an expensive + * memory fence on ARM and weakly ordered architectures. + */ + self.collecting.load(Ordering::Relaxed) + } + #[inline] + pub fn is_collecting(&self) -> bool { + self.collecting.load(Ordering::SeqCst) + } + pub(super) unsafe fn freeze_context(&self, context: &RawContext) { + assert_eq!(context.state.get(), ContextState::Active); + // TODO: Isn't this state read concurrently? + context.state.set(ContextState::Frozen); + /* + * We may need to notify others that we are frozen + * This means we are now "valid" for the purposes of + * collection ^_^ + */ + self.valid_contexts_wait.notify_all(); + } + pub(super) unsafe fn unfreeze_context(&self, context: &RawContext) { + /* + * A pending collection might be relying in the validity of this + * context's shadow stack, so unfreezing it while in progress + * could trigger undefined behavior!!!!! + */ + context.collector.prevent_collection(|_| { + assert_eq!(context.state.get(), ContextState::Frozen); + context.state.set(ContextState::Active); + }) } } @@ -59,10 +107,10 @@ pub struct RawContext { pub(crate) collector: Arc, original_thread: ThreadId, // NOTE: We are Send, not Sync - shadow_stack: UnsafeCell, + pub(super) shadow_stack: UnsafeCell, // TODO: Does the collector access this async? - state: Cell, - logger: Logger + pub(super) state: Cell, + pub(super) logger: Logger } impl Debug for RawContext { fn fmt(&self, f: &mut Formatter) -> fmt::Result { @@ -127,7 +175,7 @@ impl RawContext { let state = &mut *guard; // If there is not a active `PendingCollection` - create one if state.pending.is_none() { - assert!(!self.collector.collecting.compare_and_swap( + assert!(!self.collector.manager.collecting.compare_and_swap( false, true, Ordering::SeqCst )); let id = state.next_pending_id(); @@ -236,205 +284,6 @@ impl RawContext { &*self.shadow_stack.get() } } -/* - * These form a stack of contexts, - * which all share owns a pointer to the RawContext, - * The raw context is implicitly bound to a single thread - * and manages the state of all the contexts. - * - * https://llvm.org/docs/GarbageCollection.html#the-shadow-stack-gc - * Essentially these objects maintain a shadow stack - * - * The pointer to the RawContext must be Arc, since the - * collector maintains a weak reference to it. - * I use double indirection with a `Rc` because I want - * `recurse_context` to avoid the cost of atomic operations. - * - * SimpleCollectorContexts mirror the application stack. - * They can be stack allocated inside `recurse_context`. - * All we would need to do is internally track ownership of the original - * context. The sub-collector in `recurse_context` is very clearly - * restricted to the lifetime of the closure - * which is a subset of the parent's lifetime. - * - * We still couldn't be Send, since we use interior mutablity - * inside of RawContext that is not thread-safe. - */ -pub struct SimpleCollectorContext { - raw: *mut RawContext, - /// Whether we are the root context - /// - /// Only the root actually owns the `Arc` - /// and is responsible for dropping it - root: bool -} -impl SimpleCollectorContext { - pub(crate) unsafe fn register_root(collector: &SimpleCollector) -> Self { - SimpleCollectorContext { - raw: Box::into_raw(ManuallyDrop::into_inner( - RawContext::register_new(&collector.0) - )), - root: true, // We are responsible for unregistering - } - } - #[inline] - pub(crate) fn collector(&self) -> &RawSimpleCollector { - unsafe { &(*self.raw).collector } - } - #[inline(always)] - unsafe fn with_shadow_stack( - &self, value: *mut &mut T, func: impl FnOnce() -> R - ) -> R { - let old_link = (*(*self.raw).shadow_stack.get()).last; - let new_link = ShadowStackLink { - element: NonNull::new_unchecked( - std::mem::transmute::< - *mut dyn DynTrace, - *mut (dyn DynTrace + 'static) - >(value as *mut dyn DynTrace) - ), - prev: old_link - }; - (*(*self.raw).shadow_stack.get()).last = &new_link; - let result = func(); - debug_assert_eq!( - (*(*self.raw).shadow_stack.get()).last, - &new_link - ); - (*(*self.raw).shadow_stack.get()).last = new_link.prev; - result - } - #[cold] - unsafe fn trigger_basic_safepoint(&self, element: &mut &mut T) { - self.with_shadow_stack(element, || { - (*self.raw).trigger_safepoint(); - }) - } -} -impl Drop for SimpleCollectorContext { - #[inline] - fn drop(&mut self) { - if self.root { - let collector = self.collector(); - unsafe { - trace!( - collector.logger, "Freeing context"; - "ptr" => format_args!("{:p}", self.raw), - "current_thread" => FnValue(|_| ThreadId::current()), - "state" => ?(*self.raw).state.get() - ); - collector.free_context(ManuallyDrop::new( - Box::from_raw(self.raw) - )); - } - } - } -} -unsafe impl GcContext for SimpleCollectorContext { - type System = SimpleCollector; - - #[inline] - unsafe fn basic_safepoint(&mut self, value: &mut &mut T) { - debug_assert_eq!((*self.raw).state.get(), ContextState::Active); - if (*self.raw).collector.should_collect_relaxed() { - self.trigger_basic_safepoint(value); - } - debug_assert_eq!((*self.raw).state.get(), ContextState::Active); - } - - unsafe fn freeze(&mut self) { - assert_eq!((*self.raw).state.get(), ContextState::Active); - // TODO: Isn't this state read concurrently? - (*self.raw).state.set(ContextState::Frozen); - /* - * We may need to notify others that we are frozen - * This means we are now "valid" for the purposes of - * collection ^_^ - */ - (*self.raw).collector.valid_contexts_wait.notify_all(); - } - - unsafe fn unfreeze(&mut self) { - /* - * A pending collection might be relying in the validity of this - * context's shadow stack, so unfreezing it while in progress - * could trigger undefined behavior!!!!! - */ - self.collector().prevent_collection(|_| { - assert_eq!((*self.raw).state.get(), ContextState::Frozen); - (*self.raw).state.set(ContextState::Active); - }) - } - - #[inline] - unsafe fn recurse_context(&self, value: &mut &mut T, func: F) -> R - where T: Trace, F: for<'gc> FnOnce(&'gc mut Self, &'gc mut T) -> R { - debug_assert_eq!((*self.raw).state.get(), ContextState::Active); - self.with_shadow_stack(value, || { - let mut sub_context = ManuallyDrop::new(SimpleCollectorContext { - /* - * safe to copy because we wont drop it - * Lifetime is guarenteed to be restricted to - * the closure. - */ - raw: self.raw, - root: false /* don't drop our pointer!!! */ - }); - let result = func(&mut *sub_context, value); - debug_assert!(!sub_context.root); - // No need to run drop code on context..... - std::mem::forget(sub_context); - debug_assert_eq!((*self.raw).state.get(), ContextState::Active); - result - }) - } -} - -/// It's not safe for a context to be sent across threads. -/// -/// We use (thread-unsafe) interior mutability to maintain the -/// shadow stack. Since we could potentially be cloned via `safepoint_recurse!`, -/// implementing `Send` would allow another thread to obtain a -/// reference to our internal `&RefCell`. Further mutation/access -/// would be undefined..... -impl !Send for SimpleCollectorContext {} - -// -// Root tracking -// - -#[repr(C)] -#[derive(Debug)] -pub(crate) struct ShadowStackLink { - pub element: NonNull, - /// The previous link in the chain, - /// or NULL if there isn't any - pub prev: *const ShadowStackLink -} - -#[derive(Clone, Debug)] -pub struct ShadowStack { - /// The last element in the shadow stack, - /// or NULL if it's empty - pub(crate) last: *const ShadowStackLink -} -impl ShadowStack { - unsafe fn as_vec(&self) -> Vec<*mut dyn DynTrace> { - let mut result: Vec<_> = self.reverse_iter().collect(); - result.reverse(); - result - } - #[inline] - pub(crate) unsafe fn reverse_iter(&self) -> impl Iterator + '_ { - std::iter::successors( - self.last.as_ref(), - |link| link.prev.as_ref() - ).map(|link| link.element.as_ptr()) - } -} - -// Frozen contexts - // Pending collections /// Keeps track of a pending collection (if any) @@ -504,13 +353,18 @@ impl RawSimpleCollector { old_num_known }) } - pub unsafe fn free_context(&self, raw: ManuallyDrop>) { + pub unsafe fn free_context(&self, raw: *mut RawContext) { + trace!( + self.logger, "Freeing context"; + "ptr" => format_args!("{:p}", raw), + "state" => ?(*raw).state.get() + ); /* * TODO: Consider using regular read instead of `read_upgradable` * This is only prevented because of the fact that we may * need to mutate `valid_contexts` and `total_contexts` */ - let ptr = &**raw as *const RawContext as *mut RawContext; + let ptr = raw; // TODO - blegh let guard = self.state.upgradable_read(); match &guard.pending { None => { // No collection @@ -545,7 +399,7 @@ impl RawSimpleCollector { assert_eq!( pending.valid_contexts.iter() .find(|&&ctx| std::ptr::eq(ctx, ptr)), - None, "state = {:?}", raw.state.get() + None, "state = {:?}", (*raw).state.get() ); pending.total_contexts -= 1; assert!(guard.known_contexts.get_mut().remove(&ptr)); @@ -556,7 +410,7 @@ impl RawSimpleCollector { }, } // Now drop the Box - drop(ManuallyDrop::into_inner(raw)); + drop(Box::from_raw(raw)); /* * Notify all threads waiting for contexts to be valid. * TODO: I think this is really only useful if we're waiting.... diff --git a/libs/simple/src/handles.rs b/libs/simple/src/handles.rs index 4146be8..823612d 100644 --- a/libs/simple/src/handles.rs +++ b/libs/simple/src/handles.rs @@ -380,6 +380,7 @@ impl GcHandle { } unsafe impl ::zerogc::GcHandle for GcHandle { type Context = SimpleCollectorContext; + #[cfg(feature = "sync")] fn use_critical(&self, func: impl FnOnce(&T) -> R) -> R { let collector = self.collector.upgrade() .expect("Dead collector"); @@ -398,6 +399,10 @@ unsafe impl ::zerogc::GcHandle for GcHandle { func(&*value) }) } + #[cfg(not(feature = "sync"))] + fn use_critical(&self, _func: impl FnOnce(&T) -> R) -> R { + unimplemented!("critical sections for single-collector impl") + } } unsafe impl<'new_gc, T> GcBindHandle<'new_gc, T> for GcHandle where T: GcSafe, T: GcBrand<'new_gc, SimpleCollector>, diff --git a/libs/simple/src/lib.rs b/libs/simple/src/lib.rs index ac59e13..63ce03c 100644 --- a/libs/simple/src/lib.rs +++ b/libs/simple/src/lib.rs @@ -31,16 +31,47 @@ use std::fmt; use std::marker::PhantomData; use std::sync::Arc; use std::sync::atomic::{AtomicUsize, AtomicBool, Ordering}; -use crossbeam::atomic::AtomicCell; +#[cfg(not(feature = "sync"))] +use std::cell::Cell; + use slog::{Logger, FnValue, o, debug}; -use crate::context::{CollectorState, RawContext}; +use crate::context::{RawContext}; use crate::utils::ThreadId; pub use crate::context::SimpleCollectorContext; -use parking_lot::{Condvar, Mutex, RwLock}; use handles::GcHandleList; +#[cfg(feature = "sync")] +type AtomicCell = ::crossbeam::atomic::AtomicCell; +/// Fallback `AtomicCell` implementation when we actually +/// don't care about thread safety +#[cfg(not(feature = "sync"))] +#[derive(Default)] +struct AtomicCell(Cell); +#[cfg(not(feature = "sync"))] +impl AtomicCell { + fn new(value: T) -> Self { + AtomicCell(Cell::new(value)) + } + fn store(&self, value: T) { + self.0.set(value) + } + fn load(&self) -> T { + self.0.get() + } + fn compare_exchange(&self, expected: T, updated: T) -> Result + where T: PartialEq { + let existing = self.0.get(); + if existing == expected { + self.0.set(updated); + Ok(existing) + } else { + Err(existing) + } + } +} + mod handles; mod context; mod utils; @@ -251,12 +282,7 @@ impl SimpleCollector { pub fn with_logger(logger: Logger) -> Self { let mut collector = Arc::new(RawSimpleCollector { logger, - state: RwLock::new(CollectorState::new()), - valid_contexts_wait: Condvar::new(), - collection_wait: Condvar::new(), - valid_contexts_lock: Mutex::new(()), - collection_wait_lock: Mutex::new(()), - collecting: AtomicBool::new(false), + manager: context::CollectionManager::new(), heap: GcHeap::new(INITIAL_COLLECTION_THRESHOLD), handle_list: GcHandleList::new(), }); @@ -271,9 +297,20 @@ impl SimpleCollector { /// /// Warning: Only one collector should be created per thread. /// Doing otherwise can cause deadlocks/panics. + #[cfg(feature = "sync")] pub fn create_context(&self) -> SimpleCollectorContext { unsafe { SimpleCollectorContext::register_root(&self) } } + /// Convert this collector into a unique context + /// + /// The single-threaded implementation only allows a single context, + /// so this method is nessicarry to support it. + pub fn into_context(self) -> SimpleCollectorContext { + #[cfg(feature = "sync")] + { self.create_context() } + #[cfg(not(feature = "sync"))] + unsafe { SimpleCollectorContext::from_collector(&self) } + } } unsafe impl GcSystem for SimpleCollector {} @@ -557,45 +594,14 @@ unsafe impl Sync for RawSimpleCollector {} /// The internal data for a simple collector struct RawSimpleCollector { logger: Logger, - state: RwLock, heap: GcHeap, - /// Simple flag on whether we're currently collecting - /// - /// This should be true whenever `self.state.pending` is `Some`. - /// However if you don't hold the lock you may not always - /// see a fully consistent state. - collecting: AtomicBool, - /// The condition variable for all contexts to be valid - /// - /// In order to be valid, a context must be either frozen - /// or paused at a safepoint. - /// - /// After a collection is marked as pending, threads must wait until - /// all contexts are valid before the actual work can begin. - valid_contexts_wait: Condvar, - /// Wait until a garbage collection is over. - /// - /// This must be separate from `valid_contexts_wait`. - /// A garbage collection can only begin once all contexts are valid, - /// so this condition logically depends on `valid_contexts_wait`. - collection_wait: Condvar, - /// The mutex used alongside `valid_contexts_wait` - /// - /// This doesn't actually protect any data. It's just - /// used because [parking_lot::RwLock] doesn't support condition vars. - /// This is the [officially suggested solution](https://github.com/Amanieu/parking_lot/issues/165) - // TODO: Should we replace this with `known_collections`? - valid_contexts_lock: Mutex<()>, - /// The mutex used alongside `collection_wait` - /// - /// Like `collection_wait`, his doesn't actually protect any data. - collection_wait_lock: Mutex<()>, + manager: self::context::CollectionManager, /// Tracks object handles handle_list: GcHandleList } impl RawSimpleCollector { #[inline] - fn should_collect_relaxed(&self) -> bool { + fn should_collect(&self) -> bool { /* * Use relaxed ordering, just like `should_collect` * @@ -603,15 +609,15 @@ impl RawSimpleCollector { * More importantly this is easier for a JIT to implement inline!!! * As of this writing cranelift doesn't even seem to support fences :o */ - self.heap.should_collect_relaxed() || self.collecting - .load(Ordering::Relaxed) + self.heap.should_collect_relaxed() || + self.manager.should_trigger_collection() } #[cold] #[inline(never)] unsafe fn perform_raw_collection( &self, contexts: &[*mut RawContext] ) { - debug_assert!(self.collecting.load(Ordering::SeqCst)); + debug_assert!(self.manager.is_collecting()); let roots: Vec<*mut dyn DynTrace> = contexts.iter() .flat_map(|ctx| { (**ctx).assume_valid_shadow_stack() @@ -644,6 +650,14 @@ impl RawSimpleCollector { ); } } +impl Deref for RawSimpleCollector { + type Target = context::CollectionManager; + + #[inline] + fn deref(&self) -> &Self::Target { + &self.manager // TODO: Do this explicitly + } +} struct CollectionTask<'a> { expected_collector: *mut RawSimpleCollector, roots: Vec<*mut dyn DynTrace>, diff --git a/libs/simple/src/utils.rs b/libs/simple/src/utils.rs index 82557c0..2e004d4 100644 --- a/libs/simple/src/utils.rs +++ b/libs/simple/src/utils.rs @@ -3,6 +3,7 @@ use std::fmt; #[derive(Clone)] pub enum ThreadId { + #[allow(unused)] Nop, Enabled { id: std::thread::ThreadId,