Skip to content

Commit

Permalink
remove SharedValue (#322)
Browse files Browse the repository at this point in the history
  • Loading branch information
conradludgate authored Feb 1, 2025
1 parent 39864b2 commit c8c936c
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 129 deletions.
15 changes: 6 additions & 9 deletions src/iter.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use super::mapref::multiple::{RefMulti, RefMutMulti};
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;
Expand Down Expand Up @@ -38,7 +37,7 @@ impl<K: Eq + Hash, V, S: BuildHasher + Clone> OwningIter<K, V, S> {
}
}

type GuardOwningIter<K, V> = hashbrown::raw::RawIntoIter<(K, SharedValue<V>)>;
type GuardOwningIter<K, V> = hashbrown::raw::RawIntoIter<(K, V)>;

impl<K: Eq + Hash, V, S: BuildHasher + Clone> Iterator for OwningIter<K, V, S> {
type Item = (K, V);
Expand All @@ -47,15 +46,14 @@ impl<K: Eq + Hash, V, S: BuildHasher + Clone> Iterator for OwningIter<K, V, S> {
loop {
if let Some(current) = self.current.as_mut() {
if let Some((k, v)) = current.next() {
return Some((k, v.into_inner()));
return Some((k, v));
}
}

if self.shard_i == self.map._shard_count() {
return None;
}

//let guard = unsafe { self.map._yield_read_shard(self.shard_i) };
let mut shard_wl = unsafe { self.map._yield_write_shard(self.shard_i) };

let map = mem::take(&mut *shard_wl);
Expand All @@ -64,7 +62,6 @@ impl<K: Eq + Hash, V, S: BuildHasher + Clone> Iterator for OwningIter<K, V, S> {

let iter = map.into_iter();

//unsafe { ptr::write(&mut self.current, Some((arcee, iter))); }
self.current = Some(iter);

self.shard_i += 1;
Expand All @@ -90,12 +87,12 @@ where

type GuardIter<'a, K, V> = (
Arc<RwLockReadGuard<'a, HashMap<K, V>>>,
hashbrown::raw::RawIter<(K, SharedValue<V>)>,
hashbrown::raw::RawIter<(K, V)>,
);

type GuardIterMut<'a, K, V> = (
Arc<RwLockWriteGuard<'a, HashMap<K, V>>>,
hashbrown::raw::RawIter<(K, SharedValue<V>)>,
hashbrown::raw::RawIter<(K, V)>,
);

/// Iterator over a DashMap yielding immutable references.
Expand Down Expand Up @@ -163,7 +160,7 @@ impl<'a, K: Eq + Hash, V, S: 'a + BuildHasher + Clone, M: Map<'a, K, V, S>> Iter
return unsafe {
let (k, v) = b.as_ref();
let guard = current.0.clone();
Some(RefMulti::new(guard, k, v.get()))
Some(RefMulti::new(guard, k, v))
};
}
}
Expand Down Expand Up @@ -245,7 +242,7 @@ impl<'a, K: Eq + Hash, V, S: 'a + BuildHasher + Clone, M: Map<'a, K, V, S>> Iter
return unsafe {
let (k, v) = b.as_mut();
let guard = current.0.clone();
Some(RefMutMulti::new(guard, k, v.get_mut()))
Some(RefMutMulti::new(guard, k, v))
};
}
}
Expand Down
41 changes: 16 additions & 25 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,7 @@ use std::collections::hash_map::RandomState;
pub use t::Map;
use try_result::TryResult;

cfg_if! {
if #[cfg(feature = "raw-api")] {
pub use util::SharedValue;
} else {
use util::SharedValue;
}
}

pub(crate) type HashMap<K, V> = hashbrown::raw::RawTable<(K, SharedValue<V>)>;
pub(crate) type HashMap<K, V> = hashbrown::raw::RawTable<(K, V)>;

// Temporary reimplementation of [`std::collections::TryReserveError`]
// util [`std::collections::TryReserveError`] stabilises.
Expand Down Expand Up @@ -331,18 +323,17 @@ impl<'a, K: 'a + Eq + Hash, V: 'a, S: BuildHasher + Clone> DashMap<K, V, S> {
///
/// ```
/// use dashmap::DashMap;
/// use dashmap::SharedValue;
/// use std::hash::{Hash, Hasher, BuildHasher};
///
/// let mut map = DashMap::<i32, &'static str>::new();
/// let shard_ind = map.determine_map(&42);
/// let mut factory = map.hasher().clone();
/// let hasher = |tuple: &(i32, SharedValue<&'static str>)| {
/// let hasher = |tuple: &(i32, &'static str)| {
/// let mut hasher = factory.build_hasher();
/// tuple.0.hash(&mut hasher);
/// hasher.finish()
/// };
/// let data = (42, SharedValue::new("forty two"));
/// let data = (42, "forty two");
/// let hash = hasher(&data);
/// map.shards_mut()[shard_ind].get_mut().insert(hash, data, hasher);
/// assert_eq!(*map.get(&42).unwrap(), "forty two");
Expand Down Expand Up @@ -965,7 +956,7 @@ impl<'a, K: 'a + Eq + Hash, V: 'a, S: 'a + BuildHasher + Clone> Map<'a, K, V, S>

