Skip to content

Commit

Permalink
misc unsafe removals (#325)
Browse files Browse the repository at this point in the history 
* misc: remove unsafe from shard handling via unreachable assertion

* breaking: remove Map trait

* remove hasher from iterators

* remove unnecessary unsafe markers and remove some dead functions
  • Loading branch information
@conradludgate
conradludgate authored Feb 3, 2025
1 parent 54710a6 commit 55f7cdf
Show file tree
Hide file tree
Showing 11 changed files with 156 additions and 351 deletions.
115 changes: 34 additions & 81 deletions src/iter.rs
View file
Original file line number Diff line number Diff line change
@@ -1,13 +1,10 @@
use crossbeam_utils::CachePadded;
use hashbrown::hash_table;

use super::mapref::multiple::{RefMulti, RefMutMulti};
use crate::lock::{RwLockReadGuardDetached, RwLockWriteGuardDetached};
use crate::t::Map;
use crate::DashMap;
use core::hash::{BuildHasher, Hash};
use core::mem;
use std::collections::hash_map::RandomState;
use std::marker::PhantomData;
use crate::lock::{RwLock, RwLockReadGuardDetached, RwLockWriteGuardDetached};
use crate::{DashMap, HashMap};
use core::hash::Hash;
use std::sync::Arc;

/// Iterator over a DashMap yielding key value pairs.
Expand All @@ -23,25 +20,23 @@ use std::sync::Arc;
/// let pairs: Vec<(&'static str, &'static str)> = map.into_iter().collect();
/// assert_eq!(pairs.len(), 2);
/// ```
pub struct OwningIter<K, V, S = RandomState> {
map: DashMap<K, V, S>,
shard_i: usize,
pub struct OwningIter<K, V> {
shards: std::vec::IntoIter<CachePadded<RwLock<HashMap<K, V>>>>,
current: Option<GuardOwningIter<K, V>>,
}

impl<K: Eq + Hash, V, S: BuildHasher + Clone> OwningIter<K, V, S> {
pub(crate) fn new(map: DashMap<K, V, S>) -> Self {
impl<K: Eq + Hash, V> OwningIter<K, V> {
pub(crate) fn new<S>(map: DashMap<K, V, S>) -> Self {
Self {
map,
shard_i: 0,
shards: map.shards.into_vec().into_iter(),
current: None,
}
}
}

type GuardOwningIter<K, V> = hash_table::IntoIter<(K, V)>;

impl<K: Eq + Hash, V, S: BuildHasher + Clone> Iterator for OwningIter<K, V, S> {
impl<K: Eq + Hash, V> Iterator for OwningIter<K, V> {
type Item = (K, V);

fn next(&mut self) -> Option<Self::Item> {
Expand All @@ -52,21 +47,8 @@ impl<K: Eq + Hash, V, S: BuildHasher + Clone> Iterator for OwningIter<K, V, S> {
}
}

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

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

let map = mem::take(&mut *shard_wl);

drop(shard_wl);

let iter = map.into_iter();

let iter = self.shards.next()?.into_inner().into_inner().into_iter();
self.current = Some(iter);

self.shard_i += 1;
}
}
}
Expand All @@ -92,62 +74,49 @@ type GuardIterMut<'a, K, V> = (
/// map.insert("hello", "world");
/// assert_eq!(map.iter().count(), 1);
/// ```
pub struct Iter<'a, K, V, S = RandomState, M = DashMap<K, V, S>> {
map: &'a M,
shard_i: usize,
pub struct Iter<'a, K, V> {
shards: std::slice::Iter<'a, CachePadded<RwLock<HashMap<K, V>>>>,
current: Option<GuardIter<'a, K, V>>,
marker: PhantomData<S>,
}

impl<'i, K: Clone + Hash + Eq, V: Clone, S: Clone + BuildHasher> Clone for Iter<'i, K, V, S> {
impl<'i, K: Clone + Hash + Eq, V: Clone> Clone for Iter<'i, K, V> {
fn clone(&self) -> Self {
Iter::new(self.map)
Iter {
shards: self.shards.clone(),
current: self.current.clone(),
}
}
}

impl<'a, K: Eq + Hash + 'a, V: 'a, S: 'a + BuildHasher + Clone, M: Map<'a, K, V, S>>
Iter<'a, K, V, S, M>
{
pub(crate) fn new(map: &'a M) -> Self {
impl<'a, K: Eq + Hash + 'a, V: 'a> Iter<'a, K, V> {
pub(crate) fn new<S>(map: &'a DashMap<K, V, S>) -> Self {
Self {
map,
shard_i: 0,
shards: map.shards.iter(),
current: None,
marker: PhantomData,
}
}
}

impl<'a, K: Eq + Hash + 'a, V: 'a, S: 'a + BuildHasher + Clone, M: Map<'a, K, V, S>> Iterator
for Iter<'a, K, V, S, M>
{
impl<'a, K: Eq + Hash + 'a, V: 'a> Iterator for Iter<'a, K, V> {
type Item = RefMulti<'a, K, V>;

fn next(&mut self) -> Option<Self::Item> {
loop {
if let Some(current) = self.current.as_mut() {
if let Some((k, v)) = current.1.next() {
return unsafe {
let guard = current.0.clone();
Some(RefMulti::new(guard, k, v))
};
let guard = current.0.clone();
return Some(RefMulti::new(guard, k, v));
}
}

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

let guard = unsafe { self.map._yield_read_shard(self.shard_i) };
let guard = self.shards.next()?.read();
// SAFETY: we keep the guard alive with the shard iterator,
// and with any refs produced by the iterator
let (guard, shard) = unsafe { RwLockReadGuardDetached::detach_from(guard) };

let iter = shard.iter();

self.current = Some((Arc::new(guard), iter));

self.shard_i += 1;
}
}
}
Expand All @@ -164,47 +133,33 @@ impl<'a, K: Eq + Hash + 'a, V: 'a, S: 'a + BuildHasher + Clone, M: Map<'a, K, V,
/// map.iter_mut().for_each(|mut r| *r += 1);
/// assert_eq!(*map.get("Johnny").unwrap(), 22);
/// ```
pub struct IterMut<'a, K, V, S = RandomState, M = DashMap<K, V, S>> {
map: &'a M,
shard_i: usize,
pub struct IterMut<'a, K, V> {
shards: std::slice::Iter<'a, CachePadded<RwLock<HashMap<K, V>>>>,
current: Option<GuardIterMut<'a, K, V>>,
marker: PhantomData<S>,
}

impl<'a, K: Eq + Hash + 'a, V: 'a, S: 'a + BuildHasher + Clone, M: Map<'a, K, V, S>>
IterMut<'a, K, V, S, M>
{
pub(crate) fn new(map: &'a M) -> Self {
impl<'a, K: Eq + Hash + 'a, V: 'a> IterMut<'a, K, V> {
pub(crate) fn new<S>(map: &'a DashMap<K, V, S>) -> Self {
Self {
map,
shard_i: 0,
shards: map.shards.iter(),
current: None,
marker: PhantomData,
}
}
}

impl<'a, K: Eq + Hash + 'a, V: 'a, S: 'a + BuildHasher + Clone, M: Map<'a, K, V, S>> Iterator
for IterMut<'a, K, V, S, M>
{
impl<'a, K: Eq + Hash + 'a, V: 'a> Iterator for IterMut<'a, K, V> {
type Item = RefMutMulti<'a, K, V>;

fn next(&mut self) -> Option<Self::Item> {
loop {
if let Some(current) = self.current.as_mut() {
if let Some((k, v)) = current.1.next() {
return unsafe {
let guard = current.0.clone();
Some(RefMutMulti::new(guard, k, v))
};
let guard = current.0.clone();
return Some(RefMutMulti::new(guard, k, v));
}
}

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

let guard = unsafe { self.map._yield_write_shard(self.shard_i) };
let guard = self.shards.next()?.write();

// SAFETY: we keep the guard alive with the shard iterator,
// and with any refs produced by the iterator
Expand All @@ -213,8 +168,6 @@ impl<'a, K: Eq + Hash + 'a, V: 'a, S: 'a + BuildHasher + Clone, M: Map<'a, K, V,
let iter = shard.iter_mut();

self.current = Some((Arc::new(guard), iter));

self.shard_i += 1;
}
}
}
Expand Down
25 changes: 11 additions & 14 deletions src/iter_set.rs
View file
Original file line number Diff line number Diff line change
@@ -1,38 +1,35 @@
use crate::setref::multiple::RefMulti;
use crate::t::Map;
use core::hash::{BuildHasher, Hash};
use core::hash::Hash;

pub struct OwningIter<K, S> {
inner: crate::iter::OwningIter<K, (), S>,
pub struct OwningIter<K> {
inner: crate::iter::OwningIter<K, ()>,
}

impl<K: Eq + Hash, S: BuildHasher + Clone> OwningIter<K, S> {
pub(crate) fn new(inner: crate::iter::OwningIter<K, (), S>) -> Self {
impl<K: Eq + Hash> OwningIter<K> {
pub(crate) fn new(inner: crate::iter::OwningIter<K, ()>) -> Self {
Self { inner }
}
}

impl<K: Eq + Hash, S: BuildHasher + Clone> Iterator for OwningIter<K, S> {
impl<K: Eq + Hash> Iterator for OwningIter<K> {
type Item = K;

fn next(&mut self) -> Option<Self::Item> {
self.inner.next().map(|(k, _)| k)
}
}

pub struct Iter<'a, K, S, M> {
inner: crate::iter::Iter<'a, K, (), S, M>,
pub struct Iter<'a, K> {
inner: crate::iter::Iter<'a, K, ()>,
}

impl<'a, K: Eq + Hash + 'a, S: 'a + BuildHasher + Clone, M: Map<'a, K, (), S>> Iter<'a, K, S, M> {
pub(crate) fn new(inner: crate::iter::Iter<'a, K, (), S, M>) -> Self {
impl<'a, K: Eq + Hash + 'a> Iter<'a, K> {
pub(crate) fn new(inner: crate::iter::Iter<'a, K, ()>) -> Self {
Self { inner }
}
}

impl<'a, K: Eq + Hash + 'a, S: 'a + BuildHasher + Clone, M: Map<'a, K, (), S>> Iterator
for Iter<'a, K, S, M>
{
impl<'a, K: Eq + Hash + 'a> Iterator for Iter<'a, K> {
type Item = RefMulti<'a, K>;

fn next(&mut self) -> Option<Self::Item> {
Expand Down
Loading

0 comments on commit 55f7cdf

Please sign in to comment.