From 37785608092c995731bc4e90014668b164a8d560 Mon Sep 17 00:00:00 2001 From: Piotr Czarnecki Date: Sat, 5 Mar 2016 22:20:59 +0100 Subject: [PATCH] Refactor fn robin_hood --- src/libstd/collections/hash/map.rs | 29 ++++++++-------- src/libstd/collections/hash/table.rs | 52 +++++++++++++++++++++------- 2 files changed, 53 insertions(+), 28 deletions(-) diff --git a/src/libstd/collections/hash/map.rs b/src/libstd/collections/hash/map.rs index 91626b8d16b6..c83c432fc950 100644 --- a/src/libstd/collections/hash/map.rs +++ b/src/libstd/collections/hash/map.rs @@ -421,24 +421,30 @@ fn pop_internal(starting_bucket: FullBucketMut) -> (K, V) { /// to recalculate it. /// /// `hash`, `k`, and `v` are the elements to "robin hood" into the hashtable. -fn robin_hood<'a, K: 'a, V: 'a>(mut bucket: FullBucketMut<'a, K, V>, +fn robin_hood<'a, K: 'a, V: 'a>(bucket: FullBucketMut<'a, K, V>, mut ib: usize, mut hash: SafeHash, - mut k: K, - mut v: V) + mut key: K, + mut val: V) -> &'a mut V { let starting_index = bucket.index(); let size = { let table = bucket.table(); // FIXME "lifetime too short". table.size() }; + // Save the *starting point*. + let mut bucket = bucket.stash(); // There can be at most `size - dib` buckets to displace, because // in the worst case, there are `size` elements and we already are - // `distance` buckets away from the initial one. + // `displacement` buckets away from the initial one. let idx_end = starting_index + size - bucket.displacement(); loop { - let (old_hash, old_key, old_val) = bucket.replace(hash, k, v); + let (old_hash, old_key, old_val) = bucket.replace(hash, key, val); + hash = old_hash; + key = old_key; + val = old_val; + loop { let probe = bucket.next(); assert!(probe.index() != idx_end); @@ -446,14 +452,10 @@ fn robin_hood<'a, K: 'a, V: 'a>(mut bucket: FullBucketMut<'a, K, V>, let full_bucket = match probe.peek() { Empty(bucket) => { // Found a hole! - let b = bucket.put(old_hash, old_key, old_val); + let bucket = bucket.put(hash, key, val); // Now that it's stolen, just read the value's pointer - // right out of the table! - return Bucket::at_index(b.into_table(), starting_index) - .peek() - .expect_full() - .into_mut_refs() - .1; + // right out of the table! Go back to the *starting point*. + return bucket.into_table().into_mut_refs().1; }, Full(bucket) => bucket }; @@ -465,9 +467,6 @@ fn robin_hood<'a, K: 'a, V: 'a>(mut bucket: FullBucketMut<'a, K, V>, // Robin hood! Steal the spot. if ib < probe_ib { ib = probe_ib; - hash = old_hash; - k = old_key; - v = old_val; break; } } diff --git a/src/libstd/collections/hash/table.rs b/src/libstd/collections/hash/table.rs index 7db89dcd1283..e3acbc96fdad 100644 --- a/src/libstd/collections/hash/table.rs +++ b/src/libstd/collections/hash/table.rs @@ -220,6 +220,28 @@ impl Bucket { } } +impl Deref for FullBucket where M: Deref> { + type Target = RawTable; + fn deref(&self) -> &RawTable { + &self.table + } +} + +impl DerefMut for FullBucket where M: DerefMut> { + fn deref_mut(&mut self) -> &mut RawTable { + &mut self.table + } +} + + +/// `Put` is implemented for types which provide access to a table and cannot be invalidated +/// by filling a bucket. A similar implementation for `Take` is possible. +pub trait Put {} +impl Put for RawTable {} +impl<'t, K, V> Put for &'t mut RawTable {} +impl Put for Bucket {} +impl Put for FullBucket {} + impl>> Bucket { pub fn new(table: M, hash: SafeHash) -> Bucket { Bucket::at_index(table, hash.inspect() as usize) @@ -320,7 +342,7 @@ impl>> EmptyBucket { } } -impl> + DerefMut> EmptyBucket { +impl EmptyBucket where M: Deref> + DerefMut + Put { /// Puts given key and value pair, along with the key's hash, /// into this bucket in the hashtable. Note how `self` is 'moved' into /// this function, because this slot will no longer be empty when @@ -359,6 +381,16 @@ impl>> FullBucket { } } + /// Duplicates the current position. This can be useful for operations + /// on two or more buckets. + pub fn stash(self) -> FullBucket { + FullBucket { + raw: self.raw, + idx: self.idx, + table: self, + } + } + /// Get the distance between this bucket and the 'ideal' location /// as determined by the key's hash stored in it. /// @@ -389,12 +421,14 @@ impl>> FullBucket { } } -impl> + DerefMut> FullBucket { +// We don't need a `Take` trait currently. This is why a mutable reference +// to the table is required. +impl<'t, K, V> FullBucket> { /// Removes this bucket's key and value from the hashtable. /// /// This works similarly to `put`, building an `EmptyBucket` out of the /// taken bucket. - pub fn take(mut self) -> (EmptyBucket, K, V) { + pub fn take(mut self) -> (EmptyBucket>, K, V) { self.table.size -= 1; unsafe { @@ -410,7 +444,9 @@ impl> + DerefMut> FullBucket { ) } } +} +impl FullBucket where M: Deref> + DerefMut { pub fn replace(&mut self, h: SafeHash, k: K, v: V) -> (SafeHash, K, V) { unsafe { let old_hash = ptr::replace(self.raw.hash as *mut SafeHash, h); @@ -455,16 +491,6 @@ impl<'t, K, V, M: Deref> + DerefMut + 't> FullBucket BucketState { - // For convenience. - pub fn expect_full(self) -> FullBucket { - match self { - Full(full) => full, - Empty(..) => panic!("Expected full bucket") - } - } -} - impl>> GapThenFull { #[inline] pub fn full(&self) -> &FullBucket {