From fcaa55511ac5aa2868a593f50d559438b50da650 Mon Sep 17 00:00:00 2001 From: Joel Date: Fri, 29 Apr 2022 21:02:35 +0200 Subject: [PATCH] Address major issues (#202) --- Cargo.toml | 10 +++--- README.md | 2 -- src/iter.rs | 2 +- src/lib.rs | 8 ++++- src/lock.rs | 69 ++++++++++++++++++++++++++++++++++++++++++ src/mapref/entry.rs | 18 +++-------- src/mapref/multiple.rs | 18 +++-------- src/mapref/one.rs | 26 ++++------------ src/rayon/map.rs | 2 +- src/set.rs | 4 +-- src/setref/one.rs | 4 --- src/t.rs | 2 +- 12 files changed, 101 insertions(+), 64 deletions(-) create mode 100644 src/lock.rs diff --git a/Cargo.toml b/Cargo.toml index 0ef6208..e3e096c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,16 +13,14 @@ keywords = ["atomic", "concurrent", "hashmap"] categories = ["concurrency", "algorithms", "data-structures"] [features] -default = ["send_guard"] raw-api = [] -send_guard = ["parking_lot/send_guard"] [dependencies] -parking_lot = "0.12.0" -hashbrown = "0.11.2" +lock_api = "0.4.7" +hashbrown = "0.12.0" serde = { version = "1.0.136", optional = true, features = ["derive"] } cfg-if = "1.0.0" -rayon = { version = "1.5.1", optional = true } +rayon = { version = "1.5.2", optional = true } [package.metadata.docs.rs] -features = ["rayon", "raw-api", "serde", "send_guard"] +features = ["rayon", "raw-api", "serde"] diff --git a/README.md b/README.md index 5f821dd..580480f 100644 --- a/README.md +++ b/README.md @@ -30,8 +30,6 @@ If you have any suggestions or tips do not hesitate to open an issue or a PR. - `rayon` - Enables rayon support. -- `send_guard` - Enables the `send_guard` feature of `parking_lot`, making `Ref*` guards `Send`. This is on by default. - ## Contributing DashMap gladly accepts contributions! diff --git a/src/iter.rs b/src/iter.rs index 601ab40..2895780 100644 --- a/src/iter.rs +++ b/src/iter.rs @@ -1,11 +1,11 @@ use super::mapref::multiple::{RefMulti, RefMutMulti}; use super::util; +use crate::lock::{RwLockReadGuard, RwLockWriteGuard}; use crate::t::Map; use crate::util::SharedValue; use crate::{DashMap, HashMap}; use core::hash::{BuildHasher, Hash}; use core::mem; -use parking_lot::{RwLockReadGuard, RwLockWriteGuard}; use std::collections::hash_map::RandomState; use std::sync::Arc; diff --git a/src/lib.rs b/src/lib.rs index 53a46d2..eca142f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -2,6 +2,7 @@ pub mod iter; pub mod iter_set; +mod lock; pub mod mapref; mod read_only; #[cfg(feature = "serde")] @@ -18,6 +19,12 @@ pub mod rayon { pub mod set; } +#[cfg(not(feature = "raw-api"))] +use crate::lock::{RwLock, RwLockReadGuard, RwLockWriteGuard}; + +#[cfg(feature = "raw-api")] +pub use crate::lock::{RawRwLock, RwLock, RwLockReadGuard, RwLockWriteGuard}; + use cfg_if::cfg_if; use core::borrow::Borrow; use core::fmt; @@ -28,7 +35,6 @@ use iter::{Iter, IterMut, OwningIter}; use mapref::entry::{Entry, OccupiedEntry, VacantEntry}; use mapref::multiple::RefMulti; use mapref::one::{Ref, RefMut}; -use parking_lot::{RwLock, RwLockReadGuard, RwLockWriteGuard}; pub use read_only::ReadOnlyView; pub use set::DashSet; use std::collections::hash_map::RandomState; diff --git a/src/lock.rs b/src/lock.rs new file mode 100644 index 0000000..0437724 --- /dev/null +++ b/src/lock.rs @@ -0,0 +1,69 @@ +use lock_api::GuardSend; +use std::sync::atomic::{AtomicU32, Ordering}; + +const EXCLUSIVE_BIT: u32 = 1 << 31; + +pub type RwLock = lock_api::RwLock; +pub type RwLockReadGuard<'a, T> = lock_api::RwLockReadGuard<'a, RawRwLock, T>; +pub type RwLockWriteGuard<'a, T> = lock_api::RwLockWriteGuard<'a, RawRwLock, T>; + +pub struct RawRwLock { + data: AtomicU32, +} + +unsafe impl lock_api::RawRwLock for RawRwLock { + type GuardMarker = GuardSend; + + #[allow(clippy::declare_interior_mutable_const)] + const INIT: Self = RawRwLock { + data: AtomicU32::new(0), + }; + + fn lock_shared(&self) { + while !self.try_lock_shared() {} + } + + fn try_lock_shared(&self) -> bool { + let x = self.data.load(Ordering::SeqCst); + if x & EXCLUSIVE_BIT != 0 { + return false; + } + + let y = x + 1; + self.data + .compare_exchange(x, y, Ordering::SeqCst, Ordering::SeqCst) + .is_ok() + } + + unsafe fn unlock_shared(&self) { + self.data.fetch_sub(1, Ordering::SeqCst); + } + + fn lock_exclusive(&self) { + while !self.try_lock_exclusive() {} + } + + fn try_lock_exclusive(&self) -> bool { + self.data + .compare_exchange(0, EXCLUSIVE_BIT, Ordering::SeqCst, Ordering::SeqCst) + .is_ok() + } + + unsafe fn unlock_exclusive(&self) { + self.data.store(0, Ordering::SeqCst) + } + + fn is_locked(&self) -> bool { + self.data.load(Ordering::SeqCst) != 0 + } + + fn is_locked_exclusive(&self) -> bool { + self.data.load(Ordering::SeqCst) & EXCLUSIVE_BIT != 0 + } +} + +unsafe impl lock_api::RawRwLockDowngrade for RawRwLock { + unsafe fn downgrade(&self) { + self.data.store(1, Ordering::SeqCst); + } +} diff --git a/src/mapref/entry.rs b/src/mapref/entry.rs index fb954eb..16a42ce 100644 --- a/src/mapref/entry.rs +++ b/src/mapref/entry.rs @@ -1,11 +1,11 @@ use super::one::RefMut; +use crate::lock::RwLockWriteGuard; use crate::util; use crate::util::SharedValue; use crate::HashMap; use core::hash::{BuildHasher, Hash}; use core::mem; use core::ptr; -use parking_lot::RwLockWriteGuard; use std::collections::hash_map::RandomState; pub enum Entry<'a, K, V, S = RandomState> { @@ -89,12 +89,8 @@ pub struct VacantEntry<'a, K, V, S = RandomState> { key: K, } -unsafe impl<'a, K: Eq + Hash + Send, V: Send, S: BuildHasher> Send for VacantEntry<'a, K, V, S> {} - -unsafe impl<'a, K: Eq + Hash + Send + Sync, V: Send + Sync, S: BuildHasher> Sync - for VacantEntry<'a, K, V, S> -{ -} +unsafe impl<'a, K: Eq + Hash + Sync, V: Sync, S: BuildHasher> Send for VacantEntry<'a, K, V, S> {} +unsafe impl<'a, K: Eq + Hash + Sync, V: Sync, S: BuildHasher> Sync for VacantEntry<'a, K, V, S> {} impl<'a, K: Eq + Hash, V, S: BuildHasher> VacantEntry<'a, K, V, S> { pub(crate) unsafe fn new(shard: RwLockWriteGuard<'a, HashMap>, key: K) -> Self { @@ -136,12 +132,8 @@ pub struct OccupiedEntry<'a, K, V, S = RandomState> { key: K, } -unsafe impl<'a, K: Eq + Hash + Send, V: Send, S: BuildHasher> Send for OccupiedEntry<'a, K, V, S> {} - -unsafe impl<'a, K: Eq + Hash + Send + Sync, V: Send + Sync, S: BuildHasher> Sync - for OccupiedEntry<'a, K, V, S> -{ -} +unsafe impl<'a, K: Eq + Hash + Sync, V: Sync, S: BuildHasher> Send for OccupiedEntry<'a, K, V, S> {} +unsafe impl<'a, K: Eq + Hash + Sync, V: Sync, S: BuildHasher> Sync for OccupiedEntry<'a, K, V, S> {} impl<'a, K: Eq + Hash, V, S: BuildHasher> OccupiedEntry<'a, K, V, S> { pub(crate) unsafe fn new( diff --git a/src/mapref/multiple.rs b/src/mapref/multiple.rs index a787171..53a8a7e 100644 --- a/src/mapref/multiple.rs +++ b/src/mapref/multiple.rs @@ -1,8 +1,8 @@ +use crate::lock::{RwLockReadGuard, RwLockWriteGuard}; use crate::HashMap; use core::hash::BuildHasher; use core::hash::Hash; use core::ops::{Deref, DerefMut}; -use parking_lot::{RwLockReadGuard, RwLockWriteGuard}; use std::collections::hash_map::RandomState; use std::sync::Arc; @@ -12,12 +12,8 @@ pub struct RefMulti<'a, K, V, S = RandomState> { v: *const V, } -unsafe impl<'a, K: Eq + Hash + Send, V: Send, S: BuildHasher> Send for RefMulti<'a, K, V, S> {} - -unsafe impl<'a, K: Eq + Hash + Send + Sync, V: Send + Sync, S: BuildHasher> Sync - for RefMulti<'a, K, V, S> -{ -} +unsafe impl<'a, K: Eq + Hash + Sync, V: Sync, S: BuildHasher> Send for RefMulti<'a, K, V, S> {} +unsafe impl<'a, K: Eq + Hash + Sync, V: Sync, S: BuildHasher> Sync for RefMulti<'a, K, V, S> {} impl<'a, K: Eq + Hash, V, S: BuildHasher> RefMulti<'a, K, V, S> { pub(crate) unsafe fn new( @@ -59,12 +55,8 @@ pub struct RefMutMulti<'a, K, V, S = RandomState> { v: *mut V, } -unsafe impl<'a, K: Eq + Hash + Send, V: Send, S: BuildHasher> Send for RefMutMulti<'a, K, V, S> {} - -unsafe impl<'a, K: Eq + Hash + Send + Sync, V: Send + Sync, S: BuildHasher> Sync - for RefMutMulti<'a, K, V, S> -{ -} +unsafe impl<'a, K: Eq + Hash + Sync, V: Sync, S: BuildHasher> Send for RefMutMulti<'a, K, V, S> {} +unsafe impl<'a, K: Eq + Hash + Sync, V: Sync, S: BuildHasher> Sync for RefMutMulti<'a, K, V, S> {} impl<'a, K: Eq + Hash, V, S: BuildHasher> RefMutMulti<'a, K, V, S> { pub(crate) unsafe fn new( diff --git a/src/mapref/one.rs b/src/mapref/one.rs index 3eff85d..cf9817f 100644 --- a/src/mapref/one.rs +++ b/src/mapref/one.rs @@ -1,7 +1,7 @@ +use crate::lock::{RwLockReadGuard, RwLockWriteGuard}; use crate::HashMap; use core::hash::{BuildHasher, Hash}; use core::ops::{Deref, DerefMut}; -use parking_lot::{RwLockReadGuard, RwLockWriteGuard}; use std::collections::hash_map::RandomState; pub struct Ref<'a, K, V, S = RandomState> { @@ -10,12 +10,8 @@ pub struct Ref<'a, K, V, S = RandomState> { v: *const V, } -unsafe impl<'a, K: Eq + Hash + Send, V: Send, S: BuildHasher> Send for Ref<'a, K, V, S> {} - -unsafe impl<'a, K: Eq + Hash + Send + Sync, V: Send + Sync, S: BuildHasher> Sync - for Ref<'a, K, V, S> -{ -} +unsafe impl<'a, K: Eq + Hash + Sync, V: Sync, S: BuildHasher> Send for Ref<'a, K, V, S> {} +unsafe impl<'a, K: Eq + Hash + Sync, V: Sync, S: BuildHasher> Sync for Ref<'a, K, V, S> {} impl<'a, K: Eq + Hash, V, S: BuildHasher> Ref<'a, K, V, S> { pub(crate) unsafe fn new( @@ -57,12 +53,8 @@ pub struct RefMut<'a, K, V, S = RandomState> { v: *mut V, } -unsafe impl<'a, K: Eq + Hash + Send, V: Send, S: BuildHasher> Send for RefMut<'a, K, V, S> {} - -unsafe impl<'a, K: Eq + Hash + Send + Sync, V: Send + Sync, S: BuildHasher> Sync - for RefMut<'a, K, V, S> -{ -} +unsafe impl<'a, K: Eq + Hash + Sync, V: Sync, S: BuildHasher> Send for RefMut<'a, K, V, S> {} +unsafe impl<'a, K: Eq + Hash + Sync, V: Sync, S: BuildHasher> Sync for RefMut<'a, K, V, S> {} impl<'a, K: Eq + Hash, V, S: BuildHasher> RefMut<'a, K, V, S> { pub(crate) unsafe fn new( @@ -94,13 +86,7 @@ impl<'a, K: Eq + Hash, V, S: BuildHasher> RefMut<'a, K, V, S> { } pub fn downgrade(self) -> Ref<'a, K, V, S> { - unsafe { - Ref::new( - parking_lot::RwLockWriteGuard::downgrade(self.guard), - self.k, - self.v, - ) - } + unsafe { Ref::new(RwLockWriteGuard::downgrade(self.guard), self.k, self.v) } } } diff --git a/src/rayon/map.rs b/src/rayon/map.rs index 57f8569..c0947cf 100644 --- a/src/rayon/map.rs +++ b/src/rayon/map.rs @@ -1,8 +1,8 @@ +use crate::lock::RwLock; use crate::mapref::multiple::{RefMulti, RefMutMulti}; use crate::util; use crate::{DashMap, HashMap}; use core::hash::{BuildHasher, Hash}; -use parking_lot::RwLock; use rayon::iter::plumbing::UnindexedConsumer; use rayon::iter::{FromParallelIterator, IntoParallelIterator, ParallelExtend, ParallelIterator}; use std::collections::hash_map::RandomState; diff --git a/src/set.rs b/src/set.rs index e7ff849..12445a9 100644 --- a/src/set.rs +++ b/src/set.rs @@ -1,4 +1,6 @@ use crate::iter_set::{Iter, OwningIter}; +#[cfg(feature = "raw-api")] +use crate::lock::RwLock; use crate::setref::one::Ref; use crate::DashMap; #[cfg(feature = "raw-api")] @@ -8,8 +10,6 @@ use core::borrow::Borrow; use core::fmt; use core::hash::{BuildHasher, Hash}; use core::iter::FromIterator; -#[cfg(feature = "raw-api")] -use parking_lot::RwLock; use std::collections::hash_map::RandomState; /// DashSet is a thin wrapper around [`DashMap`] using `()` as the value type. It uses diff --git a/src/setref/one.rs b/src/setref/one.rs index ef03421..0787187 100644 --- a/src/setref/one.rs +++ b/src/setref/one.rs @@ -6,10 +6,6 @@ pub struct Ref<'a, K, S = RandomState> { inner: mapref::one::Ref<'a, K, (), S>, } -unsafe impl<'a, K: Eq + Hash + Send, S: BuildHasher> Send for Ref<'a, K, S> {} - -unsafe impl<'a, K: Eq + Hash + Send + Sync, S: BuildHasher> Sync for Ref<'a, K, S> {} - impl<'a, K: Eq + Hash, S: BuildHasher> Ref<'a, K, S> { pub(crate) fn new(inner: mapref::one::Ref<'a, K, (), S>) -> Self { Self { inner } diff --git a/src/t.rs b/src/t.rs index 48eaec5..5e1fedc 100644 --- a/src/t.rs +++ b/src/t.rs @@ -1,13 +1,13 @@ //! Central map trait to ease modifications and extensions down the road. use crate::iter::{Iter, IterMut}; +use crate::lock::{RwLockReadGuard, RwLockWriteGuard}; use crate::mapref::entry::Entry; use crate::mapref::one::{Ref, RefMut}; use crate::try_result::TryResult; use crate::HashMap; use core::borrow::Borrow; use core::hash::{BuildHasher, Hash}; -use parking_lot::{RwLockReadGuard, RwLockWriteGuard}; /// Implementation detail that is exposed due to generic constraints in public types. pub trait Map<'a, K: 'a + Eq + Hash, V: 'a, S: 'a + Clone + BuildHasher> {