RFC: Add optional Serde support for HashMap (#106)

* Make current Clippy happy

* Add optional Serde support for HashMap

The implementation of Deserialize is based on the implementation of Extend.

Cargo.toml

@@ -37,6 +37,7 @@ crossbeam-utils = { version = "0.8.12", optional = true }
 crossbeam-epoch = { version = "0.9.11", optional = true }
 crossbeam-queue = { version = "0.3.6", optional = true }
 lru = { version = "0.12", optional = true }
+serde = { version = "1.0", optional = true }
 smallvec = { version = "1.4", optional = true }
 sptr = "0.3"
 tokio = { version = "1", features = ["sync"], optional = true }
@@ -49,6 +50,7 @@ time = "0.3"
 tracing-subscriber = { version = "0.3", features = ["env-filter", "std", "fmt"] }
 uuid = "1.0"
 function_name = "0.3"
+serde_json = "1.0"
 tokio = { version = "1", features = ["rt", "macros"] }
 
 [[bench]]

benches/hashmap_benchmark.rs

@@ -19,7 +19,7 @@ extern crate criterion;
 extern crate rand;
 
 use concread::hashmap::*;
-use criterion::{criterion_group, criterion_main, BatchSize, Criterion};
+use criterion::{black_box, criterion_group, criterion_main, BatchSize, Criterion};
 use rand::{thread_rng, Rng};
 
 // ranges of counts for different benchmarks (MINs are inclusive, MAXes exclusive):
@@ -190,8 +190,7 @@ fn remove_vec<'a, V: Clone + Sync + Send + 'static>(
 fn search_vec<V: Clone + Sync + Send + 'static>(map: &HashMap<u32, V>, list: &Vec<u32>) {
     let read_txn = map.read();
     for i in list.iter() {
-        // ! This could potentially get optimized into nothing !
-        read_txn.get(&i);
+        read_txn.get(black_box(i));
     }
 }
 
@@ -265,7 +264,7 @@ fn prepare_remove<V: Clone + Sync + Send + 'static>(value: V) -> (HashMap<u32, V
     for i in random_order(insert_count, insert_count).iter() {
         // We could count on the hash function alone to make the order random, but it seems
         // better to count on every possible implementation.
-        write_txn.insert(i.clone(), value.clone());
+        write_txn.insert(*i, value.clone());
     }
     write_txn.commit();
     (map, random_order(insert_count, remove_count))

src/arcache/mod.rs

@@ -28,7 +28,6 @@ use std::sync::{Mutex, RwLock};
 
 use std::borrow::Borrow;
 use std::cell::UnsafeCell;
-use std::convert::TryFrom;
 use std::fmt::Debug;
 use std::hash::Hash;
 use std::mem;
@@ -1118,7 +1117,7 @@ impl<
                             CacheItem::Haunted(*llp)
                         }
                     };
-                    mem::swap(&mut (*ci).v, &mut next_state);
+                    mem::swap(&mut ci.v, &mut next_state);
                 } // for each item in the bucket.
             }
             // Do nothing, it must have been evicted.
@@ -1344,7 +1343,7 @@ impl<
                         };
                         // Now change the state.
                         if let Some(ref mut next_state) = next_state {
-                            mem::swap(&mut (*ci).v, next_state);
+                            mem::swap(&mut ci.v, next_state);
                         }
                     } // for each ci in slots
                 }
@@ -1704,10 +1703,10 @@ impl<
     /// the thread local cache, a `Some` is returned, else you will recieve a `None`. On a
     /// `None`, you must then consult the external data source that this structure is acting
     /// as a cache for.
-    pub fn get<Q: ?Sized>(&mut self, k: &Q) -> Option<&V>
+    pub fn get<Q>(&mut self, k: &Q) -> Option<&V>
     where
         K: Borrow<Q>,
