From debd1873c79d934e94bddb3b98c52e131563800e Mon Sep 17 00:00:00 2001 From: Mark LeMoine Date: Mon, 8 Jun 2020 02:12:57 -0700 Subject: [PATCH 1/4] Add reverse to OrderMapCore, IndexMap, IndexSet --- src/map.rs | 13 +++++++++++++ src/set.rs | 7 +++++++ 2 files changed, 20 insertions(+) diff --git a/src/map.rs b/src/map.rs index cd9a216b..59a36d51 100644 --- a/src/map.rs +++ b/src/map.rs @@ -1253,6 +1253,13 @@ where self.into_iter() } + /// Reverses the order of the map’s key-value pairs in place. + /// + /// Computes in **O(n)** time and **O(1)** space. + pub fn reverse(&mut self) { + self.core.reverse() + } + /// Clears the `IndexMap`, returning all key-value pairs as a drain iterator. /// Keeps the allocated memory for reuse. pub fn drain(&mut self, range: RangeFull) -> Drain { @@ -1752,6 +1759,12 @@ impl OrderMapCore { self.restore_hash_index(side_index); } + fn reverse(&mut self) { + let side_index = self.save_hash_index(); + self.entries.reverse(); + self.restore_hash_index(side_index); + } + 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 diff --git a/src/set.rs b/src/set.rs index 86228a4f..d856f22a 100644 --- a/src/set.rs +++ b/src/set.rs @@ -527,6 +527,13 @@ where } } + /// Reverses the order of the set’s values in place. + /// + /// Computes in **O(n)** time and **O(1)** space. + pub fn reverse(&mut self) { + self.map.reverse() + } + /// Clears the `IndexSet`, returning all values as a drain iterator. /// Keeps the allocated memory for reuse. pub fn drain(&mut self, range: RangeFull) -> Drain { From 25b0a218cc2d5283a4efa9c8a95dd81da0856b45 Mon Sep 17 00:00:00 2001 From: Mark LeMoine Date: Mon, 8 Jun 2020 12:59:15 -0700 Subject: [PATCH 2/4] Add test for .reverse() --- tests/quick.rs | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/tests/quick.rs b/tests/quick.rs index f77fe93f..c22844c6 100644 --- a/tests/quick.rs +++ b/tests/quick.rs @@ -330,6 +330,59 @@ quickcheck! { map.sort_by(|_, v1, _, v2| Ord::cmp(v1, v2)); assert_sorted_by_key(map, |t| t.1); } + + fn reverse(keyvals: Large>) -> () { + let mut map: IndexMap<_, _> = IndexMap::from_iter(keyvals.to_vec()); + + fn generate_answer(input: &Vec<(i8, i8)>) -> Vec<(i8, i8)> { + // to mimic what `IndexMap::from_iter` does: + // need to get (A) the unique keys in forward order, and (B) the + // last value of each of those keys. + + // create (A): an iterable that yields the unique keys in ltr order + let mut seen_keys = HashSet::new(); + let unique_keys_forward = input.iter().filter_map(move |(k, _)| { + if seen_keys.contains(k) { None } + else { seen_keys.insert(*k); Some(*k) } + }); + + // create (B): a mapping of keys to the last value seen for that key + // this is the same as reversing the input and taking the first + // value seen for that key! + let mut last_val_per_key = HashMap::new(); + for &(k, v) in input.iter().rev() { + if !last_val_per_key.contains_key(&k) { + last_val_per_key.insert(k, v); + } + } + + // iterate over the keys in (A) in order, and match each one with + // the corresponding last value from (B) + let mut ans: Vec<_> = unique_keys_forward + .map(|k| (k, *last_val_per_key.get(&k).unwrap())) + .collect(); + + // finally, since this test is testing `.reverse()`, reverse the + // answer in-place + ans.reverse(); + + ans + } + + let answer = generate_answer(&keyvals.0); + + // perform the work + map.reverse(); + + // check it contains all the values it should + for &(key, val) in &answer { + assert_eq!(map[&key], val); + } + + // check the order + let mapv = Vec::from_iter(map); + assert_eq!(answer, mapv); + } } fn assert_sorted_by_key(iterable: I, key: Key) From 0e4a3a2b9c50b276fabe8aca7a0d152b50098198 Mon Sep 17 00:00:00 2001 From: Mark LeMoine Date: Mon, 8 Jun 2020 15:16:49 -0700 Subject: [PATCH 3/4] Make `reverse` actually use O(1) space --- src/map.rs | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/map.rs b/src/map.rs index 59a36d51..19f8f436 100644 --- a/src/map.rs +++ b/src/map.rs @@ -1760,9 +1760,22 @@ impl OrderMapCore { } fn reverse(&mut self) { - let side_index = self.save_hash_index(); self.entries.reverse(); - self.restore_hash_index(side_index); + + // 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); + } + } + } } fn save_hash_index(&mut self) -> Vec { From a442475ab6a0ddefd886aed1938e6486e4aed1f8 Mon Sep 17 00:00:00 2001 From: Mark LeMoine Date: Mon, 8 Jun 2020 15:28:39 -0700 Subject: [PATCH 4/4] Fix off-by-one error in reindexing after `reverse` --- src/map.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/map.rs b/src/map.rs index 19f8f436..0702ef81 100644 --- a/src/map.rs +++ b/src/map.rs @@ -1772,7 +1772,7 @@ impl OrderMapCore { { for pos in indices { if let Some((i, _)) = pos.resolve::() { - pos.set_pos::(len - i); + pos.set_pos::(len - i - 1); } } }