From e1fdf8bc54f1d2a5d83697ac881d6e4a2b425133 Mon Sep 17 00:00:00 2001 From: James Bornholt Date: Mon, 22 May 2023 11:00:36 -0500 Subject: [PATCH] Make {Mutex, Condvar, RwLock}::new const (#106) These constructors have been const since Rust 1.63 (https://github.com/rust-lang/rust/issues/93740). It's pretty easy for us to make them const too, which allows code that relies on them being const to correctly compile with Shuttle. The one exception is that HashMap::new isn't const, and our Condvar implementation uses a HashMap to track waiters. I took the easy way out and just used a vector as an association list instead -- we shouldn't expect large numbers of waiters on the same condvar, so this shouldn't be too inefficient. --- Cargo.toml | 3 ++- src/runtime/task/clock.rs | 6 ++++-- src/runtime/task/mod.rs | 8 +++---- src/sync/condvar.rs | 20 +++++++++-------- src/sync/mutex.rs | 7 +++--- src/sync/rwlock.rs | 45 +++++++++++++++++++-------------------- 6 files changed, 45 insertions(+), 44 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 1b7300e6..320c783e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -9,6 +9,7 @@ keywords = ["concurrency", "lock", "thread", "async"] categories = ["asynchronous", "concurrency", "development-tools::testing"] [dependencies] +assoc = "0.1.3" bitvec = "1.0.1" generator = "0.7.1" hex = "0.4.2" @@ -17,7 +18,7 @@ rand_core = "0.6.4" rand = "0.8.5" rand_pcg = "0.3.1" scoped-tls = "1.0.0" -smallvec = "1.6.1" +smallvec = { version = "1.10.0", features = ["const_new"] } tracing = { version = "0.1.21", default-features = false, features = ["std"] } [dev-dependencies] diff --git a/src/runtime/task/clock.rs b/src/runtime/task/clock.rs index 0a18c52a..0670971e 100644 --- a/src/runtime/task/clock.rs +++ b/src/runtime/task/clock.rs @@ -8,8 +8,10 @@ pub struct VectorClock { } impl VectorClock { - pub(crate) fn new() -> Self { - Self { time: SmallVec::new() } + pub(crate) const fn new() -> Self { + Self { + time: SmallVec::new_const(), + } } #[cfg(test)] diff --git a/src/runtime/task/mod.rs b/src/runtime/task/mod.rs index fbc6265b..272c91fe 100644 --- a/src/runtime/task/mod.rs +++ b/src/runtime/task/mod.rs @@ -427,10 +427,8 @@ pub(crate) struct TaskSet { } impl TaskSet { - pub fn new() -> Self { - Self { - tasks: BitVec::from_bitslice(bits![0; DEFAULT_INLINE_TASKS]), - } + pub const fn new() -> Self { + Self { tasks: BitVec::EMPTY } } pub fn contains(&self, tid: TaskId) -> bool { @@ -446,7 +444,7 @@ impl TaskSet { /// the set did have this value present, `false` is returned. pub fn insert(&mut self, tid: TaskId) -> bool { if tid.0 >= self.tasks.len() { - self.tasks.resize(1 + tid.0, false); + self.tasks.resize(DEFAULT_INLINE_TASKS.max(1 + tid.0), false); } !std::mem::replace(&mut *self.tasks.get_mut(tid.0).unwrap(), true) } diff --git a/src/sync/condvar.rs b/src/sync/condvar.rs index 7b816fcd..822bd9e3 100644 --- a/src/sync/condvar.rs +++ b/src/sync/condvar.rs @@ -4,9 +4,9 @@ use crate::runtime::task::clock::VectorClock; use crate::runtime::task::TaskId; use crate::runtime::thread; use crate::sync::MutexGuard; +use assoc::AssocExt; use std::cell::RefCell; -use std::collections::{HashMap, VecDeque}; -use std::rc::Rc; +use std::collections::VecDeque; use std::sync::{LockResult, PoisonError}; use std::time::Duration; use tracing::trace; @@ -15,12 +15,13 @@ use tracing::trace; /// waiting for an event to occur. #[derive(Debug)] pub struct Condvar { - state: Rc>, + state: RefCell, } #[derive(Debug)] struct CondvarState { - waiters: HashMap, + // TODO: this should be a HashMap but [HashMap::new] is not const + waiters: Vec<(TaskId, CondvarWaitStatus)>, next_epoch: usize, } @@ -114,14 +115,14 @@ enum CondvarWaitStatus { // and can run in any order (because they are all contending on the same mutex). impl Condvar { /// Creates a new condition variable which is ready to be waited on and notified. - pub fn new() -> Self { + pub const fn new() -> Self { let state = CondvarState { - waiters: HashMap::new(), + waiters: Vec::new(), next_epoch: 0, }; Self { - state: Rc::new(RefCell::new(state)), + state: RefCell::new(state), } } @@ -133,7 +134,8 @@ impl Condvar { trace!(waiters=?state.waiters, next_epoch=state.next_epoch, "waiting on condvar {:p}", self); - assert!(state.waiters.insert(me, CondvarWaitStatus::Waiting).is_none()); + debug_assert!(<_ as AssocExt<_, _>>::get(&state.waiters, &me).is_none()); + state.waiters.push((me, CondvarWaitStatus::Waiting)); // TODO: Condvar::wait should allow for spurious wakeups. ExecutionState::with(|s| s.current_mut().block(false)); drop(state); @@ -144,7 +146,7 @@ impl Condvar { // After the context switch, consume whichever signal that woke this thread let mut state = self.state.borrow_mut(); trace!(waiters=?state.waiters, next_epoch=state.next_epoch, "woken from condvar {:p}", self); - let my_status = state.waiters.remove(&me).expect("should be waiting"); + let my_status = <_ as AssocExt<_, _>>::remove(&mut state.waiters, &me).expect("should be waiting"); match my_status { CondvarWaitStatus::Broadcast(clock) => { // Woken by a broadcast, so nothing to do except update the clock diff --git a/src/sync/mutex.rs b/src/sync/mutex.rs index 943c6e25..43ad066e 100644 --- a/src/sync/mutex.rs +++ b/src/sync/mutex.rs @@ -6,13 +6,12 @@ use std::cell::RefCell; use std::fmt::{Debug, Display}; use std::ops::{Deref, DerefMut}; use std::panic::{RefUnwindSafe, UnwindSafe}; -use std::rc::Rc; use std::sync::{LockResult, PoisonError, TryLockError, TryLockResult}; use tracing::trace; /// A mutex, the same as [`std::sync::Mutex`]. pub struct Mutex { - state: Rc>, + state: RefCell, inner: std::sync::Mutex, } @@ -31,7 +30,7 @@ struct MutexState { impl Mutex { /// Creates a new mutex in an unlocked state ready for use. - pub fn new(value: T) -> Self { + pub const fn new(value: T) -> Self { let state = MutexState { holder: None, waiters: TaskSet::new(), @@ -40,7 +39,7 @@ impl Mutex { Self { inner: std::sync::Mutex::new(value), - state: Rc::new(RefCell::new(state)), + state: RefCell::new(state), } } } diff --git a/src/sync/rwlock.rs b/src/sync/rwlock.rs index bf5d3fac..e046b729 100644 --- a/src/sync/rwlock.rs +++ b/src/sync/rwlock.rs @@ -6,7 +6,6 @@ use std::cell::RefCell; use std::fmt::{Debug, Display}; use std::ops::{Deref, DerefMut}; use std::panic::{RefUnwindSafe, UnwindSafe}; -use std::rc::Rc; use std::sync::{LockResult, PoisonError, TryLockError, TryLockResult}; use tracing::trace; @@ -16,7 +15,7 @@ use tracing::trace; /// `RwLock` more than once. The `std` version is ambiguous about what behavior is allowed here, so /// we choose the most conservative one. pub struct RwLock { - state: Rc>, + state: RefCell, inner: std::sync::RwLock, } @@ -43,7 +42,7 @@ enum RwLockType { impl RwLock { /// Create a new instance of an `RwLock` which is unlocked. - pub fn new(value: T) -> Self { + pub const fn new(value: T) -> Self { let state = RwLockState { holder: RwLockHolder::None, waiting_readers: TaskSet::new(), @@ -53,7 +52,7 @@ impl RwLock { Self { inner: std::sync::RwLock::new(value), - state: Rc::new(RefCell::new(state)), + state: RefCell::new(state), } } } @@ -67,12 +66,12 @@ impl RwLock { match self.inner.try_read() { Ok(guard) => Ok(RwLockReadGuard { inner: Some(guard), - state: Rc::clone(&self.state), + rwlock: self, me: ExecutionState::me(), }), Err(TryLockError::Poisoned(err)) => Err(PoisonError::new(RwLockReadGuard { inner: Some(err.into_inner()), - state: Rc::clone(&self.state), + rwlock: self, me: ExecutionState::me(), })), Err(TryLockError::WouldBlock) => panic!("rwlock state out of sync"), @@ -87,12 +86,12 @@ impl RwLock { match self.inner.try_write() { Ok(guard) => Ok(RwLockWriteGuard { inner: Some(guard), - state: Rc::clone(&self.state), + rwlock: self, me: ExecutionState::me(), }), Err(TryLockError::Poisoned(err)) => Err(PoisonError::new(RwLockWriteGuard { inner: Some(err.into_inner()), - state: Rc::clone(&self.state), + rwlock: self, me: ExecutionState::me(), })), Err(TryLockError::WouldBlock) => panic!("rwlock state out of sync"), @@ -111,12 +110,12 @@ impl RwLock { match self.inner.try_read() { Ok(guard) => Ok(RwLockReadGuard { inner: Some(guard), - state: Rc::clone(&self.state), + rwlock: self, me: ExecutionState::me(), }), Err(TryLockError::Poisoned(err)) => Err(TryLockError::Poisoned(PoisonError::new(RwLockReadGuard { inner: Some(err.into_inner()), - state: Rc::clone(&self.state), + rwlock: self, me: ExecutionState::me(), }))), Err(TryLockError::WouldBlock) => panic!("rwlock state out of sync"), @@ -135,12 +134,12 @@ impl RwLock { match self.inner.try_write() { Ok(guard) => Ok(RwLockWriteGuard { inner: Some(guard), - state: Rc::clone(&self.state), + rwlock: self, me: ExecutionState::me(), }), Err(TryLockError::Poisoned(err)) => Err(TryLockError::Poisoned(PoisonError::new(RwLockWriteGuard { inner: Some(err.into_inner()), - state: Rc::clone(&self.state), + rwlock: self, me: ExecutionState::me(), }))), Err(TryLockError::WouldBlock) => panic!("rwlock state out of sync"), @@ -175,7 +174,7 @@ impl RwLock { waiting_writers = ?state.waiting_writers, "acquiring {:?} lock on rwlock {:p}", typ, - self.state, + self, ); // We are waiting for the lock @@ -250,7 +249,7 @@ impl RwLock { waiting_writers = ?state.waiting_writers, "acquired {:?} lock on rwlock {:p}", typ, - self.state + self ); // Increment the current thread's clock and update this RwLock's clock to match. @@ -283,7 +282,7 @@ impl RwLock { waiting_writers = ?state.waiting_writers, "trying to acquire {:?} lock on rwlock {:p}", typ, - self.state, + self, ); let acquired = match (typ, &mut state.holder) { @@ -309,7 +308,7 @@ impl RwLock { "{} {:?} lock on rwlock {:p}", if acquired { "acquired" } else { "failed to acquire" }, typ, - self.state, + self, ); // Update this thread's clock with the clock stored in the RwLock. @@ -403,9 +402,9 @@ impl Debug for RwLock { /// RAII structure used to release the shared read access of a `RwLock` when dropped. pub struct RwLockReadGuard<'a, T: ?Sized> { - state: Rc>, - me: TaskId, inner: Option>, + rwlock: &'a RwLock, + me: TaskId, } impl Deref for RwLockReadGuard<'_, T> { @@ -432,14 +431,14 @@ impl Drop for RwLockReadGuard<'_, T> { fn drop(&mut self) { self.inner = None; - let mut state = self.state.borrow_mut(); + let mut state = self.rwlock.state.borrow_mut(); trace!( holder = ?state.holder, waiting_readers = ?state.waiting_readers, waiting_writers = ?state.waiting_writers, "releasing Read lock on rwlock {:p}", - self.state + self.rwlock ); match &mut state.holder { @@ -471,7 +470,7 @@ impl Drop for RwLockReadGuard<'_, T> { /// RAII structure used to release the exclusive write access of a `RwLock` when dropped. pub struct RwLockWriteGuard<'a, T: ?Sized> { inner: Option>, - state: Rc>, + rwlock: &'a RwLock, me: TaskId, } @@ -505,13 +504,13 @@ impl Drop for RwLockWriteGuard<'_, T> { fn drop(&mut self) { self.inner = None; - let mut state = self.state.borrow_mut(); + let mut state = self.rwlock.state.borrow_mut(); trace!( holder = ?state.holder, waiting_readers = ?state.waiting_readers, waiting_writers = ?state.waiting_writers, "releasing Write lock on rwlock {:p}", - self.state + self.rwlock ); assert_eq!(state.holder, RwLockHolder::Write(self.me));