Skip to content

Commit e787b3d

Browse files
committed
Add a core method to support set's replace_full
This may improve performance slightly, because it will perform a hash lookup only once, although `RawTable` does force an early reserve to accomplish that. This is the same trade-off that `insert` makes. This also lets us remove the saved key from `map::OccupiedEntry`, which may benefit all other `entry` use cases where that was unused.
1 parent d79a215 commit e787b3d

File tree

5 files changed

+45
-18
lines changed

5 files changed

+45
-18
lines changed

src/map.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -462,7 +462,7 @@ impl<K, V, S> IndexMap<K, V, S>
462462
where
463463
S: BuildHasher,
464464
{
465-
fn hash<Q: ?Sized + Hash>(&self, key: &Q) -> HashValue {
465+
pub(crate) fn hash<Q: ?Sized + Hash>(&self, key: &Q) -> HashValue {
466466
let mut h = self.hash_builder.build_hasher();
467467
key.hash(&mut h);
468468
HashValue(h.finish() as usize)

src/map/core.rs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,33 @@ impl<K, V> IndexMapCore<K, V> {
332332
}
333333
}
334334

335+
/// Same as `insert_full`, except it also replaces the key
336+
pub(crate) fn replace_full(
337+
&mut self,
338+
hash: HashValue,
339+
key: K,
340+
value: V,
341+
) -> (usize, Option<(K, V)>)
342+
where
343+
K: Eq,
344+
{
345+
match self.find_or_insert(hash, &key) {
346+
Ok(i) => {
347+
let entry = &mut self.entries[i];
348+
let kv = (
349+
mem::replace(&mut entry.key, key),
350+
mem::replace(&mut entry.value, value),
351+
);
352+
(i, Some(kv))
353+
}
354+
Err(i) => {
355+
debug_assert_eq!(i, self.entries.len());
356+
self.push_entry(hash, key, value);
357+
(i, None)
358+
}
359+
}
360+
}
361+
335362
/// Remove an entry by shifting all entries that follow it
336363
pub(crate) fn shift_remove_full<Q>(&mut self, hash: HashValue, key: &Q) -> Option<(usize, K, V)>
337364
where

src/map/core/entry.rs

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ impl<K, V> IndexMapCore<K, V> {
99
K: Eq,
1010
{
1111
match self.raw_entry(hash, |k| *k == key) {
12-
Ok(raw) => Entry::Occupied(OccupiedEntry { raw, key }),
12+
Ok(raw) => Entry::Occupied(OccupiedEntry { raw }),
1313
Err(map) => Entry::Vacant(VacantEntry { map, hash, key }),
1414
}
1515
}
@@ -124,7 +124,6 @@ impl<K: fmt::Debug, V: fmt::Debug> fmt::Debug for Entry<'_, K, V> {
124124
/// It is part of the [`Entry`] enum.
125125
pub struct OccupiedEntry<'a, K, V> {
126126
raw: RawTableEntry<'a, K, V>,
127-
key: K,
128127
}
129128

130129
impl<'a, K, V> OccupiedEntry<'a, K, V> {
@@ -162,12 +161,6 @@ impl<'a, K, V> OccupiedEntry<'a, K, V> {
162161
&mut self.raw.into_bucket().value
163162
}
164163

165-
/// Put the new key in the occupied entry's key slot
166-
pub(crate) fn replace_key(mut self) -> K {
167-
let old_key = &mut self.raw.bucket_mut().key;
168-
mem::replace(old_key, self.key)
169-
}
170-
171164
/// Sets the value of the entry to `value`, and returns the entry's old value.
172165
pub fn insert(&mut self, value: V) -> V {
173166
mem::replace(self.get_mut(), value)

src/set.rs

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -368,15 +368,10 @@ where
368368
///
369369
/// Computes in **O(1)** time (average).
370370
pub fn replace_full(&mut self, value: T) -> (usize, Option<T>) {
371-
use super::map::Entry::*;
372-
373-
match self.map.entry(value) {
374-
Vacant(e) => {
375-
let index = e.index();
376-
e.insert(());
377-
(index, None)
378-
}
379-
Occupied(e) => (e.index(), Some(e.replace_key())),
371+
let hash = self.map.hash(&value);
372+
match self.map.core.replace_full(hash, value, ()) {
373+
(i, Some((replaced, ()))) => (i, Some(replaced)),
374+
(i, None) => (i, None),
380375
}
381376
}
382377

src/set/tests.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,18 @@ fn replace_order() {
233233
}
234234
}
235235

236+
#[test]
237+
fn replace_change() {
238+
// Check pointers to make sure it really changes
239+
let mut set = indexset!(vec![42]);
240+
let old_ptr = set[0].as_ptr();
241+
let new = set[0].clone();
242+
let new_ptr = new.as_ptr();
243+
assert_ne!(old_ptr, new_ptr);
244+
let replaced = set.replace(new).unwrap();
245+
assert_eq!(replaced.as_ptr(), old_ptr);
246+
}
247+
236248
#[test]
237249
fn grow() {
238250
let insert = [0, 4, 2, 12, 8, 7, 11];

0 commit comments

Comments
 (0)