From 019872b8cc8c1fe23aa7a0f8a43c55e91b51e08b Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Thu, 17 Mar 2022 22:46:38 +0000 Subject: [PATCH 01/47] Really basic hash map which you can only add stuff to --- agb/src/hash_map.rs | 166 ++++++++++++++++++++++++++++++++++++++++++++ agb/src/lib.rs | 2 + 2 files changed, 168 insertions(+) create mode 100644 agb/src/hash_map.rs diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs new file mode 100644 index 0000000..9962f57 --- /dev/null +++ b/agb/src/hash_map.rs @@ -0,0 +1,166 @@ +use alloc::{vec, vec::Vec}; +use core::{ + hash::{BuildHasher, BuildHasherDefault, Hash, Hasher}, + mem, +}; + +use rustc_hash::FxHasher; + +type HashType = u32; + +struct Node { + hash: HashType, + distance_to_initial_bucket: u32, + key: K, + value: V, +} + +impl Node +where + K: Sized, + V: Sized, +{ + fn with_new_key_value(&self, new_key: K, new_value: V) -> Self { + Self { + hash: self.hash, + distance_to_initial_bucket: self.distance_to_initial_bucket, + key: new_key, + value: new_value, + } + } +} + +struct HashMap { + number_of_elements: usize, + max_distance_to_initial_bucket: u32, + nodes: Vec>>, + + hasher: BuildHasherDefault, +} + +impl HashMap { + pub fn new() -> Self { + Self { + number_of_elements: 0, + max_distance_to_initial_bucket: 0, + nodes: vec![ + None, None, None, None, None, None, None, None, None, None, None, None, None, None, + None, None, + ], + hasher: Default::default(), + } + } +} + +fn fast_mod(len: usize, hash: HashType) -> usize { + debug_assert!(len.is_power_of_two(), "Length must be a power of 2"); + (hash as usize) & (len - 1) +} + +impl HashMap +where + K: Eq, +{ + fn get_location(&self, key: &K, hash: HashType) -> Option { + for distance_to_initial_bucket in 0..=self.max_distance_to_initial_bucket + 1 { + let location = fast_mod(self.nodes.len(), hash + distance_to_initial_bucket); + + let node = &self.nodes[location]; + if let Some(node) = node { + if &node.key == key { + return Some(location); + } + } else { + return None; + } + } + + None + } +} + +impl HashMap +where + K: Eq + Hash, +{ + pub fn put(&mut self, key: K, value: V) { + let mut hasher = self.hasher.build_hasher(); + key.hash(&mut hasher); + let hash = hasher.finish() as HashType; + + if let Some(location) = self.get_location(&key, hash) { + let old_node = self.nodes[location].as_ref().unwrap(); + self.nodes[location] = Some(old_node.with_new_key_value(key, value)); + + return; + } + + self.insert_new(key, value, hash); + } + + pub fn get(&self, key: &K) -> Option<&V> { + let mut hasher = self.hasher.build_hasher(); + key.hash(&mut hasher); + let hash = hasher.finish() as HashType; + + self.get_location(&key, hash) + .map(|location| &self.nodes[location].as_ref().unwrap().value) + } +} + +impl HashMap { + fn insert_new(&mut self, key: K, value: V, hash: HashType) { + // if we need to resize + if self.nodes.len() * 85 / 100 < self.number_of_elements { + todo!("resize not implemented yet"); + } + + let mut new_node = Node { + hash, + distance_to_initial_bucket: 0, + key, + value, + }; + + loop { + let location = fast_mod(self.nodes.len(), hash + new_node.distance_to_initial_bucket); + let current_node = self.nodes[location].as_mut(); + + if let Some(current_node) = current_node { + if current_node.distance_to_initial_bucket <= new_node.distance_to_initial_bucket { + self.max_distance_to_initial_bucket = new_node + .distance_to_initial_bucket + .max(self.max_distance_to_initial_bucket); + + mem::swap(&mut new_node, current_node); + } + } else { + self.nodes[location] = Some(new_node); + break; + } + + new_node.distance_to_initial_bucket += 1; + } + + self.number_of_elements += 1; + } +} + +#[cfg(test)] +mod test { + use super::*; + use crate::Gba; + + #[test_case] + fn can_store_up_to_initial_capacity_elements(_gba: &mut Gba) { + let mut map = HashMap::new(); + + for i in 0..8 { + map.put(i, i % 4); + } + + for i in 0..8 { + assert_eq!(map.get(&i), Some(&(i % 4))); + } + } +} diff --git a/agb/src/lib.rs b/agb/src/lib.rs index 9eb98ee..770161d 100644 --- a/agb/src/lib.rs +++ b/agb/src/lib.rs @@ -160,6 +160,8 @@ pub mod syscall; /// Interactions with the internal timers pub mod timer; +mod hash_map; + #[cfg(not(test))] #[panic_handler] #[allow(unused_must_use)] From bfebba1ec1d6323f8802a03594c69249f81b5dfd Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Thu, 17 Mar 2022 23:18:49 +0000 Subject: [PATCH 02/47] Implement remove --- agb/src/hash_map.rs | 125 +++++++++++++++++++++++++++++++++++++------- 1 file changed, 105 insertions(+), 20 deletions(-) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index 9962f57..a44c08e 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -20,13 +20,16 @@ where K: Sized, V: Sized, { - fn with_new_key_value(&self, new_key: K, new_value: V) -> Self { - Self { - hash: self.hash, - distance_to_initial_bucket: self.distance_to_initial_bucket, - key: new_key, - value: new_value, - } + fn with_new_key_value(self, new_key: K, new_value: V) -> (Self, V) { + ( + Self { + hash: self.hash, + distance_to_initial_bucket: self.distance_to_initial_bucket, + key: new_key, + value: new_value, + }, + self.value, + ) } } @@ -50,6 +53,10 @@ impl HashMap { hasher: Default::default(), } } + + pub fn len(&self) -> usize { + self.number_of_elements + } } fn fast_mod(len: usize, hash: HashType) -> usize { @@ -62,7 +69,7 @@ where K: Eq, { fn get_location(&self, key: &K, hash: HashType) -> Option { - for distance_to_initial_bucket in 0..=self.max_distance_to_initial_bucket + 1 { + for distance_to_initial_bucket in 0..=self.max_distance_to_initial_bucket { let location = fast_mod(self.nodes.len(), hash + distance_to_initial_bucket); let node = &self.nodes[location]; @@ -83,28 +90,44 @@ impl HashMap where K: Eq + Hash, { - pub fn put(&mut self, key: K, value: V) { - let mut hasher = self.hasher.build_hasher(); - key.hash(&mut hasher); - let hash = hasher.finish() as HashType; + pub fn put(&mut self, key: K, value: V) -> Option { + let hash = self.hash(&key); if let Some(location) = self.get_location(&key, hash) { - let old_node = self.nodes[location].as_ref().unwrap(); - self.nodes[location] = Some(old_node.with_new_key_value(key, value)); + let old_node = self.nodes[location].take().unwrap(); + let (new_node, old_value) = old_node.with_new_key_value(key, value); + self.nodes[location] = Some(new_node); - return; + return Some(old_value); } self.insert_new(key, value, hash); + None } pub fn get(&self, key: &K) -> Option<&V> { + let hash = self.hash(key); + + self.get_location(key, hash) + .map(|location| &self.nodes[location].as_ref().unwrap().value) + } + + pub fn remove(&mut self, key: &K) -> Option { + let hash = self.hash(key); + + self.get_location(key, hash) + .map(|location| self.remove_from_location(location)) + } +} + +impl HashMap +where + K: Hash, +{ + fn hash(&self, key: &K) -> HashType { let mut hasher = self.hasher.build_hasher(); key.hash(&mut hasher); - let hash = hasher.finish() as HashType; - - self.get_location(&key, hash) - .map(|location| &self.nodes[location].as_ref().unwrap().value) + hasher.finish() as HashType } } @@ -144,6 +167,29 @@ impl HashMap { self.number_of_elements += 1; } + + fn remove_from_location(&mut self, location: usize) -> V { + let mut current_location = location; + + let result = loop { + let next_location = fast_mod(self.nodes.len(), (current_location + 1) as HashType); + + // if the next node is empty, then we can clear the current node + if self.nodes[next_location].is_none() { + break self.nodes[current_location].take().unwrap(); + } + + self.nodes.swap(current_location, next_location); + self.nodes[current_location] + .as_mut() + .unwrap() + .distance_to_initial_bucket -= 1; + current_location = next_location; + }; + + self.number_of_elements -= 1; + result.value + } } #[cfg(test)] @@ -152,7 +198,7 @@ mod test { use crate::Gba; #[test_case] - fn can_store_up_to_initial_capacity_elements(_gba: &mut Gba) { + fn can_store_and_retrieve_8_elements(_gba: &mut Gba) { let mut map = HashMap::new(); for i in 0..8 { @@ -163,4 +209,43 @@ mod test { assert_eq!(map.get(&i), Some(&(i % 4))); } } + + #[test_case] + fn can_get_the_length(_gba: &mut Gba) { + let mut map = HashMap::new(); + + for i in 0..8 { + map.put(i / 2, true); + } + + assert_eq!(map.len(), 4); + } + + #[test_case] + fn returns_none_if_element_does_not_exist(_gba: &mut Gba) { + let mut map = HashMap::new(); + + for i in 0..8 { + map.put(i, i % 3); + } + + assert_eq!(map.get(&12), None); + } + + #[test_case] + fn can_delete_entries(_gba: &mut Gba) { + let mut map = HashMap::new(); + + for i in 0..8 { + map.put(i, i % 3); + } + + for i in 0..4 { + map.remove(&i); + } + + assert_eq!(map.len(), 4); + assert_eq!(map.get(&3), None); + assert_eq!(map.get(&7), Some(&1)); + } } From a459a4811cf37b2a0b1bbe63efe6ba37b430dd18 Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Thu, 17 Mar 2022 23:44:57 +0000 Subject: [PATCH 03/47] Add test for iterating through all entries --- agb/src/hash_map.rs | 53 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index a44c08e..4265042 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -192,6 +192,39 @@ impl HashMap { } } +pub struct Iter<'a, K: 'a, V: 'a> { + map: &'a HashMap, + at: usize, +} + +impl<'a, K, V> Iterator for Iter<'a, K, V> { + type Item = (&'a K, &'a V); + + fn next(&mut self) -> Option { + loop { + if self.at >= self.map.nodes.len() { + return None; + } + + if let Some(node) = &self.map.nodes[self.at] { + self.at += 1; + return Some((&node.key, &node.value)); + } + + self.at += 1; + } + } +} + +impl<'a, K, V> IntoIterator for &'a HashMap { + type Item = (&'a K, &'a V); + type IntoIter = Iter<'a, K, V>; + + fn into_iter(self) -> Self::IntoIter { + Iter { map: self, at: 0 } + } +} + #[cfg(test)] mod test { use super::*; @@ -248,4 +281,24 @@ mod test { assert_eq!(map.get(&3), None); assert_eq!(map.get(&7), Some(&1)); } + + #[test_case] + fn can_iterate_through_all_entries(_gba: &mut Gba) { + let mut map = HashMap::new(); + + for i in 0..8 { + map.put(i, i); + } + + let mut max_found = -1; + let mut num_found = 0; + + for (_, value) in map.into_iter() { + max_found = max_found.max(*value); + num_found += 1; + } + + assert_eq!(num_found, 8); + assert_eq!(max_found, 7); + } } From 05b8accaec54f2c31a07c1720a7e6dd5d2852468 Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Thu, 17 Mar 2022 23:45:41 +0000 Subject: [PATCH 04/47] Make the hash_map mod public --- agb/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/agb/src/lib.rs b/agb/src/lib.rs index 770161d..efc06cb 100644 --- a/agb/src/lib.rs +++ b/agb/src/lib.rs @@ -152,6 +152,8 @@ mod memory_mapped; pub mod mgba; /// Implementation of fixnums for working with non-integer values. pub use agb_fixnum as fixnum; +/// Contains an implementation of a hashmap which suits the gameboy advance's hardware +pub mod hash_map; mod single; /// Implements sound output. pub mod sound; @@ -160,8 +162,6 @@ pub mod syscall; /// Interactions with the internal timers pub mod timer; -mod hash_map; - #[cfg(not(test))] #[panic_handler] #[allow(unused_must_use)] From dd5aad0de538e134c40481c5cc5f1c813ed4e5a3 Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Thu, 17 Mar 2022 23:46:21 +0000 Subject: [PATCH 05/47] Make HashMap struct public --- agb/src/hash_map.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index 4265042..f055a0e 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -33,7 +33,7 @@ where } } -struct HashMap { +pub struct HashMap { number_of_elements: usize, max_distance_to_initial_bucket: u32, nodes: Vec>>, From bfdca7117d81ce06f84cf0c34b3f178ce20db0d7 Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Thu, 17 Mar 2022 23:46:45 +0000 Subject: [PATCH 06/47] Make fast_mod constant --- agb/src/hash_map.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index f055a0e..3ea0823 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -59,7 +59,7 @@ impl HashMap { } } -fn fast_mod(len: usize, hash: HashType) -> usize { +const fn fast_mod(len: usize, hash: HashType) -> usize { debug_assert!(len.is_power_of_two(), "Length must be a power of 2"); (hash as usize) & (len - 1) } From c0d9f0ab415091e1c58ae24d5ad40a1f632e238e Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Thu, 17 Mar 2022 23:51:01 +0000 Subject: [PATCH 07/47] Correctly initialise to 16 elements --- agb/src/hash_map.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index 3ea0823..c691765 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -1,7 +1,7 @@ -use alloc::{vec, vec::Vec}; +use alloc::vec::Vec; use core::{ hash::{BuildHasher, BuildHasherDefault, Hash, Hasher}, - mem, + iter, mem, }; use rustc_hash::FxHasher; @@ -46,10 +46,7 @@ impl HashMap { Self { number_of_elements: 0, max_distance_to_initial_bucket: 0, - nodes: vec![ - None, None, None, None, None, None, None, None, None, None, None, None, None, None, - None, None, - ], + nodes: iter::repeat_with(|| None).take(16).collect(), hasher: Default::default(), } } From 03f5cd0953deecf4584fe34e815a886e38b8ee1c Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Thu, 17 Mar 2022 23:54:42 +0000 Subject: [PATCH 08/47] Add with_capacity method --- agb/src/hash_map.rs | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index c691765..57afd96 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -43,10 +43,16 @@ pub struct HashMap { impl HashMap { pub fn new() -> Self { + Self::with_capacity(16) + } + + pub fn with_capacity(capacity: usize) -> Self { + let next_power_of_2 = find_next_power_of_2(capacity); + Self { number_of_elements: 0, max_distance_to_initial_bucket: 0, - nodes: iter::repeat_with(|| None).take(16).collect(), + nodes: iter::repeat_with(|| None).take(next_power_of_2).collect(), hasher: Default::default(), } } @@ -61,6 +67,15 @@ const fn fast_mod(len: usize, hash: HashType) -> usize { (hash as usize) & (len - 1) } +const fn find_next_power_of_2(n: usize) -> usize { + let mut next_power_of_2 = 1; + while next_power_of_2 <= n { + next_power_of_2 *= 2; + } + + next_power_of_2 +} + impl HashMap where K: Eq, From e999b44c6774e922c6066b6455c3c287446ecf01 Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Thu, 17 Mar 2022 23:58:30 +0000 Subject: [PATCH 09/47] Put nodes in a NodeStorage --- agb/src/hash_map.rs | 42 +++++++++++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 15 deletions(-) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index 57afd96..b6cce31 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -33,10 +33,24 @@ where } } +struct NodeStorage(Vec>>); + +impl NodeStorage { + fn with_size(capacity: usize) -> Self { + let next_power_of_2 = find_next_power_of_2(capacity); + + Self(iter::repeat_with(|| None).take(next_power_of_2).collect()) + } + + fn len(&self) -> usize { + self.0.len() + } +} + pub struct HashMap { number_of_elements: usize, max_distance_to_initial_bucket: u32, - nodes: Vec>>, + nodes: NodeStorage, hasher: BuildHasherDefault, } @@ -47,12 +61,10 @@ impl HashMap { } pub fn with_capacity(capacity: usize) -> Self { - let next_power_of_2 = find_next_power_of_2(capacity); - Self { number_of_elements: 0, max_distance_to_initial_bucket: 0, - nodes: iter::repeat_with(|| None).take(next_power_of_2).collect(), + nodes: NodeStorage::with_size(capacity), hasher: Default::default(), } } @@ -84,7 +96,7 @@ where for distance_to_initial_bucket in 0..=self.max_distance_to_initial_bucket { let location = fast_mod(self.nodes.len(), hash + distance_to_initial_bucket); - let node = &self.nodes[location]; + let node = &self.nodes.0[location]; if let Some(node) = node { if &node.key == key { return Some(location); @@ -106,9 +118,9 @@ where let hash = self.hash(&key); if let Some(location) = self.get_location(&key, hash) { - let old_node = self.nodes[location].take().unwrap(); + let old_node = self.nodes.0[location].take().unwrap(); let (new_node, old_value) = old_node.with_new_key_value(key, value); - self.nodes[location] = Some(new_node); + self.nodes.0[location] = Some(new_node); return Some(old_value); } @@ -121,7 +133,7 @@ where let hash = self.hash(key); self.get_location(key, hash) - .map(|location| &self.nodes[location].as_ref().unwrap().value) + .map(|location| &self.nodes.0[location].as_ref().unwrap().value) } pub fn remove(&mut self, key: &K) -> Option { @@ -159,7 +171,7 @@ impl HashMap { loop { let location = fast_mod(self.nodes.len(), hash + new_node.distance_to_initial_bucket); - let current_node = self.nodes[location].as_mut(); + let current_node = self.nodes.0[location].as_mut(); if let Some(current_node) = current_node { if current_node.distance_to_initial_bucket <= new_node.distance_to_initial_bucket { @@ -170,7 +182,7 @@ impl HashMap { mem::swap(&mut new_node, current_node); } } else { - self.nodes[location] = Some(new_node); + self.nodes.0[location] = Some(new_node); break; } @@ -187,12 +199,12 @@ impl HashMap { let next_location = fast_mod(self.nodes.len(), (current_location + 1) as HashType); // if the next node is empty, then we can clear the current node - if self.nodes[next_location].is_none() { - break self.nodes[current_location].take().unwrap(); + if self.nodes.0[next_location].is_none() { + break self.nodes.0[current_location].take().unwrap(); } - self.nodes.swap(current_location, next_location); - self.nodes[current_location] + self.nodes.0.swap(current_location, next_location); + self.nodes.0[current_location] .as_mut() .unwrap() .distance_to_initial_bucket -= 1; @@ -218,7 +230,7 @@ impl<'a, K, V> Iterator for Iter<'a, K, V> { return None; } - if let Some(node) = &self.map.nodes[self.at] { + if let Some(node) = &self.map.nodes.0[self.at] { self.at += 1; return Some((&node.key, &node.value)); } From 8d976b49bc18d438b05b1f648fb8712ea6b661ca Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Fri, 18 Mar 2022 00:05:03 +0000 Subject: [PATCH 10/47] Extract a node_storage --- agb/src/hash_map.rs | 144 +++++++++++++++++++++++++------------------- 1 file changed, 82 insertions(+), 62 deletions(-) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index b6cce31..47ecb48 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -45,6 +45,74 @@ impl NodeStorage { fn len(&self) -> usize { self.0.len() } + + fn insert_new( + &mut self, + key: K, + value: V, + hash: HashType, + number_of_elements: usize, + max_distance_to_initial_bucket: u32, + ) -> u32 { + debug_assert!( + self.len() * 85 / 100 > number_of_elements, + "Do not have space to insert into len {} with {number_of_elements}", + self.len() + ); + + let mut new_node = Node { + hash, + distance_to_initial_bucket: 0, + key, + value, + }; + + let mut max_distance_to_initial_bucket = max_distance_to_initial_bucket; + + loop { + let location = fast_mod(self.len(), hash + new_node.distance_to_initial_bucket); + let current_node = self.0[location].as_mut(); + + if let Some(current_node) = current_node { + if current_node.distance_to_initial_bucket <= new_node.distance_to_initial_bucket { + max_distance_to_initial_bucket = new_node + .distance_to_initial_bucket + .max(max_distance_to_initial_bucket); + + mem::swap(&mut new_node, current_node); + } + } else { + self.0[location] = Some(new_node); + break; + } + + new_node.distance_to_initial_bucket += 1; + } + + max_distance_to_initial_bucket + } + + fn remove_from_location(&mut self, location: usize) -> V { + let mut current_location = location; + + let result = loop { + let next_location = fast_mod(self.len(), (current_location + 1) as HashType); + + // if the next node is empty, then we can clear the current node + if self.0[next_location].is_none() { + break self.0[current_location].take().unwrap(); + } + + self.0.swap(current_location, next_location); + self.0[current_location] + .as_mut() + .unwrap() + .distance_to_initial_bucket -= 1; + current_location = next_location; + }; + + result.value + } } pub struct HashMap { @@ -125,7 +193,14 @@ where return Some(old_value); } - self.insert_new(key, value, hash); + self.max_distance_to_initial_bucket = self.nodes.insert_new( + key, + value, + hash, + self.number_of_elements, + self.max_distance_to_initial_bucket, + ); + self.number_of_elements += 1; None } @@ -140,7 +215,11 @@ where let hash = self.hash(key); self.get_location(key, hash) - .map(|location| self.remove_from_location(location)) + .map(|location| self.nodes.remove_from_location(location)) + .map(|value| { + self.number_of_elements -= 1; + value + }) } } @@ -155,66 +234,7 @@ where } } -impl HashMap { - fn insert_new(&mut self, key: K, value: V, hash: HashType) { - // if we need to resize - if self.nodes.len() * 85 / 100 < self.number_of_elements { - todo!("resize not implemented yet"); - } - - let mut new_node = Node { - hash, - distance_to_initial_bucket: 0, - key, - value, - }; - - loop { - let location = fast_mod(self.nodes.len(), hash + new_node.distance_to_initial_bucket); - let current_node = self.nodes.0[location].as_mut(); - - if let Some(current_node) = current_node { - if current_node.distance_to_initial_bucket <= new_node.distance_to_initial_bucket { - self.max_distance_to_initial_bucket = new_node - .distance_to_initial_bucket - .max(self.max_distance_to_initial_bucket); - - mem::swap(&mut new_node, current_node); - } - } else { - self.nodes.0[location] = Some(new_node); - break; - } - - new_node.distance_to_initial_bucket += 1; - } - - self.number_of_elements += 1; - } - - fn remove_from_location(&mut self, location: usize) -> V { - let mut current_location = location; - - let result = loop { - let next_location = fast_mod(self.nodes.len(), (current_location + 1) as HashType); - - // if the next node is empty, then we can clear the current node - if self.nodes.0[next_location].is_none() { - break self.nodes.0[current_location].take().unwrap(); - } - - self.nodes.0.swap(current_location, next_location); - self.nodes.0[current_location] - .as_mut() - .unwrap() - .distance_to_initial_bucket -= 1; - current_location = next_location; - }; - - self.number_of_elements -= 1; - result.value - } -} +impl HashMap {} pub struct Iter<'a, K: 'a, V: 'a> { map: &'a HashMap, From a9115c23e859839b02f04ba393865cb2d110d297 Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Fri, 18 Mar 2022 00:18:39 +0000 Subject: [PATCH 11/47] Allow for resizing --- agb/src/hash_map.rs | 59 ++++++++++++++++++++++++++++++++++++--------- 1 file changed, 48 insertions(+), 11 deletions(-) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index 47ecb48..87f36d0 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -37,9 +37,9 @@ struct NodeStorage(Vec>>); impl NodeStorage { fn with_size(capacity: usize) -> Self { - let next_power_of_2 = find_next_power_of_2(capacity); + assert!(capacity.is_power_of_two(), "Capacity must be a power of 2"); - Self(iter::repeat_with(|| None).take(next_power_of_2).collect()) + Self(iter::repeat_with(|| None).take(capacity).collect()) } fn len(&self) -> usize { @@ -140,6 +140,35 @@ impl HashMap { pub fn len(&self) -> usize { self.number_of_elements } + + pub fn resize(&mut self, new_size: usize) { + assert!( + new_size >= self.nodes.len(), + "Can only increase the size of a hash map" + ); + if new_size == self.nodes.len() { + return; + } + + let mut new_node_storage = NodeStorage::with_size(new_size); + let mut new_max_distance_to_initial_bucket = 0; + let number_of_elements = self.number_of_elements; + + for node in self.nodes.0.drain(..) { + if let Some(node) = node { + new_max_distance_to_initial_bucket = new_node_storage.insert_new( + node.key, + node.value, + node.hash, + number_of_elements, + new_max_distance_to_initial_bucket, + ); + } + } + + self.nodes = new_node_storage; + self.max_distance_to_initial_bucket = new_max_distance_to_initial_bucket; + } } const fn fast_mod(len: usize, hash: HashType) -> usize { @@ -147,15 +176,6 @@ const fn fast_mod(len: usize, hash: HashType) -> usize { (hash as usize) & (len - 1) } -const fn find_next_power_of_2(n: usize) -> usize { - let mut next_power_of_2 = 1; - while next_power_of_2 <= n { - next_power_of_2 *= 2; - } - - next_power_of_2 -} - impl HashMap where K: Eq, @@ -193,6 +213,10 @@ where return Some(old_value); } + if self.nodes.len() * 85 / 100 <= self.number_of_elements { + self.resize(self.nodes.len() * 2); + } + self.max_distance_to_initial_bucket = self.nodes.insert_new( key, value, @@ -345,4 +369,17 @@ mod test { assert_eq!(num_found, 8); assert_eq!(max_found, 7); } + + #[test_case] + fn can_insert_more_than_initial_capacity(_gba: &mut Gba) { + let mut map = HashMap::new(); + + for i in 0..65 { + map.put(i, i % 4); + } + + for i in 0..65 { + assert_eq!(map.get(&i), Some(&(i % 4))); + } + } } From 6ff4cbe4f182b4b20cdb01198098d9d3c493ca5d Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Fri, 18 Mar 2022 00:20:24 +0000 Subject: [PATCH 12/47] Rename put to insert --- agb/src/hash_map.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index 87f36d0..104d4f7 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -202,7 +202,7 @@ impl HashMap where K: Eq + Hash, { - pub fn put(&mut self, key: K, value: V) -> Option { + pub fn insert(&mut self, key: K, value: V) -> Option { let hash = self.hash(&key); if let Some(location) = self.get_location(&key, hash) { @@ -303,7 +303,7 @@ mod test { let mut map = HashMap::new(); for i in 0..8 { - map.put(i, i % 4); + map.insert(i, i % 4); } for i in 0..8 { @@ -316,7 +316,7 @@ mod test { let mut map = HashMap::new(); for i in 0..8 { - map.put(i / 2, true); + map.insert(i / 2, true); } assert_eq!(map.len(), 4); @@ -327,7 +327,7 @@ mod test { let mut map = HashMap::new(); for i in 0..8 { - map.put(i, i % 3); + map.insert(i, i % 3); } assert_eq!(map.get(&12), None); @@ -338,7 +338,7 @@ mod test { let mut map = HashMap::new(); for i in 0..8 { - map.put(i, i % 3); + map.insert(i, i % 3); } for i in 0..4 { @@ -355,7 +355,7 @@ mod test { let mut map = HashMap::new(); for i in 0..8 { - map.put(i, i); + map.insert(i, i); } let mut max_found = -1; @@ -375,7 +375,7 @@ mod test { let mut map = HashMap::new(); for i in 0..65 { - map.put(i, i % 4); + map.insert(i, i % 4); } for i in 0..65 { From 5edd46e0850ef86d3b2cc283248cf777e26dacb3 Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Fri, 18 Mar 2022 00:21:55 +0000 Subject: [PATCH 13/47] Add default implementation --- agb/src/hash_map.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index 104d4f7..06db618 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -171,6 +171,12 @@ impl HashMap { } } +impl Default for HashMap { + fn default() -> Self { + Self::new() + } +} + const fn fast_mod(len: usize, hash: HashType) -> usize { debug_assert!(len.is_power_of_two(), "Length must be a power of 2"); (hash as usize) & (len - 1) @@ -235,6 +241,13 @@ where .map(|location| &self.nodes.0[location].as_ref().unwrap().value) } + pub fn get_mut(&mut self, key: &K) -> Option<&mut V> { + let hash = self.hash(key); + + self.get_location(key, hash) + .map(|location| &mut self.nodes.0[location].as_mut().unwrap().value) + } + pub fn remove(&mut self, key: &K) -> Option { let hash = self.hash(key); From f7eb1866c2093c8a0e820e8af0230e1781df9202 Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Fri, 18 Mar 2022 00:47:10 +0000 Subject: [PATCH 14/47] Remove hashbrown from object.rs --- agb/Cargo.toml | 1 - agb/src/display/object.rs | 51 ++++++++++++-------------- agb/src/hash_map.rs | 75 ++++++++++++++++++++++++++++++++++++--- 3 files changed, 93 insertions(+), 34 deletions(-) diff --git a/agb/Cargo.toml b/agb/Cargo.toml index feae8f6..63251e0 100644 --- a/agb/Cargo.toml +++ b/agb/Cargo.toml @@ -25,7 +25,6 @@ agb_sound_converter = { version = "0.1.0", path = "../agb-sound-converter" } agb_macros = { version = "0.1.0", path = "../agb-macros" } agb_fixnum = { version = "0.1.0", path = "../agb-fixnum" } bare-metal = "1" -hashbrown = "0.12" modular-bitfield = "0.11" rustc-hash = { version = "1", default-features = false } diff --git a/agb/src/display/object.rs b/agb/src/display/object.rs index 878c8c2..d2bd56b 100644 --- a/agb/src/display/object.rs +++ b/agb/src/display/object.rs @@ -6,9 +6,6 @@ use core::ptr::NonNull; use core::slice; use modular_bitfield::prelude::{B10, B2, B3, B4, B5, B8, B9}; use modular_bitfield::{bitfield, BitfieldSpecifier}; -use rustc_hash::FxHasher; - -use hashbrown::HashMap; const BYTES_PER_TILE_4BPP: usize = 32; @@ -18,6 +15,7 @@ use crate::agb_alloc::block_allocator::BlockAllocator; use crate::agb_alloc::bump_allocator::StartEnd; use crate::dma; use crate::fixnum::Vector2D; +use crate::hash_map::HashMap; use attributes::*; @@ -316,8 +314,8 @@ pub struct Object<'a, 'b> { } struct SpriteControllerInner { - palette: HashMap>, - sprite: HashMap>, + palette: HashMap, + sprite: HashMap, } pub struct SpriteController { @@ -629,36 +627,31 @@ impl SpriteControllerInner { } fn return_sprite(&mut self, sprite: &'static Sprite) { - self.sprite - .entry(sprite.get_id()) - .and_replace_entry_with(|_, mut storage| { - storage.count -= 1; - if storage.count == 0 { - unsafe { SPRITE_ALLOCATOR.dealloc(storage.as_sprite_ptr(), sprite.layout()) } - None - } else { - Some(storage) - } - }); + let storage = self.sprite.get_mut(&sprite.get_id()); + + if let Some(storage) = storage { + storage.count -= 1; + + if storage.count == 0 { + unsafe { SPRITE_ALLOCATOR.dealloc(storage.as_sprite_ptr(), sprite.layout()) }; + self.sprite.remove(&sprite.get_id()); + } + } self.return_palette(sprite.palette) } fn return_palette(&mut self, palette: &'static Palette16) { let id = palette.get_id(); - self.palette - .entry(id) - .and_replace_entry_with(|_, mut storage| { - storage.count -= 1; - if storage.count == 0 { - unsafe { - PALETTE_ALLOCATOR.dealloc(storage.as_palette_ptr(), Palette16::layout()); - } - None - } else { - Some(storage) - } - }); + + if let Some(storage) = self.palette.get_mut(&id) { + storage.count -= 1; + + if storage.count == 0 { + unsafe { PALETTE_ALLOCATOR.dealloc(storage.as_palette_ptr(), Palette16::layout()) }; + self.palette.remove(&id); + } + } } } diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index 06db618..273e378 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -244,8 +244,11 @@ where pub fn get_mut(&mut self, key: &K) -> Option<&mut V> { let hash = self.hash(key); - self.get_location(key, hash) - .map(|location| &mut self.nodes.0[location].as_mut().unwrap().value) + if let Some(location) = self.get_location(key, hash) { + Some(&mut self.nodes.0[location].as_mut().unwrap().value) + } else { + None + } } pub fn remove(&mut self, key: &K) -> Option { @@ -271,8 +274,6 @@ where } } -impl HashMap {} - pub struct Iter<'a, K: 'a, V: 'a> { map: &'a HashMap, at: usize, @@ -306,6 +307,72 @@ impl<'a, K, V> IntoIterator for &'a HashMap { } } +pub struct OccupiedEntry<'a, K: 'a, V: 'a> { + entry: &'a mut Node, +} + +pub struct VacantEntry<'a, K: 'a, V: 'a> { + key: K, + map: &'a mut HashMap, +} + +impl<'a, K: 'a, V: 'a> VacantEntry<'a, K, V> { + pub fn insert(self, value: V) + where + K: Hash + Eq, + { + self.map.insert(self.key, value); + } +} + +pub enum Entry<'a, K: 'a, V: 'a> { + Occupied(OccupiedEntry<'a, K, V>), + Vacant(VacantEntry<'a, K, V>), +} + +impl<'a, K, V> Entry<'a, K, V> +where + K: Hash + Eq, +{ + pub fn or_insert(self, value: V) { + match self { + Entry::Occupied(_) => {} + Entry::Vacant(e) => e.insert(value), + } + } + + pub fn and_modify(self, f: F) -> Self + where + F: FnOnce(&mut V), + { + match self { + Entry::Occupied(e) => { + f(&mut e.entry.value); + Entry::Occupied(e) + } + Entry::Vacant(e) => Entry::Vacant(e), + } + } +} + +impl<'a, K, V> HashMap +where + K: Hash + Eq, +{ + pub fn entry(&mut self, key: K) -> Entry<'_, K, V> { + let hash = self.hash(&key); + let location = self.get_location(&key, hash); + + if let Some(location) = location { + Entry::Occupied(OccupiedEntry { + entry: self.nodes.0[location].as_mut().unwrap(), + }) + } else { + Entry::Vacant(VacantEntry { key, map: self }) + } + } +} + #[cfg(test)] mod test { use super::*; From 12dab0c3babbb644ed1b961918c331fb15f31cdf Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Fri, 18 Mar 2022 00:53:18 +0000 Subject: [PATCH 15/47] Fix issue where we weren't considering 0 distance correctly --- agb/src/hash_map.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index 273e378..922f9dd 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -98,10 +98,18 @@ impl NodeStorage { let result = loop { let next_location = fast_mod(self.len(), (current_location + 1) as HashType); - // if the next node is empty, then we can clear the current node - if self.0[next_location].is_none() { + // if the next node is empty, or the next location has 0 distance to initial bucket then + // we can clear the current node + if self.0[next_location].is_none() + || self.0[next_location] + .as_ref() + .unwrap() + .distance_to_initial_bucket + == 0 + { break self.0[current_location].take().unwrap(); } + if self.0[next_location].is_none() {} self.0.swap(current_location, next_location); self.0[current_location] From a6c7eaec1add217e19be26af22d4784cf506353c Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Fri, 18 Mar 2022 01:06:07 +0000 Subject: [PATCH 16/47] Correctly calculate new distance to initial bucket --- agb/src/hash_map.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index 922f9dd..1d65ea5 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -75,10 +75,6 @@ impl NodeStorage { if let Some(current_node) = current_node { if current_node.distance_to_initial_bucket <= new_node.distance_to_initial_bucket { - max_distance_to_initial_bucket = new_node - .distance_to_initial_bucket - .max(max_distance_to_initial_bucket); - mem::swap(&mut new_node, current_node); } } else { @@ -87,6 +83,9 @@ impl NodeStorage { } new_node.distance_to_initial_bucket += 1; + max_distance_to_initial_bucket = new_node + .distance_to_initial_bucket + .max(max_distance_to_initial_bucket); } max_distance_to_initial_bucket From 89cc00f8cff797b7b157ed2cca9f018d7a17e86a Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Fri, 18 Mar 2022 20:56:33 +0000 Subject: [PATCH 17/47] Sized is implied --- agb/src/hash_map.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index 1d65ea5..77265e7 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -8,18 +8,14 @@ use rustc_hash::FxHasher; type HashType = u32; -struct Node { +struct Node { hash: HashType, distance_to_initial_bucket: u32, key: K, value: V, } -impl Node -where - K: Sized, - V: Sized, -{ +impl Node { fn with_new_key_value(self, new_key: K, new_value: V) -> (Self, V) { ( Self { From eb00563b099e35c1609034b788745b6a6d75c9f7 Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Fri, 18 Mar 2022 21:08:26 +0000 Subject: [PATCH 18/47] Add extreme test case and fix bug causing disappearing entries --- agb/src/hash_map.rs | 66 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 65 insertions(+), 1 deletion(-) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index 77265e7..65a7bb7 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -66,7 +66,10 @@ impl NodeStorage { let mut max_distance_to_initial_bucket = max_distance_to_initial_bucket; loop { - let location = fast_mod(self.len(), hash + new_node.distance_to_initial_bucket); + let location = fast_mod( + self.len(), + new_node.hash + new_node.distance_to_initial_bucket, + ); let current_node = self.0[location].as_mut(); if let Some(current_node) = current_node { @@ -465,4 +468,65 @@ mod test { assert_eq!(map.get(&i), Some(&(i % 4))); } } + + struct RandomNumberGenerator { + state: [u32; 4], + } + + impl RandomNumberGenerator { + const fn new() -> Self { + Self { + state: [1014776995, 476057059, 3301633994, 706340607], + } + } + + fn next(&mut self) -> i32 { + let result = (self.state[0].wrapping_add(self.state[3])) + .rotate_left(7) + .wrapping_mul(9); + let t = self.state[1].wrapping_shr(9); + + self.state[2] ^= self.state[0]; + self.state[3] ^= self.state[1]; + self.state[1] ^= self.state[2]; + self.state[0] ^= self.state[3]; + + self.state[2] ^= t; + self.state[3] = self.state[3].rotate_left(11); + + result as i32 + } + } + + #[test_case] + fn extreme_case(_gba: &mut Gba) { + let mut map = HashMap::new(); + let mut rng = RandomNumberGenerator::new(); + + let mut answers: [Option; 128] = [None; 128]; + + for _ in 0..5_000 { + let command = rng.next().rem_euclid(2); + let key = rng.next().rem_euclid(128); + let value = rng.next(); + + match command { + 0 => { + // insert + answers[key as usize] = Some(value); + map.insert(key, value); + } + 1 => { + // remove + answers[key as usize] = None; + map.remove(&key); + } + _ => {} + } + + for (i, answer) in answers.iter().enumerate() { + assert_eq!(map.get(&(i as i32)), answer.as_ref()); + } + } + } } From e9d3c6e5c47c96c0a5855d42caf9007f10af7c37 Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Fri, 18 Mar 2022 21:11:53 +0000 Subject: [PATCH 19/47] Fix clippy lint that we should define is_empty --- agb/src/hash_map.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index 65a7bb7..58f9daa 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -147,6 +147,10 @@ impl HashMap { self.number_of_elements } + pub fn is_empty(&self) -> bool { + self.number_of_elements == 0 + } + pub fn resize(&mut self, new_size: usize) { assert!( new_size >= self.nodes.len(), From ff709d9d57155cbf55faa42c131e6f4e3226cfb2 Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Fri, 18 Mar 2022 21:12:46 +0000 Subject: [PATCH 20/47] Use .drain(..).flatten() --- agb/src/hash_map.rs | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index 58f9daa..4703409 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -164,16 +164,14 @@ impl HashMap { let mut new_max_distance_to_initial_bucket = 0; let number_of_elements = self.number_of_elements; - for node in self.nodes.0.drain(..) { - if let Some(node) = node { - new_max_distance_to_initial_bucket = new_node_storage.insert_new( - node.key, - node.value, - node.hash, - number_of_elements, - new_max_distance_to_initial_bucket, - ); - } + for node in self.nodes.0.drain(..).flatten() { + new_max_distance_to_initial_bucket = new_node_storage.insert_new( + node.key, + node.value, + node.hash, + number_of_elements, + new_max_distance_to_initial_bucket, + ); } self.nodes = new_node_storage; From 9f6797f4ed4bea25e68266c30804e107e02de014 Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Fri, 18 Mar 2022 21:14:07 +0000 Subject: [PATCH 21/47] Update lockfiles --- book/games/pong/Cargo.lock | 50 ------------------- .../the-hat-chooses-the-wizard/Cargo.lock | 50 ------------------- examples/the-purple-night/Cargo.lock | 50 ------------------- 3 files changed, 150 deletions(-) diff --git a/book/games/pong/Cargo.lock b/book/games/pong/Cargo.lock index 090de30..20e3942 100644 --- a/book/games/pong/Cargo.lock +++ b/book/games/pong/Cargo.lock @@ -24,7 +24,6 @@ dependencies = [ "agb_sound_converter", "bare-metal", "bitflags", - "hashbrown", "modular-bitfield", "rustc-hash", ] @@ -68,17 +67,6 @@ dependencies = [ "syn", ] -[[package]] -name = "ahash" -version = "0.7.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fcb51a0695d8f838b1ee009b3fbf66bda078cd64590202a864a8f3e8c4315c47" -dependencies = [ - "getrandom", - "once_cell", - "version_check", -] - [[package]] name = "asefile" version = "0.3.2" @@ -166,26 +154,6 @@ dependencies = [ "miniz_oxide 0.4.4", ] -[[package]] -name = "getrandom" -version = "0.2.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d39cd93900197114fa1fcb7ae84ca742095eed9442088988ae74fa744e930e77" -dependencies = [ - "cfg-if", - "libc", - "wasi", -] - -[[package]] -name = "hashbrown" -version = "0.12.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8c21d40587b92fa6a6c6e3c1bdbf87d75511db5672f9c93175574b3a00df1758" -dependencies = [ - "ahash", -] - [[package]] name = "hound" version = "3.4.0" @@ -309,12 +277,6 @@ dependencies = [ "autocfg", ] -[[package]] -name = "once_cell" -version = "1.10.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "87f3e037eac156d1775da914196f0f37741a274155e34a0b7e427c35d2a2ecb9" - [[package]] name = "png" version = "0.16.8" @@ -409,15 +371,3 @@ name = "unicode-xid" version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8ccb82d61f80a663efe1f787a51b16b5a51e3314d6ac365b08639f52387b33f3" - -[[package]] -name = "version_check" -version = "0.9.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "49874b5167b65d7193b8aba1567f5c7d93d001cafc34600cee003eda787e483f" - -[[package]] -name = "wasi" -version = "0.10.2+wasi-snapshot-preview1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fd6fbd9a79829dd1ad0cc20627bf1ed606756a7f77edff7b66b7064f9cb327c6" diff --git a/examples/the-hat-chooses-the-wizard/Cargo.lock b/examples/the-hat-chooses-the-wizard/Cargo.lock index c68e876..7a396a5 100644 --- a/examples/the-hat-chooses-the-wizard/Cargo.lock +++ b/examples/the-hat-chooses-the-wizard/Cargo.lock @@ -24,7 +24,6 @@ dependencies = [ "agb_sound_converter", "bare-metal", "bitflags", - "hashbrown", "modular-bitfield", "rustc-hash", ] @@ -68,17 +67,6 @@ dependencies = [ "syn", ] -[[package]] -name = "ahash" -version = "0.7.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fcb51a0695d8f838b1ee009b3fbf66bda078cd64590202a864a8f3e8c4315c47" -dependencies = [ - "getrandom", - "once_cell", - "version_check", -] - [[package]] name = "asefile" version = "0.3.2" @@ -166,26 +154,6 @@ dependencies = [ "miniz_oxide 0.4.4", ] -[[package]] -name = "getrandom" -version = "0.2.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d39cd93900197114fa1fcb7ae84ca742095eed9442088988ae74fa744e930e77" -dependencies = [ - "cfg-if", - "libc", - "wasi", -] - -[[package]] -name = "hashbrown" -version = "0.12.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8c21d40587b92fa6a6c6e3c1bdbf87d75511db5672f9c93175574b3a00df1758" -dependencies = [ - "ahash", -] - [[package]] name = "hound" version = "3.4.0" @@ -315,12 +283,6 @@ dependencies = [ "autocfg", ] -[[package]] -name = "once_cell" -version = "1.10.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "87f3e037eac156d1775da914196f0f37741a274155e34a0b7e427c35d2a2ecb9" - [[package]] name = "png" version = "0.16.8" @@ -434,15 +396,3 @@ name = "unicode-xid" version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8ccb82d61f80a663efe1f787a51b16b5a51e3314d6ac365b08639f52387b33f3" - -[[package]] -name = "version_check" -version = "0.9.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "49874b5167b65d7193b8aba1567f5c7d93d001cafc34600cee003eda787e483f" - -[[package]] -name = "wasi" -version = "0.10.2+wasi-snapshot-preview1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fd6fbd9a79829dd1ad0cc20627bf1ed606756a7f77edff7b66b7064f9cb327c6" diff --git a/examples/the-purple-night/Cargo.lock b/examples/the-purple-night/Cargo.lock index 2214b3f..4caec95 100644 --- a/examples/the-purple-night/Cargo.lock +++ b/examples/the-purple-night/Cargo.lock @@ -24,7 +24,6 @@ dependencies = [ "agb_sound_converter", "bare-metal", "bitflags", - "hashbrown", "modular-bitfield", "rustc-hash", ] @@ -68,17 +67,6 @@ dependencies = [ "syn", ] -[[package]] -name = "ahash" -version = "0.7.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fcb51a0695d8f838b1ee009b3fbf66bda078cd64590202a864a8f3e8c4315c47" -dependencies = [ - "getrandom", - "once_cell", - "version_check", -] - [[package]] name = "asefile" version = "0.3.2" @@ -190,26 +178,6 @@ dependencies = [ "cfg-if 0.1.10", ] -[[package]] -name = "getrandom" -version = "0.2.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d39cd93900197114fa1fcb7ae84ca742095eed9442088988ae74fa744e930e77" -dependencies = [ - "cfg-if 1.0.0", - "libc", - "wasi", -] - -[[package]] -name = "hashbrown" -version = "0.12.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8c21d40587b92fa6a6c6e3c1bdbf87d75511db5672f9c93175574b3a00df1758" -dependencies = [ - "ahash", -] - [[package]] name = "hound" version = "3.4.0" @@ -345,12 +313,6 @@ dependencies = [ "autocfg", ] -[[package]] -name = "once_cell" -version = "1.10.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "87f3e037eac156d1775da914196f0f37741a274155e34a0b7e427c35d2a2ecb9" - [[package]] name = "png" version = "0.16.8" @@ -472,18 +434,6 @@ version = "0.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8ccb82d61f80a663efe1f787a51b16b5a51e3314d6ac365b08639f52387b33f3" -[[package]] -name = "version_check" -version = "0.9.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "49874b5167b65d7193b8aba1567f5c7d93d001cafc34600cee003eda787e483f" - -[[package]] -name = "wasi" -version = "0.10.2+wasi-snapshot-preview1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fd6fbd9a79829dd1ad0cc20627bf1ed606756a7f77edff7b66b7064f9cb327c6" - [[package]] name = "xml-rs" version = "0.8.4" From cc53b0a911cf6a46192e0fd74148aa12e877fa2f Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Sat, 19 Mar 2022 21:54:10 +0000 Subject: [PATCH 22/47] Change distance_to_initial_bucket to be an i32 --- agb/src/hash_map.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index 4703409..205c24e 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -10,7 +10,7 @@ type HashType = u32; struct Node { hash: HashType, - distance_to_initial_bucket: u32, + distance_to_initial_bucket: i32, key: K, value: V, } @@ -48,8 +48,8 @@ impl NodeStorage { value: V, hash: HashType, number_of_elements: usize, - max_distance_to_initial_bucket: u32, - ) -> u32 { + max_distance_to_initial_bucket: i32, + ) -> i32 { debug_assert!( self.len() * 85 / 100 > number_of_elements, "Do not have space to insert into len {} with {number_of_elements}", @@ -68,7 +68,7 @@ impl NodeStorage { loop { let location = fast_mod( self.len(), - new_node.hash + new_node.distance_to_initial_bucket, + new_node.hash + new_node.distance_to_initial_bucket as HashType, ); let current_node = self.0[location].as_mut(); @@ -123,7 +123,7 @@ impl NodeStorage { pub struct HashMap { number_of_elements: usize, - max_distance_to_initial_bucket: u32, + max_distance_to_initial_bucket: i32, nodes: NodeStorage, hasher: BuildHasherDefault, @@ -196,7 +196,10 @@ where { fn get_location(&self, key: &K, hash: HashType) -> Option { for distance_to_initial_bucket in 0..=self.max_distance_to_initial_bucket { - let location = fast_mod(self.nodes.len(), hash + distance_to_initial_bucket); + let location = fast_mod( + self.nodes.len(), + hash + distance_to_initial_bucket as HashType, + ); let node = &self.nodes.0[location]; if let Some(node) = node { From fdc2172b3d4cfeb9617731636cf2053ea31d6000 Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Sat, 19 Mar 2022 21:59:12 +0000 Subject: [PATCH 23/47] Move max_distance_to_initial_bucket to NodeStorage --- agb/src/hash_map.rs | 120 ++++++++++++++++++++------------------------ 1 file changed, 54 insertions(+), 66 deletions(-) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index 205c24e..72eda98 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -29,27 +29,26 @@ impl Node { } } -struct NodeStorage(Vec>>); +struct NodeStorage { + nodes: Vec>>, + max_distance_to_initial_bucket: i32, +} impl NodeStorage { fn with_size(capacity: usize) -> Self { assert!(capacity.is_power_of_two(), "Capacity must be a power of 2"); - Self(iter::repeat_with(|| None).take(capacity).collect()) + Self { + nodes: iter::repeat_with(|| None).take(capacity).collect(), + max_distance_to_initial_bucket: 0, + } } fn len(&self) -> usize { - self.0.len() + self.nodes.len() } - fn insert_new( - &mut self, - key: K, - value: V, - hash: HashType, - number_of_elements: usize, - max_distance_to_initial_bucket: i32, - ) -> i32 { + fn insert_new(&mut self, key: K, value: V, hash: HashType, number_of_elements: usize) { debug_assert!( self.len() * 85 / 100 > number_of_elements, "Do not have space to insert into len {} with {number_of_elements}", @@ -63,31 +62,27 @@ impl NodeStorage { value, }; - let mut max_distance_to_initial_bucket = max_distance_to_initial_bucket; - loop { let location = fast_mod( self.len(), new_node.hash + new_node.distance_to_initial_bucket as HashType, ); - let current_node = self.0[location].as_mut(); + let current_node = self.nodes[location].as_mut(); if let Some(current_node) = current_node { if current_node.distance_to_initial_bucket <= new_node.distance_to_initial_bucket { mem::swap(&mut new_node, current_node); } } else { - self.0[location] = Some(new_node); + self.nodes[location] = Some(new_node); break; } new_node.distance_to_initial_bucket += 1; - max_distance_to_initial_bucket = new_node + self.max_distance_to_initial_bucket = new_node .distance_to_initial_bucket - .max(max_distance_to_initial_bucket); + .max(self.max_distance_to_initial_bucket); } - - max_distance_to_initial_bucket } fn remove_from_location(&mut self, location: usize) -> V { @@ -98,19 +93,19 @@ impl NodeStorage { // if the next node is empty, or the next location has 0 distance to initial bucket then // we can clear the current node - if self.0[next_location].is_none() - || self.0[next_location] + if self.nodes[next_location].is_none() + || self.nodes[next_location] .as_ref() .unwrap() .distance_to_initial_bucket == 0 { - break self.0[current_location].take().unwrap(); + break self.nodes[current_location].take().unwrap(); } - if self.0[next_location].is_none() {} + if self.nodes[next_location].is_none() {} - self.0.swap(current_location, next_location); - self.0[current_location] + self.nodes.swap(current_location, next_location); + self.nodes[current_location] .as_mut() .unwrap() .distance_to_initial_bucket -= 1; @@ -119,11 +114,33 @@ impl NodeStorage { result.value } + + fn get_location(&self, key: &K, hash: HashType) -> Option + where + K: Eq, + { + for distance_to_initial_bucket in 0..=self.max_distance_to_initial_bucket { + let location = fast_mod( + self.nodes.len(), + hash + distance_to_initial_bucket as HashType, + ); + + let node = &self.nodes[location]; + if let Some(node) = node { + if &node.key == key { + return Some(location); + } + } else { + return None; + } + } + + None + } } pub struct HashMap { number_of_elements: usize, - max_distance_to_initial_bucket: i32, nodes: NodeStorage, hasher: BuildHasherDefault, @@ -137,7 +154,6 @@ impl HashMap { pub fn with_capacity(capacity: usize) -> Self { Self { number_of_elements: 0, - max_distance_to_initial_bucket: 0, nodes: NodeStorage::with_size(capacity), hasher: Default::default(), } @@ -164,18 +180,11 @@ impl HashMap { let mut new_max_distance_to_initial_bucket = 0; let number_of_elements = self.number_of_elements; - for node in self.nodes.0.drain(..).flatten() { - new_max_distance_to_initial_bucket = new_node_storage.insert_new( - node.key, - node.value, - node.hash, - number_of_elements, - new_max_distance_to_initial_bucket, - ); + for node in self.nodes.nodes.drain(..).flatten() { + new_node_storage.insert_new(node.key, node.value, node.hash, number_of_elements); } self.nodes = new_node_storage; - self.max_distance_to_initial_bucket = new_max_distance_to_initial_bucket; } } @@ -195,23 +204,7 @@ where K: Eq, { fn get_location(&self, key: &K, hash: HashType) -> Option { - for distance_to_initial_bucket in 0..=self.max_distance_to_initial_bucket { - let location = fast_mod( - self.nodes.len(), - hash + distance_to_initial_bucket as HashType, - ); - - let node = &self.nodes.0[location]; - if let Some(node) = node { - if &node.key == key { - return Some(location); - } - } else { - return None; - } - } - - None + self.nodes.get_location(key, hash) } } @@ -223,9 +216,9 @@ where let hash = self.hash(&key); if let Some(location) = self.get_location(&key, hash) { - let old_node = self.nodes.0[location].take().unwrap(); + let old_node = self.nodes.nodes[location].take().unwrap(); let (new_node, old_value) = old_node.with_new_key_value(key, value); - self.nodes.0[location] = Some(new_node); + self.nodes.nodes[location] = Some(new_node); return Some(old_value); } @@ -234,13 +227,8 @@ where self.resize(self.nodes.len() * 2); } - self.max_distance_to_initial_bucket = self.nodes.insert_new( - key, - value, - hash, - self.number_of_elements, - self.max_distance_to_initial_bucket, - ); + self.nodes + .insert_new(key, value, hash, self.number_of_elements); self.number_of_elements += 1; None } @@ -249,14 +237,14 @@ where let hash = self.hash(key); self.get_location(key, hash) - .map(|location| &self.nodes.0[location].as_ref().unwrap().value) + .map(|location| &self.nodes.nodes[location].as_ref().unwrap().value) } pub fn get_mut(&mut self, key: &K) -> Option<&mut V> { let hash = self.hash(key); if let Some(location) = self.get_location(key, hash) { - Some(&mut self.nodes.0[location].as_mut().unwrap().value) + Some(&mut self.nodes.nodes[location].as_mut().unwrap().value) } else { None } @@ -299,7 +287,7 @@ impl<'a, K, V> Iterator for Iter<'a, K, V> { return None; } - if let Some(node) = &self.map.nodes.0[self.at] { + if let Some(node) = &self.map.nodes.nodes[self.at] { self.at += 1; return Some((&node.key, &node.value)); } @@ -376,7 +364,7 @@ where if let Some(location) = location { Entry::Occupied(OccupiedEntry { - entry: self.nodes.0[location].as_mut().unwrap(), + entry: self.nodes.nodes[location].as_mut().unwrap(), }) } else { Entry::Vacant(VacantEntry { key, map: self }) From 0e89f9190f532ef3235335461d62dc7f41809205 Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Sat, 19 Mar 2022 22:01:16 +0000 Subject: [PATCH 24/47] Rename NodeStorage.len() to NodeStorage.capacity() --- agb/src/hash_map.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index 72eda98..65b01ee 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -44,15 +44,15 @@ impl NodeStorage { } } - fn len(&self) -> usize { + fn capacity(&self) -> usize { self.nodes.len() } fn insert_new(&mut self, key: K, value: V, hash: HashType, number_of_elements: usize) { debug_assert!( - self.len() * 85 / 100 > number_of_elements, + self.capacity() * 85 / 100 > number_of_elements, "Do not have space to insert into len {} with {number_of_elements}", - self.len() + self.capacity() ); let mut new_node = Node { @@ -64,7 +64,7 @@ impl NodeStorage { loop { let location = fast_mod( - self.len(), + self.capacity(), new_node.hash + new_node.distance_to_initial_bucket as HashType, ); let current_node = self.nodes[location].as_mut(); @@ -89,7 +89,7 @@ impl NodeStorage { let mut current_location = location; let result = loop { - let next_location = fast_mod(self.len(), (current_location + 1) as HashType); + let next_location = fast_mod(self.capacity(), (current_location + 1) as HashType); // if the next node is empty, or the next location has 0 distance to initial bucket then // we can clear the current node @@ -169,15 +169,15 @@ impl HashMap { pub fn resize(&mut self, new_size: usize) { assert!( - new_size >= self.nodes.len(), + new_size >= self.nodes.capacity(), "Can only increase the size of a hash map" ); - if new_size == self.nodes.len() { + if new_size == self.nodes.capacity() { return; } let mut new_node_storage = NodeStorage::with_size(new_size); - let mut new_max_distance_to_initial_bucket = 0; + let number_of_elements = self.number_of_elements; for node in self.nodes.nodes.drain(..).flatten() { @@ -223,8 +223,8 @@ where return Some(old_value); } - if self.nodes.len() * 85 / 100 <= self.number_of_elements { - self.resize(self.nodes.len() * 2); + if self.nodes.capacity() * 85 / 100 <= self.number_of_elements { + self.resize(self.nodes.capacity() * 2); } self.nodes @@ -283,7 +283,7 @@ impl<'a, K, V> Iterator for Iter<'a, K, V> { fn next(&mut self) -> Option { loop { - if self.at >= self.map.nodes.len() { + if self.at >= self.map.nodes.capacity() { return None; } From 321702531bbba5129e9c3124f3c27feb2026b2f4 Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Sat, 19 Mar 2022 22:04:11 +0000 Subject: [PATCH 25/47] Move number_of_items to NodeStorage --- agb/src/hash_map.rs | 40 +++++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index 65b01ee..999427b 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -32,6 +32,8 @@ impl Node { struct NodeStorage { nodes: Vec>>, max_distance_to_initial_bucket: i32, + + number_of_items: usize, } impl NodeStorage { @@ -41,6 +43,7 @@ impl NodeStorage { Self { nodes: iter::repeat_with(|| None).take(capacity).collect(), max_distance_to_initial_bucket: 0, + number_of_items: 0, } } @@ -48,11 +51,16 @@ impl NodeStorage { self.nodes.len() } - fn insert_new(&mut self, key: K, value: V, hash: HashType, number_of_elements: usize) { + fn len(&self) -> usize { + self.number_of_items + } + + fn insert_new(&mut self, key: K, value: V, hash: HashType) { debug_assert!( - self.capacity() * 85 / 100 > number_of_elements, - "Do not have space to insert into len {} with {number_of_elements}", - self.capacity() + self.capacity() * 85 / 100 > self.len(), + "Do not have space to insert into len {} with {}", + self.capacity(), + self.len() ); let mut new_node = Node { @@ -83,6 +91,8 @@ impl NodeStorage { .distance_to_initial_bucket .max(self.max_distance_to_initial_bucket); } + + self.number_of_items += 1; } fn remove_from_location(&mut self, location: usize) -> V { @@ -112,6 +122,8 @@ impl NodeStorage { current_location = next_location; }; + self.number_of_items -= 1; + result.value } @@ -140,7 +152,6 @@ impl NodeStorage { } pub struct HashMap { - number_of_elements: usize, nodes: NodeStorage, hasher: BuildHasherDefault, @@ -153,18 +164,17 @@ impl HashMap { pub fn with_capacity(capacity: usize) -> Self { Self { - number_of_elements: 0, nodes: NodeStorage::with_size(capacity), hasher: Default::default(), } } pub fn len(&self) -> usize { - self.number_of_elements + self.nodes.len() } pub fn is_empty(&self) -> bool { - self.number_of_elements == 0 + self.len() == 0 } pub fn resize(&mut self, new_size: usize) { @@ -178,10 +188,8 @@ impl HashMap { let mut new_node_storage = NodeStorage::with_size(new_size); - let number_of_elements = self.number_of_elements; - for node in self.nodes.nodes.drain(..).flatten() { - new_node_storage.insert_new(node.key, node.value, node.hash, number_of_elements); + new_node_storage.insert_new(node.key, node.value, node.hash); } self.nodes = new_node_storage; @@ -223,13 +231,11 @@ where return Some(old_value); } - if self.nodes.capacity() * 85 / 100 <= self.number_of_elements { + if self.nodes.capacity() * 85 / 100 <= self.len() { self.resize(self.nodes.capacity() * 2); } - self.nodes - .insert_new(key, value, hash, self.number_of_elements); - self.number_of_elements += 1; + self.nodes.insert_new(key, value, hash); None } @@ -255,10 +261,6 @@ where self.get_location(key, hash) .map(|location| self.nodes.remove_from_location(location)) - .map(|value| { - self.number_of_elements -= 1; - value - }) } } From cd9798d01fab3ce4ab29621176b1292ec9c66347 Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Sat, 19 Mar 2022 22:04:53 +0000 Subject: [PATCH 26/47] Move the implementation of get_location to NodeStorage --- agb/src/hash_map.rs | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index 999427b..b8e6a47 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -207,15 +207,6 @@ const fn fast_mod(len: usize, hash: HashType) -> usize { (hash as usize) & (len - 1) } -impl HashMap -where - K: Eq, -{ - fn get_location(&self, key: &K, hash: HashType) -> Option { - self.nodes.get_location(key, hash) - } -} - impl HashMap where K: Eq + Hash, @@ -223,7 +214,7 @@ where pub fn insert(&mut self, key: K, value: V) -> Option { let hash = self.hash(&key); - if let Some(location) = self.get_location(&key, hash) { + if let Some(location) = self.nodes.get_location(&key, hash) { let old_node = self.nodes.nodes[location].take().unwrap(); let (new_node, old_value) = old_node.with_new_key_value(key, value); self.nodes.nodes[location] = Some(new_node); @@ -242,14 +233,15 @@ where pub fn get(&self, key: &K) -> Option<&V> { let hash = self.hash(key); - self.get_location(key, hash) + self.nodes + .get_location(key, hash) .map(|location| &self.nodes.nodes[location].as_ref().unwrap().value) } pub fn get_mut(&mut self, key: &K) -> Option<&mut V> { let hash = self.hash(key); - if let Some(location) = self.get_location(key, hash) { + if let Some(location) = self.nodes.get_location(key, hash) { Some(&mut self.nodes.nodes[location].as_mut().unwrap().value) } else { None @@ -259,7 +251,8 @@ where pub fn remove(&mut self, key: &K) -> Option { let hash = self.hash(key); - self.get_location(key, hash) + self.nodes + .get_location(key, hash) .map(|location| self.nodes.remove_from_location(location)) } } @@ -362,7 +355,7 @@ where { pub fn entry(&mut self, key: K) -> Entry<'_, K, V> { let hash = self.hash(&key); - let location = self.get_location(&key, hash); + let location = self.nodes.get_location(&key, hash); if let Some(location) = location { Entry::Occupied(OccupiedEntry { From ee983ef7ece455e6228c43179bab3cdfc827c882 Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Sat, 19 Mar 2022 22:42:09 +0000 Subject: [PATCH 27/47] Use MaybeUninit to reduce memory usage by half --- agb/src/hash_map.rs | 184 +++++++++++++++++++++++++++++++------------- 1 file changed, 132 insertions(+), 52 deletions(-) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index b8e6a47..138b9c4 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -1,7 +1,9 @@ use alloc::vec::Vec; use core::{ hash::{BuildHasher, BuildHasherDefault, Hash, Hasher}, - iter, mem, + iter, + mem::{self, MaybeUninit}, + ptr, }; use rustc_hash::FxHasher; @@ -10,27 +12,118 @@ type HashType = u32; struct Node { hash: HashType, + + // distance_to_initial_bucket = -1 => key and value are uninit. + // distance_to_initial_bucket >= 0 => key and value are init distance_to_initial_bucket: i32, - key: K, - value: V, + key: MaybeUninit, + value: MaybeUninit, } impl Node { - fn with_new_key_value(self, new_key: K, new_value: V) -> (Self, V) { + fn with_new_key_value(self, new_key: K, new_value: V) -> (Self, Option) { ( Self { hash: self.hash, distance_to_initial_bucket: self.distance_to_initial_bucket, - key: new_key, - value: new_value, + key: MaybeUninit::new(new_key), + value: MaybeUninit::new(new_value), }, - self.value, + self.get_owned_value(), ) } + + fn new() -> Self { + Self { + hash: 0, + distance_to_initial_bucket: -1, + key: MaybeUninit::uninit(), + value: MaybeUninit::uninit(), + } + } + + fn new_with(key: K, value: V, hash: HashType) -> Self { + Self { + hash, + distance_to_initial_bucket: 0, + key: MaybeUninit::new(key), + value: MaybeUninit::new(value), + } + } + + fn get_value_ref(&self) -> Option<&V> { + if self.has_value() { + Some(unsafe { self.value.assume_init_ref() }) + } else { + None + } + } + + fn get_value_mut(&mut self) -> Option<&mut V> { + if self.has_value() { + Some(unsafe { self.value.assume_init_mut() }) + } else { + None + } + } + + fn get_owned_value(mut self) -> Option { + if self.has_value() { + let value = mem::replace(&mut self.value, MaybeUninit::uninit()); + self.distance_to_initial_bucket = -1; + + Some(unsafe { value.assume_init() }) + } else { + None + } + } + + fn key_ref(&self) -> Option<&K> { + if self.distance_to_initial_bucket >= 0 { + Some(unsafe { self.key.assume_init_ref() }) + } else { + None + } + } + + fn has_value(&self) -> bool { + self.distance_to_initial_bucket >= 0 + } + + fn take(&mut self) -> Self { + mem::take(self) + } + + fn take_key_value(&mut self) -> Option<(K, V, HashType)> { + if self.has_value() { + let key = mem::replace(&mut self.key, MaybeUninit::uninit()); + let value = mem::replace(&mut self.value, MaybeUninit::uninit()); + self.distance_to_initial_bucket = -1; + + Some(unsafe { (key.assume_init(), value.assume_init(), self.hash) }) + } else { + None + } + } +} + +impl Drop for Node { + fn drop(&mut self) { + if self.distance_to_initial_bucket >= 0 { + unsafe { ptr::drop_in_place(self.key.as_mut_ptr()) }; + unsafe { ptr::drop_in_place(self.value.as_mut_ptr()) }; + } + } +} + +impl Default for Node { + fn default() -> Self { + Self::new() + } } struct NodeStorage { - nodes: Vec>>, + nodes: Vec>, max_distance_to_initial_bucket: i32, number_of_items: usize, @@ -41,7 +134,7 @@ impl NodeStorage { assert!(capacity.is_power_of_two(), "Capacity must be a power of 2"); Self { - nodes: iter::repeat_with(|| None).take(capacity).collect(), + nodes: iter::repeat_with(Default::default).take(capacity).collect(), max_distance_to_initial_bucket: 0, number_of_items: 0, } @@ -63,26 +156,21 @@ impl NodeStorage { self.len() ); - let mut new_node = Node { - hash, - distance_to_initial_bucket: 0, - key, - value, - }; + let mut new_node = Node::new_with(key, value, hash); loop { let location = fast_mod( self.capacity(), new_node.hash + new_node.distance_to_initial_bucket as HashType, ); - let current_node = self.nodes[location].as_mut(); + let current_node = &mut self.nodes[location]; - if let Some(current_node) = current_node { + if current_node.has_value() { if current_node.distance_to_initial_bucket <= new_node.distance_to_initial_bucket { mem::swap(&mut new_node, current_node); } } else { - self.nodes[location] = Some(new_node); + self.nodes[location] = new_node; break; } @@ -97,34 +185,23 @@ impl NodeStorage { fn remove_from_location(&mut self, location: usize) -> V { let mut current_location = location; + self.number_of_items -= 1; - let result = loop { + loop { let next_location = fast_mod(self.capacity(), (current_location + 1) as HashType); // if the next node is empty, or the next location has 0 distance to initial bucket then // we can clear the current node - if self.nodes[next_location].is_none() - || self.nodes[next_location] - .as_ref() - .unwrap() - .distance_to_initial_bucket - == 0 + if !self.nodes[next_location].has_value() + || self.nodes[next_location].distance_to_initial_bucket == 0 { - break self.nodes[current_location].take().unwrap(); + return self.nodes[current_location].take_key_value().unwrap().1; } - if self.nodes[next_location].is_none() {} self.nodes.swap(current_location, next_location); - self.nodes[current_location] - .as_mut() - .unwrap() - .distance_to_initial_bucket -= 1; + self.nodes[current_location].distance_to_initial_bucket -= 1; current_location = next_location; - }; - - self.number_of_items -= 1; - - result.value + } } fn get_location(&self, key: &K, hash: HashType) -> Option @@ -138,8 +215,8 @@ impl NodeStorage { ); let node = &self.nodes[location]; - if let Some(node) = node { - if &node.key == key { + if let Some(node_key_ref) = node.key_ref() { + if node_key_ref == key { return Some(location); } } else { @@ -188,8 +265,10 @@ impl HashMap { let mut new_node_storage = NodeStorage::with_size(new_size); - for node in self.nodes.nodes.drain(..).flatten() { - new_node_storage.insert_new(node.key, node.value, node.hash); + for mut node in self.nodes.nodes.drain(..) { + if let Some((key, value, hash)) = node.take_key_value() { + new_node_storage.insert_new(key, value, hash); + } } self.nodes = new_node_storage; @@ -215,11 +294,11 @@ where let hash = self.hash(&key); if let Some(location) = self.nodes.get_location(&key, hash) { - let old_node = self.nodes.nodes[location].take().unwrap(); + let old_node = self.nodes.nodes[location].take(); let (new_node, old_value) = old_node.with_new_key_value(key, value); - self.nodes.nodes[location] = Some(new_node); + self.nodes.nodes[location] = new_node; - return Some(old_value); + return old_value; } if self.nodes.capacity() * 85 / 100 <= self.len() { @@ -235,14 +314,15 @@ where self.nodes .get_location(key, hash) - .map(|location| &self.nodes.nodes[location].as_ref().unwrap().value) + .map(|location| self.nodes.nodes[location].get_value_ref()) + .flatten() } pub fn get_mut(&mut self, key: &K) -> Option<&mut V> { let hash = self.hash(key); if let Some(location) = self.nodes.get_location(key, hash) { - Some(&mut self.nodes.nodes[location].as_mut().unwrap().value) + self.nodes.nodes[location].get_value_mut() } else { None } @@ -282,12 +362,12 @@ impl<'a, K, V> Iterator for Iter<'a, K, V> { return None; } - if let Some(node) = &self.map.nodes.nodes[self.at] { - self.at += 1; - return Some((&node.key, &node.value)); - } - + let node = &self.map.nodes.nodes[self.at]; self.at += 1; + + if node.has_value() { + return Some((node.key_ref().unwrap(), node.get_value_ref().unwrap())); + } } } } @@ -341,7 +421,7 @@ where { match self { Entry::Occupied(e) => { - f(&mut e.entry.value); + f(e.entry.get_value_mut().unwrap()); Entry::Occupied(e) } Entry::Vacant(e) => Entry::Vacant(e), @@ -359,7 +439,7 @@ where if let Some(location) = location { Entry::Occupied(OccupiedEntry { - entry: self.nodes.nodes[location].as_mut().unwrap(), + entry: &mut self.nodes.nodes[location], }) } else { Entry::Vacant(VacantEntry { key, map: self }) From 280e7f876d7dfa251375070584018d05eb5c0005 Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Sat, 19 Mar 2022 22:59:00 +0000 Subject: [PATCH 28/47] Add some drop tests --- agb/src/hash_map.rs | 96 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 96 insertions(+) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index 138b9c4..655aa1b 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -449,6 +449,8 @@ where #[cfg(test)] mod test { + use core::cell::RefCell; + use super::*; use crate::Gba; @@ -597,4 +599,98 @@ mod test { } } } + + struct Droppable<'a> { + id: usize, + drop_registry: &'a DropRegistry, + } + + impl<'a> Drop for Droppable<'a> { + fn drop(&mut self) { + self.drop_registry.dropped(self.id); + } + } + + struct DropRegistry { + are_dropped: RefCell>, + } + + impl DropRegistry { + pub fn new() -> Self { + Self { + are_dropped: Default::default(), + } + } + + pub fn new_droppable(&self) -> Droppable<'_> { + self.are_dropped.borrow_mut().push(false); + Droppable { + id: self.are_dropped.borrow().len() - 1, + drop_registry: self, + } + } + + pub fn dropped(&self, id: usize) { + self.are_dropped.borrow_mut()[id] = true; + } + + pub fn assert_dropped(&self, id: usize) { + assert!(self.are_dropped.borrow()[id]); + } + + pub fn assert_not_dropped(&self, id: usize) { + assert!(!self.are_dropped.borrow()[id]); + } + } + + #[test_case] + fn correctly_drops_on_remove_and_overall_drop(_gba: &mut Gba) { + let drop_registry = DropRegistry::new(); + + let droppable1 = drop_registry.new_droppable(); + let droppable2 = drop_registry.new_droppable(); + + let id1 = droppable1.id; + let id2 = droppable2.id; + + { + let mut map = HashMap::new(); + + map.insert(1, droppable1); + map.insert(2, droppable2); + + drop_registry.assert_not_dropped(id1); + drop_registry.assert_not_dropped(id2); + + map.remove(&1); + drop_registry.assert_dropped(id1); + drop_registry.assert_not_dropped(id2); + } + + drop_registry.assert_dropped(id2); + } + + #[test_case] + fn correctly_drop_on_override(_gba: &mut Gba) { + let drop_registry = DropRegistry::new(); + + let droppable1 = drop_registry.new_droppable(); + let droppable2 = drop_registry.new_droppable(); + + let id1 = droppable1.id; + let id2 = droppable2.id; + + { + let mut map = HashMap::new(); + + map.insert(1, droppable1); + drop_registry.assert_not_dropped(id1); + map.insert(1, droppable2); + + drop_registry.assert_dropped(id1); + drop_registry.assert_not_dropped(id2); + } + + drop_registry.assert_dropped(id2); + } } From e68a4d373f4cdeac067466cd478e3c16aa138b76 Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Sat, 19 Mar 2022 23:10:12 +0000 Subject: [PATCH 29/47] Replace .map().flatten() with .and_then() --- agb/src/hash_map.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index 655aa1b..30028c0 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -314,8 +314,7 @@ where self.nodes .get_location(key, hash) - .map(|location| self.nodes.nodes[location].get_value_ref()) - .flatten() + .and_then(|location| self.nodes.nodes[location].get_value_ref()) } pub fn get_mut(&mut self, key: &K) -> Option<&mut V> { From d4ed1cd2fab576a6da4969a834ececd52f7bd0e2 Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Sat, 19 Mar 2022 23:13:02 +0000 Subject: [PATCH 30/47] Allow swapping out the hasher --- agb/src/hash_map.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index 30028c0..f0b0ca8 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -228,10 +228,13 @@ impl NodeStorage { } } -pub struct HashMap { +pub struct HashMap> +where + H: BuildHasher, +{ nodes: NodeStorage, - hasher: BuildHasherDefault, + hasher: H, } impl HashMap { From 9c7d9520a7d39a086be0cb21ca1627e381b82369 Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Sun, 20 Mar 2022 13:41:31 +0000 Subject: [PATCH 31/47] Add test that we drop the key on override --- agb/src/hash_map.rs | 63 +++++++++++++++++++++++++++++++++++++-------- 1 file changed, 52 insertions(+), 11 deletions(-) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index f0b0ca8..dd5158b 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -602,19 +602,34 @@ mod test { } } + #[derive(Clone)] struct Droppable<'a> { id: usize, drop_registry: &'a DropRegistry, } - impl<'a> Drop for Droppable<'a> { + impl Hash for Droppable<'_> { + fn hash(&self, hasher: &mut H) { + hasher.write_usize(self.id); + } + } + + impl PartialEq for Droppable<'_> { + fn eq(&self, other: &Self) -> bool { + self.id == other.id + } + } + + impl Eq for Droppable<'_> {} + + impl Drop for Droppable<'_> { fn drop(&mut self) { self.drop_registry.dropped(self.id); } } struct DropRegistry { - are_dropped: RefCell>, + are_dropped: RefCell>, } impl DropRegistry { @@ -625,7 +640,7 @@ mod test { } pub fn new_droppable(&self) -> Droppable<'_> { - self.are_dropped.borrow_mut().push(false); + self.are_dropped.borrow_mut().push(0); Droppable { id: self.are_dropped.borrow().len() - 1, drop_registry: self, @@ -633,15 +648,19 @@ mod test { } pub fn dropped(&self, id: usize) { - self.are_dropped.borrow_mut()[id] = true; + self.are_dropped.borrow_mut()[id] += 1; } - pub fn assert_dropped(&self, id: usize) { - assert!(self.are_dropped.borrow()[id]); + pub fn assert_dropped_once(&self, id: usize) { + assert_eq!(self.are_dropped.borrow()[id], 1); } pub fn assert_not_dropped(&self, id: usize) { - assert!(!self.are_dropped.borrow()[id]); + assert_eq!(self.are_dropped.borrow()[id], 0); + } + + pub fn assert_dropped_n_times(&self, id: usize, num_drops: i32) { + assert_eq!(self.are_dropped.borrow()[id], num_drops); } } @@ -665,11 +684,11 @@ mod test { drop_registry.assert_not_dropped(id2); map.remove(&1); - drop_registry.assert_dropped(id1); + drop_registry.assert_dropped_once(id1); drop_registry.assert_not_dropped(id2); } - drop_registry.assert_dropped(id2); + drop_registry.assert_dropped_once(id2); } #[test_case] @@ -689,10 +708,32 @@ mod test { drop_registry.assert_not_dropped(id1); map.insert(1, droppable2); - drop_registry.assert_dropped(id1); + drop_registry.assert_dropped_once(id1); drop_registry.assert_not_dropped(id2); } - drop_registry.assert_dropped(id2); + drop_registry.assert_dropped_once(id2); + } + + #[test_case] + fn correctly_drops_key_on_override(_gba: &mut Gba) { + let drop_registry = DropRegistry::new(); + + let droppable1 = drop_registry.new_droppable(); + let droppable1a = droppable1.clone(); + + let id1 = droppable1.id; + + { + let mut map = HashMap::new(); + + map.insert(droppable1, 1); + drop_registry.assert_not_dropped(id1); + map.insert(droppable1a, 2); + + drop_registry.assert_dropped_once(id1); + } + + drop_registry.assert_dropped_n_times(id1, 2); } } From 3ae0e30d3c4f4d0cb20d3e596b1b70f47ab674ab Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Sun, 20 Mar 2022 13:42:28 +0000 Subject: [PATCH 32/47] Correctly drop key on override --- agb/src/hash_map.rs | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index dd5158b..1d53cfa 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -21,7 +21,7 @@ struct Node { } impl Node { - fn with_new_key_value(self, new_key: K, new_value: V) -> (Self, Option) { + fn with_new_key_value(mut self, new_key: K, new_value: V) -> (Self, Option) { ( Self { hash: self.hash, @@ -29,7 +29,7 @@ impl Node { key: MaybeUninit::new(new_key), value: MaybeUninit::new(new_value), }, - self.get_owned_value(), + self.take_key_value().map(|(_, v, _)| v), ) } @@ -67,17 +67,6 @@ impl Node { } } - fn get_owned_value(mut self) -> Option { - if self.has_value() { - let value = mem::replace(&mut self.value, MaybeUninit::uninit()); - self.distance_to_initial_bucket = -1; - - Some(unsafe { value.assume_init() }) - } else { - None - } - } - fn key_ref(&self) -> Option<&K> { if self.distance_to_initial_bucket >= 0 { Some(unsafe { self.key.assume_init_ref() }) From e9c56327b1148a82b8f4408f20196d9b1d610852 Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Sun, 20 Mar 2022 13:43:31 +0000 Subject: [PATCH 33/47] Avoid duplicating constant --- agb/src/hash_map.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index 1d53cfa..f3b221c 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -568,7 +568,7 @@ mod test { for _ in 0..5_000 { let command = rng.next().rem_euclid(2); - let key = rng.next().rem_euclid(128); + let key = rng.next().rem_euclid(answers.len() as i32); let value = rng.next(); match command { From 86760f76acaf803a6204b6e76cab0957e92de6c2 Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Sun, 20 Mar 2022 13:50:04 +0000 Subject: [PATCH 34/47] Move resize to a more sensible place --- agb/src/hash_map.rs | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index f3b221c..bdb63ca 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -215,6 +215,18 @@ impl NodeStorage { None } + + fn resized_to(&mut self, new_size: usize) -> Self { + let mut new_node_storage = Self::with_size(new_size); + + for mut node in self.nodes.drain(..) { + if let Some((key, value, hash)) = node.take_key_value() { + new_node_storage.insert_new(key, value, hash); + } + } + + new_node_storage + } } pub struct HashMap> @@ -255,15 +267,7 @@ impl HashMap { return; } - let mut new_node_storage = NodeStorage::with_size(new_size); - - for mut node in self.nodes.nodes.drain(..) { - if let Some((key, value, hash)) = node.take_key_value() { - new_node_storage.insert_new(key, value, hash); - } - } - - self.nodes = new_node_storage; + self.nodes = self.nodes.resized_to(new_size); } } From e0d829a4fc7c049430408f4e8398d97569ee018d Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Sun, 20 Mar 2022 13:57:41 +0000 Subject: [PATCH 35/47] Rename get_value_ref and get_value_mut to remove get --- agb/src/hash_map.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index bdb63ca..7f295f8 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -51,7 +51,7 @@ impl Node { } } - fn get_value_ref(&self) -> Option<&V> { + fn value_ref(&self) -> Option<&V> { if self.has_value() { Some(unsafe { self.value.assume_init_ref() }) } else { @@ -59,7 +59,7 @@ impl Node { } } - fn get_value_mut(&mut self) -> Option<&mut V> { + fn value_mut(&mut self) -> Option<&mut V> { if self.has_value() { Some(unsafe { self.value.assume_init_mut() }) } else { @@ -310,14 +310,14 @@ where self.nodes .get_location(key, hash) - .and_then(|location| self.nodes.nodes[location].get_value_ref()) + .and_then(|location| self.nodes.nodes[location].value_ref()) } pub fn get_mut(&mut self, key: &K) -> Option<&mut V> { let hash = self.hash(key); if let Some(location) = self.nodes.get_location(key, hash) { - self.nodes.nodes[location].get_value_mut() + self.nodes.nodes[location].value_mut() } else { None } @@ -361,7 +361,7 @@ impl<'a, K, V> Iterator for Iter<'a, K, V> { self.at += 1; if node.has_value() { - return Some((node.key_ref().unwrap(), node.get_value_ref().unwrap())); + return Some((node.key_ref().unwrap(), node.value_ref().unwrap())); } } } @@ -416,7 +416,7 @@ where { match self { Entry::Occupied(e) => { - f(e.entry.get_value_mut().unwrap()); + f(e.entry.value_mut().unwrap()); Entry::Occupied(e) } Entry::Vacant(e) => Entry::Vacant(e), From bde36c7019e51694d8b3d2b9baf6f0c525265335 Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Sun, 20 Mar 2022 13:59:01 +0000 Subject: [PATCH 36/47] Invert the order of the HashMap, Node and NodeStorage --- agb/src/hash_map.rs | 438 ++++++++++++++++++++++---------------------- 1 file changed, 219 insertions(+), 219 deletions(-) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index 7f295f8..b1e48b5 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -10,225 +10,6 @@ use rustc_hash::FxHasher; type HashType = u32; -struct Node { - hash: HashType, - - // distance_to_initial_bucket = -1 => key and value are uninit. - // distance_to_initial_bucket >= 0 => key and value are init - distance_to_initial_bucket: i32, - key: MaybeUninit, - value: MaybeUninit, -} - -impl Node { - fn with_new_key_value(mut self, new_key: K, new_value: V) -> (Self, Option) { - ( - Self { - hash: self.hash, - distance_to_initial_bucket: self.distance_to_initial_bucket, - key: MaybeUninit::new(new_key), - value: MaybeUninit::new(new_value), - }, - self.take_key_value().map(|(_, v, _)| v), - ) - } - - fn new() -> Self { - Self { - hash: 0, - distance_to_initial_bucket: -1, - key: MaybeUninit::uninit(), - value: MaybeUninit::uninit(), - } - } - - fn new_with(key: K, value: V, hash: HashType) -> Self { - Self { - hash, - distance_to_initial_bucket: 0, - key: MaybeUninit::new(key), - value: MaybeUninit::new(value), - } - } - - fn value_ref(&self) -> Option<&V> { - if self.has_value() { - Some(unsafe { self.value.assume_init_ref() }) - } else { - None - } - } - - fn value_mut(&mut self) -> Option<&mut V> { - if self.has_value() { - Some(unsafe { self.value.assume_init_mut() }) - } else { - None - } - } - - fn key_ref(&self) -> Option<&K> { - if self.distance_to_initial_bucket >= 0 { - Some(unsafe { self.key.assume_init_ref() }) - } else { - None - } - } - - fn has_value(&self) -> bool { - self.distance_to_initial_bucket >= 0 - } - - fn take(&mut self) -> Self { - mem::take(self) - } - - fn take_key_value(&mut self) -> Option<(K, V, HashType)> { - if self.has_value() { - let key = mem::replace(&mut self.key, MaybeUninit::uninit()); - let value = mem::replace(&mut self.value, MaybeUninit::uninit()); - self.distance_to_initial_bucket = -1; - - Some(unsafe { (key.assume_init(), value.assume_init(), self.hash) }) - } else { - None - } - } -} - -impl Drop for Node { - fn drop(&mut self) { - if self.distance_to_initial_bucket >= 0 { - unsafe { ptr::drop_in_place(self.key.as_mut_ptr()) }; - unsafe { ptr::drop_in_place(self.value.as_mut_ptr()) }; - } - } -} - -impl Default for Node { - fn default() -> Self { - Self::new() - } -} - -struct NodeStorage { - nodes: Vec>, - max_distance_to_initial_bucket: i32, - - number_of_items: usize, -} - -impl NodeStorage { - fn with_size(capacity: usize) -> Self { - assert!(capacity.is_power_of_two(), "Capacity must be a power of 2"); - - Self { - nodes: iter::repeat_with(Default::default).take(capacity).collect(), - max_distance_to_initial_bucket: 0, - number_of_items: 0, - } - } - - fn capacity(&self) -> usize { - self.nodes.len() - } - - fn len(&self) -> usize { - self.number_of_items - } - - fn insert_new(&mut self, key: K, value: V, hash: HashType) { - debug_assert!( - self.capacity() * 85 / 100 > self.len(), - "Do not have space to insert into len {} with {}", - self.capacity(), - self.len() - ); - - let mut new_node = Node::new_with(key, value, hash); - - loop { - let location = fast_mod( - self.capacity(), - new_node.hash + new_node.distance_to_initial_bucket as HashType, - ); - let current_node = &mut self.nodes[location]; - - if current_node.has_value() { - if current_node.distance_to_initial_bucket <= new_node.distance_to_initial_bucket { - mem::swap(&mut new_node, current_node); - } - } else { - self.nodes[location] = new_node; - break; - } - - new_node.distance_to_initial_bucket += 1; - self.max_distance_to_initial_bucket = new_node - .distance_to_initial_bucket - .max(self.max_distance_to_initial_bucket); - } - - self.number_of_items += 1; - } - - fn remove_from_location(&mut self, location: usize) -> V { - let mut current_location = location; - self.number_of_items -= 1; - - loop { - let next_location = fast_mod(self.capacity(), (current_location + 1) as HashType); - - // if the next node is empty, or the next location has 0 distance to initial bucket then - // we can clear the current node - if !self.nodes[next_location].has_value() - || self.nodes[next_location].distance_to_initial_bucket == 0 - { - return self.nodes[current_location].take_key_value().unwrap().1; - } - - self.nodes.swap(current_location, next_location); - self.nodes[current_location].distance_to_initial_bucket -= 1; - current_location = next_location; - } - } - - fn get_location(&self, key: &K, hash: HashType) -> Option - where - K: Eq, - { - for distance_to_initial_bucket in 0..=self.max_distance_to_initial_bucket { - let location = fast_mod( - self.nodes.len(), - hash + distance_to_initial_bucket as HashType, - ); - - let node = &self.nodes[location]; - if let Some(node_key_ref) = node.key_ref() { - if node_key_ref == key { - return Some(location); - } - } else { - return None; - } - } - - None - } - - fn resized_to(&mut self, new_size: usize) -> Self { - let mut new_node_storage = Self::with_size(new_size); - - for mut node in self.nodes.drain(..) { - if let Some((key, value, hash)) = node.take_key_value() { - new_node_storage.insert_new(key, value, hash); - } - } - - new_node_storage - } -} - pub struct HashMap> where H: BuildHasher, @@ -442,6 +223,225 @@ where } } +struct NodeStorage { + nodes: Vec>, + max_distance_to_initial_bucket: i32, + + number_of_items: usize, +} + +impl NodeStorage { + fn with_size(capacity: usize) -> Self { + assert!(capacity.is_power_of_two(), "Capacity must be a power of 2"); + + Self { + nodes: iter::repeat_with(Default::default).take(capacity).collect(), + max_distance_to_initial_bucket: 0, + number_of_items: 0, + } + } + + fn capacity(&self) -> usize { + self.nodes.len() + } + + fn len(&self) -> usize { + self.number_of_items + } + + fn insert_new(&mut self, key: K, value: V, hash: HashType) { + debug_assert!( + self.capacity() * 85 / 100 > self.len(), + "Do not have space to insert into len {} with {}", + self.capacity(), + self.len() + ); + + let mut new_node = Node::new_with(key, value, hash); + + loop { + let location = fast_mod( + self.capacity(), + new_node.hash + new_node.distance_to_initial_bucket as HashType, + ); + let current_node = &mut self.nodes[location]; + + if current_node.has_value() { + if current_node.distance_to_initial_bucket <= new_node.distance_to_initial_bucket { + mem::swap(&mut new_node, current_node); + } + } else { + self.nodes[location] = new_node; + break; + } + + new_node.distance_to_initial_bucket += 1; + self.max_distance_to_initial_bucket = new_node + .distance_to_initial_bucket + .max(self.max_distance_to_initial_bucket); + } + + self.number_of_items += 1; + } + + fn remove_from_location(&mut self, location: usize) -> V { + let mut current_location = location; + self.number_of_items -= 1; + + loop { + let next_location = fast_mod(self.capacity(), (current_location + 1) as HashType); + + // if the next node is empty, or the next location has 0 distance to initial bucket then + // we can clear the current node + if !self.nodes[next_location].has_value() + || self.nodes[next_location].distance_to_initial_bucket == 0 + { + return self.nodes[current_location].take_key_value().unwrap().1; + } + + self.nodes.swap(current_location, next_location); + self.nodes[current_location].distance_to_initial_bucket -= 1; + current_location = next_location; + } + } + + fn get_location(&self, key: &K, hash: HashType) -> Option + where + K: Eq, + { + for distance_to_initial_bucket in 0..=self.max_distance_to_initial_bucket { + let location = fast_mod( + self.nodes.len(), + hash + distance_to_initial_bucket as HashType, + ); + + let node = &self.nodes[location]; + if let Some(node_key_ref) = node.key_ref() { + if node_key_ref == key { + return Some(location); + } + } else { + return None; + } + } + + None + } + + fn resized_to(&mut self, new_size: usize) -> Self { + let mut new_node_storage = Self::with_size(new_size); + + for mut node in self.nodes.drain(..) { + if let Some((key, value, hash)) = node.take_key_value() { + new_node_storage.insert_new(key, value, hash); + } + } + + new_node_storage + } +} + +struct Node { + hash: HashType, + + // distance_to_initial_bucket = -1 => key and value are uninit. + // distance_to_initial_bucket >= 0 => key and value are init + distance_to_initial_bucket: i32, + key: MaybeUninit, + value: MaybeUninit, +} + +impl Node { + fn with_new_key_value(mut self, new_key: K, new_value: V) -> (Self, Option) { + ( + Self { + hash: self.hash, + distance_to_initial_bucket: self.distance_to_initial_bucket, + key: MaybeUninit::new(new_key), + value: MaybeUninit::new(new_value), + }, + self.take_key_value().map(|(_, v, _)| v), + ) + } + + fn new() -> Self { + Self { + hash: 0, + distance_to_initial_bucket: -1, + key: MaybeUninit::uninit(), + value: MaybeUninit::uninit(), + } + } + + fn new_with(key: K, value: V, hash: HashType) -> Self { + Self { + hash, + distance_to_initial_bucket: 0, + key: MaybeUninit::new(key), + value: MaybeUninit::new(value), + } + } + + fn value_ref(&self) -> Option<&V> { + if self.has_value() { + Some(unsafe { self.value.assume_init_ref() }) + } else { + None + } + } + + fn value_mut(&mut self) -> Option<&mut V> { + if self.has_value() { + Some(unsafe { self.value.assume_init_mut() }) + } else { + None + } + } + + fn key_ref(&self) -> Option<&K> { + if self.distance_to_initial_bucket >= 0 { + Some(unsafe { self.key.assume_init_ref() }) + } else { + None + } + } + + fn has_value(&self) -> bool { + self.distance_to_initial_bucket >= 0 + } + + fn take(&mut self) -> Self { + mem::take(self) + } + + fn take_key_value(&mut self) -> Option<(K, V, HashType)> { + if self.has_value() { + let key = mem::replace(&mut self.key, MaybeUninit::uninit()); + let value = mem::replace(&mut self.value, MaybeUninit::uninit()); + self.distance_to_initial_bucket = -1; + + Some(unsafe { (key.assume_init(), value.assume_init(), self.hash) }) + } else { + None + } + } +} + +impl Drop for Node { + fn drop(&mut self) { + if self.distance_to_initial_bucket >= 0 { + unsafe { ptr::drop_in_place(self.key.as_mut_ptr()) }; + unsafe { ptr::drop_in_place(self.value.as_mut_ptr()) }; + } + } +} + +impl Default for Node { + fn default() -> Self { + Self::new() + } +} + #[cfg(test)] mod test { use core::cell::RefCell; From 9df79a16bd5a99f5527e2c36b704c50d1973d39b Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Sun, 20 Mar 2022 14:04:27 +0000 Subject: [PATCH 37/47] Replace slightly dodgy replace code --- agb/src/hash_map.rs | 37 ++++++++++++++++--------------------- 1 file changed, 16 insertions(+), 21 deletions(-) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index b1e48b5..5759623 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -71,11 +71,7 @@ where let hash = self.hash(&key); if let Some(location) = self.nodes.get_location(&key, hash) { - let old_node = self.nodes.nodes[location].take(); - let (new_node, old_value) = old_node.with_new_key_value(key, value); - self.nodes.nodes[location] = new_node; - - return old_value; + return self.nodes.replace_at_location(location, key, value); } if self.nodes.capacity() * 85 / 100 <= self.len() { @@ -339,6 +335,10 @@ impl NodeStorage { new_node_storage } + + fn replace_at_location(&mut self, location: usize, key: K, value: V) -> Option { + self.nodes[location].replace(key, value).map(|(k, v)| v) + } } struct Node { @@ -352,18 +352,6 @@ struct Node { } impl Node { - fn with_new_key_value(mut self, new_key: K, new_value: V) -> (Self, Option) { - ( - Self { - hash: self.hash, - distance_to_initial_bucket: self.distance_to_initial_bucket, - key: MaybeUninit::new(new_key), - value: MaybeUninit::new(new_value), - }, - self.take_key_value().map(|(_, v, _)| v), - ) - } - fn new() -> Self { Self { hash: 0, @@ -410,10 +398,6 @@ impl Node { self.distance_to_initial_bucket >= 0 } - fn take(&mut self) -> Self { - mem::take(self) - } - fn take_key_value(&mut self) -> Option<(K, V, HashType)> { if self.has_value() { let key = mem::replace(&mut self.key, MaybeUninit::uninit()); @@ -425,6 +409,17 @@ impl Node { None } } + + fn replace(&mut self, key: K, value: V) -> Option<(K, V)> { + if self.has_value() { + let old_key = mem::replace(&mut self.key, MaybeUninit::new(key)); + let old_value = mem::replace(&mut self.value, MaybeUninit::new(value)); + + Some(unsafe { (old_key.assume_init(), old_value.assume_init()) }) + } else { + panic!("Cannot replace an uninitialised node"); + } + } } impl Drop for Node { From 3f624ee87d6d9dbe1de6037e5a0a7a20b04dd1ab Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Sun, 20 Mar 2022 14:10:38 +0000 Subject: [PATCH 38/47] Fail test in extreme case test if we drop twice --- agb/src/hash_map.rs | 44 +++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 41 insertions(+), 3 deletions(-) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index 5759623..7bafabb 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -558,6 +558,41 @@ mod test { } } + struct NoisyDrop { + i: i32, + dropped: bool, + } + + impl NoisyDrop { + fn new(i: i32) -> Self { + Self { i, dropped: false } + } + } + + impl PartialEq for NoisyDrop { + fn eq(&self, other: &Self) -> bool { + self.i == other.i + } + } + + impl Eq for NoisyDrop {} + + impl Hash for NoisyDrop { + fn hash(&self, hasher: &mut H) { + hasher.write_i32(self.i); + } + } + + impl Drop for NoisyDrop { + fn drop(&mut self) { + if self.dropped { + panic!("NoisyDropped dropped twice"); + } + + self.dropped = true; + } + } + #[test_case] fn extreme_case(_gba: &mut Gba) { let mut map = HashMap::new(); @@ -574,18 +609,21 @@ mod test { 0 => { // insert answers[key as usize] = Some(value); - map.insert(key, value); + map.insert(NoisyDrop::new(key), NoisyDrop::new(value)); } 1 => { // remove answers[key as usize] = None; - map.remove(&key); + map.remove(&NoisyDrop::new(key)); } _ => {} } for (i, answer) in answers.iter().enumerate() { - assert_eq!(map.get(&(i as i32)), answer.as_ref()); + assert_eq!( + map.get(&NoisyDrop::new(i as i32)).map(|nd| &nd.i), + answer.as_ref() + ); } } } From 9bfb8de481bc23a9e0fe337f5b52f46dfdd5b2e9 Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Sun, 20 Mar 2022 14:14:22 +0000 Subject: [PATCH 39/47] Wrap distance_to_initial_bucket a bit better --- agb/src/hash_map.rs | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index 7bafabb..ecec173 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -258,12 +258,12 @@ impl NodeStorage { loop { let location = fast_mod( self.capacity(), - new_node.hash + new_node.distance_to_initial_bucket as HashType, + new_node.hash + new_node.get_distance() as HashType, ); let current_node = &mut self.nodes[location]; if current_node.has_value() { - if current_node.distance_to_initial_bucket <= new_node.distance_to_initial_bucket { + if current_node.get_distance() <= new_node.get_distance() { mem::swap(&mut new_node, current_node); } } else { @@ -271,9 +271,9 @@ impl NodeStorage { break; } - new_node.distance_to_initial_bucket += 1; + new_node.increment_distance(); self.max_distance_to_initial_bucket = new_node - .distance_to_initial_bucket + .get_distance() .max(self.max_distance_to_initial_bucket); } @@ -290,13 +290,13 @@ impl NodeStorage { // if the next node is empty, or the next location has 0 distance to initial bucket then // we can clear the current node if !self.nodes[next_location].has_value() - || self.nodes[next_location].distance_to_initial_bucket == 0 + || self.nodes[next_location].get_distance() == 0 { return self.nodes[current_location].take_key_value().unwrap().1; } self.nodes.swap(current_location, next_location); - self.nodes[current_location].distance_to_initial_bucket -= 1; + self.nodes[current_location].decrement_distance(); current_location = next_location; } } @@ -420,6 +420,21 @@ impl Node { panic!("Cannot replace an uninitialised node"); } } + + fn increment_distance(&mut self) { + self.distance_to_initial_bucket += 1; + } + + fn decrement_distance(&mut self) { + self.distance_to_initial_bucket -= 1; + if self.distance_to_initial_bucket < 0 { + panic!("Cannot decrement distance to below 0"); + } + } + + fn get_distance(&self) -> i32 { + self.distance_to_initial_bucket + } } impl Drop for Node { From ab80f200e8b5e9bc03c31164d4f4425a34943ddb Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Sun, 20 Mar 2022 14:15:24 +0000 Subject: [PATCH 40/47] Encaspulate a bit better --- agb/src/hash_map.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index ecec173..2b335f6 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -439,7 +439,7 @@ impl Node { impl Drop for Node { fn drop(&mut self) { - if self.distance_to_initial_bucket >= 0 { + if self.has_value() { unsafe { ptr::drop_in_place(self.key.as_mut_ptr()) }; unsafe { ptr::drop_in_place(self.value.as_mut_ptr()) }; } From 86635752af66104c503d9738ef18de6a07e5aafc Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Sun, 20 Mar 2022 14:21:45 +0000 Subject: [PATCH 41/47] Also implement or_insert_with_key --- agb/src/hash_map.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index 2b335f6..4307815 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -187,6 +187,19 @@ where } } + pub fn or_insert_with_key(self, f: F) + where + F: FnOnce(&K) -> V, + { + match self { + Entry::Occupied(_) => {} + Entry::Vacant(e) => { + let value = f(&e.key); + e.map.insert(e.key, value); + } + } + } + pub fn and_modify(self, f: F) -> Self where F: FnOnce(&mut V), From 2706b2494d56bb9066d5ec5e779e3b7013615501 Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Sun, 20 Mar 2022 14:22:43 +0000 Subject: [PATCH 42/47] Some wiggling --- agb/src/hash_map.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index 4307815..0f3c02c 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -163,7 +163,7 @@ pub struct VacantEntry<'a, K: 'a, V: 'a> { } impl<'a, K: 'a, V: 'a> VacantEntry<'a, K, V> { - pub fn insert(self, value: V) + fn insert(self, value: V) where K: Hash + Eq, { @@ -195,7 +195,7 @@ where Entry::Occupied(_) => {} Entry::Vacant(e) => { let value = f(&e.key); - e.map.insert(e.key, value); + e.insert(value); } } } From 921a338c59c79738f4f7fea7b064822b7afbbf75 Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Sun, 20 Mar 2022 14:54:53 +0000 Subject: [PATCH 43/47] Implement the entirity of the entry api --- agb/src/hash_map.rs | 80 +++++++++++++++++++++++++++++++++++---------- 1 file changed, 63 insertions(+), 17 deletions(-) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index 0f3c02c..9b9ff51 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -67,19 +67,21 @@ impl HashMap where K: Eq + Hash, { - pub fn insert(&mut self, key: K, value: V) -> Option { + pub fn insert(&mut self, key: K, value: V) -> &mut V { let hash = self.hash(&key); - if let Some(location) = self.nodes.get_location(&key, hash) { - return self.nodes.replace_at_location(location, key, value); - } + let location = if let Some(location) = self.nodes.get_location(&key, hash) { + self.nodes.replace_at_location(location, key, value); + location + } else { + if self.nodes.capacity() * 85 / 100 <= self.len() { + self.resize(self.nodes.capacity() * 2); + } - if self.nodes.capacity() * 85 / 100 <= self.len() { - self.resize(self.nodes.capacity() * 2); - } + self.nodes.insert_new(key, value, hash) + }; - self.nodes.insert_new(key, value, hash); - None + self.nodes.nodes[location].value_mut().unwrap() } pub fn get(&self, key: &K) -> Option<&V> { @@ -154,20 +156,27 @@ impl<'a, K, V> IntoIterator for &'a HashMap { } pub struct OccupiedEntry<'a, K: 'a, V: 'a> { + key: K, entry: &'a mut Node, } +impl<'a, K: 'a, V: 'a> OccupiedEntry<'a, K, V> { + fn value_mut(self) -> &'a mut V { + self.entry.value_mut().unwrap() + } +} + pub struct VacantEntry<'a, K: 'a, V: 'a> { key: K, map: &'a mut HashMap, } impl<'a, K: 'a, V: 'a> VacantEntry<'a, K, V> { - fn insert(self, value: V) + fn insert(self, value: V) -> &'a mut V where K: Hash + Eq, { - self.map.insert(self.key, value); + self.map.insert(self.key, value) } } @@ -180,22 +189,32 @@ impl<'a, K, V> Entry<'a, K, V> where K: Hash + Eq, { - pub fn or_insert(self, value: V) { + pub fn or_insert(self, value: V) -> &'a mut V { match self { - Entry::Occupied(_) => {} + Entry::Occupied(e) => e.value_mut(), Entry::Vacant(e) => e.insert(value), } } - pub fn or_insert_with_key(self, f: F) + pub fn or_insert_with(self, f: F) -> &'a mut V + where + F: FnOnce() -> V, + { + match self { + Entry::Occupied(e) => e.value_mut(), + Entry::Vacant(e) => e.insert(f()), + } + } + + pub fn or_insert_with_key(self, f: F) -> &'a mut V where F: FnOnce(&K) -> V, { match self { - Entry::Occupied(_) => {} + Entry::Occupied(e) => e.value_mut(), Entry::Vacant(e) => { let value = f(&e.key); - e.insert(value); + e.insert(value) } } } @@ -212,6 +231,23 @@ where Entry::Vacant(e) => Entry::Vacant(e), } } + + pub fn or_default(self) -> &'a mut V + where + V: Default, + { + match self { + Entry::Occupied(e) => e.value_mut(), + Entry::Vacant(e) => e.insert(Default::default()), + } + } + + pub fn key(&self) -> &K { + match self { + Entry::Occupied(e) => &e.key, + Entry::Vacant(e) => &e.key, + } + } } impl<'a, K, V> HashMap @@ -224,6 +260,7 @@ where if let Some(location) = location { Entry::Occupied(OccupiedEntry { + key, entry: &mut self.nodes.nodes[location], }) } else { @@ -258,7 +295,7 @@ impl NodeStorage { self.number_of_items } - fn insert_new(&mut self, key: K, value: V, hash: HashType) { + fn insert_new(&mut self, key: K, value: V, hash: HashType) -> usize { debug_assert!( self.capacity() * 85 / 100 > self.len(), "Do not have space to insert into len {} with {}", @@ -267,6 +304,7 @@ impl NodeStorage { ); let mut new_node = Node::new_with(key, value, hash); + let mut inserted_location = usize::MAX; loop { let location = fast_mod( @@ -278,9 +316,16 @@ impl NodeStorage { if current_node.has_value() { if current_node.get_distance() <= new_node.get_distance() { mem::swap(&mut new_node, current_node); + + if inserted_location == usize::MAX { + inserted_location = location; + } } } else { self.nodes[location] = new_node; + if inserted_location == usize::MAX { + inserted_location = location; + } break; } @@ -291,6 +336,7 @@ impl NodeStorage { } self.number_of_items += 1; + inserted_location } fn remove_from_location(&mut self, location: usize) -> V { From 631e1e9bc296a080104e27c114e667f28632858f Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Sun, 20 Mar 2022 15:07:25 +0000 Subject: [PATCH 44/47] Start implementing the individual entry types --- agb/src/hash_map.rs | 41 +++++++++++++++++++++++++++++++---------- 1 file changed, 31 insertions(+), 10 deletions(-) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index 9b9ff51..9cea8fe 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -161,9 +161,21 @@ pub struct OccupiedEntry<'a, K: 'a, V: 'a> { } impl<'a, K: 'a, V: 'a> OccupiedEntry<'a, K, V> { - fn value_mut(self) -> &'a mut V { + pub fn key(&self) -> &K { + &self.key + } + + pub fn get(&self) -> &V { + self.entry.value_ref().unwrap() + } + + pub fn into_mut(self) -> &'a mut V { self.entry.value_mut().unwrap() } + + pub fn insert(&mut self, value: V) -> V { + self.entry.replace_value(value) + } } pub struct VacantEntry<'a, K: 'a, V: 'a> { @@ -172,7 +184,7 @@ pub struct VacantEntry<'a, K: 'a, V: 'a> { } impl<'a, K: 'a, V: 'a> VacantEntry<'a, K, V> { - fn insert(self, value: V) -> &'a mut V + pub fn insert(self, value: V) -> &'a mut V where K: Hash + Eq, { @@ -191,7 +203,7 @@ where { pub fn or_insert(self, value: V) -> &'a mut V { match self { - Entry::Occupied(e) => e.value_mut(), + Entry::Occupied(e) => e.into_mut(), Entry::Vacant(e) => e.insert(value), } } @@ -201,7 +213,7 @@ where F: FnOnce() -> V, { match self { - Entry::Occupied(e) => e.value_mut(), + Entry::Occupied(e) => e.into_mut(), Entry::Vacant(e) => e.insert(f()), } } @@ -211,7 +223,7 @@ where F: FnOnce(&K) -> V, { match self { - Entry::Occupied(e) => e.value_mut(), + Entry::Occupied(e) => e.into_mut(), Entry::Vacant(e) => { let value = f(&e.key); e.insert(value) @@ -237,7 +249,7 @@ where V: Default, { match self { - Entry::Occupied(e) => e.value_mut(), + Entry::Occupied(e) => e.into_mut(), Entry::Vacant(e) => e.insert(Default::default()), } } @@ -395,8 +407,8 @@ impl NodeStorage { new_node_storage } - fn replace_at_location(&mut self, location: usize, key: K, value: V) -> Option { - self.nodes[location].replace(key, value).map(|(k, v)| v) + fn replace_at_location(&mut self, location: usize, key: K, value: V) -> V { + self.nodes[location].replace(key, value).1 } } @@ -469,12 +481,21 @@ impl Node { } } - fn replace(&mut self, key: K, value: V) -> Option<(K, V)> { + fn replace_value(&mut self, value: V) -> V { + if self.has_value() { + let old_value = mem::replace(&mut self.value, MaybeUninit::new(value)); + unsafe { old_value.assume_init() } + } else { + panic!("Cannot replace an unininitalised node"); + } + } + + fn replace(&mut self, key: K, value: V) -> (K, V) { if self.has_value() { let old_key = mem::replace(&mut self.key, MaybeUninit::new(key)); let old_value = mem::replace(&mut self.value, MaybeUninit::new(value)); - Some(unsafe { (old_key.assume_init(), old_value.assume_init()) }) + unsafe { (old_key.assume_init(), old_value.assume_init()) } } else { panic!("Cannot replace an uninitialised node"); } From 6bc3816b29b9569c95d947a381a07815347817b4 Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Sun, 20 Mar 2022 15:13:22 +0000 Subject: [PATCH 45/47] Fully implement OccupiedEntry --- agb/src/hash_map.rs | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index 9cea8fe..71e906f 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -157,7 +157,8 @@ impl<'a, K, V> IntoIterator for &'a HashMap { pub struct OccupiedEntry<'a, K: 'a, V: 'a> { key: K, - entry: &'a mut Node, + map: &'a mut HashMap, + location: usize, } impl<'a, K: 'a, V: 'a> OccupiedEntry<'a, K, V> { @@ -165,16 +166,29 @@ impl<'a, K: 'a, V: 'a> OccupiedEntry<'a, K, V> { &self.key } + pub fn remove_entry(self) -> (K, V) { + let old_value = self.map.nodes.remove_from_location(self.location); + (self.key, old_value) + } + pub fn get(&self) -> &V { - self.entry.value_ref().unwrap() + self.map.nodes.nodes[self.location].value_ref().unwrap() + } + + pub fn get_mut(&mut self) -> &mut V { + self.map.nodes.nodes[self.location].value_mut().unwrap() } pub fn into_mut(self) -> &'a mut V { - self.entry.value_mut().unwrap() + self.map.nodes.nodes[self.location].value_mut().unwrap() } pub fn insert(&mut self, value: V) -> V { - self.entry.replace_value(value) + self.map.nodes.nodes[self.location].replace_value(value) + } + + pub fn remove(self) -> V { + self.map.nodes.remove_from_location(self.location) } } @@ -236,8 +250,8 @@ where F: FnOnce(&mut V), { match self { - Entry::Occupied(e) => { - f(e.entry.value_mut().unwrap()); + Entry::Occupied(mut e) => { + f(e.get_mut()); Entry::Occupied(e) } Entry::Vacant(e) => Entry::Vacant(e), @@ -273,7 +287,8 @@ where if let Some(location) = location { Entry::Occupied(OccupiedEntry { key, - entry: &mut self.nodes.nodes[location], + location, + map: self, }) } else { Entry::Vacant(VacantEntry { key, map: self }) From 2b75ce6cbd2a5d94058aa476613d62904f68402b Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Sun, 20 Mar 2022 15:14:44 +0000 Subject: [PATCH 46/47] Implement all of VacantEntry --- agb/src/hash_map.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index 71e906f..a695ce2 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -198,6 +198,14 @@ pub struct VacantEntry<'a, K: 'a, V: 'a> { } impl<'a, K: 'a, V: 'a> VacantEntry<'a, K, V> { + pub fn key(&self) -> &K { + &self.key + } + + pub fn into_key(self) -> K { + self.key + } + pub fn insert(self, value: V) -> &'a mut V where K: Hash + Eq, From 4a1d99f14337c5fbb6edd195c0e7c39b61e70ed4 Mon Sep 17 00:00:00 2001 From: Gwilym Kuiper Date: Sun, 20 Mar 2022 15:38:39 +0000 Subject: [PATCH 47/47] Implement Index and FromIterator and add some tests lifted from rust stdlib --- agb/src/hash_map.rs | 153 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 152 insertions(+), 1 deletion(-) diff --git a/agb/src/hash_map.rs b/agb/src/hash_map.rs index a695ce2..c6f9afe 100644 --- a/agb/src/hash_map.rs +++ b/agb/src/hash_map.rs @@ -1,8 +1,9 @@ use alloc::vec::Vec; use core::{ hash::{BuildHasher, BuildHasherDefault, Hash, Hasher}, - iter, + iter::{self, FromIterator}, mem::{self, MaybeUninit}, + ops::Index, ptr, }; @@ -304,6 +305,50 @@ where } } +impl FromIterator<(K, V)> for HashMap +where + K: Eq + Hash, +{ + fn from_iter>(iter: T) -> Self { + let mut map = HashMap::new(); + map.extend(iter); + map + } +} + +impl Extend<(K, V)> for HashMap +where + K: Eq + Hash, +{ + fn extend>(&mut self, iter: T) { + for (k, v) in iter { + self.insert(k, v); + } + } +} + +impl Index<&K> for HashMap +where + K: Eq + Hash, +{ + type Output = V; + + fn index(&self, key: &K) -> &V { + self.get(key).expect("no entry found for key") + } +} + +impl Index for HashMap +where + K: Eq + Hash, +{ + type Output = V; + + fn index(&self, key: K) -> &V { + self.get(&key).expect("no entry found for key") + } +} + struct NodeStorage { nodes: Vec>, max_distance_to_initial_bucket: i32, @@ -880,4 +925,110 @@ mod test { drop_registry.assert_dropped_n_times(id1, 2); } + + // Following test cases copied from the rust source + // https://github.com/rust-lang/rust/blob/master/library/std/src/collections/hash/map/tests.rs + mod rust_std_tests { + use crate::{ + hash_map::{Entry::*, HashMap}, + Gba, + }; + + #[test_case] + fn test_entry(_gba: &mut Gba) { + let xs = [(1, 10), (2, 20), (3, 30), (4, 40), (5, 50), (6, 60)]; + + let mut map: HashMap<_, _> = xs.iter().cloned().collect(); + + // Existing key (insert) + match map.entry(1) { + Vacant(_) => unreachable!(), + Occupied(mut view) => { + assert_eq!(view.get(), &10); + assert_eq!(view.insert(100), 10); + } + } + assert_eq!(map.get(&1).unwrap(), &100); + assert_eq!(map.len(), 6); + + // Existing key (update) + match map.entry(2) { + Vacant(_) => unreachable!(), + Occupied(mut view) => { + let v = view.get_mut(); + let new_v = (*v) * 10; + *v = new_v; + } + } + assert_eq!(map.get(&2).unwrap(), &200); + assert_eq!(map.len(), 6); + + // Existing key (take) + match map.entry(3) { + Vacant(_) => unreachable!(), + Occupied(view) => { + assert_eq!(view.remove(), 30); + } + } + assert_eq!(map.get(&3), None); + assert_eq!(map.len(), 5); + + // Inexistent key (insert) + match map.entry(10) { + Occupied(_) => unreachable!(), + Vacant(view) => { + assert_eq!(*view.insert(1000), 1000); + } + } + assert_eq!(map.get(&10).unwrap(), &1000); + assert_eq!(map.len(), 6); + } + + #[test_case] + fn test_occupied_entry_key(_gba: &mut Gba) { + let mut a = HashMap::new(); + let key = "hello there"; + let value = "value goes here"; + assert!(a.is_empty()); + a.insert(key, value); + assert_eq!(a.len(), 1); + assert_eq!(a[key], value); + + match a.entry(key) { + Vacant(_) => panic!(), + Occupied(e) => assert_eq!(key, *e.key()), + } + assert_eq!(a.len(), 1); + assert_eq!(a[key], value); + } + + #[test_case] + fn test_vacant_entry_key(_gba: &mut Gba) { + let mut a = HashMap::new(); + let key = "hello there"; + let value = "value goes here"; + + assert!(a.is_empty()); + match a.entry(key) { + Occupied(_) => panic!(), + Vacant(e) => { + assert_eq!(key, *e.key()); + e.insert(value); + } + } + assert_eq!(a.len(), 1); + assert_eq!(a[key], value); + } + + #[test_case] + fn test_index(_gba: &mut Gba) { + let mut map = HashMap::new(); + + map.insert(1, 2); + map.insert(2, 1); + map.insert(3, 4); + + assert_eq!(map[&2], 1); + } + } }