0.0.4-alpha

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "flashmap"
-version = "0.0.3-alpha"
+version = "0.0.4-alpha"
 authors = ["Cassy343"]
 edition = "2021"
 

src/core/mod.rs

@@ -18,12 +18,11 @@ use crate::{
     },
     util::{likely, lock, Alias},
 };
-use crate::{util::CachePadded, Map, ReadHandle, WriteHandle};
-use crate::{Builder, BuilderArgs};
+use crate::{util::CachePadded, BuilderArgs, Map, ReadHandle, WriteHandle};
 use std::hash::{BuildHasher, Hash};
 use std::marker::PhantomData;
 use std::process::abort;
-use std::ptr::{self, NonNull};
+use std::ptr::NonNull;
 
 pub struct Core<K, V, S = DefaultHashBuilder> {
     residual: AtomicIsize,
@@ -42,19 +41,17 @@ where
     K: Eq + Hash,
     S: BuildHasher,
 {
-    pub unsafe fn build_map(options: Builder<S>) -> (WriteHandle<K, V, S>, ReadHandle<K, V, S>) {
-        let BuilderArgs { capacity, h1, h2 } = options.into_args();
+    pub(crate) unsafe fn build_map(
+        args: BuilderArgs<S>,
+    ) -> (WriteHandle<K, V, S>, ReadHandle<K, V, S>) {
+        let BuilderArgs { capacity, h1, h2 } = args;
 
         let maps = Box::new([
             CachePadded::new(UnsafeCell::new(Map::with_capacity_and_hasher(capacity, h1))),
             CachePadded::new(UnsafeCell::new(Map::with_capacity_and_hasher(capacity, h2))),
         ]);
 
-        #[cfg(not(miri))]
-        let init_refcount_capacity = num_cpus::get();
-
-        #[cfg(miri)]
-        let init_refcount_capacity = 1;
+        let init_refcount_capacity = if cfg!(not(miri)) { num_cpus::get() } else { 1 };
 
         let me = Arc::new(Self {
             residual: AtomicIsize::new(0),
@@ -65,7 +62,7 @@ where
             _not_send_sync: PhantomData,
         });
 
-        let write_handle = WriteHandle::new(Arc::clone(&me));
+        let write_handle = unsafe { WriteHandle::new(Arc::clone(&me)) };
         let read_handle = Self::new_reader(me);
 
         (write_handle, read_handle)
@@ -94,7 +91,8 @@ impl<K, V, S> Core<K, V, S> {
     pub unsafe fn release_residual(&self) {
         let last_residual = self.residual.fetch_sub(1, Ordering::AcqRel);
 
-        // If we were not the last residual reader, we do nothing.
+        // If we were not the last residual reader, or the writer is not currently waiting for the
+        // last reader, we do nothing.
         if last_residual != isize::MIN + 1 {
             return;
         }
@@ -111,7 +109,7 @@ impl<K, V, S> Core<K, V, S> {
             None => {
                 #[cfg(debug_assertions)]
                 {
-                    unreachable!("WAITING_ON_READERS state observed when writer_thread is None");
+                    unreachable!("Writer is waiting on readers but writer_thread is None");
                 }
 
                 #[cfg(not(debug_assertions))]
@@ -128,10 +126,7 @@ impl<K, V, S> Core<K, V, S> {
 
         if residual != 0 {
             let current = Some(thread::current());
-            let old = self
-                .writer_thread
-                .with_mut(|ptr| unsafe { ptr::replace(ptr, current) });
-            drop(old);
+            self.writer_thread.with_mut(|ptr| unsafe { *ptr = current });
 
             let latest_residual = self.residual.fetch_add(isize::MIN, Ordering::AcqRel);
 
@@ -162,13 +157,13 @@ impl<K, V, S> Core<K, V, S> {
     pub unsafe fn finish_write(&self) {
         debug_assert_eq!(self.residual.load(Ordering::Relaxed), 0);
 
+        fence(Ordering::Release);
+
         let guard = lock(&self.refcounts);
 
         // This needs to be within the mutex
         self.writer_map.set(self.writer_map.get().other());
 
-        fence(Ordering::Release);
-
         let mut initial_residual = 0isize;
 
         // Clippy doesn't like that we're iterating over something in a mutex apparently
@@ -185,11 +180,11 @@ impl<K, V, S> Core<K, V, S> {
             }
         }
 
-        fence(Ordering::Acquire);
-
         drop(guard);
 
-        self.residual.fetch_add(initial_residual, Ordering::AcqRel);
+        self.residual.fetch_add(initial_residual, Ordering::Relaxed);
+
+        fence(Ordering::Acquire);
     }
 }
 

src/lib.rs

@@ -84,7 +84,7 @@ impl<S> Debug for Builder<S> {
 
 impl Builder<RandomState> {
     /// Creates a new builder with a [`RandomState`](std::collections::hash_map::RandomState)
-    /// hasher.
+    /// hasher, and an initial capacity of zero.
     pub fn new() -> Self {
         Self {
             capacity: 0,
@@ -135,13 +135,13 @@ impl<S> Builder<S> {
     /// # Safety
     ///
     /// See [`crate::with_hasher`](crate::with_hasher).
-    pub unsafe fn with_hasher_fn<H>(self, make: fn() -> H) -> Builder<H>
+    pub unsafe fn with_hasher_generator<H>(self, gen: fn() -> H) -> Builder<H>
     where
         H: BuildHasher,
     {
         Builder {
             capacity: self.capacity,
-            hasher: HasherGen::MakeOne(make),
+            hasher: HasherGen::Generate(gen),
         }
     }
 
@@ -171,13 +171,14 @@ impl<S> Builder<S> {
     ///
     /// # Safety
     ///
-    /// The implementations of `Hash` and `Eq` for the key type **must** be deterministic.
+    /// The implementations of `Hash` and `Eq` for the key type **must** be deterministic. See
+    /// [`TrustedHashEq`](crate::TrustedHashEq) for details.
     pub unsafe fn build_assert_trusted<K, V>(self) -> (WriteHandle<K, V, S>, ReadHandle<K, V, S>)
     where
         K: Hash + Eq,
         S: BuildHasher,
     {
-        unsafe { Core::build_map(self) }
+        unsafe { Core::build_map(self.into_args()) }
     }
 
     pub(crate) fn into_args(self) -> BuilderArgs<S> {
@@ -192,15 +193,15 @@ impl<S> Builder<S> {
 
 #[derive(Clone, Copy)]
 enum HasherGen<S> {
-    MakeOne(fn() -> S),
+    Generate(fn() -> S),
     MakeBoth(fn() -> (S, S)),
     Clone(S, fn(&S) -> S),
 }
 
 impl<S> HasherGen<S> {
     fn generate(self) -> (S, S) {
         match self {
-            Self::MakeOne(make) => (make(), make()),
+            Self::Generate(gen) => (gen(), gen()),
             Self::MakeBoth(make_both) => make_both(),
             Self::Clone(hasher, clone) => (clone(&hasher), hasher),
         }

src/write.rs

@@ -61,7 +61,7 @@ where
     K: Hash + Eq,
     S: BuildHasher,
 {
-    pub(crate) fn new(core: Arc<Core<K, V, S>>) -> Self {
+    pub(crate) unsafe fn new(core: Arc<Core<K, V, S>>) -> Self {
         Self {
             core,
             operations: UnsafeCell::new(Vec::new()),
@@ -475,10 +475,9 @@ enum RawOperation<K, V> {
 /// value. This also means that you're responsible for manually dropping it. See
 /// [`leak`](crate::Evicted::leak) and [`Leaked`](crate::Leaked) for more information.
 pub struct Evicted<'a, K, V> {
-    value: Alias<V>,
+    leaked: Leaked<V>,
     operations: &'a UnsafeCell<Vec<Operation<K, V>>>,
     operation: usize,
-    handle_uid: WriterUid,
 }
 
 impl<'a, K, V> Evicted<'a, K, V> {
@@ -492,10 +491,12 @@ impl<'a, K, V> Evicted<'a, K, V> {
         let operation = operations.with(|ops_ptr| unsafe { &*ops_ptr }.len() - 1);
 
         Self {
-            value,
+            leaked: Leaked {
+                value,
+                handle_uid: guard.handle_uid,
+            },
             operations,
             operation,
-            handle_uid: guard.handle_uid,
         }
     }
 
@@ -534,24 +535,20 @@ impl<'a, K, V> Evicted<'a, K, V> {
     /// // Lazily drop another
     /// write.guard().drop_lazily(b);
     /// ```
-    #[must_use = "Not using a leaked value may cause a memory leak"]
     pub fn leak(evicted: Self) -> Leaked<V> {
         evicted
             .operations
             .with_mut(|ptr| unsafe { (*ptr).get_unchecked_mut(evicted.operation) }.make_leaky());
 
-        Leaked {
-            value: evicted.value,
-            handle_uid: evicted.handle_uid,
-        }
+        evicted.leaked
     }
 }
 
 impl<K, V> Deref for Evicted<'_, K, V> {
     type Target = V;
 
     fn deref(&self) -> &Self::Target {
-        &self.value
+        &self.leaked
     }
 }
 
@@ -564,6 +561,7 @@ impl<K, V> Deref for Evicted<'_, K, V> {
 /// value is not dropped if the wrapper is dropped. See [`leak`](crate::Evicted::leak) for how to
 /// safely drop or take ownership of a leaked value. See [`into_inner`](crate::Leaked::into_inner)
 /// for details on how to unsafely take ownership of a leaked value.
+#[must_use = "Not using a leaked value may cause a memory leak"]
 pub struct Leaked<V> {
     value: Alias<V>,
     handle_uid: WriterUid,

test.sh

@@ -0,0 +1,4 @@
+#!/usr/bin/env bash
+cargo test
+cargo miri test -- --nocapture
+RUST_BACKTRACE=full RUSTFLAGS="--cfg loom" cargo test --test loom --profile loomtest -- --nocapture

tests/functionality.rs

@@ -6,7 +6,7 @@
 
 mod util;
 
-use util::dderef;
+use std::ops::Deref;
 
 #[test]
 pub fn insert() {
@@ -41,6 +41,10 @@ fn test_empty_iter() {
 
 #[test]
 fn test_lots_of_insertions() {
+    if cfg!(miri) {
+        println!("This test will take a few seconds when run with miri.");
+    }
+
     let (mut write, read) = flashmap::new::<Box<i32>, Box<i32>>();
 
     const MIN: i32 = 0;
@@ -55,11 +59,11 @@ fn test_lots_of_insertions() {
             assert!(write.guard().insert(Box::new(i), Box::new(i)).is_none());
 
             for j in MIN..=i {
-                assert_eq!(read.guard().get(&j).map(dderef), Some(&j));
+                assert_eq!(read.guard().get(&j).map(Deref::deref), Some(&j));
             }
 
             for j in i + 1..MID {
-                assert_eq!(read.guard().get(&j).map(dderef), None);
+                assert_eq!(read.guard().get(&j).map(Deref::deref), None);
             }
         }
 
@@ -116,7 +120,7 @@ fn test_replace() {
         .guard()
         .replace(Box::new(5), |_| Box::new(new))
         .unwrap();
-    assert_eq!(read.guard().get(&5).map(dderef), Some(&new));
+    assert_eq!(read.guard().get(&5).map(Deref::deref), Some(&new));
 }
 
 #[test]

tests/miri.rs

@@ -3,8 +3,7 @@
 mod util;
 
 use flashmap::Evicted;
-use std::thread;
-use util::dderef;
+use std::{ops::Deref, thread};
 
 #[test]
 pub fn nested_readers() {
@@ -19,15 +18,15 @@ pub fn nested_readers() {
         let outer = read.guard();
         let forty = outer.get(&20);
         let middle = read.guard();
-        assert!(matches!(forty.map(dderef), Some(40) | None));
+        assert!(matches!(forty.map(Deref::deref), Some(40) | None));
         let inner = read.guard();
         let twenty = middle.get(&10);
-        assert!(matches!(forty.map(dderef), Some(40) | None));
+        assert!(matches!(forty.map(Deref::deref), Some(40) | None));
         drop(outer);
-        assert!(matches!(twenty.map(dderef), Some(20) | None));
+        assert!(matches!(twenty.map(Deref::deref), Some(20) | None));
         assert_eq!(inner.get(&10), twenty);
         drop(inner);
-        assert!(matches!(twenty.map(dderef), Some(20) | None));
+        assert!(matches!(twenty.map(Deref::deref), Some(20) | None));
         drop(middle);
     });
 

tests/util.rs

@@ -7,11 +7,6 @@ pub use std::{sync, thread};
 
 pub use track_access::*;
 
-#[allow(clippy::borrowed_box)]
-pub fn dderef<T>(x: &Box<T>) -> &T {
-    x
-}
-
 pub fn maybe_loom_model<F>(test: F)
 where
     F: Fn() + Send + Sync + 'static,