-        Q: Hash + Eq + Ord,
+        Q: Hash + Eq + Ord + ?Sized,
     {
         let k_hash: u64 = self.cache.prehash(k);
 
@@ -1773,10 +1772,10 @@ impl<
     ///
     /// Since you are mutating the state of the value, if you have sized insertions you MAY
     /// break this since you can change the weight of the value to be inconsistent
-    pub fn get_mut<Q: ?Sized>(&mut self, k: &Q, make_dirty: bool) -> Option<&mut V>
+    pub fn get_mut<Q>(&mut self, k: &Q, make_dirty: bool) -> Option<&mut V>
     where
         K: Borrow<Q>,
-        Q: Hash + Eq + Ord,
+        Q: Hash + Eq + Ord + ?Sized,
     {
         // If we were requested to clear, we can not copy to the tlocal cache.
         let is_cleared = unsafe {
@@ -1814,10 +1813,10 @@ impl<
     }
 
     /// Determine if this cache contains the following key.
-    pub fn contains_key<Q: ?Sized>(&mut self, k: &Q) -> bool
+    pub fn contains_key<Q>(&mut self, k: &Q) -> bool
     where
         K: Borrow<Q>,
-        Q: Hash + Eq + Ord,
+        Q: Hash + Eq + Ord + ?Sized,
     {
         self.get(k).is_some()
     }
@@ -2047,10 +2046,10 @@ impl<
     /// the thread local cache, a `Some` is returned, else you will recieve a `None`. On a
     /// `None`, you must then consult the external data source that this structure is acting
     /// as a cache for.
-    pub fn get<Q: ?Sized>(&mut self, k: &Q) -> Option<&V>
+    pub fn get<Q>(&mut self, k: &Q) -> Option<&V>
     where
         K: Borrow<Q>,
-        Q: Hash + Eq + Ord,
+        Q: Hash + Eq + Ord + ?Sized,
     {
         let k_hash: u64 = self.cache.prehash(k);
 
@@ -2108,10 +2107,10 @@ impl<
     }
 
     /// Determine if this cache contains the following key.
-    pub fn contains_key<Q: ?Sized>(&mut self, k: &Q) -> bool
+    pub fn contains_key<Q>(&mut self, k: &Q) -> bool
     where
         K: Borrow<Q>,
-        Q: Hash + Eq + Ord,
+        Q: Hash + Eq + Ord + ?Sized,
     {
         self.get(k).is_some()
     }
@@ -2220,8 +2219,8 @@ mod tests {
             .expect("Invalid cache parameters!");
         let mut wr_txn = arc.write();
 
-        assert!(wr_txn.get(&1) == None);
-        assert!(wr_txn.peek_hit().len() == 0);
+        assert!(wr_txn.get(&1).is_none());
+        assert!(wr_txn.peek_hit().is_empty());
         wr_txn.insert(1, 1);
         assert!(wr_txn.get(&1) == Some(&1));
         assert!(wr_txn.peek_hit().len() == 1);
@@ -2483,7 +2482,7 @@ mod tests {
         let gfreq_set: Vec<usize> = wr_txn.iter_ghost_freq().take(4).copied().collect();
 
         gfreq_set.iter().for_each(|i| wr_txn.insert(*i, *i));
-        let _stats = wr_txn.commit();
+        wr_txn.commit();
 
         println!("== 9");
         let wr_txn = arc.write();
@@ -2659,12 +2658,12 @@ mod tests {
         wr_txn.insert(1, 1);
 
         // assert 1 is not in rd.
-        assert!(rd_txn.get(&1) == None);
+        assert!(rd_txn.get(&1).is_none());
 
         // Commit wr
         wr_txn.commit();
         // Even after the commit, it's not in rd.
-        assert!(rd_txn.get(&1) == None);
+        assert!(rd_txn.get(&1).is_none());
         // begin wr
         let mut wr_txn = arc.write();
         // We now need to flood the cache, to cause ghost rec eviction.
@@ -2684,7 +2683,7 @@ mod tests {
         // assert that 1 is haunted.
         assert!(wr_txn.peek_cache(&1) == CacheState::Haunted);
         // assert 1 is not in rd.
-        assert!(rd_txn.get(&1) == None);
+        assert!(rd_txn.get(&1).is_none());
         // now that 1 is hanuted, in rd attempt to insert 1, X
         rd_txn.insert(1, 100);
         // commit wr

src/bptree/asynch.rs

@@ -40,7 +40,6 @@ mod tests {
     use crate::internals::bptree::node::{assert_released, L_CAPACITY};
     // use rand::prelude::*;
     use rand::seq::SliceRandom;
-    use std::iter::FromIterator;
 
     #[tokio::test]
     async fn test_bptree2_map_basic_write() {
@@ -197,7 +196,7 @@ mod tests {
 
         let r3 = map.read().await;
         // println!("{:?}", r3.len());
-        assert!(r3.len() == 0);
+        assert!(r3.is_empty());
 
         std::mem::drop(r);
         std::mem::drop(r2);
@@ -261,9 +260,9 @@ mod tests {
         let rd = map.read().await;
 
         wr.insert(1, 1);
-        assert!(rd.get(&1) == None);
+        assert!(rd.get(&1).is_none());
         wr.commit().await;
-        assert!(rd.get(&1) == None);
+        assert!(rd.get(&1).is_none());
     }
 
     #[tokio::test]

src/bptree/impl.rs

@@ -165,19 +165,19 @@ impl<K: Clone + Ord + Debug + Sync + Send + 'static, V: Clone + Sync + Send + 's
 
     /// Retrieve a value from the tree. If the value exists, a reference is returned
     /// as `Some(&V)`, otherwise if not present `None` is returned.
-    pub fn get<Q: ?Sized>(&self, k: &Q) -> Option<&V>
+    pub fn get<Q>(&self, k: &Q) -> Option<&V>
     where
         K: Borrow<Q>,
-        Q: Ord,
+        Q: Ord + ?Sized,
     {
         self.inner.as_ref().search(k)
     }
 
     /// Assert if a key exists in the tree.
-    pub fn contains_key<Q: ?Sized>(&self, k: &Q) -> bool
+    pub fn contains_key<Q>(&self, k: &Q) -> bool
     where
         K: Borrow<Q>,
-        Q: Ord,
+        Q: Ord + ?Sized,
     {
         self.inner.as_ref().contains_key(k)
     }
@@ -317,19 +317,19 @@ impl<K: Clone + Ord + Debug + Sync + Send + 'static, V: Clone + Sync + Send + 's
 {
     /// Retrieve a value from the tree. If the value exists, a reference is returned
     /// as `Some(&V)`, otherwise if not present `None` is returned.
-    pub fn get<Q: ?Sized>(&self, k: &Q) -> Option<&V>
+    pub fn get<Q>(&self, k: &Q) -> Option<&V>
     where
         K: Borrow<Q>,
-        Q: Ord,
+        Q: Ord + ?Sized,
     {
         self.inner.as_ref().search(k)
     }
 
     /// Assert if a key exists in the tree.
-    pub fn contains_key<Q: ?Sized>(&self, k: &Q) -> bool
+    pub fn contains_key<Q>(&self, k: &Q) -> bool
     where
         K: Borrow<Q>,
-        Q: Ord,
+        Q: Ord + ?Sized,
     {
         self.inner.as_ref().contains_key(k)
     }
@@ -404,10 +404,10 @@ impl<K: Clone + Ord + Debug + Sync + Send + 'static, V: Clone + Sync + Send + 's
 {
     /// Retrieve a value from the tree. If the value exists, a reference is returned
     /// as `Some(&V)`, otherwise if not present `None` is returned.
-    pub fn get<Q: ?Sized>(&self, k: &Q) -> Option<&V>
+    pub fn get<Q>(&self, k: &Q) -> Option<&V>
     where
         K: Borrow<Q>,
-        Q: Ord,
+        Q: Ord + ?Sized,
     {
         match self.inner {
             SnapshotType::R(inner) => inner.search(k),
@@ -416,10 +416,10 @@ impl<K: Clone + Ord + Debug + Sync + Send + 'static, V: Clone + Sync + Send + 's
     }
 
     /// Assert if a key exists in the tree.
-    pub fn contains_key<Q: ?Sized>(&self, k: &Q) -> bool
+    pub fn contains_key<Q>(&self, k: &Q) -> bool
     where
         K: Borrow<Q>,
-        Q: Ord,
+        Q: Ord + ?Sized,
     {
         match self.inner {
             SnapshotType::R(inner) => inner.contains_key(k),

src/bptree/mod.rs

@@ -43,7 +43,6 @@ mod tests {
     use crate::internals::bptree::node::{assert_released, L_CAPACITY};
     // use rand::prelude::*;
     use rand::seq::SliceRandom;
-    use std::iter::FromIterator;
 
     #[test]
     fn test_bptree2_map_basic_write() {
@@ -200,7 +199,7 @@ mod tests {
 
         let r3 = map.read();
         // println!("{:?}", r3.len());
-        assert!(r3.len() == 0);
+        assert!(r3.is_empty());
 
         std::mem::drop(r);
         std::mem::drop(r2);
@@ -264,9 +263,9 @@ mod tests {
         let rd = map.read();
 
         wr.insert(1, 1);
-        assert!(rd.get(&1) == None);
+        assert!(rd.get(&1).is_none());
         wr.commit();
-        assert!(rd.get(&1) == None);
+        assert!(rd.get(&1).is_none());
     }
 
     #[test]

src/cowcell/asynch.rs

@@ -184,7 +184,7 @@ where
     fn deref(&self) -> &T {
         match &self.work {
             Some(v) => v,
-            None => &(*self.read),
+            None => &self.read,
         }
     }
 }
@@ -285,7 +285,7 @@ mod tests {
                 let mut cc_wrtxn = cc.write().await;
                 {
                     let mut_ptr = cc_wrtxn.get_mut();
-                    mut_ptr.data = mut_ptr.data + 1;
+                    mut_ptr.data += 1;
                 }
                 cc_wrtxn.commit().await;
             }

src/cowcell/mod.rs

@@ -216,7 +216,7 @@ where
     fn deref(&self) -> &T {
         match &self.work {
             Some(v) => v,
-            None => &(*self.read),
+            None => &self.read,
         }
     }
 }
@@ -335,7 +335,7 @@ mod tests {
                                 let mut_ptr = cc_wrtxn.get_mut();
                                 assert!(*mut_ptr >= last_value);
                                 last_value = *mut_ptr;
-                                *mut_ptr = *mut_ptr + 1;
+                                *mut_ptr += 1;
                             }
                             cc_wrtxn.commit();
                         }
@@ -378,7 +378,7 @@ mod tests {
                 let mut cc_wrtxn = cc.write();
                 {
                     let mut_ptr = cc_wrtxn.get_mut();
-                    mut_ptr.data = mut_ptr.data + 1;
+                    mut_ptr.data += 1;
                 }
                 cc_wrtxn.commit();
             }

src/ebrcell/mod.rs

@@ -19,7 +19,6 @@ use crossbeam_epoch as epoch;
 use crossbeam_epoch::{Atomic, Guard, Owned};
 use std::sync::atomic::Ordering::{Acquire, Relaxed, Release};
 
-use std::marker::Send;
 use std::mem;
 use std::ops::{Deref, DerefMut};
 use std::sync::{Mutex, MutexGuard};
@@ -359,7 +358,7 @@ mod tests {
                                 let mut_ptr = cc_wrtxn.get_mut();
                                 assert!(*mut_ptr >= last_value);
                                 last_value = *mut_ptr;
-                                *mut_ptr = *mut_ptr + 1;
+                                *mut_ptr += 1;
                             }
                             cc_wrtxn.commit();
                         }
@@ -401,7 +400,7 @@ mod tests {
                 let mut cc_wrtxn = cc.write();
                 {
                     let mut_ptr = cc_wrtxn.get_mut();
-                    mut_ptr.data = mut_ptr.data + 1;
+                    mut_ptr.data += 1;
                 }
                 cc_wrtxn.commit();
             }
@@ -494,7 +493,7 @@ mod tests_linear {
             let mut cc_wrtxn = cc.write();
             {
                 let mut_ptr = cc_wrtxn.get_mut();
-                mut_ptr.data = mut_ptr.data + 1;
+                mut_ptr.data += 1;
             }
             cc_wrtxn.commit();
         }
@@ -505,7 +504,7 @@ mod tests_linear {
             let mut cc_wrtxn = cc.write();
             {
                 let mut_ptr = cc_wrtxn.get_mut();
-                mut_ptr.data = mut_ptr.data + 1;
+                mut_ptr.data += 1;
             }
             cc_wrtxn.commit();
         }