From 74ffdbc0c458d9c74619e6758316b1210b0bfde2 Mon Sep 17 00:00:00 2001 From: Sujay Jayakar Date: Fri, 15 Mar 2019 17:21:12 -0700 Subject: [PATCH 1/4] Implement `insert_with_hasher` for map --- src/map.rs | 332 ++++++++++++++++++++++++++++------------------------- 1 file changed, 175 insertions(+), 157 deletions(-) diff --git a/src/map.rs b/src/map.rs index 665c1e2856..63d11ae49a 100644 --- a/src/map.rs +++ b/src/map.rs @@ -197,7 +197,7 @@ fn make_hash(hash_builder: &impl BuildHasher, val: &K) -> u64 state.finish() } -impl HashMap { +impl HashMap { /// Creates an empty `HashMap`. /// /// The hash map is initially created with a capacity of 0, so it will not allocate until it @@ -233,7 +233,6 @@ impl HashMap { impl HashMap where - K: Eq + Hash, S: BuildHasher, { /// Creates an empty `HashMap` which will use the given hash builder to hash @@ -329,108 +328,6 @@ where self.table.capacity() } - /// Reserves capacity for at least `additional` more elements to be inserted - /// in the `HashMap`. The collection may reserve more space to avoid - /// frequent reallocations. - /// - /// # Panics - /// - /// Panics if the new allocation size overflows [`usize`]. - /// - /// [`usize`]: https://doc.rust-lang.org/std/primitive.usize.html - /// - /// # Examples - /// - /// ``` - /// use hashbrown::HashMap; - /// let mut map: HashMap<&str, i32> = HashMap::new(); - /// map.reserve(10); - /// ``` - #[inline] - pub fn reserve(&mut self, additional: usize) { - let hash_builder = &self.hash_builder; - self.table - .reserve(additional, |x| make_hash(hash_builder, &x.0)); - } - - /// Tries to reserve capacity for at least `additional` more elements to be inserted - /// in the given `HashMap`. The collection may reserve more space to avoid - /// frequent reallocations. - /// - /// # Errors - /// - /// If the capacity overflows, or the allocator reports a failure, then an error - /// is returned. - /// - /// # Examples - /// - /// ``` - /// use hashbrown::HashMap; - /// let mut map: HashMap<&str, isize> = HashMap::new(); - /// map.try_reserve(10).expect("why is the test harness OOMing on 10 bytes?"); - /// ``` - #[inline] - pub fn try_reserve(&mut self, additional: usize) -> Result<(), CollectionAllocErr> { - let hash_builder = &self.hash_builder; - self.table - .try_reserve(additional, |x| make_hash(hash_builder, &x.0)) - } - - /// Shrinks the capacity of the map as much as possible. It will drop - /// down as much as possible while maintaining the internal rules - /// and possibly leaving some space in accordance with the resize policy. - /// - /// # Examples - /// - /// ``` - /// use hashbrown::HashMap; - /// - /// let mut map: HashMap = HashMap::with_capacity(100); - /// map.insert(1, 2); - /// map.insert(3, 4); - /// assert!(map.capacity() >= 100); - /// map.shrink_to_fit(); - /// assert!(map.capacity() >= 2); - /// ``` - #[inline] - pub fn shrink_to_fit(&mut self) { - let hash_builder = &self.hash_builder; - self.table.shrink_to(0, |x| make_hash(hash_builder, &x.0)); - } - - /// Shrinks the capacity of the map with a lower limit. It will drop - /// down no lower than the supplied limit while maintaining the internal rules - /// and possibly leaving some space in accordance with the resize policy. - /// - /// Panics if the current capacity is smaller than the supplied - /// minimum capacity. - /// - /// # Examples - /// - /// ``` - /// use hashbrown::HashMap; - /// - /// let mut map: HashMap = HashMap::with_capacity(100); - /// map.insert(1, 2); - /// map.insert(3, 4); - /// assert!(map.capacity() >= 100); - /// map.shrink_to(10); - /// assert!(map.capacity() >= 10); - /// map.shrink_to(0); - /// assert!(map.capacity() >= 2); - /// ``` - #[inline] - pub fn shrink_to(&mut self, min_capacity: usize) { - assert!( - self.capacity() >= min_capacity, - "Tried to shrink to a larger capacity" - ); - - let hash_builder = &self.hash_builder; - self.table - .shrink_to(min_capacity, |x| make_hash(hash_builder, &x.0)); - } - /// An iterator visiting all keys in arbitrary order. /// The iterator element type is `&'a K`. /// @@ -566,43 +463,6 @@ where } } - /// Gets the given key's corresponding entry in the map for in-place manipulation. - /// - /// # Examples - /// - /// ``` - /// use hashbrown::HashMap; - /// - /// let mut letters = HashMap::new(); - /// - /// for ch in "a short treatise on fungi".chars() { - /// let counter = letters.entry(ch).or_insert(0); - /// *counter += 1; - /// } - /// - /// assert_eq!(letters[&'s'], 2); - /// assert_eq!(letters[&'t'], 3); - /// assert_eq!(letters[&'u'], 1); - /// assert_eq!(letters.get(&'y'), None); - /// ``` - #[inline] - pub fn entry(&mut self, key: K) -> Entry { - let hash = make_hash(&self.hash_builder, &key); - if let Some(elem) = self.table.find(hash, |q| q.0.eq(&key)) { - Entry::Occupied(OccupiedEntry { - key: Some(key), - elem, - table: self, - }) - } else { - Entry::Vacant(VacantEntry { - hash, - key, - table: self, - }) - } - } - #[cfg(test)] #[inline] fn raw_capacity(&self) -> usize { @@ -689,6 +549,151 @@ where pub fn clear(&mut self) { self.table.clear(); } +} + +impl HashMap +where + K: Eq + Hash, + S: BuildHasher, +{ + /// Reserves capacity for at least `additional` more elements to be inserted + /// in the `HashMap`. The collection may reserve more space to avoid + /// frequent reallocations. + /// + /// # Panics + /// + /// Panics if the new allocation size overflows [`usize`]. + /// + /// [`usize`]: https://doc.rust-lang.org/std/primitive.usize.html + /// + /// # Examples + /// + /// ``` + /// use hashbrown::HashMap; + /// let mut map: HashMap<&str, i32> = HashMap::new(); + /// map.reserve(10); + /// ``` + #[inline] + pub fn reserve(&mut self, additional: usize) { + let hash_builder = &self.hash_builder; + self.table + .reserve(additional, |x| make_hash(hash_builder, &x.0)); + } + + /// Tries to reserve capacity for at least `additional` more elements to be inserted + /// in the given `HashMap`. The collection may reserve more space to avoid + /// frequent reallocations. + /// + /// # Errors + /// + /// If the capacity overflows, or the allocator reports a failure, then an error + /// is returned. + /// + /// # Examples + /// + /// ``` + /// use hashbrown::HashMap; + /// let mut map: HashMap<&str, isize> = HashMap::new(); + /// map.try_reserve(10).expect("why is the test harness OOMing on 10 bytes?"); + /// ``` + #[inline] + pub fn try_reserve(&mut self, additional: usize) -> Result<(), CollectionAllocErr> { + let hash_builder = &self.hash_builder; + self.table + .try_reserve(additional, |x| make_hash(hash_builder, &x.0)) + } + + /// Shrinks the capacity of the map as much as possible. It will drop + /// down as much as possible while maintaining the internal rules + /// and possibly leaving some space in accordance with the resize policy. + /// + /// # Examples + /// + /// ``` + /// use hashbrown::HashMap; + /// + /// let mut map: HashMap = HashMap::with_capacity(100); + /// map.insert(1, 2); + /// map.insert(3, 4); + /// assert!(map.capacity() >= 100); + /// map.shrink_to_fit(); + /// assert!(map.capacity() >= 2); + /// ``` + #[inline] + pub fn shrink_to_fit(&mut self) { + let hash_builder = &self.hash_builder; + self.table.shrink_to(0, |x| make_hash(hash_builder, &x.0)); + } + + /// Shrinks the capacity of the map with a lower limit. It will drop + /// down no lower than the supplied limit while maintaining the internal rules + /// and possibly leaving some space in accordance with the resize policy. + /// + /// Panics if the current capacity is smaller than the supplied + /// minimum capacity. + /// + /// # Examples + /// + /// ``` + /// use hashbrown::HashMap; + /// + /// let mut map: HashMap = HashMap::with_capacity(100); + /// map.insert(1, 2); + /// map.insert(3, 4); + /// assert!(map.capacity() >= 100); + /// map.shrink_to(10); + /// assert!(map.capacity() >= 10); + /// map.shrink_to(0); + /// assert!(map.capacity() >= 2); + /// ``` + #[inline] + pub fn shrink_to(&mut self, min_capacity: usize) { + assert!( + self.capacity() >= min_capacity, + "Tried to shrink to a larger capacity" + ); + + let hash_builder = &self.hash_builder; + self.table + .shrink_to(min_capacity, |x| make_hash(hash_builder, &x.0)); + } + + /// Gets the given key's corresponding entry in the map for in-place manipulation. + /// + /// # Examples + /// + /// ``` + /// use hashbrown::HashMap; + /// + /// let mut letters = HashMap::new(); + /// + /// for ch in "a short treatise on fungi".chars() { + /// let counter = letters.entry(ch).or_insert(0); + /// *counter += 1; + /// } + /// + /// assert_eq!(letters[&'s'], 2); + /// assert_eq!(letters[&'t'], 3); + /// assert_eq!(letters[&'u'], 1); + /// assert_eq!(letters.get(&'y'), None); + /// ``` + #[inline] + pub fn entry(&mut self, key: K) -> Entry { + let hash = make_hash(&self.hash_builder, &key); + if let Some(elem) = self.table.find(hash, |q| q.0.eq(&key)) { + Entry::Occupied(OccupiedEntry { + key: Some(key), + elem, + table: self, + }) + } else { + Entry::Vacant(VacantEntry { + hash, + key, + table: self, + }) + } + } /// Returns a reference to the value corresponding to the key. /// @@ -954,7 +959,6 @@ where impl HashMap where - K: Eq + Hash, S: BuildHasher, { /// Creates a raw entry builder for the HashMap. @@ -990,7 +994,6 @@ where /// are free to assume this doesn't happen (within the limits of memory-safety). #[inline] pub fn raw_entry_mut(&mut self) -> RawEntryBuilderMut { - self.reserve(1); RawEntryBuilderMut { map: self } } @@ -1041,7 +1044,7 @@ where impl Debug for HashMap where - K: Eq + Hash + Debug, + K: Debug, V: Debug, S: BuildHasher, { @@ -1052,7 +1055,6 @@ where impl Default for HashMap where - K: Eq + Hash, S: BuildHasher + Default, { /// Creates an empty `HashMap`, with the `Default` value for the hasher. @@ -1323,6 +1325,21 @@ where { self.from_hash(hash, |q| q.borrow().eq(k)) } +} + +impl<'a, K, V, S> RawEntryBuilderMut<'a, K, V, S> +where + S: BuildHasher +{ + /// Create a `RawEntryMut` from the given hash. + #[inline] + #[allow(clippy::wrong_self_convention)] + pub fn from_hash(self, hash: u64, is_match: F) -> RawEntryMut<'a, K, V, S> + where + for<'b> F: FnMut(&'b K) -> bool, + { + self.search(hash, is_match) + } #[inline] fn search(self, hash: u64, mut is_match: F) -> RawEntryMut<'a, K, V, S> @@ -1340,16 +1357,6 @@ where }), } } - - /// Create a `RawEntryMut` from the given hash. - #[inline] - #[allow(clippy::wrong_self_convention)] - pub fn from_hash(self, hash: u64, is_match: F) -> RawEntryMut<'a, K, V, S> - where - for<'b> F: FnMut(&'b K) -> bool, - { - self.search(hash, is_match) - } } impl<'a, K, V, S> RawEntryBuilder<'a, K, V, S> @@ -1624,11 +1631,22 @@ impl<'a, K, V, S> RawVacantEntryMut<'a, K, V, S> { K: Hash, S: BuildHasher, { + let hash_builder = self.hash_builder; + self.insert_with_hasher(hash, key, value, |k| make_hash(hash_builder, k)) + } + + /// Set the value of an entry with a custom hasher function. + #[inline] + pub fn insert_with_hasher(self, hash: u64, key: K, value: V, hasher: H) -> (&'a mut K, &'a mut V) + where + S: BuildHasher, + H: Fn(&K) -> u64, + { + self.table.reserve(1, |x| hasher(&x.0)); unsafe { - let hash_builder = self.hash_builder; let elem = self .table - .insert(hash, (key, value), |x| make_hash(hash_builder, &x.0)); + .insert(hash, (key, value), |x| hasher(&x.0)); let &mut (ref mut key, ref mut value) = elem.as_mut(); (key, value) } From 8481ce3619354ef32a9ff2effd06d74d8d8b5f65 Mon Sep 17 00:00:00 2001 From: Sujay Jayakar Date: Fri, 15 Mar 2019 17:32:50 -0700 Subject: [PATCH 2/4] Add new test --- src/map.rs | 46 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/src/map.rs b/src/map.rs index 63d11ae49a..e035c4b505 100644 --- a/src/map.rs +++ b/src/map.rs @@ -2535,7 +2535,7 @@ fn assert_covariance() { mod test_map { use super::DefaultHashBuilder; use super::Entry::{Occupied, Vacant}; - use super::HashMap; + use super::{HashMap, RawEntryMut}; use rand::{rngs::SmallRng, Rng, SeedableRng}; use std::cell::RefCell; use std::usize; @@ -3531,4 +3531,48 @@ mod test_map { } } + #[test] + fn test_key_without_hash_impl() { + #[derive(Debug)] + struct IntWrapper(u64); + + let mut m: HashMap = HashMap::new(); + { + assert!(m.raw_entry().from_hash(0, |k| k.0 == 0).is_none()); + } + { + let vacant_entry = match m.raw_entry_mut().from_hash(0, |k| k.0 == 0) { + RawEntryMut::Occupied(..) => panic!("Found entry for key 0"), + RawEntryMut::Vacant(e) => e, + }; + vacant_entry.insert_with_hasher(0, IntWrapper(0), (), |k| k.0); + } + { + assert!(m.raw_entry().from_hash(0, |k| k.0 == 0).is_some()); + assert!(m.raw_entry().from_hash(1, |k| k.0 == 1).is_none()); + assert!(m.raw_entry().from_hash(2, |k| k.0 == 2).is_none()); + } + { + let vacant_entry = match m.raw_entry_mut().from_hash(1, |k| k.0 == 1) { + RawEntryMut::Occupied(..) => panic!("Found entry for key 1"), + RawEntryMut::Vacant(e) => e, + }; + vacant_entry.insert_with_hasher(1, IntWrapper(1), (), |k| k.0); + } + { + assert!(m.raw_entry().from_hash(0, |k| k.0 == 0).is_some()); + assert!(m.raw_entry().from_hash(1, |k| k.0 == 1).is_some()); + assert!(m.raw_entry().from_hash(2, |k| k.0 == 2).is_none()); + } + { + let occupied_entry = match m.raw_entry_mut().from_hash(0, |k| k.0 == 0) { + RawEntryMut::Occupied(e) => e, + RawEntryMut::Vacant(..) => panic!("Couldn't find entry for key 0"), + }; + occupied_entry.remove(); + } + assert!(m.raw_entry().from_hash(0, |k| k.0 == 0).is_none()); + assert!(m.raw_entry().from_hash(1, |k| k.0 == 1).is_some()); + assert!(m.raw_entry().from_hash(2, |k| k.0 == 2).is_none()); + } } From e1d1a17ac4eeb997e0d3fa8cd5b80322fce4fd99 Mon Sep 17 00:00:00 2001 From: Sujay Jayakar Date: Sat, 16 Mar 2019 12:20:20 -0700 Subject: [PATCH 3/4] cargo fmt --- src/map.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/map.rs b/src/map.rs index e035c4b505..440cd88780 100644 --- a/src/map.rs +++ b/src/map.rs @@ -1329,7 +1329,7 @@ where impl<'a, K, V, S> RawEntryBuilderMut<'a, K, V, S> where - S: BuildHasher + S: BuildHasher, { /// Create a `RawEntryMut` from the given hash. #[inline] @@ -1637,16 +1637,20 @@ impl<'a, K, V, S> RawVacantEntryMut<'a, K, V, S> { /// Set the value of an entry with a custom hasher function. #[inline] - pub fn insert_with_hasher(self, hash: u64, key: K, value: V, hasher: H) -> (&'a mut K, &'a mut V) + pub fn insert_with_hasher( + self, + hash: u64, + key: K, + value: V, + hasher: H, + ) -> (&'a mut K, &'a mut V) where S: BuildHasher, H: Fn(&K) -> u64, { self.table.reserve(1, |x| hasher(&x.0)); unsafe { - let elem = self - .table - .insert(hash, (key, value), |x| hasher(&x.0)); + let elem = self.table.insert(hash, (key, value), |x| hasher(&x.0)); let &mut (ref mut key, ref mut value) = elem.as_mut(); (key, value) } From b3717fd99a0223eff16ecc64b232214adf233dcb Mon Sep 17 00:00:00 2001 From: Sujay Jayakar Date: Sun, 17 Mar 2019 12:31:26 -0700 Subject: [PATCH 4/4] Remove extraneous reservation --- src/map.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/map.rs b/src/map.rs index 440cd88780..cda348be90 100644 --- a/src/map.rs +++ b/src/map.rs @@ -1648,7 +1648,6 @@ impl<'a, K, V, S> RawVacantEntryMut<'a, K, V, S> { S: BuildHasher, H: Fn(&K) -> u64, { - self.table.reserve(1, |x| hasher(&x.0)); unsafe { let elem = self.table.insert(hash, (key, value), |x| hasher(&x.0)); let &mut (ref mut key, ref mut value) = elem.as_mut();