From 3f955e4e9244c16bec0e0fa5b92ae0aafe6e5ac2 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Thu, 4 Jun 2020 15:58:36 -0700 Subject: [PATCH 01/14] Switch to hashbrown's RawTable internally --- Cargo.toml | 5 + README.rst | 8 +- src/lib.rs | 7 +- src/map.rs | 228 ++++++---- src/map_core.rs | 1084 +++++++++++------------------------------------ src/set.rs | 51 ++- src/util.rs | 7 - 7 files changed, 439 insertions(+), 951 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 68fc639e..00957e35 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -35,6 +35,11 @@ autocfg = "1" serde = { version = "1.0", optional = true, default-features = false } rayon = { version = "1.0", optional = true } +[dependencies.hashbrown] +version = "0.7" +default-features = false +features = ["inline-more", "raw"] + [dev-dependencies] itertools = "0.8" rand = {version = "0.7", features = ["small_rng"] } diff --git a/README.rst b/README.rst index afc95dc8..ee753048 100644 --- a/README.rst +++ b/README.rst @@ -15,8 +15,7 @@ indexmap .. |rustc| image:: https://img.shields.io/badge/rust-1.18%2B-orange.svg .. _rustc: https://img.shields.io/badge/rust-1.18%2B-orange.svg -A safe, pure-Rust hash table which preserves (in a limited sense) insertion -order. +A pure-Rust hash table which preserves (in a limited sense) insertion order. This crate implements compact map and set data-structures, where the iteration order of the keys is independent from their hash or @@ -45,11 +44,6 @@ was indexmap, a hash table that has following properties: - It's the usual backwards shift deletion, but only on the index vector, so it's cheaper because it's moving less memory around. -Does not implement (Yet) ------------------------- - -- ``.reserve()`` exists but does not have a complete implementation - Performance ----------- diff --git a/src/lib.rs b/src/lib.rs index 246a097f..b5a6c6ad 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -80,9 +80,10 @@ //! [def]: map/struct.IndexMap.html#impl-Default #[cfg(not(has_std))] -#[macro_use(vec)] extern crate alloc; +extern crate hashbrown; + #[cfg(not(has_std))] pub(crate) mod std { pub use core::*; @@ -129,8 +130,8 @@ struct HashValue(usize); impl HashValue { #[inline(always)] - fn get(self) -> usize { - self.0 + fn get(self) -> u64 { + self.0 as u64 } } diff --git a/src/map.rs b/src/map.rs index 7937d147..a5473c6d 100644 --- a/src/map.rs +++ b/src/map.rs @@ -1,8 +1,6 @@ //! `IndexMap` is a hash table where the iteration order of the key-value //! pairs is independent of the hash values of the keys. -#[cfg(not(has_std))] -use alloc::boxed::Box; #[cfg(not(has_std))] use std::vec::Vec; @@ -30,12 +28,6 @@ use {Bucket, Entries, HashValue}; pub use map_core::{Entry, OccupiedEntry, VacantEntry}; -fn hash_elem_using(build: &B, k: &K) -> HashValue { - let mut h = build.build_hasher(); - k.hash(&mut h); - HashValue(h.finish() as usize) -} - /// A hash table where the iteration order of the key-value pairs is independent /// of the hash values of the keys. /// @@ -110,14 +102,17 @@ where impl Entries for IndexMap { type Entry = Bucket; + #[inline] fn into_entries(self) -> Vec { self.core.into_entries() } + #[inline] fn as_entries(&self) -> &[Self::Entry] { self.core.as_entries() } + #[inline] fn as_entries_mut(&mut self) -> &mut [Self::Entry] { self.core.as_entries_mut() } @@ -137,18 +132,21 @@ where S: BuildHasher, { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - f.debug_map().entries(self.iter()).finish()?; - if cfg!(feature = "test_debug") { - writeln!(f)?; - self.core.debug_entries(f)?; + if cfg!(not(feature = "test_debug")) { + f.debug_map().entries(self.iter()).finish() + } else { + // Let the inner `IndexMapCore` print all of its details + f.debug_struct("IndexMap") + .field("core", &self.core) + .finish() } - Ok(()) } } #[cfg(has_std)] impl IndexMap { /// Create a new map. (Does not allocate.) + #[inline] pub fn new() -> Self { Self::with_capacity(0) } @@ -157,6 +155,7 @@ impl IndexMap { /// allocate if `n` is zero.) /// /// Computes in **O(n)** time. + #[inline] pub fn with_capacity(n: usize) -> Self { Self::with_capacity_and_hasher(n, <_>::default()) } @@ -167,6 +166,7 @@ impl IndexMap { /// allocate if `n` is zero.) /// /// Computes in **O(n)** time. + #[inline] pub fn with_capacity_and_hasher(n: usize, hash_builder: S) -> Self where S: BuildHasher, @@ -187,6 +187,7 @@ impl IndexMap { /// Return the number of key-value pairs in the map. /// /// Computes in **O(1)** time. + #[inline] pub fn len(&self) -> usize { self.core.len() } @@ -194,6 +195,7 @@ impl IndexMap { /// Returns true if the map contains no elements. /// /// Computes in **O(1)** time. + #[inline] pub fn is_empty(&self) -> bool { self.len() == 0 } @@ -234,11 +236,22 @@ where /// Reserve capacity for `additional` more key-value pairs. /// - /// FIXME Not implemented fully yet. + /// Computes in **O(n)** time. pub fn reserve(&mut self, additional: usize) { - if additional > 0 { - self.core.reserve_one(); - } + self.core.reserve(additional); + } + + /// Shrink the capacity of the map as much as possible. + /// + /// Computes in **O(n)** time. + pub fn shrink_to_fit(&mut self) { + self.core.shrink_to_fit(); + } + + fn hash(&self, key: &Q) -> HashValue { + let mut h = self.hash_builder.build_hasher(); + key.hash(&mut h); + HashValue(h.finish() as usize) } /// Insert a key-value pair in the map. @@ -272,7 +285,7 @@ where /// See also [`entry`](#method.entry) if you you want to insert *or* modify /// or if you need to get the index of the corresponding key-value pair. pub fn insert_full(&mut self, key: K, value: V) -> (usize, Option) { - let hash = hash_elem_using(&self.hash_builder, &key); + let hash = self.hash(&key); self.core.insert_full(hash, key, value) } @@ -281,7 +294,7 @@ where /// /// Computes in **O(1)** time (amortized average). pub fn entry(&mut self, key: K) -> Entry { - let hash = hash_elem_using(&self.hash_builder, &key); + let hash = self.hash(&key); self.core.entry(hash, key) } @@ -328,7 +341,7 @@ where where Q: Hash + Equivalent, { - self.find(key).is_some() + self.get_index_of(key).is_some() } /// Return a reference to the value stored for `key`, if it is present, @@ -339,7 +352,12 @@ where where Q: Hash + Equivalent, { - self.get_full(key).map(third) + if let Some(i) = self.get_index_of(key) { + let entry = &self.as_entries()[i]; + Some(&entry.value) + } else { + None + } } /// Return references to the key-value pair stored for `key`, @@ -363,9 +381,9 @@ where where Q: Hash + Equivalent, { - if let Some(found) = self.get_index_of(key) { - let entry = &self.as_entries()[found]; - Some((found, &entry.key, &entry.value)) + if let Some(i) = self.get_index_of(key) { + let entry = &self.as_entries()[i]; + Some((i, &entry.key, &entry.value)) } else { None } @@ -376,10 +394,11 @@ where where Q: Hash + Equivalent, { - if let Some((_, found)) = self.find(key) { - Some(found) - } else { + if self.is_empty() { None + } else { + let hash = self.hash(key); + self.core.get_index_of(hash, key) } } @@ -387,14 +406,24 @@ where where Q: Hash + Equivalent, { - self.get_full_mut(key).map(third) + if let Some(i) = self.get_index_of(key) { + let entry = &mut self.as_entries_mut()[i]; + Some(&mut entry.value) + } else { + None + } } pub fn get_full_mut(&mut self, key: &Q) -> Option<(usize, &K, &mut V)> where Q: Hash + Equivalent, { - self.get_full_mut2_impl(key).map(|(i, k, v)| (i, &*k, v)) + if let Some(i) = self.get_index_of(key) { + let entry = &mut self.as_entries_mut()[i]; + Some((i, &entry.key, &mut entry.value)) + } else { + None + } } pub(crate) fn get_full_mut2_impl( @@ -404,27 +433,14 @@ where where Q: Hash + Equivalent, { - if let Some((_, found)) = self.find(key) { - let entry = &mut self.as_entries_mut()[found]; - Some((found, &mut entry.key, &mut entry.value)) + if let Some(i) = self.get_index_of(key) { + let entry = &mut self.as_entries_mut()[i]; + Some((i, &mut entry.key, &mut entry.value)) } else { None } } - /// Return probe (indices) and position (entries) - fn find(&self, key: &Q) -> Option<(usize, usize)> - where - Q: Hash + Equivalent, - { - if self.is_empty() { - return None; - } - let h = hash_elem_using(&self.hash_builder, key); - self.core - .find_using(h, move |entry| Q::equivalent(key, &entry.key)) - } - /// Remove the key-value pair equivalent to `key` and return /// its value. /// @@ -504,12 +520,11 @@ where where Q: Hash + Equivalent, { - let (probe, found) = match self.find(key) { - None => return None, - Some(t) => t, - }; - let (k, v) = self.core.swap_remove_found(probe, found); - Some((found, k, v)) + if self.is_empty() { + return None; + } + let hash = self.hash(key); + self.core.swap_remove_full(hash, key) } /// Remove the key-value pair equivalent to `key` and return @@ -562,19 +577,18 @@ where where Q: Hash + Equivalent, { - let (probe, found) = match self.find(key) { - None => return None, - Some(t) => t, - }; - let (k, v) = self.core.shift_remove_found(probe, found); - Some((found, k, v)) + if self.is_empty() { + return None; + } + let hash = self.hash(key); + self.core.shift_remove_full(hash, key) } /// Remove the last key-value pair /// /// Computes in **O(1)** time (average). pub fn pop(&mut self) -> Option<(K, V)> { - self.core.pop_impl() + self.core.pop() } /// Scan through each key-value pair in the map and keep those where the @@ -588,7 +602,7 @@ where where F: FnMut(&K, &mut V) -> bool, { - self.retain_mut(move |k, v| keep(k, v)); + self.core.retain_in_order(move |k, v| keep(k, v)); } pub(crate) fn retain_mut(&mut self, keep: F) @@ -605,7 +619,9 @@ where where K: Ord, { - self.core.sort_by(key_cmp) + self.with_entries(|entries| { + entries.sort_by(|a, b| Ord::cmp(&a.key, &b.key)); + }); } /// Sort the map’s key-value pairs in place using the comparison @@ -616,11 +632,13 @@ where /// /// Computes in **O(n log n + c)** time and **O(n)** space where *n* is /// the length of the map and *c* the capacity. The sort is stable. - pub fn sort_by(&mut self, compare: F) + pub fn sort_by(&mut self, mut cmp: F) where F: FnMut(&K, &V, &K, &V) -> Ordering, { - self.core.sort_by(compare) + self.with_entries(|entries| { + entries.sort_by(|a, b| cmp(&a.key, &a.value, &b.key, &b.value)); + }); } /// Sort the key-value pairs of the map and return a by value iterator of @@ -632,7 +650,7 @@ where F: FnMut(&K, &V, &K, &V) -> Ordering, { let mut entries = self.into_entries(); - entries.sort_by(move |a, b| cmp(&a.key, &a.value, &b.key, &b.value)); + entries.sort_by(|a, b| cmp(&a.key, &a.value, &b.key, &b.value)); IntoIter { iter: entries.into_iter(), } @@ -654,13 +672,6 @@ where } } -fn key_cmp(k1: &K, _v1: &V, k2: &K, _v2: &V) -> Ordering -where - K: Ord, -{ - Ord::cmp(k1, k2) -} - impl IndexMap { /// Get a key-value pair by index /// @@ -690,11 +701,7 @@ impl IndexMap { /// /// Computes in **O(1)** time (average). pub fn swap_remove_index(&mut self, index: usize) -> Option<(K, V)> { - let (probe, found) = match self.as_entries().get(index) { - Some(e) => self.core.find_existing_entry(e), - None => return None, - }; - Some(self.core.swap_remove_found(probe, found)) + self.core.swap_remove_index(index) } /// Remove the key-value pair by index @@ -707,11 +714,7 @@ impl IndexMap { /// /// Computes in **O(n)** time (average). pub fn shift_remove_index(&mut self, index: usize) -> Option<(K, V)> { - let (probe, found) = match self.as_entries().get(index) { - Some(e) => self.core.find_existing_entry(e), - None => return None, - }; - Some(self.core.shift_remove_found(probe, found)) + self.core.shift_remove_index(index) } } @@ -1020,11 +1023,7 @@ where /// ***Panics*** if `key` is not present in the map. fn index(&self, key: &'a Q) -> &V { - if let Some(v) = self.get(key) { - v - } else { - panic!("IndexMap: key not found") - } + self.get(key).expect("IndexMap: key not found") } } @@ -1040,11 +1039,7 @@ where { /// ***Panics*** if `key` is not present in the map. fn index_mut(&mut self, key: &'a Q) -> &mut V { - if let Some(v) = self.get_mut(key) { - v - } else { - panic!("IndexMap: key not found") - } + self.get_mut(key).expect("IndexMap: key not found") } } @@ -1082,9 +1077,21 @@ where /// equivalents of a key occur more than once, the last corresponding value /// prevails. fn extend>(&mut self, iterable: I) { - for (k, v) in iterable { + // (Note: this is a copy of `std`/`hashbrown`'s reservation logic.) + // Keys may be already present or show multiple times in the iterator. + // Reserve the entire hint lower bound if the map is empty. + // Otherwise reserve half the hint (rounded up), so the map + // will only resize twice in the worst case. + let iter = iterable.into_iter(); + let reserve = if self.is_empty() { + iter.size_hint().0 + } else { + (iter.size_hint().0 + 1) / 2 + }; + self.reserve(reserve); + iter.for_each(move |(k, v)| { self.insert(k, v); - } + }); } } @@ -1279,6 +1286,43 @@ mod tests { } } + #[test] + fn reserve() { + let mut map = IndexMap::::new(); + assert_eq!(map.capacity(), 0); + map.reserve(100); + let capacity = map.capacity(); + assert!(capacity >= 100); + for i in 0..capacity { + assert_eq!(map.len(), i); + map.insert(i, i * i); + assert_eq!(map.len(), i + 1); + assert_eq!(map.capacity(), capacity); + assert_eq!(map.get(&i), Some(&(i * i))); + } + map.insert(capacity, std::usize::MAX); + assert_eq!(map.len(), capacity + 1); + assert!(map.capacity() > capacity); + assert_eq!(map.get(&capacity), Some(&std::usize::MAX)); + } + + #[test] + fn shrink_to_fit() { + let mut map = IndexMap::::new(); + assert_eq!(map.capacity(), 0); + for i in 0..100 { + assert_eq!(map.len(), i); + map.insert(i, i * i); + assert_eq!(map.len(), i + 1); + assert!(map.capacity() >= i + 1); + assert_eq!(map.get(&i), Some(&(i * i))); + map.shrink_to_fit(); + assert_eq!(map.len(), i + 1); + assert_eq!(map.capacity(), i + 1); + assert_eq!(map.get(&i), Some(&(i * i))); + } + } + #[test] fn remove() { let insert = [0, 4, 2, 12, 8, 7, 11, 5, 3, 17, 19, 22, 23]; diff --git a/src/map_core.rs b/src/map_core.rs index 2d8a74ee..bf4f7a65 100644 --- a/src/map_core.rs +++ b/src/map_core.rs @@ -1,406 +1,101 @@ +#![allow(unsafe_code)] //! This is the core implementation that doesn't depend on the hasher at all. //! -//! The methods of `IndexMapCore` don't use any properties (Hash / Eq) of K. +//! The methods of `IndexMapCore` don't use any Hash properties of K. //! //! It's cleaner to separate them out, then the compiler checks that we are not -//! using Hash + Eq at all in these methods. +//! using Hash at all in these methods. //! //! However, we should probably not let this show in the public API or docs. -#[cfg(not(has_std))] -use alloc::boxed::Box; #[cfg(not(has_std))] use std::vec::Vec; -use std::cmp::{max, Ordering}; +use hashbrown::raw::RawTable; + +use std::cmp; use std::fmt; -use std::iter::FromIterator; -use std::marker::PhantomData; use std::mem::replace; use std::ops::RangeFull; use std::vec::Drain; -use util::{enumerate, ptrdistance}; +use equivalent::Equivalent; +use util::enumerate; use {Bucket, Entries, HashValue}; -/// Trait for the "size class". Either u32 or u64 depending on the index -/// size needed to address an entry's index in self.core.entries. -trait Size { - fn is_64_bit() -> bool; - fn is_same_size() -> bool { - Self::is_64_bit() == T::is_64_bit() - } -} - -impl Size for u32 { - #[inline] - fn is_64_bit() -> bool { - false - } -} - -impl Size for u64 { - #[inline] - fn is_64_bit() -> bool { - true - } -} - -/// Call self.method(args) with `::` or `::` depending on `self` -/// size class. -/// -/// The u32 or u64 is *prepended* to the type parameter list! -macro_rules! dispatch_32_vs_64 { - // self.methodname with other explicit type params, - // size is prepended - ($self_:ident . $method:ident::<$($t:ty),*>($($arg:expr),*)) => { - if $self_.size_class_is_64bit() { - $self_.$method::($($arg),*) - } else { - $self_.$method::($($arg),*) - } - }; - // self.methodname with only one type param, the size. - ($self_:ident . $method:ident ($($arg:expr),*)) => { - if $self_.size_class_is_64bit() { - $self_.$method::($($arg),*) - } else { - $self_.$method::($($arg),*) - } - }; - // functionname with size_class_is_64bit as the first argument, only one - // type param, the size. - ($self_:ident => $function:ident ($($arg:expr),*)) => { - if $self_.size_class_is_64bit() { - $function::($($arg),*) - } else { - $function::($($arg),*) - } - }; -} - -/// A possibly truncated hash value. -/// -#[derive(Debug)] -struct ShortHash(usize, PhantomData); - -impl ShortHash { - /// Pretend this is a full HashValue, which - /// is completely ok w.r.t determining bucket index - /// - /// - Sz = u32: 32-bit hash is enough to select bucket index - /// - Sz = u64: hash is not truncated - fn into_hash(self) -> HashValue { - HashValue(self.0) - } -} +type RawBucket = hashbrown::raw::Bucket; -impl Copy for ShortHash {} -impl Clone for ShortHash { - #[inline] - fn clone(&self) -> Self { - *self - } +/// Core of the map that does not depend on S +pub(crate) struct IndexMapCore { + /// indices mapping from the entry hash to its index. + indices: RawTable, + /// entries is a dense vec of entries in their order. + entries: Vec>, } -impl PartialEq for ShortHash { - #[inline] - fn eq(&self, rhs: &Self) -> bool { - self.0 == rhs.0 - } +#[inline(always)] +fn get_hash(entries: &[Bucket]) -> impl Fn(&usize) -> u64 + '_ { + move |&i| entries[i].hash.get() } -// Compare ShortHash == HashValue by truncating appropriately -// if applicable before the comparison -impl PartialEq for ShortHash +impl Clone for IndexMapCore where - Sz: Size, + K: Clone, + V: Clone, { - #[inline] - fn eq(&self, rhs: &HashValue) -> bool { - if Sz::is_64_bit() { - self.0 == rhs.0 - } else { - lo32(self.0 as u64) == lo32(rhs.0 as u64) - } - } -} -impl From> for HashValue { - fn from(x: ShortHash) -> Self { - HashValue(x.0) - } -} - -/// `Pos` is stored in the `indices` array and it points to the index of a -/// `Bucket` in self.core.entries. -/// -/// Pos can be interpreted either as a 64-bit index, or as a 32-bit index and -/// a 32-bit hash. -/// -/// Storing the truncated hash next to the index saves loading the hash from the -/// entry, increasing the cache efficiency. -/// -/// Note that the lower 32 bits of the hash is enough to compute desired -/// position and probe distance in a hash map with less than 2**32 buckets. -/// -/// The IndexMap will simply query its **current raw capacity** to see what its -/// current size class is, and dispatch to the 32-bit or 64-bit lookup code as -/// appropriate. Only the growth code needs some extra logic to handle the -/// transition from one class to another -#[derive(Copy)] -struct Pos { - index: u64, -} - -impl Clone for Pos { - #[inline(always)] fn clone(&self) -> Self { - *self - } -} - -impl fmt::Debug for Pos { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match self.pos() { - Some(i) => write!(f, "Pos({} / {:x})", i, self.index), - None => write!(f, "Pos(None)"), - } - } -} - -impl Pos { - #[inline] - fn none() -> Self { - Pos { index: !0 } - } - - #[inline] - fn is_none(&self) -> bool { - self.index == !0 - } - - /// Return the index part of the Pos value inside `Some(_)` if the position - /// is not none, otherwise return `None`. - #[inline] - fn pos(&self) -> Option { - if self.index == !0 { - None - } else { - Some(lo32(self.index as u64)) - } - } - - /// Set the index part of the Pos value to `i` - #[inline] - fn set_pos(&mut self, i: usize) - where - Sz: Size, - { - debug_assert!(!self.is_none()); - if Sz::is_64_bit() { - self.index = i as u64; - } else { - self.index = i as u64 | ((self.index >> 32) << 32) - } - } - - #[inline] - fn with_hash(i: usize, hash: HashValue) -> Self - where - Sz: Size, - { - if Sz::is_64_bit() { - Pos { index: i as u64 } - } else { - Pos { - index: i as u64 | ((hash.0 as u64) << 32), - } - } - } - - /// “Resolve” the Pos into a combination of its index value and - /// a proxy value to the hash (whether it contains the hash or not - /// depends on the size class of the hash map). - #[inline] - fn resolve(&self) -> Option<(usize, ShortHashProxy)> - where - Sz: Size, - { - if Sz::is_64_bit() { - if !self.is_none() { - Some((self.index as usize, ShortHashProxy::new(0))) - } else { - None - } - } else { - if !self.is_none() { - let (i, hash) = split_lo_hi(self.index); - Some((i as usize, ShortHashProxy::new(hash as usize))) - } else { - None - } - } + let indices = self.indices.clone(); + let mut entries = Vec::with_capacity(indices.capacity()); + entries.clone_from(&self.entries); + IndexMapCore { indices, entries } } - /// Like resolve, but the Pos **must** be non-none. Return its index. - #[inline] - fn resolve_existing_index(&self) -> usize - where - Sz: Size, - { - debug_assert!( - !self.is_none(), - "datastructure inconsistent: none where valid Pos expected" - ); - if Sz::is_64_bit() { - self.index as usize - } else { - let (i, _) = split_lo_hi(self.index); - i as usize + fn clone_from(&mut self, other: &Self) { + let hasher = get_hash(&other.entries); + self.indices.clone_from_with_hasher(&other.indices, hasher); + if self.entries.capacity() < other.entries.len() { + // If we must resize, match the indices capacity + self.reserve_entries(); } + self.entries.clone_from(&other.entries); } } -#[inline] -fn lo32(x: u64) -> usize { - (x & 0xFFFF_FFFF) as usize -} - -// split into low, hi parts -#[inline] -fn split_lo_hi(x: u64) -> (u32, u32) { - (x as u32, (x >> 32) as u32) -} - -// Possibly contains the truncated hash value for an entry, depending on -// the size class. -struct ShortHashProxy(usize, PhantomData); - -impl ShortHashProxy +impl fmt::Debug for IndexMapCore where - Sz: Size, + K: fmt::Debug, + V: fmt::Debug, { - fn new(x: usize) -> Self { - ShortHashProxy(x, PhantomData) - } - - /// Get the hash from either `self` or from a lookup into `entries`, - /// depending on `Sz`. - fn get_short_hash(&self, entries: &[Bucket], index: usize) -> ShortHash { - if Sz::is_64_bit() { - ShortHash(entries[index].hash.0, PhantomData) - } else { - ShortHash(self.0, PhantomData) - } - } -} - -#[inline] -fn usable_capacity(cap: usize) -> usize { - cap - cap / 4 -} - -#[inline] -fn to_raw_capacity(n: usize) -> usize { - n + n / 3 -} - -#[inline(always)] -fn desired_pos(mask: usize, hash: HashValue) -> usize { - hash.0 & mask -} - -/// The number of steps that `current` is forward of the desired position for hash -#[inline(always)] -fn probe_distance(mask: usize, hash: HashValue, current: usize) -> usize { - current.wrapping_sub(desired_pos(mask, hash)) & mask -} - -// this could not be captured in an efficient iterator -macro_rules! probe_loop { - ($probe_var: ident < $len: expr, $body: expr) => { - loop { - if $probe_var < $len { - $body - $probe_var += 1; - } else { - $probe_var = 0; + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + struct DebugIndices<'a>(&'a RawTable); + impl fmt::Debug for DebugIndices<'_> { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + let indices = unsafe { self.0.iter().map(|raw_bucket| raw_bucket.read()) }; + f.debug_list().entries(indices).finish() } } - } -} - -/// Find, in the indices, an entry that already exists at a known position -/// inside self.entries in the IndexMap. -/// -/// This is effectively reverse lookup, from the entries into the hash buckets. -/// -/// Return the probe index (into self.indices) -/// -/// + indices: The self.indices of the map, -/// + hash: The full hash value from the bucket -/// + mask: self.mask. -/// + entry_index: The index of the entry in self.entries -fn find_existing_entry_at( - indices: &[Pos], - hash: HashValue, - mask: usize, - entry_index: usize, -) -> usize -where - Sz: Size, -{ - let mut probe = desired_pos(mask, hash); - probe_loop!(probe < indices.len(), { - // the entry *must* be present; if we hit a Pos::none this was not true - // and there is a debug assertion in resolve_existing_index for that. - let i = indices[probe].resolve_existing_index::(); - if i == entry_index { - return probe; - } - }); -} - -/// Core of the map that does not depend on S -pub(crate) struct IndexMapCore { - mask: usize, - /// indices are the buckets. indices.len() == raw capacity - indices: Box<[Pos]>, - /// entries is a dense vec of entries in their order. entries.len() == len - entries: Vec>, -} - -impl Clone for IndexMapCore -where - K: Clone, - V: Clone, -{ - fn clone(&self) -> Self { - IndexMapCore { - mask: self.mask, - indices: self.indices.clone(), - entries: self.entries.clone(), - } - } - fn clone_from(&mut self, other: &Self) { - self.mask = other.mask; - self.indices.clone_from(&other.indices); - self.entries.clone_from(&other.entries); + f.debug_struct("IndexMapCore") + .field("indices", &DebugIndices(&self.indices)) + .field("entries", &self.entries) + .finish() } } impl Entries for IndexMapCore { type Entry = Bucket; + #[inline] fn into_entries(self) -> Vec { self.entries } + #[inline] fn as_entries(&self) -> &[Self::Entry] { &self.entries } + #[inline] fn as_entries_mut(&mut self) -> &mut [Self::Entry] { &mut self.entries } @@ -409,9 +104,8 @@ impl Entries for IndexMapCore { where F: FnOnce(&mut [Self::Entry]), { - let side_index = self.save_hash_index(); f(&mut self.entries); - self.restore_hash_index(side_index); + self.rebuild_hash_table(); } } @@ -419,437 +113,257 @@ impl IndexMapCore { #[inline] pub(crate) fn new() -> Self { IndexMapCore { - mask: 0, - indices: Box::new([]), + indices: RawTable::new(), entries: Vec::new(), } } #[inline] pub(crate) fn with_capacity(n: usize) -> Self { - let raw = to_raw_capacity(n); - let raw_cap = max(raw.next_power_of_two(), 8); IndexMapCore { - mask: raw_cap.wrapping_sub(1), - indices: vec![Pos::none(); raw_cap].into_boxed_slice(), - entries: Vec::with_capacity(usable_capacity(raw_cap)), + indices: RawTable::with_capacity(n), + entries: Vec::with_capacity(n), } } - // Return whether we need 32 or 64 bits to specify a bucket or entry index - #[cfg(not(feature = "test_low_transition_point"))] - fn size_class_is_64bit(&self) -> bool { - usize::max_value() > u32::max_value() as usize - && self.raw_capacity() >= u32::max_value() as usize - } - - // for testing - #[cfg(feature = "test_low_transition_point")] - fn size_class_is_64bit(&self) -> bool { - self.raw_capacity() >= 64 - } - - #[inline(always)] - fn raw_capacity(&self) -> usize { - self.indices.len() - } - + #[inline] pub(crate) fn len(&self) -> usize { - self.entries.len() + self.indices.len() } + #[inline] pub(crate) fn capacity(&self) -> usize { - usable_capacity(self.raw_capacity()) + cmp::min(self.indices.capacity(), self.entries.capacity()) } pub(crate) fn clear(&mut self) { + self.indices.clear_no_drop(); self.entries.clear(); - self.clear_indices(); } pub(crate) fn drain(&mut self, range: RangeFull) -> Drain> { - self.clear_indices(); + self.indices.clear_no_drop(); self.entries.drain(range) } - // clear self.indices to the same state as "no elements" - fn clear_indices(&mut self) { - for pos in self.indices.iter_mut() { - *pos = Pos::none(); - } + /// Reserve capacity for `additional` more key-value pairs. + pub(crate) fn reserve(&mut self, additional: usize) { + self.indices.reserve(additional, get_hash(&self.entries)); + self.reserve_entries(); } - fn first_allocation(&mut self) { - debug_assert_eq!(self.len(), 0); - let raw_cap = 8usize; - self.mask = raw_cap.wrapping_sub(1); - self.indices = vec![Pos::none(); raw_cap].into_boxed_slice(); - self.entries = Vec::with_capacity(usable_capacity(raw_cap)); + /// Reserve entries capacity to match the indices + fn reserve_entries(&mut self) { + let additional = self.indices.capacity() - self.entries.len(); + self.entries.reserve_exact(additional); } - pub(crate) fn reserve_one(&mut self) { - if self.len() == self.capacity() { - dispatch_32_vs_64!(self.double_capacity()); - } + /// Shrink the capacity of the map as much as possible. + pub(crate) fn shrink_to_fit(&mut self) { + self.indices.shrink_to(0, get_hash(&self.entries)); + self.entries.shrink_to_fit(); } - #[inline(never)] - // `Sz` is *current* Size class, before grow - fn double_capacity(&mut self) - where - Sz: Size, - { - debug_assert!(self.raw_capacity() == 0 || self.len() > 0); - if self.raw_capacity() == 0 { - return self.first_allocation(); - } - - // find first ideally placed element -- start of cluster - let mut first_ideal = 0; - for (i, index) in enumerate(&*self.indices) { - if let Some(pos) = index.pos() { - if 0 == probe_distance(self.mask, self.entries[pos].hash, i) { - first_ideal = i; - break; - } - } - } - - // visit the entries in an order where we can simply reinsert them - // into self.indices without any bucket stealing. - let new_raw_cap = self.indices.len() * 2; - let old_indices = replace( - &mut self.indices, - vec![Pos::none(); new_raw_cap].into_boxed_slice(), - ); - self.mask = new_raw_cap.wrapping_sub(1); - - // `Sz` is the old size class, and either u32 or u64 is the new - for &pos in &old_indices[first_ideal..] { - dispatch_32_vs_64!(self.reinsert_entry_in_order::(pos)); - } - - for &pos in &old_indices[..first_ideal] { - dispatch_32_vs_64!(self.reinsert_entry_in_order::(pos)); + /// Remove the last key-value pair + pub(crate) fn pop(&mut self) -> Option<(K, V)> { + if let Some(entry) = self.entries.pop() { + let last = self.entries.len(); + let raw_bucket = self.find_index(entry.hash, last).unwrap(); + unsafe { self.indices.erase_no_drop(&raw_bucket) }; + Some((entry.key, entry.value)) + } else { + None } - let more = self.capacity() - self.len(); - self.entries.reserve_exact(more); } - // write to self.indices - // read from self.entries at `pos` - // - // reinserting rewrites all `Pos` entries anyway. This handles transitioning - // from u32 to u64 size class if needed by using the two type parameters. - fn reinsert_entry_in_order(&mut self, pos: Pos) - where - SzNew: Size, - SzOld: Size, - { - if let Some((i, hash_proxy)) = pos.resolve::() { - // only if the size class is conserved can we use the short hash - let entry_hash = if SzOld::is_same_size::() { - hash_proxy.get_short_hash(&self.entries, i).into_hash() - } else { - self.entries[i].hash - }; - // find first empty bucket and insert there - let mut probe = desired_pos(self.mask, entry_hash); - probe_loop!(probe < self.indices.len(), { - if self.indices[probe].is_none() { - // empty bucket, insert here - self.indices[probe] = Pos::with_hash::(i, entry_hash); - return; - } - }); + /// Append a key-value pair, *without* checking whether it already exists. + fn push(&mut self, hash: HashValue, key: K, value: V) -> usize { + let i = self.entries.len(); + self.indices.insert(hash.get(), i, get_hash(&self.entries)); + if i == self.entries.capacity() { + // Reserve our own capacity synced to the indices, + // rather than letting `Vec::push` just double it. + self.reserve_entries(); } + self.entries.push(Bucket { hash, key, value }); + i } - pub(crate) fn pop_impl(&mut self) -> Option<(K, V)> { - let (probe, found) = match self.as_entries().last() { - Some(e) => self.find_existing_entry(e), - None => return None, - }; - debug_assert_eq!(found, self.entries.len() - 1); - Some(self.swap_remove_found(probe, found)) - } - - fn insert_phase_1<'a, Sz, A>(&'a mut self, hash: HashValue, key: K, action: A) -> A::Output + /// Return the index in `entries` where an equivalent key can be found + pub(crate) fn get_index_of(&self, hash: HashValue, key: &Q) -> Option where - Sz: Size, - K: Eq, - A: ProbeAction<'a, Sz, K, V>, - { - let mut probe = desired_pos(self.mask, hash); - let mut dist = 0; - debug_assert!(self.len() < self.raw_capacity()); - probe_loop!(probe < self.indices.len(), { - if let Some((i, hash_proxy)) = self.indices[probe].resolve::() { - let entry_hash = hash_proxy.get_short_hash(&self.entries, i); - // if existing element probed less than us, swap - let their_dist = probe_distance(self.mask, entry_hash.into_hash(), probe); - if their_dist < dist { - // robin hood: steal the spot if it's better for us - return action.steal(VacantEntry { - map: self, - hash: hash, - key: key, - probe: probe, - }); - } else if entry_hash == hash && self.entries[i].key == key { - return action.hit(OccupiedEntry { - map: self, - key: key, - probe: probe, - index: i, - }); - } - } else { - // empty bucket, insert here - return action.empty(VacantEntry { - map: self, - hash: hash, - key: key, - probe: probe, - }); - } - dist += 1; - }); - } - - /// phase 2 is post-insert where we forward-shift `Pos` in the indices. - fn insert_phase_2(&mut self, mut probe: usize, mut old_pos: Pos) - where - Sz: Size, + Q: ?Sized + Equivalent, { - probe_loop!(probe < self.indices.len(), { - let pos = &mut self.indices[probe]; - if pos.is_none() { - *pos = old_pos; - break; - } else { - old_pos = replace(pos, old_pos); - } - }); + match self.find_equivalent(hash, key) { + Some(raw_bucket) => Some(unsafe { raw_bucket.read() }), + None => None, + } } pub(crate) fn insert_full(&mut self, hash: HashValue, key: K, value: V) -> (usize, Option) where K: Eq, { - self.reserve_one(); - dispatch_32_vs_64!(self.insert_phase_1::<_>(hash, key, InsertValue(value))) + match self.get_index_of(hash, &key) { + Some(i) => (i, Some(replace(&mut self.entries[i].value, value))), + None => (self.push(hash, key, value), None), + } } pub(crate) fn entry(&mut self, hash: HashValue, key: K) -> Entry where K: Eq, { - self.reserve_one(); - dispatch_32_vs_64!(self.insert_phase_1::<_>(hash, key, MakeEntry)) - } - - /// Return probe (indices) and position (entries) - pub(crate) fn find_using(&self, hash: HashValue, key_eq: F) -> Option<(usize, usize)> + match self.find_equivalent(hash, &key) { + Some(raw_bucket) => Entry::Occupied(OccupiedEntry { + map: self, + raw_bucket, + key, + }), + None => Entry::Vacant(VacantEntry { + map: self, + hash, + key, + }), + } + } + + /// Return the raw bucket with an equivalent key + fn find_equivalent(&self, hash: HashValue, key: &Q) -> Option where - F: Fn(&Bucket) -> bool, + Q: ?Sized + Equivalent, { - dispatch_32_vs_64!(self.find_using_impl::<_>(hash, key_eq)) + self.indices.find(hash.get(), { + |&i| Q::equivalent(key, &self.entries[i].key) + }) } - fn find_using_impl(&self, hash: HashValue, key_eq: F) -> Option<(usize, usize)> - where - F: Fn(&Bucket) -> bool, - Sz: Size, - { - debug_assert!(self.len() > 0); - let mut probe = desired_pos(self.mask, hash); - let mut dist = 0; - probe_loop!(probe < self.indices.len(), { - if let Some((i, hash_proxy)) = self.indices[probe].resolve::() { - let entry_hash = hash_proxy.get_short_hash(&self.entries, i); - if dist > probe_distance(self.mask, entry_hash.into_hash(), probe) { - // give up when probe distance is too long - return None; - } else if entry_hash == hash && key_eq(&self.entries[i]) { - return Some((probe, i)); - } - } else { - return None; - } - dist += 1; - }); + /// Return the raw bucket for the given index + fn find_index(&self, hash: HashValue, index: usize) -> Option { + self.indices.find(hash.get(), |&i| i == index) } - /// Find `entry` which is already placed inside self.entries; - /// return its probe and entry index. - pub(crate) fn find_existing_entry(&self, entry: &Bucket) -> (usize, usize) { - debug_assert!(self.len() > 0); - - let hash = entry.hash; - let actual_pos = ptrdistance(&self.entries[0], entry); - let probe = dispatch_32_vs_64!(self => - find_existing_entry_at(&self.indices, hash, self.mask, actual_pos)); - (probe, actual_pos) + /// Remove an entry by shifting all entries that follow it + pub(crate) fn shift_remove_full(&mut self, hash: HashValue, key: &Q) -> Option<(usize, K, V)> + where + Q: ?Sized + Equivalent, + { + match self.find_equivalent(hash, key) { + Some(raw_bucket) => Some(self.shift_remove_bucket(raw_bucket)), + None => None, + } } /// Remove an entry by shifting all entries that follow it - pub(crate) fn shift_remove_found(&mut self, probe: usize, found: usize) -> (K, V) { - dispatch_32_vs_64!(self.shift_remove_found_impl(probe, found)) + pub(crate) fn shift_remove_index(&mut self, index: usize) -> Option<(K, V)> { + let raw_bucket = match self.entries.get(index) { + Some(entry) => self.find_index(entry.hash, index).unwrap(), + None => return None, + }; + let (_, key, value) = self.shift_remove_bucket(raw_bucket); + Some((key, value)) } - fn shift_remove_found_impl(&mut self, probe: usize, found: usize) -> (K, V) - where - Sz: Size, - { - // index `probe` and entry `found` is to be removed + /// Remove an entry by shifting all entries that follow it + fn shift_remove_bucket(&mut self, raw_bucket: RawBucket) -> (usize, K, V) { // use Vec::remove, but then we need to update the indices that point // to all of the other entries that have to move - self.indices[probe] = Pos::none(); - let entry = self.entries.remove(found); + let index = unsafe { + self.indices.erase_no_drop(&raw_bucket); + raw_bucket.read() + }; + let entry = self.entries.remove(index); // correct indices that point to the entries that followed the removed entry. - // use a heuristic between a full sweep vs. a `probe_loop!` for every shifted item. - if self.indices.len() < (self.entries.len() - found) * 2 { - // shift all indices greater than `found` - for pos in self.indices.iter_mut() { - if let Some((i, _)) = pos.resolve::() { - if i > found { - // shift the index - pos.set_pos::(i - 1); + // use a heuristic between a full sweep vs. a `find()` for every shifted item. + let raw_capacity = self.indices.buckets(); + let shifted_entries = &self.entries[index..]; + if shifted_entries.len() > raw_capacity / 2 { + // shift all indices greater than `index` + unsafe { + for raw_bucket in self.indices.iter() { + let i = raw_bucket.read(); + if i > index { + raw_bucket.write(i - 1); } } } } else { // find each following entry to shift its index - for (offset, entry) in enumerate(&self.entries[found..]) { - let index = found + offset; - let mut probe = desired_pos(self.mask, entry.hash); - probe_loop!(probe < self.indices.len(), { - let pos = &mut self.indices[probe]; - if let Some((i, _)) = pos.resolve::() { - if i == index + 1 { - // found it, shift it - pos.set_pos::(index); - break; - } - } - }); + for (i, entry) in (index + 1..).zip(shifted_entries) { + let raw_bucket = self.find_index(entry.hash, i).unwrap(); + unsafe { raw_bucket.write(i - 1) }; } } - self.backward_shift_after_removal::(probe); + (index, entry.key, entry.value) + } - (entry.key, entry.value) + /// Remove an entry by swapping it with the last + pub(crate) fn swap_remove_full(&mut self, hash: HashValue, key: &Q) -> Option<(usize, K, V)> + where + Q: ?Sized + Equivalent, + { + match self.find_equivalent(hash, key) { + Some(raw_bucket) => Some(self.swap_remove_bucket(raw_bucket)), + None => None, + } } /// Remove an entry by swapping it with the last - pub(crate) fn swap_remove_found(&mut self, probe: usize, found: usize) -> (K, V) { - dispatch_32_vs_64!(self.swap_remove_found_impl(probe, found)) + pub(crate) fn swap_remove_index(&mut self, index: usize) -> Option<(K, V)> { + let raw_bucket = match self.entries.get(index) { + Some(entry) => self.find_index(entry.hash, index).unwrap(), + None => return None, + }; + let (_, key, value) = self.swap_remove_bucket(raw_bucket); + Some((key, value)) } - fn swap_remove_found_impl(&mut self, probe: usize, found: usize) -> (K, V) - where - Sz: Size, - { - // index `probe` and entry `found` is to be removed + /// Remove an entry by swapping it with the last + fn swap_remove_bucket(&mut self, raw_bucket: RawBucket) -> (usize, K, V) { // use swap_remove, but then we need to update the index that points // to the other entry that has to move - self.indices[probe] = Pos::none(); - let entry = self.entries.swap_remove(found); + let index = unsafe { + self.indices.erase_no_drop(&raw_bucket); + raw_bucket.read() + }; + let entry = self.entries.swap_remove(index); // correct index that points to the entry that had to swap places - if let Some(entry) = self.entries.get(found) { + if let Some(entry) = self.entries.get(index) { // was not last element - // examine new element in `found` and find it in indices - let mut probe = desired_pos(self.mask, entry.hash); - probe_loop!(probe < self.indices.len(), { - let pos = &mut self.indices[probe]; - if let Some((i, _)) = pos.resolve::() { - if i >= self.entries.len() { - // found it - pos.set_pos::(found); - break; - } - } - }); + // examine new element in `index` and find it in indices + let last = self.entries.len(); + let raw_bucket = self.find_index(entry.hash, last).unwrap(); + unsafe { raw_bucket.write(index) }; } - self.backward_shift_after_removal::(probe); - - (entry.key, entry.value) + (index, entry.key, entry.value) } - fn backward_shift_after_removal(&mut self, probe_at_remove: usize) - where - Sz: Size, - { - // backward shift deletion in self.indices - // after probe, shift all non-ideally placed indices backward - let mut last_probe = probe_at_remove; - let mut probe = probe_at_remove + 1; - probe_loop!(probe < self.indices.len(), { - if let Some((i, hash_proxy)) = self.indices[probe].resolve::() { - let entry_hash = hash_proxy.get_short_hash(&self.entries, i); - if probe_distance(self.mask, entry_hash.into_hash(), probe) > 0 { - self.indices[last_probe] = self.indices[probe]; - self.indices[probe] = Pos::none(); - } else { - break; - } - } else { - break; - } - last_probe = probe; - }); - } - - pub(crate) fn retain_in_order(&mut self, keep: F) + pub(crate) fn retain_in_order(&mut self, mut keep: F) where F: FnMut(&mut K, &mut V) -> bool, { - dispatch_32_vs_64!(self.retain_in_order_impl::<_>(keep)); - } - - fn retain_in_order_impl(&mut self, mut keep: F) - where - F: FnMut(&mut K, &mut V) -> bool, - Sz: Size, - { - // Like Vec::retain in self.entries; for each removed key-value pair, - // we clear its corresponding spot in self.indices, and run the - // usual backward shift in self.indices. + // Like Vec::retain in self.entries, but with mutable K and V. + // We swap-shift all the items we want to keep, truncate the rest, + // then rebuild the raw hash table with the new indexes. let len = self.entries.len(); let mut n_deleted = 0; for i in 0..len { - let will_keep; - let hash; - { - let ent = &mut self.entries[i]; - hash = ent.hash; - will_keep = keep(&mut ent.key, &mut ent.value); + let will_keep = { + let entry = &mut self.entries[i]; + keep(&mut entry.key, &mut entry.value) }; - let probe = find_existing_entry_at::(&self.indices, hash, self.mask, i); if !will_keep { n_deleted += 1; - self.indices[probe] = Pos::none(); - self.backward_shift_after_removal::(probe); } else if n_deleted > 0 { - self.indices[probe].set_pos::(i - n_deleted); self.entries.swap(i - n_deleted, i); } } self.entries.truncate(len - n_deleted); - } - - pub(crate) fn sort_by(&mut self, mut compare: F) - where - F: FnMut(&K, &V, &K, &V) -> Ordering, - { - let side_index = self.save_hash_index(); - self.entries - .sort_by(move |ei, ej| compare(&ei.key, &ei.value, &ej.key, &ej.value)); - self.restore_hash_index(side_index); + self.rebuild_hash_table(); } pub(crate) fn reverse(&mut self) { @@ -857,137 +371,22 @@ impl IndexMapCore { // No need to save hash indices, can easily calculate what they should // be, given that this is an in-place reversal. - dispatch_32_vs_64!(self => apply_new_index(&mut self.indices, self.entries.len())); - - fn apply_new_index(indices: &mut [Pos], len: usize) - where - Sz: Size, - { - for pos in indices { - if let Some((i, _)) = pos.resolve::() { - pos.set_pos::(len - i - 1); - } - } - } - } - - fn save_hash_index(&mut self) -> Vec { - // Temporarily use the hash field in a bucket to store the old index. - // Save the old hash values in `side_index`. Then we can sort - // `self.entries` in place. - Vec::from_iter( - enumerate(&mut self.entries).map(|(i, elt)| replace(&mut elt.hash, HashValue(i)).get()), - ) - } - - fn restore_hash_index(&mut self, mut side_index: Vec) { - // Write back the hash values from side_index and fill `side_index` with - // a mapping from the old to the new index instead. - for (i, ent) in enumerate(&mut self.entries) { - let old_index = ent.hash.get(); - ent.hash = HashValue(replace(&mut side_index[old_index], i)); - } - - // Apply new index to self.indices - dispatch_32_vs_64!(self => apply_new_index(&mut self.indices, &side_index)); - - fn apply_new_index(indices: &mut [Pos], new_index: &[usize]) - where - Sz: Size, - { - for pos in indices { - if let Some((i, _)) = pos.resolve::() { - pos.set_pos::(new_index[i]); - } + let len = self.entries.len(); + unsafe { + for raw_bucket in self.indices.iter() { + let i = raw_bucket.read(); + raw_bucket.write(len - i - 1); } } } - pub(crate) fn debug_entries(&self, f: &mut fmt::Formatter) -> fmt::Result - where - K: fmt::Debug, - { - for (i, index) in enumerate(&*self.indices) { - write!(f, "{}: {:?}", i, index)?; - if let Some(pos) = index.pos() { - let hash = self.entries[pos].hash; - let key = &self.entries[pos].key; - let desire = desired_pos(self.mask, hash); - write!( - f, - ", desired={}, probe_distance={}, key={:?}", - desire, - probe_distance(self.mask, hash, i), - key - )?; - } - writeln!(f)?; + fn rebuild_hash_table(&mut self) { + self.indices.clear_no_drop(); + debug_assert!(self.indices.capacity() >= self.entries.len()); + for (i, entry) in enumerate(&self.entries) { + // We should never have to reallocate, so there's no need for a real hasher. + self.indices.insert(entry.hash.get(), i, |_| unreachable!()); } - writeln!( - f, - "cap={}, raw_cap={}, entries.cap={}", - self.capacity(), - self.raw_capacity(), - self.entries.capacity() - )?; - Ok(()) - } -} - -trait ProbeAction<'a, Sz: Size, K, V>: Sized { - type Output; - // handle an occupied spot in the map - fn hit(self, entry: OccupiedEntry<'a, K, V>) -> Self::Output; - // handle an empty spot in the map - fn empty(self, entry: VacantEntry<'a, K, V>) -> Self::Output; - // robin hood: handle a spot that you should steal because it's better for you - fn steal(self, entry: VacantEntry<'a, K, V>) -> Self::Output; -} - -struct InsertValue(V); - -impl<'a, Sz: Size, K, V> ProbeAction<'a, Sz, K, V> for InsertValue { - type Output = (usize, Option); - - fn hit(self, entry: OccupiedEntry<'a, K, V>) -> Self::Output { - let old = replace(&mut entry.map.entries[entry.index].value, self.0); - (entry.index, Some(old)) - } - - fn empty(self, entry: VacantEntry<'a, K, V>) -> Self::Output { - let pos = &mut entry.map.indices[entry.probe]; - let index = entry.map.entries.len(); - *pos = Pos::with_hash::(index, entry.hash); - entry.map.entries.push(Bucket { - hash: entry.hash, - key: entry.key, - value: self.0, - }); - (index, None) - } - - fn steal(self, entry: VacantEntry<'a, K, V>) -> Self::Output { - let index = entry.map.entries.len(); - entry.insert_impl::(self.0); - (index, None) - } -} - -struct MakeEntry; - -impl<'a, Sz: Size, K: 'a, V: 'a> ProbeAction<'a, Sz, K, V> for MakeEntry { - type Output = Entry<'a, K, V>; - - fn hit(self, entry: OccupiedEntry<'a, K, V>) -> Self::Output { - Entry::Occupied(entry) - } - - fn empty(self, entry: VacantEntry<'a, K, V>) -> Self::Output { - Entry::Vacant(entry) - } - - fn steal(self, entry: VacantEntry<'a, K, V>) -> Self::Output { - Entry::Vacant(entry) } } @@ -1079,34 +478,44 @@ impl<'a, K: 'a + fmt::Debug, V: 'a + fmt::Debug> fmt::Debug for Entry<'a, K, V> /// [`Entry`]: enum.Entry.html pub struct OccupiedEntry<'a, K: 'a, V: 'a> { map: &'a mut IndexMapCore, + raw_bucket: RawBucket, key: K, - probe: usize, - index: usize, } +// `hashbrown::raw::Bucket` is only `Send`, not `Sync`. +// SAFETY: `&self` only accesses the bucket to read it. +unsafe impl Sync for OccupiedEntry<'_, K, V> {} + impl<'a, K, V> OccupiedEntry<'a, K, V> { pub fn key(&self) -> &K { &self.key } + pub fn get(&self) -> &V { - &self.map.entries[self.index].value + &self.map.entries[self.index()].value } + pub fn get_mut(&mut self) -> &mut V { - &mut self.map.entries[self.index].value + let index = self.index(); + &mut self.map.entries[index].value } /// Put the new key in the occupied entry's key slot pub(crate) fn replace_key(self) -> K { - let old_key = &mut self.map.entries[self.index].key; + let index = self.index(); + let old_key = &mut self.map.entries[index].key; replace(old_key, self.key) } /// Return the index of the key-value pair + #[inline] pub fn index(&self) -> usize { - self.index + unsafe { self.raw_bucket.read() } } + pub fn into_mut(self) -> &'a mut V { - &mut self.map.entries[self.index].value + let index = self.index(); + &mut self.map.entries[index].value } /// Sets the value of the entry to `value`, and returns the entry's old value. @@ -1158,7 +567,8 @@ impl<'a, K, V> OccupiedEntry<'a, K, V> { /// /// Computes in **O(1)** time (average). pub fn swap_remove_entry(self) -> (K, V) { - self.map.swap_remove_found(self.probe, self.index) + let (_, key, value) = self.map.swap_remove_bucket(self.raw_bucket); + (key, value) } /// Remove and return the key, value pair stored in the map for this entry @@ -1169,7 +579,8 @@ impl<'a, K, V> OccupiedEntry<'a, K, V> { /// /// Computes in **O(n)** time (average). pub fn shift_remove_entry(self) -> (K, V) { - self.map.shift_remove_found(self.probe, self.index) + let (_, key, value) = self.map.shift_remove_bucket(self.raw_bucket); + (key, value) } } @@ -1188,43 +599,27 @@ impl<'a, K: 'a + fmt::Debug, V: 'a + fmt::Debug> fmt::Debug for OccupiedEntry<'a /// [`Entry`]: enum.Entry.html pub struct VacantEntry<'a, K: 'a, V: 'a> { map: &'a mut IndexMapCore, - key: K, hash: HashValue, - probe: usize, + key: K, } impl<'a, K, V> VacantEntry<'a, K, V> { pub fn key(&self) -> &K { &self.key } + pub fn into_key(self) -> K { self.key } + /// Return the index where the key-value pair will be inserted. pub fn index(&self) -> usize { self.map.len() } - pub fn insert(self, value: V) -> &'a mut V { - if self.map.size_class_is_64bit() { - self.insert_impl::(value) - } else { - self.insert_impl::(value) - } - } - fn insert_impl(self, value: V) -> &'a mut V - where - Sz: Size, - { - let index = self.map.entries.len(); - self.map.entries.push(Bucket { - hash: self.hash, - key: self.key, - value, - }); - let old_pos = Pos::with_hash::(index, self.hash); - self.map.insert_phase_2::(self.probe, old_pos); - &mut { self.map }.entries[index].value + pub fn insert(self, value: V) -> &'a mut V { + let i = self.map.push(self.hash, self.key, value); + &mut self.map.entries[i].value } } @@ -1235,3 +630,10 @@ impl<'a, K: 'a + fmt::Debug, V: 'a> fmt::Debug for VacantEntry<'a, K, V> { .finish() } } + +#[test] +fn assert_send_sync() { + fn assert_send_sync() {} + assert_send_sync::>(); + assert_send_sync::>(); +} diff --git a/src/set.rs b/src/set.rs index 9600960e..28ee8640 100644 --- a/src/set.rs +++ b/src/set.rs @@ -91,14 +91,17 @@ where impl Entries for IndexSet { type Entry = Bucket; + #[inline] fn into_entries(self) -> Vec { self.map.into_entries() } + #[inline] fn as_entries(&self) -> &[Self::Entry] { self.map.as_entries() } + #[inline] fn as_entries_mut(&mut self) -> &mut [Self::Entry] { self.map.as_entries_mut() } @@ -210,11 +213,20 @@ where self.map.clear(); } - /// FIXME Not implemented fully yet + /// Reserve capacity for `additional` more values. + /// + /// Computes in **O(n)** time. pub fn reserve(&mut self, additional: usize) { self.map.reserve(additional); } + /// Shrink the capacity of the set as much as possible. + /// + /// Computes in **O(n)** time. + pub fn shrink_to_fit(&mut self) { + self.map.shrink_to_fit(); + } + /// Insert the value into the set. /// /// If an equivalent item already exists in the set, it returns @@ -1308,6 +1320,43 @@ mod tests { } } + #[test] + fn reserve() { + let mut set = IndexSet::::new(); + assert_eq!(set.capacity(), 0); + set.reserve(100); + let capacity = set.capacity(); + assert!(capacity >= 100); + for i in 0..capacity { + assert_eq!(set.len(), i); + set.insert(i); + assert_eq!(set.len(), i + 1); + assert_eq!(set.capacity(), capacity); + assert_eq!(set.get(&i), Some(&i)); + } + set.insert(capacity); + assert_eq!(set.len(), capacity + 1); + assert!(set.capacity() > capacity); + assert_eq!(set.get(&capacity), Some(&capacity)); + } + + #[test] + fn shrink_to_fit() { + let mut set = IndexSet::::new(); + assert_eq!(set.capacity(), 0); + for i in 0..100 { + assert_eq!(set.len(), i); + set.insert(i); + assert_eq!(set.len(), i + 1); + assert!(set.capacity() >= i + 1); + assert_eq!(set.get(&i), Some(&i)); + set.shrink_to_fit(); + assert_eq!(set.len(), i + 1); + assert_eq!(set.capacity(), i + 1); + assert_eq!(set.get(&i), Some(&i)); + } + } + #[test] fn remove() { let insert = [0, 4, 2, 12, 8, 7, 11, 5, 3, 17, 19, 22, 23]; diff --git a/src/util.rs b/src/util.rs index 5613c2b8..4174316f 100644 --- a/src/util.rs +++ b/src/util.rs @@ -1,5 +1,4 @@ use std::iter::Enumerate; -use std::mem::size_of; pub(crate) fn third(t: (A, B, C)) -> C { t.2 @@ -11,9 +10,3 @@ where { iterable.into_iter().enumerate() } - -/// return the number of steps from a to b -pub(crate) fn ptrdistance(a: *const T, b: *const T) -> usize { - debug_assert!(a as usize <= b as usize); - (b as usize - a as usize) / size_of::() -} From 262c97a8536ce8528664bf2223993f310492c52e Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Tue, 9 Jun 2020 13:14:55 -0700 Subject: [PATCH 02/14] Raise the MSRV to 1.32 for hashbrown --- .travis.yml | 4 ---- Cargo.toml | 2 +- README.rst | 4 ++-- src/lib.rs | 5 ++--- 4 files changed, 5 insertions(+), 10 deletions(-) diff --git a/.travis.yml b/.travis.yml index b5a04224..9c3c0004 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,10 +2,6 @@ language: rust sudo: false matrix: include: - # MSRV is lower for non-dev builds - - rust: 1.18.0 - env: - - SKIP_TEST=1 - rust: 1.32.0 - rust: 1.34.2 - rust: stable diff --git a/Cargo.toml b/Cargo.toml index 00957e35..1374a6ef 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -41,7 +41,7 @@ default-features = false features = ["inline-more", "raw"] [dev-dependencies] -itertools = "0.8" +itertools = "0.9" rand = {version = "0.7", features = ["small_rng"] } quickcheck = { version = "0.9", default-features = false } fnv = "1.0" diff --git a/README.rst b/README.rst index ee753048..6ec2210f 100644 --- a/README.rst +++ b/README.rst @@ -12,8 +12,8 @@ indexmap .. |docs| image:: https://docs.rs/indexmap/badge.svg .. _docs: https://docs.rs/indexmap -.. |rustc| image:: https://img.shields.io/badge/rust-1.18%2B-orange.svg -.. _rustc: https://img.shields.io/badge/rust-1.18%2B-orange.svg +.. |rustc| image:: https://img.shields.io/badge/rust-1.32%2B-orange.svg +.. _rustc: https://img.shields.io/badge/rust-1.32%2B-orange.svg A pure-Rust hash table which preserves (in a limited sense) insertion order. diff --git a/src/lib.rs b/src/lib.rs index b5a6c6ad..87a1abe2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -53,9 +53,8 @@ //! //! ### Rust Version //! -//! This version of indexmap requires Rust 1.18 or later, or 1.32+ for -//! development builds, and Rust 1.36+ for using with `alloc` (without `std`), -//! see below. +//! This version of indexmap requires Rust 1.32 or later, or Rust 1.36+ for +//! using with `alloc` (without `std`), see below. //! //! The indexmap 1.x release series will use a carefully considered version //! upgrade policy, where in a later 1.x version, we will raise the minimum From e3c03a5e5e584a42b154416f010a14766abb33e7 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Wed, 10 Jun 2020 11:54:31 -0700 Subject: [PATCH 03/14] Add a top-level comment about allowed unsafe --- src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lib.rs b/src/lib.rs index 87a1abe2..655d16ad 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,3 +1,4 @@ +// We *mostly* avoid unsafe code, but `mod map_core` allows it to use `RawTable`. #![deny(unsafe_code)] #![doc(html_root_url = "https://docs.rs/indexmap/1/")] #![cfg_attr(not(has_std), no_std)] From 118d3271eca3cc867c951ad940f8fff0674f4665 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Wed, 10 Jun 2020 12:42:56 -0700 Subject: [PATCH 04/14] Move cmp into the sorting closures --- src/map.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/map.rs b/src/map.rs index a5473c6d..91eba752 100644 --- a/src/map.rs +++ b/src/map.rs @@ -636,8 +636,8 @@ where where F: FnMut(&K, &V, &K, &V) -> Ordering, { - self.with_entries(|entries| { - entries.sort_by(|a, b| cmp(&a.key, &a.value, &b.key, &b.value)); + self.with_entries(move |entries| { + entries.sort_by(move |a, b| cmp(&a.key, &a.value, &b.key, &b.value)); }); } @@ -650,7 +650,7 @@ where F: FnMut(&K, &V, &K, &V) -> Ordering, { let mut entries = self.into_entries(); - entries.sort_by(|a, b| cmp(&a.key, &a.value, &b.key, &b.value)); + entries.sort_by(move |a, b| cmp(&a.key, &a.value, &b.key, &b.value)); IntoIter { iter: entries.into_iter(), } From 1db5e114eef43c71eabdc75ba255caa01652f2f5 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Thu, 11 Jun 2020 10:42:07 -0700 Subject: [PATCH 05/14] Only rebuild if retain actually changed something --- src/map_core.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/map_core.rs b/src/map_core.rs index bf4f7a65..466a1ee6 100644 --- a/src/map_core.rs +++ b/src/map_core.rs @@ -362,8 +362,10 @@ impl IndexMapCore { self.entries.swap(i - n_deleted, i); } } - self.entries.truncate(len - n_deleted); - self.rebuild_hash_table(); + if n_deleted > 0 { + self.entries.truncate(len - n_deleted); + self.rebuild_hash_table(); + } } pub(crate) fn reverse(&mut self) { From 8771a6c79faaba9e1abe0330e154071addd9a0ff Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Thu, 18 Jun 2020 10:08:57 -0700 Subject: [PATCH 06/14] Upgrade to hashbrown 0.8 --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 1374a6ef..f35436cf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,7 +36,7 @@ serde = { version = "1.0", optional = true, default-features = false } rayon = { version = "1.0", optional = true } [dependencies.hashbrown] -version = "0.7" +version = "0.8" default-features = false features = ["inline-more", "raw"] From 89eefd64b000dca4774d47e04a4c0d135d212884 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Thu, 25 Jun 2020 11:57:42 -0700 Subject: [PATCH 07/14] Document the return value of IndexMapCore::push --- src/map_core.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/map_core.rs b/src/map_core.rs index 466a1ee6..98232d38 100644 --- a/src/map_core.rs +++ b/src/map_core.rs @@ -176,7 +176,8 @@ impl IndexMapCore { } } - /// Append a key-value pair, *without* checking whether it already exists. + /// Append a key-value pair, *without* checking whether it already exists, + /// and return the pair's new index. fn push(&mut self, hash: HashValue, key: K, value: V) -> usize { let i = self.entries.len(); self.indices.insert(hash.get(), i, get_hash(&self.entries)); From 39618a2b6862c0ebc0506757d65ddf720a31aa9a Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Thu, 25 Jun 2020 12:06:03 -0700 Subject: [PATCH 08/14] Don't shadow raw_bucket variables --- src/map_core.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/map_core.rs b/src/map_core.rs index 98232d38..2024af09 100644 --- a/src/map_core.rs +++ b/src/map_core.rs @@ -282,18 +282,18 @@ impl IndexMapCore { if shifted_entries.len() > raw_capacity / 2 { // shift all indices greater than `index` unsafe { - for raw_bucket in self.indices.iter() { - let i = raw_bucket.read(); + for bucket in self.indices.iter() { + let i = bucket.read(); if i > index { - raw_bucket.write(i - 1); + bucket.write(i - 1); } } } } else { // find each following entry to shift its index for (i, entry) in (index + 1..).zip(shifted_entries) { - let raw_bucket = self.find_index(entry.hash, i).unwrap(); - unsafe { raw_bucket.write(i - 1) }; + let shifted_bucket = self.find_index(entry.hash, i).unwrap(); + unsafe { shifted_bucket.write(i - 1) }; } } @@ -336,8 +336,8 @@ impl IndexMapCore { // was not last element // examine new element in `index` and find it in indices let last = self.entries.len(); - let raw_bucket = self.find_index(entry.hash, last).unwrap(); - unsafe { raw_bucket.write(index) }; + let swapped_bucket = self.find_index(entry.hash, last).unwrap(); + unsafe { swapped_bucket.write(index) }; } (index, entry.key, entry.value) From 379a07ae2a2818dafa8cfd6a448de5b0dec7a37d Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Thu, 25 Jun 2020 13:27:55 -0700 Subject: [PATCH 09/14] Don't enable hashbrown/inline-more --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index f35436cf..caadda88 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -38,7 +38,7 @@ rayon = { version = "1.0", optional = true } [dependencies.hashbrown] version = "0.8" default-features = false -features = ["inline-more", "raw"] +features = ["raw"] [dev-dependencies] itertools = "0.9" From 66cb0af437b1a880fe199208d682004be5d3175d Mon Sep 17 00:00:00 2001 From: bluss Date: Sun, 28 Jun 2020 10:55:08 +0200 Subject: [PATCH 10/14] Make methods that get RawBucket parameters unsafe; add safety comments These methods trust their caller to pass correct RawBucket values, so we mark them unsafe to use the common safe/unsafe distinction. I used allow(unused_unsafe) to write the functions in the (hopefully) future style of internal unsafe blocks in unsafe functions. --- src/map_core.rs | 44 ++++++++++++++++++++++++++++++++------------ 1 file changed, 32 insertions(+), 12 deletions(-) diff --git a/src/map_core.rs b/src/map_core.rs index 2024af09..20c00d1e 100644 --- a/src/map_core.rs +++ b/src/map_core.rs @@ -216,6 +216,8 @@ impl IndexMapCore { K: Eq, { match self.find_equivalent(hash, &key) { + // Safety: The entry is created with a live raw bucket, at the same time we have a &mut + // reference to the map, so it can not be modified further. Some(raw_bucket) => Entry::Occupied(OccupiedEntry { map: self, raw_bucket, @@ -250,7 +252,7 @@ impl IndexMapCore { Q: ?Sized + Equivalent, { match self.find_equivalent(hash, key) { - Some(raw_bucket) => Some(self.shift_remove_bucket(raw_bucket)), + Some(raw_bucket) => unsafe { Some(self.shift_remove_bucket(raw_bucket)) }, None => None, } } @@ -261,12 +263,17 @@ impl IndexMapCore { Some(entry) => self.find_index(entry.hash, index).unwrap(), None => return None, }; - let (_, key, value) = self.shift_remove_bucket(raw_bucket); - Some((key, value)) + unsafe { + let (_, key, value) = self.shift_remove_bucket(raw_bucket); + Some((key, value)) + } } /// Remove an entry by shifting all entries that follow it - fn shift_remove_bucket(&mut self, raw_bucket: RawBucket) -> (usize, K, V) { + /// + /// Safety: The caller must pass a live `raw_bucket`. + #[allow(unused_unsafe)] + unsafe fn shift_remove_bucket(&mut self, raw_bucket: RawBucket) -> (usize, K, V) { // use Vec::remove, but then we need to update the indices that point // to all of the other entries that have to move let index = unsafe { @@ -306,7 +313,7 @@ impl IndexMapCore { Q: ?Sized + Equivalent, { match self.find_equivalent(hash, key) { - Some(raw_bucket) => Some(self.swap_remove_bucket(raw_bucket)), + Some(raw_bucket) => unsafe { Some(self.swap_remove_bucket(raw_bucket)) }, None => None, } } @@ -317,12 +324,17 @@ impl IndexMapCore { Some(entry) => self.find_index(entry.hash, index).unwrap(), None => return None, }; - let (_, key, value) = self.swap_remove_bucket(raw_bucket); - Some((key, value)) + unsafe { + let (_, key, value) = self.swap_remove_bucket(raw_bucket); + Some((key, value)) + } } /// Remove an entry by swapping it with the last - fn swap_remove_bucket(&mut self, raw_bucket: RawBucket) -> (usize, K, V) { + /// + /// Safety: The caller must pass a live `raw_bucket`. + #[allow(unused_unsafe)] + unsafe fn swap_remove_bucket(&mut self, raw_bucket: RawBucket) -> (usize, K, V) { // use swap_remove, but then we need to update the index that points // to the other entry that has to move let index = unsafe { @@ -570,8 +582,12 @@ impl<'a, K, V> OccupiedEntry<'a, K, V> { /// /// Computes in **O(1)** time (average). pub fn swap_remove_entry(self) -> (K, V) { - let (_, key, value) = self.map.swap_remove_bucket(self.raw_bucket); - (key, value) + // This is safe because it can only happen once (self is consumed) + // and map.indices have not been modified since entry construction + unsafe { + let (_, key, value) = self.map.swap_remove_bucket(self.raw_bucket); + (key, value) + } } /// Remove and return the key, value pair stored in the map for this entry @@ -582,8 +598,12 @@ impl<'a, K, V> OccupiedEntry<'a, K, V> { /// /// Computes in **O(n)** time (average). pub fn shift_remove_entry(self) -> (K, V) { - let (_, key, value) = self.map.shift_remove_bucket(self.raw_bucket); - (key, value) + // This is safe because it can only happen once (self is consumed) + // and map.indices have not been modified since entry construction + unsafe { + let (_, key, value) = self.map.shift_remove_bucket(self.raw_bucket); + (key, value) + } } } From b0c9577ad2aee065e5670c5b48aef6bd5b1d9127 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Mon, 29 Jun 2020 13:13:31 -0700 Subject: [PATCH 11/14] Encapsulate unsafe code in a raw module --- src/lib.rs | 4 +- src/map.rs | 6 +- src/{map_core.rs => map/core.rs} | 265 +---------------------------- src/map/core/raw.rs | 276 +++++++++++++++++++++++++++++++ 4 files changed, 287 insertions(+), 264 deletions(-) rename src/{map_core.rs => map/core.rs} (56%) create mode 100644 src/map/core/raw.rs diff --git a/src/lib.rs b/src/lib.rs index 655d16ad..3def5d6b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,4 +1,4 @@ -// We *mostly* avoid unsafe code, but `mod map_core` allows it to use `RawTable`. +// We *mostly* avoid unsafe code, but `map::core::raw` allows it to use `RawTable` buckets. #![deny(unsafe_code)] #![doc(html_root_url = "https://docs.rs/indexmap/1/")] #![cfg_attr(not(has_std), no_std)] @@ -107,8 +107,6 @@ mod mutable_keys; mod serde; mod util; -mod map_core; - pub mod map; pub mod set; diff --git a/src/map.rs b/src/map.rs index 91eba752..c8518447 100644 --- a/src/map.rs +++ b/src/map.rs @@ -1,6 +1,8 @@ //! `IndexMap` is a hash table where the iteration order of the key-value //! pairs is independent of the hash values of the keys. +mod core; + #[cfg(not(has_std))] use std::vec::Vec; @@ -21,12 +23,12 @@ use std::collections::hash_map::RandomState; use std::cmp::Ordering; use std::fmt; +use self::core::IndexMapCore; use equivalent::Equivalent; -use map_core::IndexMapCore; use util::third; use {Bucket, Entries, HashValue}; -pub use map_core::{Entry, OccupiedEntry, VacantEntry}; +pub use self::core::{Entry, OccupiedEntry, VacantEntry}; /// A hash table where the iteration order of the key-value pairs is independent /// of the hash values of the keys. diff --git a/src/map_core.rs b/src/map/core.rs similarity index 56% rename from src/map_core.rs rename to src/map/core.rs index 20c00d1e..e5b82679 100644 --- a/src/map_core.rs +++ b/src/map/core.rs @@ -1,4 +1,3 @@ -#![allow(unsafe_code)] //! This is the core implementation that doesn't depend on the hasher at all. //! //! The methods of `IndexMapCore` don't use any Hash properties of K. @@ -8,6 +7,8 @@ //! //! However, we should probably not let this show in the public API or docs. +mod raw; + #[cfg(not(has_std))] use std::vec::Vec; @@ -23,8 +24,6 @@ use equivalent::Equivalent; use util::enumerate; use {Bucket, Entries, HashValue}; -type RawBucket = hashbrown::raw::Bucket; - /// Core of the map that does not depend on S pub(crate) struct IndexMapCore { /// indices mapping from the entry hash to its index. @@ -67,16 +66,8 @@ where V: fmt::Debug, { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - struct DebugIndices<'a>(&'a RawTable); - impl fmt::Debug for DebugIndices<'_> { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - let indices = unsafe { self.0.iter().map(|raw_bucket| raw_bucket.read()) }; - f.debug_list().entries(indices).finish() - } - } - f.debug_struct("IndexMapCore") - .field("indices", &DebugIndices(&self.indices)) + .field("indices", &raw::DebugIndices(&self.indices)) .field("entries", &self.entries) .finish() } @@ -168,8 +159,7 @@ impl IndexMapCore { pub(crate) fn pop(&mut self) -> Option<(K, V)> { if let Some(entry) = self.entries.pop() { let last = self.entries.len(); - let raw_bucket = self.find_index(entry.hash, last).unwrap(); - unsafe { self.indices.erase_no_drop(&raw_bucket) }; + self.erase_index(entry.hash, last); Some((entry.key, entry.value)) } else { None @@ -190,17 +180,6 @@ impl IndexMapCore { i } - /// Return the index in `entries` where an equivalent key can be found - pub(crate) fn get_index_of(&self, hash: HashValue, key: &Q) -> Option - where - Q: ?Sized + Equivalent, - { - match self.find_equivalent(hash, key) { - Some(raw_bucket) => Some(unsafe { raw_bucket.read() }), - None => None, - } - } - pub(crate) fn insert_full(&mut self, hash: HashValue, key: K, value: V) -> (usize, Option) where K: Eq, @@ -211,150 +190,6 @@ impl IndexMapCore { } } - pub(crate) fn entry(&mut self, hash: HashValue, key: K) -> Entry - where - K: Eq, - { - match self.find_equivalent(hash, &key) { - // Safety: The entry is created with a live raw bucket, at the same time we have a &mut - // reference to the map, so it can not be modified further. - Some(raw_bucket) => Entry::Occupied(OccupiedEntry { - map: self, - raw_bucket, - key, - }), - None => Entry::Vacant(VacantEntry { - map: self, - hash, - key, - }), - } - } - - /// Return the raw bucket with an equivalent key - fn find_equivalent(&self, hash: HashValue, key: &Q) -> Option - where - Q: ?Sized + Equivalent, - { - self.indices.find(hash.get(), { - |&i| Q::equivalent(key, &self.entries[i].key) - }) - } - - /// Return the raw bucket for the given index - fn find_index(&self, hash: HashValue, index: usize) -> Option { - self.indices.find(hash.get(), |&i| i == index) - } - - /// Remove an entry by shifting all entries that follow it - pub(crate) fn shift_remove_full(&mut self, hash: HashValue, key: &Q) -> Option<(usize, K, V)> - where - Q: ?Sized + Equivalent, - { - match self.find_equivalent(hash, key) { - Some(raw_bucket) => unsafe { Some(self.shift_remove_bucket(raw_bucket)) }, - None => None, - } - } - - /// Remove an entry by shifting all entries that follow it - pub(crate) fn shift_remove_index(&mut self, index: usize) -> Option<(K, V)> { - let raw_bucket = match self.entries.get(index) { - Some(entry) => self.find_index(entry.hash, index).unwrap(), - None => return None, - }; - unsafe { - let (_, key, value) = self.shift_remove_bucket(raw_bucket); - Some((key, value)) - } - } - - /// Remove an entry by shifting all entries that follow it - /// - /// Safety: The caller must pass a live `raw_bucket`. - #[allow(unused_unsafe)] - unsafe fn shift_remove_bucket(&mut self, raw_bucket: RawBucket) -> (usize, K, V) { - // use Vec::remove, but then we need to update the indices that point - // to all of the other entries that have to move - let index = unsafe { - self.indices.erase_no_drop(&raw_bucket); - raw_bucket.read() - }; - let entry = self.entries.remove(index); - - // correct indices that point to the entries that followed the removed entry. - // use a heuristic between a full sweep vs. a `find()` for every shifted item. - let raw_capacity = self.indices.buckets(); - let shifted_entries = &self.entries[index..]; - if shifted_entries.len() > raw_capacity / 2 { - // shift all indices greater than `index` - unsafe { - for bucket in self.indices.iter() { - let i = bucket.read(); - if i > index { - bucket.write(i - 1); - } - } - } - } else { - // find each following entry to shift its index - for (i, entry) in (index + 1..).zip(shifted_entries) { - let shifted_bucket = self.find_index(entry.hash, i).unwrap(); - unsafe { shifted_bucket.write(i - 1) }; - } - } - - (index, entry.key, entry.value) - } - - /// Remove an entry by swapping it with the last - pub(crate) fn swap_remove_full(&mut self, hash: HashValue, key: &Q) -> Option<(usize, K, V)> - where - Q: ?Sized + Equivalent, - { - match self.find_equivalent(hash, key) { - Some(raw_bucket) => unsafe { Some(self.swap_remove_bucket(raw_bucket)) }, - None => None, - } - } - - /// Remove an entry by swapping it with the last - pub(crate) fn swap_remove_index(&mut self, index: usize) -> Option<(K, V)> { - let raw_bucket = match self.entries.get(index) { - Some(entry) => self.find_index(entry.hash, index).unwrap(), - None => return None, - }; - unsafe { - let (_, key, value) = self.swap_remove_bucket(raw_bucket); - Some((key, value)) - } - } - - /// Remove an entry by swapping it with the last - /// - /// Safety: The caller must pass a live `raw_bucket`. - #[allow(unused_unsafe)] - unsafe fn swap_remove_bucket(&mut self, raw_bucket: RawBucket) -> (usize, K, V) { - // use swap_remove, but then we need to update the index that points - // to the other entry that has to move - let index = unsafe { - self.indices.erase_no_drop(&raw_bucket); - raw_bucket.read() - }; - let entry = self.entries.swap_remove(index); - - // correct index that points to the entry that had to swap places - if let Some(entry) = self.entries.get(index) { - // was not last element - // examine new element in `index` and find it in indices - let last = self.entries.len(); - let swapped_bucket = self.find_index(entry.hash, last).unwrap(); - unsafe { swapped_bucket.write(index) }; - } - - (index, entry.key, entry.value) - } - pub(crate) fn retain_in_order(&mut self, mut keep: F) where F: FnMut(&mut K, &mut V) -> bool, @@ -381,20 +216,6 @@ impl IndexMapCore { } } - pub(crate) fn reverse(&mut self) { - self.entries.reverse(); - - // No need to save hash indices, can easily calculate what they should - // be, given that this is an in-place reversal. - let len = self.entries.len(); - unsafe { - for raw_bucket in self.indices.iter() { - let i = raw_bucket.read(); - raw_bucket.write(len - i - 1); - } - } - } - fn rebuild_hash_table(&mut self) { self.indices.clear_no_drop(); debug_assert!(self.indices.capacity() >= self.entries.len()); @@ -487,52 +308,10 @@ impl<'a, K: 'a + fmt::Debug, V: 'a + fmt::Debug> fmt::Debug for Entry<'a, K, V> } } -/// A view into an occupied entry in a `IndexMap`. -/// It is part of the [`Entry`] enum. -/// -/// [`Entry`]: enum.Entry.html -pub struct OccupiedEntry<'a, K: 'a, V: 'a> { - map: &'a mut IndexMapCore, - raw_bucket: RawBucket, - key: K, -} - -// `hashbrown::raw::Bucket` is only `Send`, not `Sync`. -// SAFETY: `&self` only accesses the bucket to read it. -unsafe impl Sync for OccupiedEntry<'_, K, V> {} +pub use self::raw::OccupiedEntry; +// Extra methods that don't threaten the unsafe encapsulation. impl<'a, K, V> OccupiedEntry<'a, K, V> { - pub fn key(&self) -> &K { - &self.key - } - - pub fn get(&self) -> &V { - &self.map.entries[self.index()].value - } - - pub fn get_mut(&mut self) -> &mut V { - let index = self.index(); - &mut self.map.entries[index].value - } - - /// Put the new key in the occupied entry's key slot - pub(crate) fn replace_key(self) -> K { - let index = self.index(); - let old_key = &mut self.map.entries[index].key; - replace(old_key, self.key) - } - - /// Return the index of the key-value pair - #[inline] - pub fn index(&self) -> usize { - unsafe { self.raw_bucket.read() } - } - - pub fn into_mut(self) -> &'a mut V { - let index = self.index(); - &mut self.map.entries[index].value - } - /// Sets the value of the entry to `value`, and returns the entry's old value. pub fn insert(&mut self, value: V) -> V { replace(self.get_mut(), value) @@ -573,38 +352,6 @@ impl<'a, K, V> OccupiedEntry<'a, K, V> { pub fn remove_entry(self) -> (K, V) { self.swap_remove_entry() } - - /// Remove and return the key, value pair stored in the map for this entry - /// - /// Like `Vec::swap_remove`, the pair is removed by swapping it with the - /// last element of the map and popping it off. **This perturbs - /// the postion of what used to be the last element!** - /// - /// Computes in **O(1)** time (average). - pub fn swap_remove_entry(self) -> (K, V) { - // This is safe because it can only happen once (self is consumed) - // and map.indices have not been modified since entry construction - unsafe { - let (_, key, value) = self.map.swap_remove_bucket(self.raw_bucket); - (key, value) - } - } - - /// Remove and return the key, value pair stored in the map for this entry - /// - /// Like `Vec::remove`, the pair is removed by shifting all of the - /// elements that follow it, preserving their relative order. - /// **This perturbs the index of all of those elements!** - /// - /// Computes in **O(n)** time (average). - pub fn shift_remove_entry(self) -> (K, V) { - // This is safe because it can only happen once (self is consumed) - // and map.indices have not been modified since entry construction - unsafe { - let (_, key, value) = self.map.shift_remove_bucket(self.raw_bucket); - (key, value) - } - } } impl<'a, K: 'a + fmt::Debug, V: 'a + fmt::Debug> fmt::Debug for OccupiedEntry<'a, K, V> { diff --git a/src/map/core/raw.rs b/src/map/core/raw.rs new file mode 100644 index 00000000..8d7ba82a --- /dev/null +++ b/src/map/core/raw.rs @@ -0,0 +1,276 @@ +#![allow(unsafe_code)] +//! This module encapsulates the `unsafe` access to `hashbrown::raw::RawTable`, +//! mostly in dealing with its bucket "pointers". + +use super::{Entry, Equivalent, HashValue, IndexMapCore, VacantEntry}; +use hashbrown::raw::RawTable; +use std::fmt; +use std::mem::replace; + +type RawBucket = hashbrown::raw::Bucket; + +pub(super) struct DebugIndices<'a>(pub &'a RawTable); +impl fmt::Debug for DebugIndices<'_> { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + let indices = unsafe { self.0.iter().map(|raw_bucket| raw_bucket.read()) }; + f.debug_list().entries(indices).finish() + } +} + +impl IndexMapCore { + /// Return the raw bucket with an equivalent key + fn find_equivalent(&self, hash: HashValue, key: &Q) -> Option + where + Q: ?Sized + Equivalent, + { + self.indices.find(hash.get(), { + move |&i| Q::equivalent(key, &self.entries[i].key) + }) + } + + /// Return the raw bucket for the given index + fn find_index(&self, hash: HashValue, index: usize) -> Option { + self.indices.find(hash.get(), move |&i| i == index) + } + + /// Return the index in `entries` where an equivalent key can be found + pub(crate) fn get_index_of(&self, hash: HashValue, key: &Q) -> Option + where + Q: ?Sized + Equivalent, + { + match self.find_equivalent(hash, key) { + Some(raw_bucket) => Some(unsafe { raw_bucket.read() }), + None => None, + } + } + + pub(super) fn erase_index(&mut self, hash: HashValue, index: usize) { + let raw_bucket = self.find_index(hash, index).unwrap(); + unsafe { self.indices.erase_no_drop(&raw_bucket) }; + } + + pub(crate) fn entry(&mut self, hash: HashValue, key: K) -> Entry + where + K: Eq, + { + match self.find_equivalent(hash, &key) { + // Safety: The entry is created with a live raw bucket, at the same time we have a &mut + // reference to the map, so it can not be modified further. + Some(raw_bucket) => Entry::Occupied(OccupiedEntry { + map: self, + raw_bucket, + key, + }), + None => Entry::Vacant(VacantEntry { + map: self, + hash, + key, + }), + } + } + + /// Remove an entry by shifting all entries that follow it + pub(crate) fn shift_remove_full(&mut self, hash: HashValue, key: &Q) -> Option<(usize, K, V)> + where + Q: ?Sized + Equivalent, + { + match self.find_equivalent(hash, key) { + Some(raw_bucket) => unsafe { Some(self.shift_remove_bucket(raw_bucket)) }, + None => None, + } + } + + /// Remove an entry by shifting all entries that follow it + pub(crate) fn shift_remove_index(&mut self, index: usize) -> Option<(K, V)> { + let raw_bucket = match self.entries.get(index) { + Some(entry) => self.find_index(entry.hash, index).unwrap(), + None => return None, + }; + unsafe { + let (_, key, value) = self.shift_remove_bucket(raw_bucket); + Some((key, value)) + } + } + + /// Remove an entry by shifting all entries that follow it + /// + /// Safety: The caller must pass a live `raw_bucket`. + #[allow(unused_unsafe)] + unsafe fn shift_remove_bucket(&mut self, raw_bucket: RawBucket) -> (usize, K, V) { + // use Vec::remove, but then we need to update the indices that point + // to all of the other entries that have to move + let index = unsafe { + self.indices.erase_no_drop(&raw_bucket); + raw_bucket.read() + }; + let entry = self.entries.remove(index); + + // correct indices that point to the entries that followed the removed entry. + // use a heuristic between a full sweep vs. a `find()` for every shifted item. + let raw_capacity = self.indices.buckets(); + let shifted_entries = &self.entries[index..]; + if shifted_entries.len() > raw_capacity / 2 { + // shift all indices greater than `index` + unsafe { + for bucket in self.indices.iter() { + let i = bucket.read(); + if i > index { + bucket.write(i - 1); + } + } + } + } else { + // find each following entry to shift its index + for (i, entry) in (index + 1..).zip(shifted_entries) { + let shifted_bucket = self.find_index(entry.hash, i).unwrap(); + unsafe { shifted_bucket.write(i - 1) }; + } + } + + (index, entry.key, entry.value) + } + + /// Remove an entry by swapping it with the last + pub(crate) fn swap_remove_full(&mut self, hash: HashValue, key: &Q) -> Option<(usize, K, V)> + where + Q: ?Sized + Equivalent, + { + match self.find_equivalent(hash, key) { + Some(raw_bucket) => unsafe { Some(self.swap_remove_bucket(raw_bucket)) }, + None => None, + } + } + + /// Remove an entry by swapping it with the last + pub(crate) fn swap_remove_index(&mut self, index: usize) -> Option<(K, V)> { + let raw_bucket = match self.entries.get(index) { + Some(entry) => self.find_index(entry.hash, index).unwrap(), + None => return None, + }; + unsafe { + let (_, key, value) = self.swap_remove_bucket(raw_bucket); + Some((key, value)) + } + } + + /// Remove an entry by swapping it with the last + /// + /// Safety: The caller must pass a live `raw_bucket`. + #[allow(unused_unsafe)] + unsafe fn swap_remove_bucket(&mut self, raw_bucket: RawBucket) -> (usize, K, V) { + // use swap_remove, but then we need to update the index that points + // to the other entry that has to move + let index = unsafe { + self.indices.erase_no_drop(&raw_bucket); + raw_bucket.read() + }; + let entry = self.entries.swap_remove(index); + + // correct index that points to the entry that had to swap places + if let Some(entry) = self.entries.get(index) { + // was not last element + // examine new element in `index` and find it in indices + let last = self.entries.len(); + let swapped_bucket = self.find_index(entry.hash, last).unwrap(); + unsafe { swapped_bucket.write(index) }; + } + + (index, entry.key, entry.value) + } + + pub(crate) fn reverse(&mut self) { + self.entries.reverse(); + + // No need to save hash indices, can easily calculate what they should + // be, given that this is an in-place reversal. + let len = self.entries.len(); + unsafe { + for raw_bucket in self.indices.iter() { + let i = raw_bucket.read(); + raw_bucket.write(len - i - 1); + } + } + } +} + +/// A view into an occupied entry in a `IndexMap`. +/// It is part of the [`Entry`] enum. +/// +/// [`Entry`]: enum.Entry.html +// SAFETY: The lifetime of the map reference also constrains the raw bucket, +// which is essentially a raw pointer into the map indices. +pub struct OccupiedEntry<'a, K: 'a, V: 'a> { + map: &'a mut IndexMapCore, + raw_bucket: RawBucket, + key: K, +} + +// `hashbrown::raw::Bucket` is only `Send`, not `Sync`. +// SAFETY: `&self` only accesses the bucket to read it. +unsafe impl Sync for OccupiedEntry<'_, K, V> {} + +// The parent module also adds methods that don't threaten the unsafe encapsulation. +impl<'a, K, V> OccupiedEntry<'a, K, V> { + pub fn key(&self) -> &K { + &self.key + } + + pub fn get(&self) -> &V { + &self.map.entries[self.index()].value + } + + pub fn get_mut(&mut self) -> &mut V { + let index = self.index(); + &mut self.map.entries[index].value + } + + /// Put the new key in the occupied entry's key slot + pub(crate) fn replace_key(self) -> K { + let index = self.index(); + let old_key = &mut self.map.entries[index].key; + replace(old_key, self.key) + } + + /// Return the index of the key-value pair + #[inline] + pub fn index(&self) -> usize { + unsafe { self.raw_bucket.read() } + } + + pub fn into_mut(self) -> &'a mut V { + let index = self.index(); + &mut self.map.entries[index].value + } + + /// Remove and return the key, value pair stored in the map for this entry + /// + /// Like `Vec::swap_remove`, the pair is removed by swapping it with the + /// last element of the map and popping it off. **This perturbs + /// the postion of what used to be the last element!** + /// + /// Computes in **O(1)** time (average). + pub fn swap_remove_entry(self) -> (K, V) { + // This is safe because it can only happen once (self is consumed) + // and map.indices have not been modified since entry construction + unsafe { + let (_, key, value) = self.map.swap_remove_bucket(self.raw_bucket); + (key, value) + } + } + + /// Remove and return the key, value pair stored in the map for this entry + /// + /// Like `Vec::remove`, the pair is removed by shifting all of the + /// elements that follow it, preserving their relative order. + /// **This perturbs the index of all of those elements!** + /// + /// Computes in **O(n)** time (average). + pub fn shift_remove_entry(self) -> (K, V) { + // This is safe because it can only happen once (self is consumed) + // and map.indices have not been modified since entry construction + unsafe { + let (_, key, value) = self.map.shift_remove_bucket(self.raw_bucket); + (key, value) + } + } +} From 604a2b68a591550de409f052616391cd9bb188d9 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Thu, 2 Jul 2020 12:11:55 -0700 Subject: [PATCH 12/14] Use plain clear instead of clear_no_drop It was an over-optimization to use `clear_no_drop`, which hurts the possibility of using griddle as an alternate table implementation. --- src/map/core.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/map/core.rs b/src/map/core.rs index e5b82679..22edc1e1 100644 --- a/src/map/core.rs +++ b/src/map/core.rs @@ -128,12 +128,12 @@ impl IndexMapCore { } pub(crate) fn clear(&mut self) { - self.indices.clear_no_drop(); + self.indices.clear(); self.entries.clear(); } pub(crate) fn drain(&mut self, range: RangeFull) -> Drain> { - self.indices.clear_no_drop(); + self.indices.clear(); self.entries.drain(range) } @@ -217,7 +217,7 @@ impl IndexMapCore { } fn rebuild_hash_table(&mut self) { - self.indices.clear_no_drop(); + self.indices.clear(); debug_assert!(self.indices.capacity() >= self.entries.len()); for (i, entry) in enumerate(&self.entries) { // We should never have to reallocate, so there's no need for a real hasher. From 603c326aa82284ffd6d8d11426b76ccc96189bbe Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Thu, 16 Jul 2020 14:38:26 -0700 Subject: [PATCH 13/14] Release 1.5.0 --- Cargo.toml | 2 +- README.rst | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index caadda88..915e4d28 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "indexmap" -version = "1.4.0" +version = "1.5.0" authors = [ "bluss", "Josh Stone " diff --git a/README.rst b/README.rst index 6ec2210f..b1917e40 100644 --- a/README.rst +++ b/README.rst @@ -80,6 +80,25 @@ which is roughly: Recent Changes ============== +- 1.5.0 + + - **MSRV**: Rust 1.32 or later is now required. + + - The inner hash table is now based on ``hashbrown`` by @cuviper in PR 131_. + This also completes the method ``reserve`` and adds ``shrink_to_fit``. + + - Add new methods ``get_key_value``, ``remove_entry``, ``swap_remove_entry``, + and ``shift_remove_entry``, by @cuviper in PR 136_ + + - ``Clone::clone_from`` reuses allocations by @cuviper in PR 125_ + + - Add new method ``reverse`` by @linclelinkpart5 in PR 128_ + +.. _125: https://github.com/bluss/indexmap/pull/125 +.. _128: https://github.com/bluss/indexmap/pull/128 +.. _131: https://github.com/bluss/indexmap/pull/131 +.. _136: https://github.com/bluss/indexmap/pull/136 + - 1.4.0 - Add new method ``get_index_of`` by @Thermatrix in PR 115_ and 120_ From 1999fa2534be62c9719b2381f260c131ff0aff1c Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Thu, 16 Jul 2020 14:56:52 -0700 Subject: [PATCH 14/14] Use new methods in hashbrown 0.8.1 --- Cargo.toml | 2 +- src/map/core.rs | 2 +- src/map/core/raw.rs | 12 +++--------- 3 files changed, 5 insertions(+), 11 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 915e4d28..9199733e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,7 +36,7 @@ serde = { version = "1.0", optional = true, default-features = false } rayon = { version = "1.0", optional = true } [dependencies.hashbrown] -version = "0.8" +version = "0.8.1" default-features = false features = ["raw"] diff --git a/src/map/core.rs b/src/map/core.rs index 22edc1e1..89c20ff7 100644 --- a/src/map/core.rs +++ b/src/map/core.rs @@ -221,7 +221,7 @@ impl IndexMapCore { debug_assert!(self.indices.capacity() >= self.entries.len()); for (i, entry) in enumerate(&self.entries) { // We should never have to reallocate, so there's no need for a real hasher. - self.indices.insert(entry.hash.get(), i, |_| unreachable!()); + self.indices.insert_no_grow(entry.hash.get(), i); } } } diff --git a/src/map/core/raw.rs b/src/map/core/raw.rs index 8d7ba82a..c06f8c33 100644 --- a/src/map/core/raw.rs +++ b/src/map/core/raw.rs @@ -46,7 +46,7 @@ impl IndexMapCore { pub(super) fn erase_index(&mut self, hash: HashValue, index: usize) { let raw_bucket = self.find_index(hash, index).unwrap(); - unsafe { self.indices.erase_no_drop(&raw_bucket) }; + unsafe { self.indices.erase(raw_bucket) }; } pub(crate) fn entry(&mut self, hash: HashValue, key: K) -> Entry @@ -99,10 +99,7 @@ impl IndexMapCore { unsafe fn shift_remove_bucket(&mut self, raw_bucket: RawBucket) -> (usize, K, V) { // use Vec::remove, but then we need to update the indices that point // to all of the other entries that have to move - let index = unsafe { - self.indices.erase_no_drop(&raw_bucket); - raw_bucket.read() - }; + let index = unsafe { self.indices.remove(raw_bucket) }; let entry = self.entries.remove(index); // correct indices that point to the entries that followed the removed entry. @@ -160,10 +157,7 @@ impl IndexMapCore { unsafe fn swap_remove_bucket(&mut self, raw_bucket: RawBucket) -> (usize, K, V) { // use swap_remove, but then we need to update the index that points // to the other entry that has to move - let index = unsafe { - self.indices.erase_no_drop(&raw_bucket); - raw_bucket.read() - }; + let index = unsafe { self.indices.remove(raw_bucket) }; let entry = self.entries.swap_remove(index); // correct index that points to the entry that had to swap places