From 77b2cf724e2897f7d2549217a0bfad9836b0c07b Mon Sep 17 00:00:00 2001 From: Askar Safin Date: Sat, 8 Feb 2025 11:55:41 +0300 Subject: [PATCH 1/4] compiler/rustc_data_structures/src/sync.rs: these RwLock methods are never called, so let's remove them --- compiler/rustc_data_structures/src/sync.rs | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/compiler/rustc_data_structures/src/sync.rs b/compiler/rustc_data_structures/src/sync.rs index bea87a6685d8d..779127ce32f75 100644 --- a/compiler/rustc_data_structures/src/sync.rs +++ b/compiler/rustc_data_structures/src/sync.rs @@ -203,12 +203,6 @@ impl RwLock { } } - #[inline(always)] - #[track_caller] - pub fn with_read_lock R, R>(&self, f: F) -> R { - f(&*self.read()) - } - #[inline(always)] pub fn try_write(&self) -> Result, ()> { self.0.try_write().ok_or(()) @@ -223,12 +217,6 @@ impl RwLock { } } - #[inline(always)] - #[track_caller] - pub fn with_write_lock R, R>(&self, f: F) -> R { - f(&mut *self.write()) - } - #[inline(always)] #[track_caller] pub fn borrow(&self) -> ReadGuard<'_, T> { @@ -240,14 +228,6 @@ impl RwLock { pub fn borrow_mut(&self) -> WriteGuard<'_, T> { self.write() } - - #[inline(always)] - pub fn leak(&self) -> &T { - let guard = self.read(); - let ret = unsafe { &*(&raw const *guard) }; - std::mem::forget(guard); - ret - } } // FIXME: Probably a bad idea From 1cc23fbcb7c460bbab2e73f2e333873a7e04bbc0 Mon Sep 17 00:00:00 2001 From: Askar Safin Date: Sun, 9 Feb 2025 08:10:08 +0300 Subject: [PATCH 2/4] compiler/rustc_data_structures/src/sync.rs: remove "impl Clone for RwLock" parking_lot::RwLock doesn't have "impl Clone", so we should not have, either --- compiler/rustc_data_structures/src/sync.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/compiler/rustc_data_structures/src/sync.rs b/compiler/rustc_data_structures/src/sync.rs index 779127ce32f75..ab457a639c7a3 100644 --- a/compiler/rustc_data_structures/src/sync.rs +++ b/compiler/rustc_data_structures/src/sync.rs @@ -229,11 +229,3 @@ impl RwLock { self.write() } } - -// FIXME: Probably a bad idea -impl Clone for RwLock { - #[inline] - fn clone(&self) -> Self { - RwLock::new(self.borrow().clone()) - } -} From 60af2ab92d829aaf6e2f59ea0dd86cc7af3999c0 Mon Sep 17 00:00:00 2001 From: Askar Safin Date: Sun, 9 Feb 2025 08:42:48 +0300 Subject: [PATCH 3/4] rustc_data_structures: sync: implement RwLock as enum { Sync(parking_lot::RwLock), NoSync(RefCell) } Also we remove ERROR_CHECKING, because RefCell already panics on borrow errors --- compiler/rustc_data_structures/src/sync.rs | 70 +---- .../rustc_data_structures/src/sync/rwlock.rs | 271 ++++++++++++++++++ 2 files changed, 276 insertions(+), 65 deletions(-) create mode 100644 compiler/rustc_data_structures/src/sync/rwlock.rs diff --git a/compiler/rustc_data_structures/src/sync.rs b/compiler/rustc_data_structures/src/sync.rs index ab457a639c7a3..703b30e237bc3 100644 --- a/compiler/rustc_data_structures/src/sync.rs +++ b/compiler/rustc_data_structures/src/sync.rs @@ -111,11 +111,7 @@ pub use std::sync::atomic::{AtomicBool, AtomicU32, AtomicUsize}; pub use std::sync::{OnceLock, Weak}; pub use mode::{is_dyn_thread_safe, set_dyn_thread_safe_mode}; -pub use parking_lot::{ - MappedMutexGuard as MappedLockGuard, MappedRwLockReadGuard as MappedReadGuard, - MappedRwLockWriteGuard as MappedWriteGuard, RwLockReadGuard as ReadGuard, - RwLockWriteGuard as WriteGuard, -}; +pub use parking_lot::MappedMutexGuard as MappedLockGuard; #[cfg(not(target_has_atomic = "64"))] pub use portable_atomic::AtomicU64; @@ -151,12 +147,6 @@ impl MTLock { } } -use parking_lot::RwLock as InnerRwLock; - -/// This makes locks panic if they are already held. -/// It is only useful when you are running in a single thread -const ERROR_CHECKING: bool = false; - pub type MTLockRef<'a, T> = LRef<'a, MTLock>; #[derive(Default)] @@ -175,57 +165,7 @@ impl HashMapExt for HashMap } } -#[derive(Debug, Default)] -pub struct RwLock(InnerRwLock); - -impl RwLock { - #[inline(always)] - pub fn new(inner: T) -> Self { - RwLock(InnerRwLock::new(inner)) - } - - #[inline(always)] - pub fn into_inner(self) -> T { - self.0.into_inner() - } - - #[inline(always)] - pub fn get_mut(&mut self) -> &mut T { - self.0.get_mut() - } - - #[inline(always)] - pub fn read(&self) -> ReadGuard<'_, T> { - if ERROR_CHECKING { - self.0.try_read().expect("lock was already held") - } else { - self.0.read() - } - } - - #[inline(always)] - pub fn try_write(&self) -> Result, ()> { - self.0.try_write().ok_or(()) - } - - #[inline(always)] - pub fn write(&self) -> WriteGuard<'_, T> { - if ERROR_CHECKING { - self.0.try_write().expect("lock was already held") - } else { - self.0.write() - } - } - - #[inline(always)] - #[track_caller] - pub fn borrow(&self) -> ReadGuard<'_, T> { - self.read() - } - - #[inline(always)] - #[track_caller] - pub fn borrow_mut(&self) -> WriteGuard<'_, T> { - self.write() - } -} +mod rwlock; +pub use rwlock::{ + MappedReadGuard, MappedWriteGuard, ReadError, ReadGuard, RwLock, WriteError, WriteGuard, +}; diff --git a/compiler/rustc_data_structures/src/sync/rwlock.rs b/compiler/rustc_data_structures/src/sync/rwlock.rs new file mode 100644 index 0000000000000..755903437de0e --- /dev/null +++ b/compiler/rustc_data_structures/src/sync/rwlock.rs @@ -0,0 +1,271 @@ +use std::cell::{Ref, RefCell, RefMut}; +use std::intrinsics::unlikely; +use std::ops::{Deref, DerefMut}; + +use crate::sync::mode::might_be_dyn_thread_safe; + +#[derive(Debug)] +pub enum RwLock { + Sync(parking_lot::RwLock), + NoSync(RefCell), +} + +#[clippy::has_significant_drop] +#[must_use = "if unused the RwLock will immediately unlock"] +#[derive(Debug)] +pub enum ReadGuard<'a, T> { + Sync(parking_lot::RwLockReadGuard<'a, T>), + NoSync(Ref<'a, T>), +} + +#[clippy::has_significant_drop] +#[must_use = "if unused the RwLock will immediately unlock"] +#[derive(Debug)] +pub enum WriteGuard<'a, T> { + Sync(parking_lot::RwLockWriteGuard<'a, T>), + NoSync(RefMut<'a, T>), +} + +#[clippy::has_significant_drop] +#[must_use = "if unused the RwLock will immediately unlock"] +#[derive(Debug)] +pub enum MappedReadGuard<'a, T> { + Sync(parking_lot::MappedRwLockReadGuard<'a, T>), + NoSync(Ref<'a, T>), +} + +#[clippy::has_significant_drop] +#[must_use = "if unused the RwLock will immediately unlock"] +#[derive(Debug)] +pub enum MappedWriteGuard<'a, T> { + Sync(parking_lot::MappedRwLockWriteGuard<'a, T>), + NoSync(RefMut<'a, T>), +} + +#[derive(Debug)] +pub struct ReadError; + +#[derive(Debug)] +pub struct WriteError; + +impl RwLock { + #[inline(always)] + pub fn new(inner: T) -> Self { + if unlikely(might_be_dyn_thread_safe()) { + RwLock::Sync(parking_lot::RwLock::new(inner)) + } else { + RwLock::NoSync(RefCell::new(inner)) + } + } + + #[inline(always)] + pub fn into_inner(self) -> T { + match self { + RwLock::Sync(inner) => parking_lot::RwLock::into_inner(inner), + RwLock::NoSync(inner) => RefCell::into_inner(inner), + } + } + + #[inline(always)] + pub fn get_mut(&mut self) -> &mut T { + match self { + RwLock::Sync(inner) => parking_lot::RwLock::get_mut(inner), + RwLock::NoSync(inner) => RefCell::get_mut(inner), + } + } + + #[inline(always)] + #[track_caller] + pub fn read(&self) -> ReadGuard<'_, T> { + match self { + RwLock::Sync(inner) => ReadGuard::Sync(inner.read()), + RwLock::NoSync(inner) => ReadGuard::NoSync(inner.borrow()), + } + } + + #[inline(always)] + pub fn try_read(&self) -> Result, ReadError> { + match self { + RwLock::Sync(inner) => Ok(ReadGuard::Sync(inner.try_read().ok_or(ReadError)?)), + RwLock::NoSync(inner) => { + Ok(ReadGuard::NoSync(inner.try_borrow().map_err(|_| ReadError)?)) + } + } + } + + #[inline(always)] + #[track_caller] + pub fn write(&self) -> WriteGuard<'_, T> { + match self { + RwLock::Sync(inner) => WriteGuard::Sync(inner.write()), + RwLock::NoSync(inner) => WriteGuard::NoSync(inner.borrow_mut()), + } + } + + #[inline(always)] + pub fn try_write(&self) -> Result, WriteError> { + match self { + RwLock::Sync(inner) => Ok(WriteGuard::Sync(inner.try_write().ok_or(WriteError)?)), + RwLock::NoSync(inner) => { + Ok(WriteGuard::NoSync(inner.try_borrow_mut().map_err(|_| WriteError)?)) + } + } + } + + #[inline(always)] + #[track_caller] + pub fn borrow(&self) -> ReadGuard<'_, T> { + self.read() + } + + #[inline(always)] + pub fn try_borrow(&self) -> Result, ReadError> { + self.try_read() + } + + #[inline(always)] + #[track_caller] + pub fn borrow_mut(&self) -> WriteGuard<'_, T> { + self.write() + } + + #[inline(always)] + pub fn try_borrow_mut(&self) -> Result, WriteError> { + self.try_write() + } +} + +impl Default for RwLock { + #[inline(always)] + fn default() -> Self { + RwLock::::new(Default::default()) + } +} + +impl<'a, T> ReadGuard<'a, T> { + #[inline(always)] + pub fn map(s: Self, f: F) -> MappedReadGuard<'a, U> + where + F: FnOnce(&T) -> &U, + { + match s { + ReadGuard::Sync(guard) => { + MappedReadGuard::Sync(parking_lot::RwLockReadGuard::map(guard, f)) + } + ReadGuard::NoSync(guard) => MappedReadGuard::NoSync(Ref::map(guard, f)), + } + } +} + +impl<'a, T> WriteGuard<'a, T> { + #[inline(always)] + pub fn map(s: Self, f: F) -> MappedWriteGuard<'a, U> + where + F: FnOnce(&mut T) -> &mut U, + { + match s { + WriteGuard::Sync(guard) => { + MappedWriteGuard::Sync(parking_lot::RwLockWriteGuard::map(guard, f)) + } + WriteGuard::NoSync(guard) => MappedWriteGuard::NoSync(RefMut::map(guard, f)), + } + } +} + +impl<'a, T> Deref for ReadGuard<'a, T> { + type Target = T; + + #[inline(always)] + fn deref(&self) -> &T { + match self { + ReadGuard::Sync(guard) => Deref::deref(guard), + ReadGuard::NoSync(guard) => Deref::deref(guard), + } + } +} + +impl<'a, T> Deref for WriteGuard<'a, T> { + type Target = T; + + #[inline(always)] + fn deref(&self) -> &T { + match self { + WriteGuard::Sync(guard) => Deref::deref(guard), + WriteGuard::NoSync(guard) => Deref::deref(guard), + } + } +} + +impl<'a, T> DerefMut for WriteGuard<'a, T> { + #[inline(always)] + fn deref_mut(&mut self) -> &mut T { + match self { + WriteGuard::Sync(guard) => DerefMut::deref_mut(guard), + WriteGuard::NoSync(guard) => DerefMut::deref_mut(guard), + } + } +} + +impl<'a, T> MappedReadGuard<'a, T> { + #[inline(always)] + pub fn map(s: Self, f: F) -> MappedReadGuard<'a, U> + where + F: FnOnce(&T) -> &U, + { + match s { + MappedReadGuard::Sync(guard) => { + MappedReadGuard::Sync(parking_lot::MappedRwLockReadGuard::map(guard, f)) + } + MappedReadGuard::NoSync(guard) => MappedReadGuard::NoSync(Ref::map(guard, f)), + } + } +} + +impl<'a, T> MappedWriteGuard<'a, T> { + #[inline(always)] + pub fn map(s: Self, f: F) -> MappedWriteGuard<'a, U> + where + F: FnOnce(&mut T) -> &mut U, + { + match s { + MappedWriteGuard::Sync(guard) => { + MappedWriteGuard::Sync(parking_lot::MappedRwLockWriteGuard::map(guard, f)) + } + MappedWriteGuard::NoSync(guard) => MappedWriteGuard::NoSync(RefMut::map(guard, f)), + } + } +} + +impl<'a, T> Deref for MappedReadGuard<'a, T> { + type Target = T; + + #[inline(always)] + fn deref(&self) -> &T { + match self { + MappedReadGuard::Sync(guard) => Deref::deref(guard), + MappedReadGuard::NoSync(guard) => Deref::deref(guard), + } + } +} + +impl<'a, T> Deref for MappedWriteGuard<'a, T> { + type Target = T; + + #[inline(always)] + fn deref(&self) -> &T { + match self { + MappedWriteGuard::Sync(guard) => Deref::deref(guard), + MappedWriteGuard::NoSync(guard) => Deref::deref(guard), + } + } +} + +impl<'a, T> DerefMut for MappedWriteGuard<'a, T> { + #[inline(always)] + fn deref_mut(&mut self) -> &mut T { + match self { + MappedWriteGuard::Sync(guard) => DerefMut::deref_mut(guard), + MappedWriteGuard::NoSync(guard) => DerefMut::deref_mut(guard), + } + } +} From 95ce6d04812885a7455f849a78745987e03bb62d Mon Sep 17 00:00:00 2001 From: Askar Safin Date: Sun, 9 Feb 2025 14:01:24 +0300 Subject: [PATCH 4/4] compiler/rustc_data_structures/src/sync/vec.rs: convert AppendOnlyVec to our RwLock --- compiler/rustc_data_structures/src/sync/vec.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_data_structures/src/sync/vec.rs b/compiler/rustc_data_structures/src/sync/vec.rs index 21ec5cf6c13bf..3da5008428221 100644 --- a/compiler/rustc_data_structures/src/sync/vec.rs +++ b/compiler/rustc_data_structures/src/sync/vec.rs @@ -2,6 +2,8 @@ use std::marker::PhantomData; use rustc_index::Idx; +use crate::sync::RwLock; + #[derive(Default)] pub struct AppendOnlyIndexVec { vec: elsa::sync::LockFreeFrozenVec, @@ -26,7 +28,7 @@ impl AppendOnlyIndexVec { #[derive(Default)] pub struct AppendOnlyVec { - vec: parking_lot::RwLock>, + vec: RwLock>, } impl AppendOnlyVec {