if let Some(bucket) = shard.find(hash, |(k, _v)| key == k.borrow()) {
let ((k, v), _) = unsafe { shard.remove(bucket) };
Some((k, v.into_inner()))
Some((k, v))
} else {
None
}
Expand All @@ -984,9 +975,9 @@ impl<'a, K: 'a + Eq + Hash, V: 'a, S: 'a + BuildHasher + Clone> Map<'a, K, V, S>

if let Some(bucket) = shard.find(hash, |(k, _v)| key == k.borrow()) {
let (k, v) = unsafe { bucket.as_ref() };
if f(k, v.get()) {
if f(k, v) {
let ((k, v), _) = unsafe { shard.remove(bucket) };
Some((k, v.into_inner()))
Some((k, v))
} else {
None
}
Expand All @@ -1008,9 +999,9 @@ impl<'a, K: 'a + Eq + Hash, V: 'a, S: 'a + BuildHasher + Clone> Map<'a, K, V, S>

if let Some(bucket) = shard.find(hash, |(k, _v)| key == k.borrow()) {
let (k, v) = unsafe { bucket.as_mut() };
if f(k, v.get_mut()) {
if f(k, v) {
let ((k, v), _) = unsafe { shard.remove(bucket) };
Some((k, v.into_inner()))
Some((k, v))
} else {
None
}
Expand Down Expand Up @@ -1041,7 +1032,7 @@ impl<'a, K: 'a + Eq + Hash, V: 'a, S: 'a + BuildHasher + Clone> Map<'a, K, V, S>
if let Some(bucket) = shard.find(hash, |(k, _v)| key == k.borrow()) {
unsafe {
let (k, v) = bucket.as_ref();
Some(Ref::new(shard, k, v.as_ptr()))
Some(Ref::new(shard, k, v))
}
} else {
None
Expand All @@ -1061,8 +1052,8 @@ impl<'a, K: 'a + Eq + Hash, V: 'a, S: 'a + BuildHasher + Clone> Map<'a, K, V, S>

if let Some(bucket) = shard.find(hash, |(k, _v)| key == k.borrow()) {
unsafe {
let (k, v) = bucket.as_ref();
Some(RefMut::new(shard, k, v.as_ptr()))
let (k, v) = bucket.as_mut();
Some(RefMut::new(shard, k, v))
}
} else {
None
Expand All @@ -1086,7 +1077,7 @@ impl<'a, K: 'a + Eq + Hash, V: 'a, S: 'a + BuildHasher + Clone> Map<'a, K, V, S>
if let Some(bucket) = shard.find(hash, |(k, _v)| key == k.borrow()) {
unsafe {
let (k, v) = bucket.as_ref();
TryResult::Present(Ref::new(shard, k, v.as_ptr()))
TryResult::Present(Ref::new(shard, k, v))
}
} else {
TryResult::Absent
Expand All @@ -1109,8 +1100,8 @@ impl<'a, K: 'a + Eq + Hash, V: 'a, S: 'a + BuildHasher + Clone> Map<'a, K, V, S>

if let Some(bucket) = shard.find(hash, |(k, _v)| key == k.borrow()) {
unsafe {
let (k, v) = bucket.as_ref();
TryResult::Present(RefMut::new(shard, k, v.as_ptr()))
let (k, v) = bucket.as_mut();
TryResult::Present(RefMut::new(shard, k, v))
}
} else {
TryResult::Absent
Expand All @@ -1136,7 +1127,7 @@ impl<'a, K: 'a + Eq + Hash, V: 'a, S: 'a + BuildHasher + Clone> Map<'a, K, V, S>
// Here we only use `iter` as a temporary, preventing use-after-free
for bucket in shard.iter() {
let (k, v) = bucket.as_mut();
if !f(&*k, v.get_mut()) {
if !f(&*k, v) {
shard.erase(bucket);
}
}
Expand Down Expand Up @@ -1362,7 +1353,7 @@ where
let entry_size_iter = iter.map(|bucket| {
// Safety: The iterator returns buckets with valid pointers to entries
let (key, value) = unsafe { bucket.as_ref() };
key.extra_size() + value.get().extra_size()
key.extra_size() + value.extra_size()
});

core::mem::size_of::<CachePadded<RwLock<HashMap<K, V>>>>()
Expand Down
44 changes: 18 additions & 26 deletions src/mapref/entry.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use super::one::RefMut;
use crate::lock::RwLockWriteGuard;
use crate::util::SharedValue;
use crate::HashMap;
use core::hash::Hash;
use core::mem;
Expand Down Expand Up @@ -138,15 +137,13 @@ impl<'a, K: Eq + Hash, V> VacantEntry<'a, K, V> {

pub fn insert(mut self, value: V) -> RefMut<'a, K, V> {
unsafe {
let occupied = self.shard.insert_in_slot(
self.hash,
self.slot,
(self.key, SharedValue::new(value)),
);
let occupied = self
.shard
.insert_in_slot(self.hash, self.slot, (self.key, value));

let (k, v) = occupied.as_ref();
let (k, v) = occupied.as_mut();

RefMut::new(self.shard, k, v.as_ptr())
RefMut::new(self.shard, k, v)
}
}

Expand All @@ -156,11 +153,9 @@ impl<'a, K: Eq + Hash, V> VacantEntry<'a, K, V> {
K: Clone,
{
unsafe {
let bucket = self.shard.insert_in_slot(
self.hash,
self.slot,
(self.key.clone(), SharedValue::new(value)),
);
let bucket = self
.shard
.insert_in_slot(self.hash, self.slot, (self.key.clone(), value));

OccupiedEntry::new(self.shard, self.key, bucket)
}
Expand All @@ -177,7 +172,7 @@ impl<'a, K: Eq + Hash, V> VacantEntry<'a, K, V> {

pub struct OccupiedEntry<'a, K, V> {
shard: RwLockWriteGuard<'a, HashMap<K, V>>,
bucket: hashbrown::raw::Bucket<(K, SharedValue<V>)>,
bucket: hashbrown::raw::Bucket<(K, V)>,
key: K,
}

Expand All @@ -188,17 +183,17 @@ impl<'a, K: Eq + Hash, V> OccupiedEntry<'a, K, V> {
pub(crate) unsafe fn new(
shard: RwLockWriteGuard<'a, HashMap<K, V>>,
key: K,
bucket: hashbrown::raw::Bucket<(K, SharedValue<V>)>,
bucket: hashbrown::raw::Bucket<(K, V)>,
) -> Self {
Self { shard, bucket, key }
}

pub fn get(&self) -> &V {
unsafe { self.bucket.as_ref().1.get() }
unsafe { &self.bucket.as_ref().1 }
}

pub fn get_mut(&mut self) -> &mut V {
unsafe { self.bucket.as_mut().1.get_mut() }
unsafe { &mut self.bucket.as_mut().1 }
}

pub fn insert(&mut self, value: V) -> V {
Expand All @@ -207,8 +202,8 @@ impl<'a, K: Eq + Hash, V> OccupiedEntry<'a, K, V> {

pub fn into_ref(self) -> RefMut<'a, K, V> {
unsafe {
let (k, v) = self.bucket.as_ref();
RefMut::new(self.shard, k, v.as_ptr())
let (k, v) = self.bucket.as_mut();
RefMut::new(self.shard, k, v)
}
}

Expand All @@ -222,20 +217,17 @@ impl<'a, K: Eq + Hash, V> OccupiedEntry<'a, K, V> {

pub fn remove(mut self) -> V {
let ((_k, v), _) = unsafe { self.shard.remove(self.bucket) };
v.into_inner()
v
}

pub fn remove_entry(mut self) -> (K, V) {
let ((k, v), _) = unsafe { self.shard.remove(self.bucket) };
(k, v.into_inner())
(k, v)
}

pub fn replace_entry(self, value: V) -> (K, V) {
let (k, v) = mem::replace(
unsafe { self.bucket.as_mut() },
(self.key, SharedValue::new(value)),
);
(k, v.into_inner())
let (k, v) = mem::replace(unsafe { self.bucket.as_mut() }, (self.key, value));
(k, v)
}
}

Expand Down
12 changes: 3 additions & 9 deletions src/rayon/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,13 +95,7 @@ where
{
Vec::from(self.shards)
.into_par_iter()
.flat_map_iter(|shard| {
shard
.into_inner()
.into_inner()
.into_iter()
.map(|(k, v)| (k, v.into_inner()))
})
.flat_map_iter(|shard| shard.into_inner().into_inner().into_iter())
.drive_unindexed(consumer)
}
}
Expand Down Expand Up @@ -145,7 +139,7 @@ where
guard.iter().map(move |b| {
let guard = Arc::clone(&guard);
let (k, v) = b.as_ref();
RefMulti::new(guard, k, v.get())
RefMulti::new(guard, k, v)
})
})
.drive_unindexed(consumer)
Expand Down Expand Up @@ -203,7 +197,7 @@ where
guard.iter().map(move |b| {
let guard = Arc::clone(&guard);
let (k, v) = b.as_mut();
RefMutMulti::new(guard, k, v.get_mut())
RefMutMulti::new(guard, k, v)
})
})
.drive_unindexed(consumer)
Expand Down
4 changes: 2 additions & 2 deletions src/read_only.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ impl<'a, K: 'a + Eq + Hash, V: 'a, S: BuildHasher + Clone> ReadOnlyView<K, V, S>

shard.find(hash, |(k, _v)| key == k.borrow()).map(|b| {
let (k, v) = unsafe { b.as_ref() };
(k, v.get())
(k, v)
})
}

Expand All @@ -100,7 +100,7 @@ impl<'a, K: 'a + Eq + Hash, V: 'a, S: BuildHasher + Clone> ReadOnlyView<K, V, S>
.flat_map(|shard| shard.iter())
.map(|b| {
let (k, v) = b.as_ref();
(k, v.get())
(k, v)
})
}
}
Expand Down
Loading

0 comments on commit c8c936c

Please sign in to comment.