From 7a4cc8ae3c650c11c04570b9b087ff4b2d37465a Mon Sep 17 00:00:00 2001 From: Nikita Masych Date: Fri, 2 Feb 2024 13:31:02 +0200 Subject: [PATCH 01/12] feat: added hashmap --- noir_stdlib/src/collections.nr | 1 + noir_stdlib/src/collections/map.nr | 245 ++++++++++++++++++ noir_stdlib/src/hash.nr | 109 ++++++++ noir_stdlib/src/hash/pedersen.nr | 25 ++ .../execution_success/hashmap/Nargo.toml | 6 + .../execution_success/hashmap/src/main.nr | 68 +++++ 6 files changed, 454 insertions(+) create mode 100644 noir_stdlib/src/collections/map.nr create mode 100644 noir_stdlib/src/hash/pedersen.nr create mode 100644 test_programs/execution_success/hashmap/Nargo.toml create mode 100644 test_programs/execution_success/hashmap/src/main.nr diff --git a/noir_stdlib/src/collections.nr b/noir_stdlib/src/collections.nr index 177ca96816f..2d952f4d6cd 100644 --- a/noir_stdlib/src/collections.nr +++ b/noir_stdlib/src/collections.nr @@ -1,2 +1,3 @@ mod vec; mod bounded_vec; +mod map; diff --git a/noir_stdlib/src/collections/map.nr b/noir_stdlib/src/collections/map.nr new file mode 100644 index 00000000000..00148c9c13a --- /dev/null +++ b/noir_stdlib/src/collections/map.nr @@ -0,0 +1,245 @@ +use crate::cmp::Eq; +use crate::collections::vec::Vec; +use crate::option::Option; +use crate::default::Default; +use crate::hash::{Hash, Hasher, BuildHasher}; + +// Generic trait for one-directional key -> value data structure. +// May be implemented by either hash tables or graphs for instance. +trait Map { + fn len(self) -> u64; + fn get(self, key: K) -> Option; + fn insert(&mut self, key: K, value: V); + fn remove(&mut self, key: K); +} + +// Hash table with open addressing and quadratic probing. +// Size of the underlying table must be known at compile time. +// It is advised to select capacity N as a power of two, or a prime number +// because utilized probing scheme is best tailored for it. +struct HashMap { + _table: [Slot; N], + + // Amount of valid elements in the map. + _len: u64, + + _build_hasher: B +} + +// Data unit in the HashMap table. +// In case Noir would support enums in future, this +// better to be refactored to use it with three states: +// 1. (key, value) +// 2. (empty) +// 3. (deleted) +struct Slot { + _key_value: Option<(K, V)>, + _is_deleted: bool, +} + +impl Default for Slot{ + fn default() -> Self{ + Slot{ + _key_value: Option::none(), + _is_deleted: false + } + } +} + +impl Slot{ + fn is_valid(self) -> bool{ + !self._is_deleted & self._key_value.is_some() + } + + fn is_available(self) -> bool{ + self._is_deleted | self._key_value.is_none() + } + + fn key_value_unchecked(self) -> (K, V){ + self._key_value.unwrap_unchecked() + } + + fn set(&mut self, key: K, value: V){ + self._key_value = Option::some((key, value)); + self._is_deleted = false; + } + + // Shall not override `_key_value` with Option::none(), + // because we must we able to differentiate empty + // and deleted slots for lookup. + fn mark_deleted(&mut self){ + self._is_deleted = true; + } +} + +impl HashMap { + // Creates a new instance of HashMap with specified BuildHasher. + pub fn with_hasher(_build_hasher: B) -> Self + where + B: BuildHasher + { + let _table = [Slot::default(); N]; + let _len = 0; + Self{_table, _len, _build_hasher} + } + + fn hash(self, key: K) -> u64 + where + K: Hash, + B: BuildHasher, + H: Hasher + { + let mut hasher = self._build_hasher.build_wrapper(); + key.hash(&mut hasher); + hasher.finish() as u64 + } + + // Probing scheme: quadratic function. + // We use 0.5 constant near variadic attempt and attempt^2 monomials. + // This ensures good uniformity of distribution for table sizes + // equal to prime numbers or powers of two. + fn quadratic_probe(self, hash: u64, attempt: u64) -> u64 { + (hash + (attempt + attempt * attempt) / 2 ) % N + } +} + +// While conducting lookup, we iterate attempt from 0 to N - 1 due to heuristic, +// that if we have went that far without finding desired, +// it is very unlikely to be after - performance in that case would be drastical. +impl Map for HashMap +where + K: Eq + Hash, + B: BuildHasher, + H: Hasher +{ + fn len(self) -> u64{ + self._len + } + + fn get(self, key: K) -> Option{ + let mut result = Option::none(); + + let hash = self.hash(key); + let mut break = false; + + for attempt in 0..N{ + if !break{ + let index = self.quadratic_probe(hash, attempt as u64); + let slot = self._table[index]; + + // Not marked as deleted and has key-value. + if slot.is_valid(){ + let (current_key, value) = slot.key_value_unchecked(); + if current_key == key{ + result = Option::some(value); + break = true; + } + } + } + } + + result + } + + fn insert(&mut self, key: K, value: V){ + let hash = self.hash(key); + let mut break = false; + + for attempt in 0..N{ + if !break{ + let index = self.quadratic_probe(hash, attempt as u64); + let mut slot = self._table[index]; + let mut insert = false; + + // Either marked as deleted or has unset key-value. + if slot.is_available(){ + insert = true; + self._len += 1; + }else{ + let (current_key, _) = slot.key_value_unchecked(); + if current_key == key{ + insert = true; + } + } + + if insert{ + slot.set(key, value); + self._table[index] = slot; + break = true; + } + } + } + } + + fn remove(&mut self, key: K){ + let hash = self.hash(key); + let mut break = false; + + for attempt in 0..N{ + if !break{ + let index = self.quadratic_probe(hash, attempt as u64); + let mut slot = self._table[index]; + + // Not marked as deleted and has key-value. + if slot.is_valid(){ + let (current_key, _) = slot.key_value_unchecked(); + if current_key == key{ + slot.mark_deleted(); + self._table[index] = slot; + self._len -= 1; + break = true; + } + } + } + } + } +} + +// Equality class on HashMap has to test that they have +// equal sets of key-value pairs, +// thus one is a subset of the other and vice versa. +impl Eq for HashMap +where + K: Eq + Hash, + V: Eq, + B: BuildHasher, + H: Hasher +{ + fn eq(self, other: HashMap) -> bool{ + let mut equal = false; + + if self.len() == other.len(){ + equal = true; + for slot in self._table{ + // Not marked as deleted and has key-value. + if equal & slot.is_valid(){ + let (key, value) = slot.key_value_unchecked(); + let other_value = other.get(key); + + if other_value.is_none(){ + equal = false; + }else{ + let other_value = other_value.unwrap_unchecked(); + if value != other_value{ + equal = false; + } + } + } + } + } + + equal + } +} + +impl Default for HashMap +where + B: BuildHasher + Default, + H: Hasher + Default +{ + fn default() -> Self{ + let _build_hasher = B::default(); + let map: HashMap = HashMap::with_hasher(_build_hasher); + map + } +} diff --git a/noir_stdlib/src/hash.nr b/noir_stdlib/src/hash.nr index cc864039a90..285ade9b356 100644 --- a/noir_stdlib/src/hash.nr +++ b/noir_stdlib/src/hash.nr @@ -1,5 +1,8 @@ mod poseidon; mod mimc; +mod pedersen; + +use crate::default::Default; #[foreign(sha256)] // docs:start:sha256 @@ -74,3 +77,109 @@ pub fn poseidon2_permutation(_input: [u8; N], _state_length: u32) -> [u8; N] #[foreign(sha256_compression)] pub fn sha256_compression(_input: [u32; 16], _state: [u32; 8]) -> [u32; 8] {} + +// Generic hashing support. +// Partially ported and impacted by rust. + +// Hash trait shall be implemented per type. +// We use HasherWrapper since we desire to have `&mut Hasher` in `Hash` trait functions prototype, +// but Noir can not correctly infer generic into pointer receiver, ergo such a hack is utilized +// and shall be removed upon generics improvement. +trait Hash{ + fn hash(self, state: &mut HasherWrapper) where H: Hasher; + + fn hash_slice(data: [Self], state: &mut HasherWrapper) where H: Hasher{ + for piece in data{ + piece.hash(state); + } + } +} + +// Hasher trait shall be implemented by algorithms to provide hash-agnostic means. +trait Hasher{ + fn finish(self) -> Field; + + fn write(self, input: [Field]) -> Self; +} + +// BuildHasher is a factory trait, responsible for production of specific Hasher. +trait BuildHasher where H: Hasher{ + fn build_hasher(self) -> H; + + fn build_wrapper(self) -> HasherWrapper{ + let hasher = self.build_hasher(); + HasherWrapper::new(hasher) + } +} + +struct HasherWrapper{ + _inner: H +} + +impl HasherWrapper +{ + pub fn new(_inner: H) -> Self{ + HasherWrapper{_inner} + } + + pub fn unwrap(self) -> H{ + self._inner + } + + pub fn finish(self) -> Field + where + H: Hasher + { + self._inner.finish() + } + + pub fn write(&mut self, input: [Field]) + where + H: Hasher + { + self._inner = self._inner.write(input); + } +} + +impl Default for HasherWrapper +where + H: Hasher + Default +{ + fn default() -> Self{ + HasherWrapper{ + _inner: H::default() + } + } +} + +struct BuildHasherDefault; + +impl BuildHasher for BuildHasherDefault +where + H: Hasher + Default +{ + fn build_hasher(self) -> H{ + H::default() + } + + fn build_wrapper(self) -> HasherWrapper{ + let wrapper: HasherWrapper = HasherWrapper::default(); + wrapper + } +} + +impl Default for BuildHasherDefault +where + H: Hasher + Default +{ + fn default() -> Self{ + BuildHasherDefault{} + } +} + +impl Hash for Field{ + fn hash(self, state: &mut HasherWrapper) where H: Hasher{ + let input: [Field] = [self]; + state.write(input); + } +} diff --git a/noir_stdlib/src/hash/pedersen.nr b/noir_stdlib/src/hash/pedersen.nr new file mode 100644 index 00000000000..85c3b9b4b85 --- /dev/null +++ b/noir_stdlib/src/hash/pedersen.nr @@ -0,0 +1,25 @@ +use crate::hash::{Hasher, pedersen_hash}; +use crate::default::Default; + +struct PedersenHasher{ + _state: [Field] +} + +impl Hasher for PedersenHasher { + fn finish(self) -> Field { + pedersen_hash(self._state) + } + + fn write(self, input: [Field]) -> Self{ + let _state = self._state.append(input); + PedersenHasher{_state} + } +} + +impl Default for PedersenHasher{ + fn default() -> Self{ + PedersenHasher{ + _state: [] + } + } +} diff --git a/test_programs/execution_success/hashmap/Nargo.toml b/test_programs/execution_success/hashmap/Nargo.toml new file mode 100644 index 00000000000..c09debc9833 --- /dev/null +++ b/test_programs/execution_success/hashmap/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "hashmap" +type = "bin" +authors = [""] + +[dependencies] \ No newline at end of file diff --git a/test_programs/execution_success/hashmap/src/main.nr b/test_programs/execution_success/hashmap/src/main.nr new file mode 100644 index 00000000000..733e967ad2d --- /dev/null +++ b/test_programs/execution_success/hashmap/src/main.nr @@ -0,0 +1,68 @@ +use dep::std::collections::map::HashMap; +use dep::std::hash::BuildHasherDefault; +use dep::std::hash::pedersen::PedersenHasher; + +fn main() { + test_sequential(); + test_multiple_same_insert(); + test_value_override(); +} + +// Insert, get, remove. +fn test_sequential() { + let mut hashmap: HashMap> = HashMap::default(); + assert(hashmap.len() == 0, "new hashmap should be empty"); + + let (key, value): (Field, Field) = (1, 2); + + hashmap.insert(key, value); + assert(hashmap.len() == 1, "hashmap after one insert should have a length of 1 element"); + + let retrieved_value = hashmap.get(key); + assert(retrieved_value.is_some(), "retrived value is none"); + + let retrieved_value = retrieved_value.unwrap_unchecked(); + assert(value == retrieved_value, "retrieved value does not match inserted"); + + hashmap.remove(key); + assert( + hashmap.len() == 0, "hashmap after one insert and corresponding removal should have a lenght of 1 element" + ); + + let retrieved_value = hashmap.get(key); + assert(retrieved_value.is_none(), "value has been removed, but is still available"); +} + +// Insert same pair several times. +fn test_multiple_same_insert() { + let mut hashmap: HashMap> = HashMap::default(); + assert(hashmap.len() == 0, "new hashmap should be empty"); + + let (key, value): (Field, Field) = (1, 2); + hashmap.insert(key, value); + hashmap.insert(key, value); + hashmap.insert(key, value); + assert(hashmap.len() == 1, "hashmap length is invalid"); + + let retrieved_value = hashmap.get(key); + assert(retrieved_value.is_some(), "retrieved value is none"); + let retrieved_value = retrieved_value.unwrap_unchecked(); + assert(retrieved_value == value, "retrieved value is not correct"); +} + +// Override value for existing pair. +fn test_value_override() { + let mut hashmap: HashMap> = HashMap::default(); + assert(hashmap.len() == 0, "new hashmap should be empty"); + + let (key, value): (Field, Field) = (1, 2); + hashmap.insert(key, value); + let value = 3; + hashmap.insert(key, value); + assert(hashmap.len() == 1, "hashmap length is invalid"); + + let retrieved_value = hashmap.get(key); + assert(retrieved_value.is_some(), "retrieved value is none"); + let retrieved_value = retrieved_value.unwrap_unchecked(); + assert(retrieved_value == 3, "retrieved value is not correct"); +} From b96bb1baf4c4aba3b673c2415a7af4a9831910a2 Mon Sep 17 00:00:00 2001 From: Nikita Masych Date: Fri, 2 Feb 2024 16:59:44 +0200 Subject: [PATCH 02/12] feat: added utility methods and testing --- noir_stdlib/src/collections/map.nr | 60 ++++++++++++++ .../execution_success/hashmap/src/main.nr | 79 ++++++++++++++++--- 2 files changed, 127 insertions(+), 12 deletions(-) diff --git a/noir_stdlib/src/collections/map.nr b/noir_stdlib/src/collections/map.nr index 00148c9c13a..e8fd272065a 100644 --- a/noir_stdlib/src/collections/map.nr +++ b/noir_stdlib/src/collections/map.nr @@ -83,6 +83,66 @@ impl HashMap { Self{_table, _len, _build_hasher} } + // Clears the map, removing all key-value pairs. + pub fn clear(&mut self){ + self._table = [Slot::default(); N]; + self._len = 0; + } + + // Returns true if the map contains a value for the specified key. + pub fn contains_key(self, key: K) -> bool + where + K: Hash + Eq, + B: BuildHasher, + H: Hasher + { + self.get(key).is_some() + } + + // Returns true if the map contains no elements. + pub fn is_empty(self) -> bool{ + self._len == 0 + } + + // Returns a vector of all active keys. + pub fn keys(self) -> Vec{ + let mut keys = Vec::new(); + for slot in self._table{ + if slot.is_valid(){ + let (key, _) = slot.key_value_unchecked(); + keys.push(key); + } + } + keys + } + + // Returns a vector of all active values. Not necessarily unique. + pub fn values(self) -> Vec{ + let mut values = Vec::new(); + for slot in self._table{ + if slot.is_valid(){ + let (_, value) = slot.key_value_unchecked(); + values.push(value); + } + } + values + } + + // Retains only the elements specified by the predicate. + pub fn retain(&mut self, f: fn(K, V) -> bool){ + for index in 0..N{ + let mut slot = self._table[index]; + if slot.is_valid(){ + let (key, value) = slot.key_value_unchecked(); + if !f(key, value){ + slot.mark_deleted(); + self._len -= 1; + self._table[index] = slot; + } + } + } + } + fn hash(self, key: K) -> u64 where K: Hash, diff --git a/test_programs/execution_success/hashmap/src/main.nr b/test_programs/execution_success/hashmap/src/main.nr index 733e967ad2d..de393ced7c5 100644 --- a/test_programs/execution_success/hashmap/src/main.nr +++ b/test_programs/execution_success/hashmap/src/main.nr @@ -4,39 +4,35 @@ use dep::std::hash::pedersen::PedersenHasher; fn main() { test_sequential(); - test_multiple_same_insert(); + test_multiple_equal_insert(); test_value_override(); + test_insert_and_methods(); + test_retain(); } // Insert, get, remove. fn test_sequential() { let mut hashmap: HashMap> = HashMap::default(); - assert(hashmap.len() == 0, "new hashmap should be empty"); + assert(hashmap.is_empty(), "new hashmap should be empty"); let (key, value): (Field, Field) = (1, 2); hashmap.insert(key, value); assert(hashmap.len() == 1, "hashmap after one insert should have a length of 1 element"); - let retrieved_value = hashmap.get(key); assert(retrieved_value.is_some(), "retrived value is none"); - let retrieved_value = retrieved_value.unwrap_unchecked(); assert(value == retrieved_value, "retrieved value does not match inserted"); - hashmap.remove(key); - assert( - hashmap.len() == 0, "hashmap after one insert and corresponding removal should have a lenght of 1 element" - ); - + assert(hashmap.is_empty(), "hashmap after one insert and corresponding removal should be empty"); let retrieved_value = hashmap.get(key); assert(retrieved_value.is_none(), "value has been removed, but is still available"); } // Insert same pair several times. -fn test_multiple_same_insert() { +fn test_multiple_equal_insert() { let mut hashmap: HashMap> = HashMap::default(); - assert(hashmap.len() == 0, "new hashmap should be empty"); + assert(hashmap.is_empty(), "new hashmap should be empty"); let (key, value): (Field, Field) = (1, 2); hashmap.insert(key, value); @@ -53,7 +49,7 @@ fn test_multiple_same_insert() { // Override value for existing pair. fn test_value_override() { let mut hashmap: HashMap> = HashMap::default(); - assert(hashmap.len() == 0, "new hashmap should be empty"); + assert(hashmap.is_empty(), "new hashmap should be empty"); let (key, value): (Field, Field) = (1, 2); hashmap.insert(key, value); @@ -66,3 +62,62 @@ fn test_value_override() { let retrieved_value = retrieved_value.unwrap_unchecked(); assert(retrieved_value == 3, "retrieved value is not correct"); } + +// Insert several distinct pairs and test auxiliary methods. +fn test_insert_and_methods() { + let mut hashmap: HashMap> = HashMap::default(); + assert(hashmap.is_empty(), "new hashmap should be empty"); + + let (key, value) = (5, 11); + hashmap.insert(key, value); + let (key, value) = (2, 13); + hashmap.insert(key, value); + let (key, value) = (3, 7); + hashmap.insert(key, value); + + assert(hashmap.len() == 3, "hashmap length should be 3"); + + assert(hashmap.contains_key(5), "hashmap does not contain key 5"); + assert(hashmap.contains_key(2), "hashmap does not contain key 2"); + assert(hashmap.contains_key(3), "hashmap does not contain key 3"); + + let mut keys = hashmap.keys(); + let mut accumulator = 1; + for _ in 0..3 { + let key = keys.pop(); + assert((key == 5) | (key == 2) | (key == 3), "found unexpected key"); + accumulator *= key; + } + assert(accumulator == 30, "got invalid set of keys"); + + let mut values = hashmap.values(); + let mut accumulator = 1; + for _ in 0..3 { + let value = values.pop(); + assert((value == 11) | (value == 13) | (value == 7), "found unexpected value"); + accumulator *= value; + } + assert(accumulator == 1001, "got invalid set of values"); + + hashmap.clear(); + assert(hashmap.is_empty(), "hashmap after clear should be empty"); +} + +// Insert several pairs and test retaining. +fn test_retain() { + let mut hashmap: HashMap> = HashMap::default(); + assert(hashmap.is_empty(), "new hashmap should be empty"); + + let (key, value) = (5, 11); + hashmap.insert(key, value); + let (key, value) = (2, 13); + hashmap.insert(key, value); + let (key, value) = (11, 5); + hashmap.insert(key, value); + + let predicate = |key: Field, value: Field| -> bool {key * value == 55}; + hashmap.retain(predicate); + + assert(hashmap.len() == 2, "HashMap should have retained 2 elements"); + assert(hashmap.get(2).is_none(), "pair should have been removed, since it does not match predicate"); +} From dcb7de15090d1dec40d99dbfe093bee49892505b Mon Sep 17 00:00:00 2001 From: Nikita Masych Date: Fri, 2 Feb 2024 18:06:57 +0200 Subject: [PATCH 03/12] feat: added equality test --- .../execution_success/hashmap/src/main.nr | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/test_programs/execution_success/hashmap/src/main.nr b/test_programs/execution_success/hashmap/src/main.nr index de393ced7c5..0c178ec8776 100644 --- a/test_programs/execution_success/hashmap/src/main.nr +++ b/test_programs/execution_success/hashmap/src/main.nr @@ -8,6 +8,7 @@ fn main() { test_value_override(); test_insert_and_methods(); test_retain(); + test_hashmaps_equality(); } // Insert, get, remove. @@ -121,3 +122,29 @@ fn test_retain() { assert(hashmap.len() == 2, "HashMap should have retained 2 elements"); assert(hashmap.get(2).is_none(), "pair should have been removed, since it does not match predicate"); } + +// Equality trait check. +fn test_hashmaps_equality() { + let mut hashmap_1: HashMap> = HashMap::default(); + let mut hashmap_2: HashMap> = HashMap::default(); + + let (key, value) = (5, 11); + hashmap_1.insert(key, value); + let (key, value) = (2, 13); + hashmap_1.insert(key, value); + let (key, value) = (11, 5); + hashmap_1.insert(key, value); + + let (key, value) = (5, 11); + hashmap_2.insert(key, value); + let (key, value) = (2, 13); + hashmap_2.insert(key, value); + let (key, value) = (11, 5); + hashmap_2.insert(key, value); + + assert(hashmap_1 == hashmap_2, "HashMaps should be equal"); + + hashmap_2.remove(11); + + assert(hashmap_1 != hashmap_2, "HashMaps should not be equal"); +} From 8178e2b7676897f4d3480d11c6e37465aa97be7d Mon Sep 17 00:00:00 2001 From: Nikita Masych Date: Fri, 2 Feb 2024 19:36:08 +0200 Subject: [PATCH 04/12] chore: removed hasher wrapper --- noir_stdlib/src/collections/map.nr | 2 +- noir_stdlib/src/hash.nr | 73 ++++-------------------------- noir_stdlib/src/hash/pedersen.nr | 5 +- 3 files changed, 13 insertions(+), 67 deletions(-) diff --git a/noir_stdlib/src/collections/map.nr b/noir_stdlib/src/collections/map.nr index e8fd272065a..e62e6026423 100644 --- a/noir_stdlib/src/collections/map.nr +++ b/noir_stdlib/src/collections/map.nr @@ -149,7 +149,7 @@ impl HashMap { B: BuildHasher, H: Hasher { - let mut hasher = self._build_hasher.build_wrapper(); + let mut hasher = self._build_hasher.build_hasher(); key.hash(&mut hasher); hasher.finish() as u64 } diff --git a/noir_stdlib/src/hash.nr b/noir_stdlib/src/hash.nr index 285ade9b356..121a32ec5f3 100644 --- a/noir_stdlib/src/hash.nr +++ b/noir_stdlib/src/hash.nr @@ -78,17 +78,14 @@ pub fn poseidon2_permutation(_input: [u8; N], _state_length: u32) -> [u8; N] #[foreign(sha256_compression)] pub fn sha256_compression(_input: [u32; 16], _state: [u32; 8]) -> [u32; 8] {} -// Generic hashing support. +// Generic hashing support. // Partially ported and impacted by rust. // Hash trait shall be implemented per type. -// We use HasherWrapper since we desire to have `&mut Hasher` in `Hash` trait functions prototype, -// but Noir can not correctly infer generic into pointer receiver, ergo such a hack is utilized -// and shall be removed upon generics improvement. trait Hash{ - fn hash(self, state: &mut HasherWrapper) where H: Hasher; + fn hash(self, state: &mut H) where H: Hasher; - fn hash_slice(data: [Self], state: &mut HasherWrapper) where H: Hasher{ + fn hash_slice(data: [Self], state: &mut H) where H: Hasher{ for piece in data{ piece.hash(state); } @@ -98,88 +95,38 @@ trait Hash{ // Hasher trait shall be implemented by algorithms to provide hash-agnostic means. trait Hasher{ fn finish(self) -> Field; - - fn write(self, input: [Field]) -> Self; + + fn write(&mut self, input: [Field]); } // BuildHasher is a factory trait, responsible for production of specific Hasher. trait BuildHasher where H: Hasher{ fn build_hasher(self) -> H; - - fn build_wrapper(self) -> HasherWrapper{ - let hasher = self.build_hasher(); - HasherWrapper::new(hasher) - } -} - -struct HasherWrapper{ - _inner: H -} - -impl HasherWrapper -{ - pub fn new(_inner: H) -> Self{ - HasherWrapper{_inner} - } - - pub fn unwrap(self) -> H{ - self._inner - } - - pub fn finish(self) -> Field - where - H: Hasher - { - self._inner.finish() - } - - pub fn write(&mut self, input: [Field]) - where - H: Hasher - { - self._inner = self._inner.write(input); - } -} - -impl Default for HasherWrapper -where - H: Hasher + Default -{ - fn default() -> Self{ - HasherWrapper{ - _inner: H::default() - } - } } struct BuildHasherDefault; impl BuildHasher for BuildHasherDefault -where +where H: Hasher + Default { fn build_hasher(self) -> H{ H::default() } - - fn build_wrapper(self) -> HasherWrapper{ - let wrapper: HasherWrapper = HasherWrapper::default(); - wrapper - } } impl Default for BuildHasherDefault -where +where H: Hasher + Default { fn default() -> Self{ BuildHasherDefault{} - } + } } impl Hash for Field{ - fn hash(self, state: &mut HasherWrapper) where H: Hasher{ + fn hash(self, state: &mut H) where H: Hasher{ let input: [Field] = [self]; - state.write(input); + H::write(state, input); } } diff --git a/noir_stdlib/src/hash/pedersen.nr b/noir_stdlib/src/hash/pedersen.nr index 85c3b9b4b85..ace6851099d 100644 --- a/noir_stdlib/src/hash/pedersen.nr +++ b/noir_stdlib/src/hash/pedersen.nr @@ -10,9 +10,8 @@ impl Hasher for PedersenHasher { pedersen_hash(self._state) } - fn write(self, input: [Field]) -> Self{ - let _state = self._state.append(input); - PedersenHasher{_state} + fn write(&mut self, input: [Field]){ + self._state = self._state.append(input); } } From 7df33dbb0484e5f2de46fb4048499620f9a9e70b Mon Sep 17 00:00:00 2001 From: Nikita Masych Date: Mon, 5 Feb 2024 16:27:50 +0200 Subject: [PATCH 05/12] fix(stdlib): applied hashmap review notes; improvements in testing --- noir_stdlib/src/collections/map.nr | 245 ++++++++++++++---- noir_stdlib/src/hash.nr | 8 +- .../hashmap_load_factor/Nargo.toml | 6 + .../hashmap_load_factor/Prover.toml | 26 ++ .../hashmap_load_factor/src/main.nr | 35 +++ .../execution_success/hashmap/Prover.toml | 26 ++ .../execution_success/hashmap/src/main.nr | 178 ++++++------- 7 files changed, 369 insertions(+), 155 deletions(-) create mode 100644 test_programs/compile_failure/hashmap_load_factor/Nargo.toml create mode 100644 test_programs/compile_failure/hashmap_load_factor/Prover.toml create mode 100644 test_programs/compile_failure/hashmap_load_factor/src/main.nr create mode 100644 test_programs/execution_success/hashmap/Prover.toml diff --git a/noir_stdlib/src/collections/map.nr b/noir_stdlib/src/collections/map.nr index e62e6026423..e2568fcd6f9 100644 --- a/noir_stdlib/src/collections/map.nr +++ b/noir_stdlib/src/collections/map.nr @@ -2,16 +2,14 @@ use crate::cmp::Eq; use crate::collections::vec::Vec; use crate::option::Option; use crate::default::Default; +use crate::unsafe::zeroed; use crate::hash::{Hash, Hasher, BuildHasher}; -// Generic trait for one-directional key -> value data structure. -// May be implemented by either hash tables or graphs for instance. -trait Map { - fn len(self) -> u64; - fn get(self, key: K) -> Option; - fn insert(&mut self, key: K, value: V); - fn remove(&mut self, key: K); -} +// We use load factor α_max = 0.75. +// Upon exceeding it, assert will fail in order to inform the user +// about performance degradation, so that he can adjust the capacity. +global MAX_LOAD_FACTOR_NUMERATOR = 3; +global MAX_LOAD_FACTOR_DEN0MINATOR = 4; // Hash table with open addressing and quadratic probing. // Size of the underlying table must be known at compile time. @@ -27,8 +25,8 @@ struct HashMap { } // Data unit in the HashMap table. -// In case Noir would support enums in future, this -// better to be refactored to use it with three states: +// In case Noir adds support for enums in the future, this +// should be refactored to have three states: // 1. (key, value) // 2. (empty) // 3. (deleted) @@ -65,13 +63,16 @@ impl Slot{ } // Shall not override `_key_value` with Option::none(), - // because we must we able to differentiate empty + // because we must be able to differentiate empty // and deleted slots for lookup. fn mark_deleted(&mut self){ self._is_deleted = true; } } +// While conducting lookup, we iterate attempt from 0 to N - 1 due to heuristic, +// that if we have went that far without finding desired, +// it is very unlikely to be after - performance will be heavily degraded. impl HashMap { // Creates a new instance of HashMap with specified BuildHasher. pub fn with_hasher(_build_hasher: B) -> Self @@ -104,30 +105,146 @@ impl HashMap { self._len == 0 } - // Returns a vector of all active keys. - pub fn keys(self) -> Vec{ - let mut keys = Vec::new(); + // Returns array of all active key-value pairs. + // Array size is restricted to N because of + // compile time limitations and nested slices temporary ban, + // hovewer first len() positioned with pairs + // and afterwards all elements are zeroed(). + pub fn iter(self) -> [(K, V); N]{ + let mut pairs = [zeroed(); N]; + let mut i = 0; + + for slot in self._table{ + if slot.is_valid(){ + pairs[i] = slot.key_value_unchecked(); + i += 1; + } + } + + assert( + i == self._len, + f"Must have iterated {self._len} times, instead did {i}." + ); + + pairs + } + + // Returns array of all active keys. + // Array size is restricted to N because of + // compile time limitations and nested slices temporary ban, + // hovewer first len() positioned with keys + // and afterwards all elements are zeroed(). + pub fn keys(self) -> [K; N]{ + let mut keys = [zeroed(); N]; + let mut i = 0; + for slot in self._table{ if slot.is_valid(){ let (key, _) = slot.key_value_unchecked(); - keys.push(key); + keys[i] = key; + i += 1; } } + + assert( + i == self._len, + f"Must have iterated {self._len} times, instead did {i}." + ); + keys - } + } + + // Returns array of all active values. + // Array size is restricted to N because of + // compile time limitations and nested slices temporary ban, + // hovewer first len() positioned with values + // and afterwards all elements are zeroed(). + pub fn values(self) -> [V; N]{ + let mut values = [zeroed(); N]; + let mut i = 0; - // Returns a vector of all active values. Not necessarily unique. - pub fn values(self) -> Vec{ - let mut values = Vec::new(); for slot in self._table{ if slot.is_valid(){ let (_, value) = slot.key_value_unchecked(); - values.push(value); + values[i] = value; + i += 1; } } + + assert( + i == self._len, + f"Must have iterated {self._len} times, instead did {i}." + ); + values + } + + // For each key-value pair applies mutator function. + pub fn iter_mut(&mut self, f: fn(K, V) -> (K, V)) + where + K: Eq + Hash, + B: BuildHasher, + H: Hasher + { + let mut pairs = self.iter(); + for i in 0..N{ + if i < self._len{ + let (key, value) = pairs[i]; + pairs[i] = f(key, value); + } + } + + let mut new_map = HashMap::with_hasher(self._build_hasher); + + for i in 0..N{ + if i < self._len{ + let (key, value) = pairs[i]; + new_map.insert(key, value); + } + } + + self._table = new_map._table; } + // For each key applies mutator function. + pub fn mut_keys(&mut self, f: fn(K) -> K) + where + K: Eq + Hash, + B: BuildHasher, + H: Hasher + { + let mut pairs = self.iter(); + for i in 0..N{ + if i < self._len{ + let (key, value) = pairs[i]; + pairs[i] = (f(key), value); + } + } + + let mut new_map = HashMap::with_hasher(self._build_hasher); + + for i in 0..N{ + if i < self._len{ + let (key, value) = pairs[i]; + new_map.insert(key, value); + } + } + + self._table = new_map._table; + } + + // For each value applies mutator function. + pub fn mut_values(&mut self, f: fn(V) -> V) { + for i in 0..N{ + let mut slot = self._table[i]; + if slot.is_valid(){ + let (key, value) = slot.key_value_unchecked(); + slot.set(key, f(value)); + self._table[i] = slot; + } + } + } + // Retains only the elements specified by the predicate. pub fn retain(&mut self, f: fn(K, V) -> bool){ for index in 0..N{ @@ -143,40 +260,18 @@ impl HashMap { } } - fn hash(self, key: K) -> u64 + // Amount of active key-value pairs. + pub fn len(self) -> u64{ + self._len + } + + // Get the value by key. If it does not exist, returns none(). + pub fn get(self, key: K) -> Option where - K: Hash, + K: Eq + Hash, B: BuildHasher, H: Hasher { - let mut hasher = self._build_hasher.build_hasher(); - key.hash(&mut hasher); - hasher.finish() as u64 - } - - // Probing scheme: quadratic function. - // We use 0.5 constant near variadic attempt and attempt^2 monomials. - // This ensures good uniformity of distribution for table sizes - // equal to prime numbers or powers of two. - fn quadratic_probe(self, hash: u64, attempt: u64) -> u64 { - (hash + (attempt + attempt * attempt) / 2 ) % N - } -} - -// While conducting lookup, we iterate attempt from 0 to N - 1 due to heuristic, -// that if we have went that far without finding desired, -// it is very unlikely to be after - performance in that case would be drastical. -impl Map for HashMap -where - K: Eq + Hash, - B: BuildHasher, - H: Hasher -{ - fn len(self) -> u64{ - self._len - } - - fn get(self, key: K) -> Option{ let mut result = Option::none(); let hash = self.hash(key); @@ -201,7 +296,15 @@ where result } - fn insert(&mut self, key: K, value: V){ + // Insert key-value pair. In case key was already present, value is overridden. + pub fn insert(&mut self, key: K, value: V) + where + K: Eq + Hash, + B: BuildHasher, + H: Hasher + { + self.assert_load_factor(); + let hash = self.hash(key); let mut break = false; @@ -231,7 +334,13 @@ where } } - fn remove(&mut self, key: K){ + // Remove key-value entry. If key is not present, HashMap remains unchanged. + pub fn remove(&mut self, key: K) + where + K: Eq + Hash, + B: BuildHasher, + H: Hasher + { let hash = self.hash(key); let mut break = false; @@ -253,6 +362,38 @@ where } } } + + // Apply HashMap's hasher onto key to obtain pre-hash for probing. + fn hash(self, key: K) -> u64 + where + K: Hash, + B: BuildHasher, + H: Hasher + { + let mut hasher = self._build_hasher.build_hasher(); + key.hash(&mut hasher); + hasher.finish() as u64 + } + + // Probing scheme: quadratic function. + // We use 0.5 constant near variadic attempt and attempt^2 monomials. + // This ensures good uniformity of distribution for table sizes + // equal to prime numbers or powers of two. + fn quadratic_probe(self, hash: u64, attempt: u64) -> u64 { + (hash + (attempt + attempt * attempt) / 2 ) % N + } + + // Amount of elements in the table in relation to available slots exceeds α_max. + // To avoid division with potential incorrectness due to floating point values, + // we conduct cross-multiplication. + // n / m >= MAX_LOAD_FACTOR_NUMERATOR / MAX_LOAD_FACTOR_DEN0MINATOR + // n * MAX_LOAD_FACTOR_NUMERATOR >= m * MAX_LOAD_FACTOR_DEN0MINATOR + fn assert_load_factor(self){ + let lhs = self._len * MAX_LOAD_FACTOR_DEN0MINATOR; + let rhs = self._table.len() as u64 * MAX_LOAD_FACTOR_NUMERATOR; + let exceeded = lhs >= rhs; + assert(!exceeded, "Load factor is exceeded, consider increasing the capacity."); + } } // Equality class on HashMap has to test that they have diff --git a/noir_stdlib/src/hash.nr b/noir_stdlib/src/hash.nr index 121a32ec5f3..a35e62edd17 100644 --- a/noir_stdlib/src/hash.nr +++ b/noir_stdlib/src/hash.nr @@ -84,15 +84,10 @@ pub fn sha256_compression(_input: [u32; 16], _state: [u32; 8]) -> [u32; 8] {} // Hash trait shall be implemented per type. trait Hash{ fn hash(self, state: &mut H) where H: Hasher; - - fn hash_slice(data: [Self], state: &mut H) where H: Hasher{ - for piece in data{ - piece.hash(state); - } - } } // Hasher trait shall be implemented by algorithms to provide hash-agnostic means. +// TODO: consider making the types generic here ([u8], [Field], etc.) trait Hasher{ fn finish(self) -> Field; @@ -124,6 +119,7 @@ where } } +// TODO: add implementations for the remainder of primitive types. impl Hash for Field{ fn hash(self, state: &mut H) where H: Hasher{ let input: [Field] = [self]; diff --git a/test_programs/compile_failure/hashmap_load_factor/Nargo.toml b/test_programs/compile_failure/hashmap_load_factor/Nargo.toml new file mode 100644 index 00000000000..92da5a357f4 --- /dev/null +++ b/test_programs/compile_failure/hashmap_load_factor/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "hashmap_load_factor" +type = "bin" +authors = [""] + +[dependencies] \ No newline at end of file diff --git a/test_programs/compile_failure/hashmap_load_factor/Prover.toml b/test_programs/compile_failure/hashmap_load_factor/Prover.toml new file mode 100644 index 00000000000..e54319c61e9 --- /dev/null +++ b/test_programs/compile_failure/hashmap_load_factor/Prover.toml @@ -0,0 +1,26 @@ +# Expected 6 key-value entries for hashmap capacity of 8. +# These must be distinct (both key-to-key, and value-to-value) for correct testing. + +[[input]] +key = 2 +value = 17 + +[[input]] +key = 3 +value = 19 + +[[input]] +key = 5 +value = 23 + +[[input]] +key = 7 +value = 29 + +[[input]] +key = 11 +value = 31 + +[[input]] +key = 41 +value = 43 \ No newline at end of file diff --git a/test_programs/compile_failure/hashmap_load_factor/src/main.nr b/test_programs/compile_failure/hashmap_load_factor/src/main.nr new file mode 100644 index 00000000000..ade43f898e1 --- /dev/null +++ b/test_programs/compile_failure/hashmap_load_factor/src/main.nr @@ -0,0 +1,35 @@ +use dep::std::collections::map::HashMap; +use dep::std::hash::BuildHasherDefault; +use dep::std::hash::pedersen::PedersenHasher; + +struct Entry{ + key: Field, + value: Field +} + +global HASHMAP_CAP = 8; +global HASHMAP_LEN = 6; + +fn allocate_hashmap() -> HashMap> { + HashMap::default() +} + +fn main(input: [Entry; HASHMAP_LEN]) { + test_load_factor(input); +} + +// In this test we exceed load factor: +// α_max = 0.75, thus for capacity of 8 and lenght of 6 +// insertion of new unique key (7-th) should throw assertion error. +fn test_load_factor(input: [Entry; HASHMAP_LEN]) { + let mut hashmap = allocate_hashmap(); + + for entry in input { + hashmap.insert(entry.key, entry.value); + } + + // We use prime numbers for testing, + // therefore it is guaranteed that doubling key we get unique value. + let key = input[0].key * 2; + hashmap.insert(key, input[0].value); +} diff --git a/test_programs/execution_success/hashmap/Prover.toml b/test_programs/execution_success/hashmap/Prover.toml new file mode 100644 index 00000000000..84d4c0733e4 --- /dev/null +++ b/test_programs/execution_success/hashmap/Prover.toml @@ -0,0 +1,26 @@ +# Input: 6 key-value entries for hashmap capacity of 8. +# These must be distinct (both key-to-key, and value-to-value) for correct testing. + +[[input]] +key = 2 +value = 17 + +[[input]] +key = 3 +value = 19 + +[[input]] +key = 5 +value = 23 + +[[input]] +key = 7 +value = 29 + +[[input]] +key = 11 +value = 31 + +[[input]] +key = 41 +value = 43 \ No newline at end of file diff --git a/test_programs/execution_success/hashmap/src/main.nr b/test_programs/execution_success/hashmap/src/main.nr index 0c178ec8776..441509b4ac9 100644 --- a/test_programs/execution_success/hashmap/src/main.nr +++ b/test_programs/execution_success/hashmap/src/main.nr @@ -2,112 +2,105 @@ use dep::std::collections::map::HashMap; use dep::std::hash::BuildHasherDefault; use dep::std::hash::pedersen::PedersenHasher; -fn main() { - test_sequential(); - test_multiple_equal_insert(); - test_value_override(); - test_insert_and_methods(); +struct Entry{ + key: Field, + value: Field +} + +global HASHMAP_CAP = 8; +global HASHMAP_LEN = 6; + +fn allocate_hashmap() -> HashMap> { + HashMap::default() +} + +fn main(input: [Entry; HASHMAP_LEN]) { + test_sequential(input[0].key, input[0].value); + test_multiple_equal_insert(input[1].key, input[1].value); + test_value_override(input[2].key, input[2].value, input[3].value); + test_insert_and_methods(input); + test_hashmaps_equality(input); test_retain(); - test_hashmaps_equality(); + + // FAIL + test_load_factor(input); } // Insert, get, remove. -fn test_sequential() { - let mut hashmap: HashMap> = HashMap::default(); - assert(hashmap.is_empty(), "new hashmap should be empty"); - - let (key, value): (Field, Field) = (1, 2); +fn test_sequential(key: Field, value: Field) { + let mut hashmap = allocate_hashmap(); + assert(hashmap.is_empty(), "New HashMap should be empty."); hashmap.insert(key, value); - assert(hashmap.len() == 1, "hashmap after one insert should have a length of 1 element"); - let retrieved_value = hashmap.get(key); - assert(retrieved_value.is_some(), "retrived value is none"); - let retrieved_value = retrieved_value.unwrap_unchecked(); - assert(value == retrieved_value, "retrieved value does not match inserted"); + assert(hashmap.len() == 1, "HashMap after one insert should have a length of 1 element."); + + let got = hashmap.get(key); + assert(got.is_some(), "Got none value."); + let got = got.unwrap_unchecked(); + assert(value == got, f"Inserted {value} but got {got} for the same key."); + hashmap.remove(key); - assert(hashmap.is_empty(), "hashmap after one insert and corresponding removal should be empty"); - let retrieved_value = hashmap.get(key); - assert(retrieved_value.is_none(), "value has been removed, but is still available"); + assert(hashmap.is_empty(), "HashMap after one insert and corresponding removal should be empty."); + let got = hashmap.get(key); + assert(got.is_none(), "Value has been removed, but is still available (not none)."); } // Insert same pair several times. -fn test_multiple_equal_insert() { - let mut hashmap: HashMap> = HashMap::default(); - assert(hashmap.is_empty(), "new hashmap should be empty"); +fn test_multiple_equal_insert(key: Field, value: Field) { + let mut hashmap = allocate_hashmap(); + assert(hashmap.is_empty(), "New HashMap should be empty."); - let (key, value): (Field, Field) = (1, 2); - hashmap.insert(key, value); - hashmap.insert(key, value); - hashmap.insert(key, value); - assert(hashmap.len() == 1, "hashmap length is invalid"); + for _ in 0..HASHMAP_LEN { + hashmap.insert(key, value); + } + + let len = hashmap.len(); + assert(len == 1, f"HashMap length must be 1, got {len}."); - let retrieved_value = hashmap.get(key); - assert(retrieved_value.is_some(), "retrieved value is none"); - let retrieved_value = retrieved_value.unwrap_unchecked(); - assert(retrieved_value == value, "retrieved value is not correct"); + let got = hashmap.get(key); + assert(got.is_some(), "Got none value."); + let got = got.unwrap_unchecked(); + assert(value == got, f"Inserted {value} but got {got} for the same key."); } // Override value for existing pair. -fn test_value_override() { - let mut hashmap: HashMap> = HashMap::default(); - assert(hashmap.is_empty(), "new hashmap should be empty"); +fn test_value_override(key: Field, value: Field, new_value: Field) { + let mut hashmap = allocate_hashmap(); + assert(hashmap.is_empty(), "New hashmap should be empty."); - let (key, value): (Field, Field) = (1, 2); hashmap.insert(key, value); - let value = 3; - hashmap.insert(key, value); - assert(hashmap.len() == 1, "hashmap length is invalid"); + hashmap.insert(key, new_value); + assert(hashmap.len() == 1, "HashMap length is invalid."); - let retrieved_value = hashmap.get(key); - assert(retrieved_value.is_some(), "retrieved value is none"); - let retrieved_value = retrieved_value.unwrap_unchecked(); - assert(retrieved_value == 3, "retrieved value is not correct"); + let got = hashmap.get(key); + assert(got.is_some(), "Got none value."); + let got = got.unwrap_unchecked(); + assert(got == new_value, f"Expected {new_value}, but got {got}."); } // Insert several distinct pairs and test auxiliary methods. -fn test_insert_and_methods() { - let mut hashmap: HashMap> = HashMap::default(); - assert(hashmap.is_empty(), "new hashmap should be empty"); - - let (key, value) = (5, 11); - hashmap.insert(key, value); - let (key, value) = (2, 13); - hashmap.insert(key, value); - let (key, value) = (3, 7); - hashmap.insert(key, value); +fn test_insert_and_methods(input: [Entry; HASHMAP_LEN]) { + let mut hashmap = allocate_hashmap(); + assert(hashmap.is_empty(), "New HashMap should be empty."); - assert(hashmap.len() == 3, "hashmap length should be 3"); + for entry in input { + hashmap.insert(entry.key, entry.value); + } - assert(hashmap.contains_key(5), "hashmap does not contain key 5"); - assert(hashmap.contains_key(2), "hashmap does not contain key 2"); - assert(hashmap.contains_key(3), "hashmap does not contain key 3"); + assert(hashmap.len() == HASHMAP_LEN, "hashmap.len() does not match input lenght."); - let mut keys = hashmap.keys(); - let mut accumulator = 1; - for _ in 0..3 { - let key = keys.pop(); - assert((key == 5) | (key == 2) | (key == 3), "found unexpected key"); - accumulator *= key; + for entry in input { + assert(hashmap.contains_key(entry.key), f"Not found inserted key {entry.key}."); } - assert(accumulator == 30, "got invalid set of keys"); - - let mut values = hashmap.values(); - let mut accumulator = 1; - for _ in 0..3 { - let value = values.pop(); - assert((value == 11) | (value == 13) | (value == 7), "found unexpected value"); - accumulator *= value; - } - assert(accumulator == 1001, "got invalid set of values"); hashmap.clear(); - assert(hashmap.is_empty(), "hashmap after clear should be empty"); + assert(hashmap.is_empty(), "HashMap after clear() should be empty."); } // Insert several pairs and test retaining. fn test_retain() { - let mut hashmap: HashMap> = HashMap::default(); - assert(hashmap.is_empty(), "new hashmap should be empty"); + let mut hashmap = allocate_hashmap(); + assert(hashmap.is_empty(), "New HashMap should be empty."); let (key, value) = (5, 11); hashmap.insert(key, value); @@ -119,32 +112,23 @@ fn test_retain() { let predicate = |key: Field, value: Field| -> bool {key * value == 55}; hashmap.retain(predicate); - assert(hashmap.len() == 2, "HashMap should have retained 2 elements"); - assert(hashmap.get(2).is_none(), "pair should have been removed, since it does not match predicate"); + assert(hashmap.len() == 2, "HashMap should have retained 2 elements."); + assert(hashmap.get(2).is_none(), "Pair should have been removed, since it does not match predicate."); } // Equality trait check. -fn test_hashmaps_equality() { - let mut hashmap_1: HashMap> = HashMap::default(); - let mut hashmap_2: HashMap> = HashMap::default(); - - let (key, value) = (5, 11); - hashmap_1.insert(key, value); - let (key, value) = (2, 13); - hashmap_1.insert(key, value); - let (key, value) = (11, 5); - hashmap_1.insert(key, value); +fn test_hashmaps_equality(input: [Entry; HASHMAP_LEN]) { + let mut hashmap_1 = allocate_hashmap(); + let mut hashmap_2 = allocate_hashmap(); - let (key, value) = (5, 11); - hashmap_2.insert(key, value); - let (key, value) = (2, 13); - hashmap_2.insert(key, value); - let (key, value) = (11, 5); - hashmap_2.insert(key, value); + for entry in input { + hashmap_1.insert(entry.key, entry.value); + hashmap_2.insert(entry.key, entry.value); + } - assert(hashmap_1 == hashmap_2, "HashMaps should be equal"); + assert(hashmap_1 == hashmap_2, "HashMaps should be equal."); - hashmap_2.remove(11); + hashmap_2.remove(input[0].key); - assert(hashmap_1 != hashmap_2, "HashMaps should not be equal"); -} + assert(hashmap_1 != hashmap_2, "HashMaps should not be equal."); +} \ No newline at end of file From 9eb689c49d47e21e2050b8ea8590958e14ba7484 Mon Sep 17 00:00:00 2001 From: Nikita Masych Date: Tue, 13 Feb 2024 14:03:03 +0200 Subject: [PATCH 06/12] fix: removed failing test from execution_success --- test_programs/execution_success/hashmap/src/main.nr | 3 --- 1 file changed, 3 deletions(-) diff --git a/test_programs/execution_success/hashmap/src/main.nr b/test_programs/execution_success/hashmap/src/main.nr index 441509b4ac9..45e76ad84a9 100644 --- a/test_programs/execution_success/hashmap/src/main.nr +++ b/test_programs/execution_success/hashmap/src/main.nr @@ -21,9 +21,6 @@ fn main(input: [Entry; HASHMAP_LEN]) { test_insert_and_methods(input); test_hashmaps_equality(input); test_retain(); - - // FAIL - test_load_factor(input); } // Insert, get, remove. From f1708d024af29976e0b4adbb1916a6cf66a8d186 Mon Sep 17 00:00:00 2001 From: Nikita Masych Date: Tue, 13 Feb 2024 15:02:44 +0200 Subject: [PATCH 07/12] feat: added hashmap iterators tests --- .../execution_success/hashmap/src/main.nr | 56 ++++++++++++++++++- .../execution_success/hashmap/src/utils.nr | 10 ++++ 2 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 test_programs/execution_success/hashmap/src/utils.nr diff --git a/test_programs/execution_success/hashmap/src/main.nr b/test_programs/execution_success/hashmap/src/main.nr index 45e76ad84a9..5bac982a1b2 100644 --- a/test_programs/execution_success/hashmap/src/main.nr +++ b/test_programs/execution_success/hashmap/src/main.nr @@ -1,7 +1,11 @@ +mod utils; + use dep::std::collections::map::HashMap; use dep::std::hash::BuildHasherDefault; use dep::std::hash::pedersen::PedersenHasher; +use utils::cut; + struct Entry{ key: Field, value: Field @@ -10,6 +14,9 @@ struct Entry{ global HASHMAP_CAP = 8; global HASHMAP_LEN = 6; +global FIELD_CMP = |a: Field, b: Field| a.lt(b); +global PAIR_CMP = |a: (Field, Field), b: (Field, Field)| a.0.lt(b.0); + fn allocate_hashmap() -> HashMap> { HashMap::default() } @@ -21,6 +28,8 @@ fn main(input: [Entry; HASHMAP_LEN]) { test_insert_and_methods(input); test_hashmaps_equality(input); test_retain(); + test_iterators(); + test_mut_iterators(); } // Insert, get, remove. @@ -128,4 +137,49 @@ fn test_hashmaps_equality(input: [Entry; HASHMAP_LEN]) { hashmap_2.remove(input[0].key); assert(hashmap_1 != hashmap_2, "HashMaps should not be equal."); -} \ No newline at end of file +} + +// Test Iter, Keys, Values. +fn test_iterators() { + let mut hashmap = allocate_hashmap(); + + hashmap.insert(2, 3); + hashmap.insert(5, 7); + hashmap.insert(11, 13); + + let keys: [Field; 3] = cut(hashmap.keys()).sort_via(FIELD_CMP); + let values: [Field; 3] = cut(hashmap.values()).sort_via(FIELD_CMP); + let entries: [(Field, Field); 3] = cut(hashmap.iter()).sort_via(PAIR_CMP); + + assert(keys == [2, 5, 11], "Got incorrect iteration of keys."); + assert(values == [3, 7, 13], "Got incorrect iteration of values."); + assert(entries == [(2, 3), (5, 7), (11, 13)], "Got incorrect iteration of entries."); +} + +// Test mutable Iter, Keys, Values. +fn test_mut_iterators() { + let mut hashmap = allocate_hashmap(); + + hashmap.insert(2, 3); + hashmap.insert(5, 7); + hashmap.insert(11, 13); + + let f = |k: Field| -> Field{ k * 3}; + hashmap.mut_keys(f); + + let f = |v: Field| -> Field{ v * 5}; + hashmap.mut_values(f); + + let keys: [Field; 3] = cut(hashmap.keys()).sort_via(FIELD_CMP); + let values: [Field; 3] = cut(hashmap.values()).sort_via(FIELD_CMP); + + assert(keys == [6, 15, 33], f"Got incorrect iteration of keys: {keys}"); + assert(values == [15, 35, 65], "Got incorrect iteration of values."); + + let f = |k: Field, v: Field| -> (Field, Field){(k * 2, v * 2)}; + hashmap.iter_mut(f); + + let entries: [(Field, Field); 3] = cut(hashmap.iter()).sort_via(PAIR_CMP); + + assert(entries == [(12, 30), (30, 70), (66, 130)], "Got incorrect iteration of entries."); +} diff --git a/test_programs/execution_success/hashmap/src/utils.nr b/test_programs/execution_success/hashmap/src/utils.nr new file mode 100644 index 00000000000..45c9ca9bbf7 --- /dev/null +++ b/test_programs/execution_success/hashmap/src/utils.nr @@ -0,0 +1,10 @@ +// Compile-time: cuts the M first elements from the [T; N] array. +pub(crate) fn cut(input: [T; N]) -> [T; M] { + assert(M as u64 < N as u64, "M should be less than N."); + + let mut new = [dep::std::unsafe::zeroed(); M]; + for i in 0..M { + new[i] = input[i]; + } + new +} From 81e2218f1ee39a7ce717f651da8e9c19ef869916 Mon Sep 17 00:00:00 2001 From: Nikita Masych Date: Wed, 14 Feb 2024 04:17:57 +0200 Subject: [PATCH 08/12] fix: changed iterator functions, improved testing organization --- noir_stdlib/src/collections/map.nr | 309 +++++++++--------- .../execution_success/hashmap/src/main.nr | 67 ++-- 2 files changed, 197 insertions(+), 179 deletions(-) diff --git a/noir_stdlib/src/collections/map.nr b/noir_stdlib/src/collections/map.nr index e2568fcd6f9..0a184969789 100644 --- a/noir_stdlib/src/collections/map.nr +++ b/noir_stdlib/src/collections/map.nr @@ -44,20 +44,24 @@ impl Default for Slot{ } } -impl Slot{ - fn is_valid(self) -> bool{ +impl Slot { + fn is_valid(self) -> bool { !self._is_deleted & self._key_value.is_some() } - fn is_available(self) -> bool{ + fn is_available(self) -> bool { self._is_deleted | self._key_value.is_none() } - fn key_value_unchecked(self) -> (K, V){ + fn key_value(self) -> Option<(K, V)> { + self._key_value + } + + fn key_value_unchecked(self) -> (K, V) { self._key_value.unwrap_unchecked() } - fn set(&mut self, key: K, value: V){ + fn set(&mut self, key: K, value: V) { self._key_value = Option::some((key, value)); self._is_deleted = false; } @@ -65,7 +69,7 @@ impl Slot{ // Shall not override `_key_value` with Option::none(), // because we must be able to differentiate empty // and deleted slots for lookup. - fn mark_deleted(&mut self){ + fn mark_deleted(&mut self) { self._is_deleted = true; } } @@ -73,132 +77,129 @@ impl Slot{ // While conducting lookup, we iterate attempt from 0 to N - 1 due to heuristic, // that if we have went that far without finding desired, // it is very unlikely to be after - performance will be heavily degraded. -impl HashMap { +impl HashMap { // Creates a new instance of HashMap with specified BuildHasher. pub fn with_hasher(_build_hasher: B) -> Self where - B: BuildHasher - { + B: BuildHasher { let _table = [Slot::default(); N]; let _len = 0; - Self{_table, _len, _build_hasher} + Self { _table, _len, _build_hasher } } - // Clears the map, removing all key-value pairs. - pub fn clear(&mut self){ + // Clears the map, removing all key-value entries. + pub fn clear(&mut self) { self._table = [Slot::default(); N]; self._len = 0; } // Returns true if the map contains a value for the specified key. - pub fn contains_key(self, key: K) -> bool + pub fn contains_key( + self, + key: K + ) -> bool where K: Hash + Eq, B: BuildHasher, - H: Hasher - { + H: Hasher { self.get(key).is_some() } // Returns true if the map contains no elements. - pub fn is_empty(self) -> bool{ + pub fn is_empty(self) -> bool { self._len == 0 } - // Returns array of all active key-value pairs. - // Array size is restricted to N because of - // compile time limitations and nested slices temporary ban, - // hovewer first len() positioned with pairs - // and afterwards all elements are zeroed(). - pub fn iter(self) -> [(K, V); N]{ - let mut pairs = [zeroed(); N]; - let mut i = 0; - - for slot in self._table{ - if slot.is_valid(){ - pairs[i] = slot.key_value_unchecked(); - i += 1; + // Get the Option<(K, V) array of valid entries + // with a length of map capacity. First len() elements + // are safe to unwrap_unchecked(), whilst remaining + // are guaranteed to be Option::none(). + // + // This design is reasoned by compile-time limitations and + // temporary nested slices ban. + pub fn entries(self) -> [Option<(K, V)>; N] { + let mut entries = [Option::none(); N]; + let mut valid_amount = 0; + + for slot in self._table { + if slot.is_valid() { + entries[valid_amount] = slot.key_value(); + valid_amount += 1; } } - - assert( - i == self._len, - f"Must have iterated {self._len} times, instead did {i}." - ); - - pairs - } - - // Returns array of all active keys. - // Array size is restricted to N because of - // compile time limitations and nested slices temporary ban, - // hovewer first len() positioned with keys - // and afterwards all elements are zeroed(). - pub fn keys(self) -> [K; N]{ - let mut keys = [zeroed(); N]; - let mut i = 0; - - for slot in self._table{ - if slot.is_valid(){ + + let msg = f"Amount of valid elements should have been {self._len} times, but got {valid_amount}."; + assert(valid_amount == self._len, msg); + + entries + } + + // Get the Option array of valid keys + // with a length of map capacity. First len() elements + // are safe to unwrap_unchecked(), whilst remaining + // are guaranteed to be Option::none(). + // + // This design is reasoned by compile-time limitations and + // temporary nested slices ban. + pub fn keys(self) -> [Option; N] { + let mut keys = [Option::none(); N]; + let mut valid_amount = 0; + + for slot in self._table { + if slot.is_valid() { let (key, _) = slot.key_value_unchecked(); - keys[i] = key; - i += 1; + keys[valid_amount] = Option::some(key); + valid_amount += 1; } } - - assert( - i == self._len, - f"Must have iterated {self._len} times, instead did {i}." - ); + + let msg = f"Amount of valid elements should have been {self._len} times, but got {valid_amount}."; + assert(valid_amount == self._len, msg); keys - } - - // Returns array of all active values. - // Array size is restricted to N because of - // compile time limitations and nested slices temporary ban, - // hovewer first len() positioned with values - // and afterwards all elements are zeroed(). - pub fn values(self) -> [V; N]{ - let mut values = [zeroed(); N]; - let mut i = 0; - - for slot in self._table{ - if slot.is_valid(){ + } + + // Get the Option array of valid values + // with a length of map capacity. First len() elements + // are safe to unwrap_unchecked(), whilst remaining + // are guaranteed to be Option::none(). + // + // This design is reasoned by compile-time limitations and + // temporary nested slices ban. + pub fn values(self) -> [Option; N] { + let mut values = [Option::none(); N]; + let mut valid_amount = 0; + + for slot in self._table { + if slot.is_valid() { let (_, value) = slot.key_value_unchecked(); - values[i] = value; - i += 1; + values[valid_amount] = Option::some(value); + valid_amount += 1; } } - - assert( - i == self._len, - f"Must have iterated {self._len} times, instead did {i}." - ); + + let msg = f"Amount of valid elements should have been {self._len} times, but got {valid_amount}."; + assert(valid_amount == self._len, msg); values - } + } - // For each key-value pair applies mutator function. - pub fn iter_mut(&mut self, f: fn(K, V) -> (K, V)) + // For each key-value entry applies mutator function. + pub fn iter_mut( + &mut self, + f: fn(K, V) -> (K, V) + ) where K: Eq + Hash, B: BuildHasher, - H: Hasher - { - let mut pairs = self.iter(); - for i in 0..N{ - if i < self._len{ - let (key, value) = pairs[i]; - pairs[i] = f(key, value); - } - } - + H: Hasher { + let mut entries = self.entries(); let mut new_map = HashMap::with_hasher(self._build_hasher); - for i in 0..N{ - if i < self._len{ - let (key, value) = pairs[i]; + for i in 0..N { + if i < self._len { + let entry = entries[i].unwrap_unchecked(); + let (key, value) = f(entry.0, entry.1); new_map.insert(key, value); } } @@ -207,25 +208,21 @@ impl HashMap { } // For each key applies mutator function. - pub fn mut_keys(&mut self, f: fn(K) -> K) + pub fn iter_keys_mut( + &mut self, + f: fn(K) -> K + ) where K: Eq + Hash, B: BuildHasher, - H: Hasher - { - let mut pairs = self.iter(); - for i in 0..N{ - if i < self._len{ - let (key, value) = pairs[i]; - pairs[i] = (f(key), value); - } - } - + H: Hasher { + let mut entries = self.entries(); let mut new_map = HashMap::with_hasher(self._build_hasher); - for i in 0..N{ - if i < self._len{ - let (key, value) = pairs[i]; + for i in 0..N { + if i < self._len { + let entry = entries[i].unwrap_unchecked(); + let (key, value) = (f(entry.0), entry.1); new_map.insert(key, value); } } @@ -234,24 +231,24 @@ impl HashMap { } // For each value applies mutator function. - pub fn mut_values(&mut self, f: fn(V) -> V) { - for i in 0..N{ + pub fn iter_values_mut(&mut self, f: fn(V) -> V) { + for i in 0..N { let mut slot = self._table[i]; - if slot.is_valid(){ + if slot.is_valid() { let (key, value) = slot.key_value_unchecked(); slot.set(key, f(value)); self._table[i] = slot; } } } - + // Retains only the elements specified by the predicate. - pub fn retain(&mut self, f: fn(K, V) -> bool){ - for index in 0..N{ + pub fn retain(&mut self, f: fn(K, V) -> bool) { + for index in 0..N { let mut slot = self._table[index]; - if slot.is_valid(){ + if slot.is_valid() { let (key, value) = slot.key_value_unchecked(); - if !f(key, value){ + if !f(key, value) { slot.mark_deleted(); self._len -= 1; self._table[index] = slot; @@ -260,32 +257,39 @@ impl HashMap { } } - // Amount of active key-value pairs. - pub fn len(self) -> u64{ + // Amount of active key-value entries. + pub fn len(self) -> u64 { self._len } + // Get the compile-time map capacity. + pub fn capacity(self) -> u64 { + N + } + // Get the value by key. If it does not exist, returns none(). - pub fn get(self, key: K) -> Option + pub fn get( + self, + key: K + ) -> Option where K: Eq + Hash, B: BuildHasher, - H: Hasher - { + H: Hasher { let mut result = Option::none(); let hash = self.hash(key); let mut break = false; - for attempt in 0..N{ - if !break{ + for attempt in 0..N { + if !break { let index = self.quadratic_probe(hash, attempt as u64); let slot = self._table[index]; // Not marked as deleted and has key-value. - if slot.is_valid(){ + if slot.is_valid() { let (current_key, value) = slot.key_value_unchecked(); - if current_key == key{ + if current_key == key { result = Option::some(value); break = true; } @@ -296,36 +300,39 @@ impl HashMap { result } - // Insert key-value pair. In case key was already present, value is overridden. - pub fn insert(&mut self, key: K, value: V) + // Insert key-value entry. In case key was already present, value is overridden. + pub fn insert( + &mut self, + key: K, + value: V + ) where K: Eq + Hash, B: BuildHasher, - H: Hasher - { + H: Hasher { self.assert_load_factor(); let hash = self.hash(key); let mut break = false; - for attempt in 0..N{ - if !break{ + for attempt in 0..N { + if !break { let index = self.quadratic_probe(hash, attempt as u64); let mut slot = self._table[index]; let mut insert = false; // Either marked as deleted or has unset key-value. - if slot.is_available(){ + if slot.is_available() { insert = true; self._len += 1; - }else{ + } else { let (current_key, _) = slot.key_value_unchecked(); - if current_key == key{ + if current_key == key { insert = true; } } - if insert{ + if insert { slot.set(key, value); self._table[index] = slot; break = true; @@ -335,24 +342,26 @@ impl HashMap { } // Remove key-value entry. If key is not present, HashMap remains unchanged. - pub fn remove(&mut self, key: K) + pub fn remove( + &mut self, + key: K + ) where K: Eq + Hash, B: BuildHasher, - H: Hasher - { + H: Hasher { let hash = self.hash(key); let mut break = false; - for attempt in 0..N{ - if !break{ + for attempt in 0..N { + if !break { let index = self.quadratic_probe(hash, attempt as u64); let mut slot = self._table[index]; // Not marked as deleted and has key-value. - if slot.is_valid(){ + if slot.is_valid() { let (current_key, _) = slot.key_value_unchecked(); - if current_key == key{ + if current_key == key { slot.mark_deleted(); self._table[index] = slot; self._len -= 1; @@ -364,12 +373,14 @@ impl HashMap { } // Apply HashMap's hasher onto key to obtain pre-hash for probing. - fn hash(self, key: K) -> u64 + fn hash( + self, + key: K + ) -> u64 where K: Hash, B: BuildHasher, - H: Hasher - { + H: Hasher { let mut hasher = self._build_hasher.build_hasher(); key.hash(&mut hasher); hasher.finish() as u64 @@ -380,15 +391,15 @@ impl HashMap { // This ensures good uniformity of distribution for table sizes // equal to prime numbers or powers of two. fn quadratic_probe(self, hash: u64, attempt: u64) -> u64 { - (hash + (attempt + attempt * attempt) / 2 ) % N + (hash + (attempt + attempt * attempt) / 2) % N } // Amount of elements in the table in relation to available slots exceeds α_max. - // To avoid division with potential incorrectness due to floating point values, - // we conduct cross-multiplication. + // To avoid a comparatively more expensive division operation + // we conduct cross-multiplication instead. // n / m >= MAX_LOAD_FACTOR_NUMERATOR / MAX_LOAD_FACTOR_DEN0MINATOR - // n * MAX_LOAD_FACTOR_NUMERATOR >= m * MAX_LOAD_FACTOR_DEN0MINATOR - fn assert_load_factor(self){ + // n * MAX_LOAD_FACTOR_DEN0MINATOR >= m * MAX_LOAD_FACTOR_NUMERATOR + fn assert_load_factor(self) { let lhs = self._len * MAX_LOAD_FACTOR_DEN0MINATOR; let rhs = self._table.len() as u64 * MAX_LOAD_FACTOR_NUMERATOR; let exceeded = lhs >= rhs; @@ -397,7 +408,7 @@ impl HashMap { } // Equality class on HashMap has to test that they have -// equal sets of key-value pairs, +// equal sets of key-value entries, // thus one is a subset of the other and vice versa. impl Eq for HashMap where diff --git a/test_programs/execution_success/hashmap/src/main.nr b/test_programs/execution_success/hashmap/src/main.nr index 5bac982a1b2..9465aa5da8c 100644 --- a/test_programs/execution_success/hashmap/src/main.nr +++ b/test_programs/execution_success/hashmap/src/main.nr @@ -3,23 +3,30 @@ mod utils; use dep::std::collections::map::HashMap; use dep::std::hash::BuildHasherDefault; use dep::std::hash::pedersen::PedersenHasher; +use dep::std::cmp::Eq; use utils::cut; +type K = Field; +type V = Field; + +// It is more convenient and readable to use structs as input. struct Entry{ - key: Field, - value: Field + key: K, + value: V } global HASHMAP_CAP = 8; global HASHMAP_LEN = 6; global FIELD_CMP = |a: Field, b: Field| a.lt(b); -global PAIR_CMP = |a: (Field, Field), b: (Field, Field)| a.0.lt(b.0); -fn allocate_hashmap() -> HashMap> { - HashMap::default() -} +global K_CMP = FIELD_CMP; +global V_CMP = FIELD_CMP; +global KV_CMP = |a: (K, V), b: (K, V)| a.0.lt(b.0); + +global ALLOCATE_HASHMAP = || -> HashMap> + HashMap::default(); fn main(input: [Entry; HASHMAP_LEN]) { test_sequential(input[0].key, input[0].value); @@ -33,8 +40,8 @@ fn main(input: [Entry; HASHMAP_LEN]) { } // Insert, get, remove. -fn test_sequential(key: Field, value: Field) { - let mut hashmap = allocate_hashmap(); +fn test_sequential(key: K, value: V) { + let mut hashmap = ALLOCATE_HASHMAP(); assert(hashmap.is_empty(), "New HashMap should be empty."); hashmap.insert(key, value); @@ -52,8 +59,8 @@ fn test_sequential(key: Field, value: Field) { } // Insert same pair several times. -fn test_multiple_equal_insert(key: Field, value: Field) { - let mut hashmap = allocate_hashmap(); +fn test_multiple_equal_insert(key: K, value: V) { + let mut hashmap = ALLOCATE_HASHMAP(); assert(hashmap.is_empty(), "New HashMap should be empty."); for _ in 0..HASHMAP_LEN { @@ -70,8 +77,8 @@ fn test_multiple_equal_insert(key: Field, value: Field) { } // Override value for existing pair. -fn test_value_override(key: Field, value: Field, new_value: Field) { - let mut hashmap = allocate_hashmap(); +fn test_value_override(key: K, value: V, new_value: V) { + let mut hashmap = ALLOCATE_HASHMAP(); assert(hashmap.is_empty(), "New hashmap should be empty."); hashmap.insert(key, value); @@ -86,7 +93,7 @@ fn test_value_override(key: Field, value: Field, new_value: Field) { // Insert several distinct pairs and test auxiliary methods. fn test_insert_and_methods(input: [Entry; HASHMAP_LEN]) { - let mut hashmap = allocate_hashmap(); + let mut hashmap = ALLOCATE_HASHMAP(); assert(hashmap.is_empty(), "New HashMap should be empty."); for entry in input { @@ -105,7 +112,7 @@ fn test_insert_and_methods(input: [Entry; HASHMAP_LEN]) { // Insert several pairs and test retaining. fn test_retain() { - let mut hashmap = allocate_hashmap(); + let mut hashmap = ALLOCATE_HASHMAP(); assert(hashmap.is_empty(), "New HashMap should be empty."); let (key, value) = (5, 11); @@ -115,7 +122,7 @@ fn test_retain() { let (key, value) = (11, 5); hashmap.insert(key, value); - let predicate = |key: Field, value: Field| -> bool {key * value == 55}; + let predicate = |key: K, value: V| -> bool {key * value == 55}; hashmap.retain(predicate); assert(hashmap.len() == 2, "HashMap should have retained 2 elements."); @@ -124,8 +131,8 @@ fn test_retain() { // Equality trait check. fn test_hashmaps_equality(input: [Entry; HASHMAP_LEN]) { - let mut hashmap_1 = allocate_hashmap(); - let mut hashmap_2 = allocate_hashmap(); + let mut hashmap_1 = ALLOCATE_HASHMAP(); + let mut hashmap_2 = ALLOCATE_HASHMAP(); for entry in input { hashmap_1.insert(entry.key, entry.value); @@ -141,15 +148,15 @@ fn test_hashmaps_equality(input: [Entry; HASHMAP_LEN]) { // Test Iter, Keys, Values. fn test_iterators() { - let mut hashmap = allocate_hashmap(); + let mut hashmap = ALLOCATE_HASHMAP(); hashmap.insert(2, 3); hashmap.insert(5, 7); hashmap.insert(11, 13); - let keys: [Field; 3] = cut(hashmap.keys()).sort_via(FIELD_CMP); - let values: [Field; 3] = cut(hashmap.values()).sort_via(FIELD_CMP); - let entries: [(Field, Field); 3] = cut(hashmap.iter()).sort_via(PAIR_CMP); + let keys: [K; 3] = cut(hashmap.keys()).map(|k: Option| k.unwrap_unchecked()).sort_via(K_CMP); + let values: [V; 3] = cut(hashmap.values()).map(|v: Option| v.unwrap_unchecked()).sort_via(V_CMP); + let entries: [(K, V); 3] = cut(hashmap.entries()).map(|e: Option<(K, V)>| e.unwrap_unchecked()).sort_via(KV_CMP); assert(keys == [2, 5, 11], "Got incorrect iteration of keys."); assert(values == [3, 7, 13], "Got incorrect iteration of values."); @@ -158,28 +165,28 @@ fn test_iterators() { // Test mutable Iter, Keys, Values. fn test_mut_iterators() { - let mut hashmap = allocate_hashmap(); + let mut hashmap = ALLOCATE_HASHMAP(); hashmap.insert(2, 3); hashmap.insert(5, 7); hashmap.insert(11, 13); - let f = |k: Field| -> Field{ k * 3}; - hashmap.mut_keys(f); + let f = |k: K| -> K{ k * 3}; + hashmap.iter_keys_mut(f); - let f = |v: Field| -> Field{ v * 5}; - hashmap.mut_values(f); + let f = |v: V| -> V{ v * 5}; + hashmap.iter_values_mut(f); - let keys: [Field; 3] = cut(hashmap.keys()).sort_via(FIELD_CMP); - let values: [Field; 3] = cut(hashmap.values()).sort_via(FIELD_CMP); + let keys: [K; 3] = cut(hashmap.keys()).map(|k: Option| k.unwrap_unchecked()).sort_via(K_CMP); + let values: [V; 3] = cut(hashmap.values()).map(|v: Option| v.unwrap_unchecked()).sort_via(V_CMP); assert(keys == [6, 15, 33], f"Got incorrect iteration of keys: {keys}"); assert(values == [15, 35, 65], "Got incorrect iteration of values."); - let f = |k: Field, v: Field| -> (Field, Field){(k * 2, v * 2)}; + let f = |k: K, v: V| -> (K, V){(k * 2, v * 2)}; hashmap.iter_mut(f); - let entries: [(Field, Field); 3] = cut(hashmap.iter()).sort_via(PAIR_CMP); + let entries: [(K, V); 3] = cut(hashmap.entries()).map(|e: Option<(K, V)>| e.unwrap_unchecked()).sort_via(KV_CMP); assert(entries == [(12, 30), (30, 70), (66, 130)], "Got incorrect iteration of entries."); } From c3e8fa855fb4186bcae6037d7691a4c2c2bb5470 Mon Sep 17 00:00:00 2001 From: Nikita Masych Date: Wed, 14 Feb 2024 04:24:46 +0200 Subject: [PATCH 09/12] chore: actualized comments in hashmap tests --- test_programs/execution_success/hashmap/src/main.nr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test_programs/execution_success/hashmap/src/main.nr b/test_programs/execution_success/hashmap/src/main.nr index 9465aa5da8c..5d1c427ab4c 100644 --- a/test_programs/execution_success/hashmap/src/main.nr +++ b/test_programs/execution_success/hashmap/src/main.nr @@ -146,7 +146,7 @@ fn test_hashmaps_equality(input: [Entry; HASHMAP_LEN]) { assert(hashmap_1 != hashmap_2, "HashMaps should not be equal."); } -// Test Iter, Keys, Values. +// Test entries, keys, values. fn test_iterators() { let mut hashmap = ALLOCATE_HASHMAP(); @@ -163,7 +163,7 @@ fn test_iterators() { assert(entries == [(2, 3), (5, 7), (11, 13)], "Got incorrect iteration of entries."); } -// Test mutable Iter, Keys, Values. +// Test mutable iteration over keys, values and entries. fn test_mut_iterators() { let mut hashmap = ALLOCATE_HASHMAP(); From 3f89780ddd8caad66bc59b01363259c16f6e0ebc Mon Sep 17 00:00:00 2001 From: Nikita Masych Date: Wed, 14 Feb 2024 17:49:56 +0200 Subject: [PATCH 10/12] fix(test): removed aliased type in input entry --- test_programs/execution_success/hashmap/src/main.nr | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test_programs/execution_success/hashmap/src/main.nr b/test_programs/execution_success/hashmap/src/main.nr index 5d1c427ab4c..597a5c0b7de 100644 --- a/test_programs/execution_success/hashmap/src/main.nr +++ b/test_programs/execution_success/hashmap/src/main.nr @@ -12,8 +12,8 @@ type V = Field; // It is more convenient and readable to use structs as input. struct Entry{ - key: K, - value: V + key: Field, + value: Field } global HASHMAP_CAP = 8; From 4910e20602c6bedd07ccd4cdd02ef946123a85bf Mon Sep 17 00:00:00 2001 From: Nikita Masych Date: Wed, 14 Feb 2024 18:09:19 +0200 Subject: [PATCH 11/12] chore: removed unused use zeroed in map --- noir_stdlib/src/collections/map.nr | 1 - 1 file changed, 1 deletion(-) diff --git a/noir_stdlib/src/collections/map.nr b/noir_stdlib/src/collections/map.nr index 0a184969789..d245d67260c 100644 --- a/noir_stdlib/src/collections/map.nr +++ b/noir_stdlib/src/collections/map.nr @@ -2,7 +2,6 @@ use crate::cmp::Eq; use crate::collections::vec::Vec; use crate::option::Option; use crate::default::Default; -use crate::unsafe::zeroed; use crate::hash::{Hash, Hasher, BuildHasher}; // We use load factor α_max = 0.75. From de97923738415db4ff08fdf93510592f37d749b4 Mon Sep 17 00:00:00 2001 From: Nikita Masych Date: Thu, 22 Feb 2024 17:52:40 +0200 Subject: [PATCH 12/12] fix(warning): corrected unused self issue --- noir_stdlib/src/collections/map.nr | 4 ++-- noir_stdlib/src/hash.nr | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/noir_stdlib/src/collections/map.nr b/noir_stdlib/src/collections/map.nr index d245d67260c..d9eb83ff5dc 100644 --- a/noir_stdlib/src/collections/map.nr +++ b/noir_stdlib/src/collections/map.nr @@ -262,7 +262,7 @@ impl HashMap { } // Get the compile-time map capacity. - pub fn capacity(self) -> u64 { + pub fn capacity(_self: Self) -> u64 { N } @@ -389,7 +389,7 @@ impl HashMap { // We use 0.5 constant near variadic attempt and attempt^2 monomials. // This ensures good uniformity of distribution for table sizes // equal to prime numbers or powers of two. - fn quadratic_probe(self, hash: u64, attempt: u64) -> u64 { + fn quadratic_probe(_self: Self, hash: u64, attempt: u64) -> u64 { (hash + (attempt + attempt * attempt) / 2) % N } diff --git a/noir_stdlib/src/hash.nr b/noir_stdlib/src/hash.nr index a35e62edd17..7a931f7c047 100644 --- a/noir_stdlib/src/hash.nr +++ b/noir_stdlib/src/hash.nr @@ -105,7 +105,7 @@ impl BuildHasher for BuildHasherDefault where H: Hasher + Default { - fn build_hasher(self) -> H{ + fn build_hasher(_self: Self) -> H{ H::default() } }