Skip to content

Add move_index to change the position of an entry #176

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions RELEASES.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@
- The new `IndexMap::shrink_to` and `IndexSet::shrink_to` methods shrink
the capacity with a lower bound.

- The new `IndexMap::move_index` and `IndexSet::move_index` methods change
the position of an item from one index to another, shifting the items
between to accommodate the move.

- The new `map::Slice<K, V>` and `set::Slice<T>` offer a linear view of maps
and sets, behaving a lot like normal `[(K, V)]` and `[T]` slices. Notably,
comparison traits like `Eq` only consider items in order, rather than hash
Expand Down
13 changes: 13 additions & 0 deletions src/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -876,6 +876,19 @@ impl<K, V, S> IndexMap<K, V, S> {
self.core.shift_remove_index(index)
}

/// Moves the position of a key-value pair from one index to another
/// by shifting all other pairs in-between.
///
/// * If `from < to`, the other pairs will shift down while the targeted pair moves up.
/// * If `from > to`, the other pairs will shift up while the targeted pair moves down.
///
/// ***Panics*** if `from` or `to` are out of bounds.
///
/// Computes in **O(n)** time (average).
pub fn move_index(&mut self, from: usize, to: usize) {
self.core.move_index(from, to)
}

/// Swaps the position of two key-value pairs in the map.
///
/// ***Panics*** if `a` or `b` are out of bounds.
Expand Down
72 changes: 60 additions & 12 deletions src/map/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,29 +283,77 @@ impl<K, V> IndexMapCore<K, V> {
///
/// The index should already be removed from `self.indices`.
fn shift_remove_finish(&mut self, index: 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
// Correct indices that point to the entries that followed the removed entry.
self.decrement_indices(index + 1, self.entries.len());

// Use Vec::remove to actually remove the entry.
let entry = self.entries.remove(index);
(entry.key, entry.value)
}

// 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`
/// Decrement all indices in the range `start..end`.
///
/// The index `start - 1` should not exist in `self.indices`.
/// All entries should still be in their original positions.
fn decrement_indices(&mut self, start: usize, end: usize) {
// Use a heuristic between a full sweep vs. a `find()` for every shifted item.
let shifted_entries = &self.entries[start..end];
if shifted_entries.len() > self.indices.buckets() / 2 {
// Shift all indices in range.
for i in self.indices_mut() {
if *i > index {
if start <= *i && *i < end {
*i -= 1;
}
}
} else {
// find each following entry to shift its index
for (i, entry) in (index + 1..).zip(shifted_entries) {
// Find each entry in range to shift its index.
for (i, entry) in (start..end).zip(shifted_entries) {
update_index(&mut self.indices, entry.hash, i, i - 1);
}
}
}

(entry.key, entry.value)
/// Increment all indices in the range `start..end`.
///
/// The index `end` should not exist in `self.indices`.
/// All entries should still be in their original positions.
fn increment_indices(&mut self, start: usize, end: usize) {
// Use a heuristic between a full sweep vs. a `find()` for every shifted item.
let shifted_entries = &self.entries[start..end];
if shifted_entries.len() > self.indices.buckets() / 2 {
// Shift all indices in range.
for i in self.indices_mut() {
if start <= *i && *i < end {
*i += 1;
}
}
} else {
// Find each entry in range to shift its index, updated in reverse so
// we never have duplicated indices that might have a hash collision.
for (i, entry) in (start..end).zip(shifted_entries).rev() {
update_index(&mut self.indices, entry.hash, i, i + 1);
}
}
}

pub(super) fn move_index(&mut self, from: usize, to: usize) {
let from_hash = self.entries[from].hash;
if from != to {
// Use a sentinal index so other indices don't collide.
update_index(&mut self.indices, from_hash, from, usize::MAX);

// Update all other indices and rotate the entry positions.
if from < to {
self.decrement_indices(from + 1, to + 1);
self.entries[from..=to].rotate_left(1);
} else if to < from {
self.increment_indices(to, from);
self.entries[to..=from].rotate_right(1);
}

// Change the sentinal index to its final position.
update_index(&mut self.indices, from_hash, usize::MAX, to);
}
}

/// Remove an entry by swapping it with the last
Expand Down
13 changes: 13 additions & 0 deletions src/set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -731,6 +731,19 @@ impl<T, S> IndexSet<T, S> {
self.map.shift_remove_index(index).map(|(x, ())| x)
}

/// Moves the position of a value from one index to another
/// by shifting all other values in-between.
///
/// * If `from < to`, the other values will shift down while the targeted value moves up.
/// * If `from > to`, the other values will shift up while the targeted value moves down.
///
/// ***Panics*** if `from` or `to` are out of bounds.
///
/// Computes in **O(n)** time (average).
pub fn move_index(&mut self, from: usize, to: usize) {
self.map.move_index(from, to)
}

/// Swaps the position of two values in the set.
///
/// ***Panics*** if `a` or `b` are out of bounds.
Expand Down
47 changes: 47 additions & 0 deletions tests/quick.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,53 @@ quickcheck_limit! {
map[&key] == value && map[i] == value
})
}

// Use `u8` test indices so quickcheck is less likely to go out of bounds.
fn swap_indices(vec: Vec<u8>, a: u8, b: u8) -> TestResult {
let mut set = IndexSet::<u8>::from_iter(vec);
let a = usize::from(a);
let b = usize::from(b);

if a >= set.len() || b >= set.len() {
return TestResult::discard();
}

let mut vec = Vec::from_iter(set.iter().cloned());
vec.swap(a, b);

set.swap_indices(a, b);

// Check both iteration order and hash lookups
assert!(set.iter().eq(vec.iter()));
assert!(vec.iter().enumerate().all(|(i, x)| {
set.get_index_of(x) == Some(i)
}));
TestResult::passed()
}

// Use `u8` test indices so quickcheck is less likely to go out of bounds.
fn move_index(vec: Vec<u8>, from: u8, to: u8) -> TestResult {
let mut set = IndexSet::<u8>::from_iter(vec);
let from = usize::from(from);
let to = usize::from(to);

if from >= set.len() || to >= set.len() {
return TestResult::discard();
}

let mut vec = Vec::from_iter(set.iter().cloned());
let x = vec.remove(from);
vec.insert(to, x);

set.move_index(from, to);

// Check both iteration order and hash lookups
assert!(set.iter().eq(vec.iter()));
assert!(vec.iter().enumerate().all(|(i, x)| {
set.get_index_of(x) == Some(i)
}));
TestResult::passed()
}
}

use crate::Op::*;
Expand Down