feat: Add HashMap to the stdlib

[NikitaMasych]

Description

This PR shall bring HashMap into the stdlib of Noir.

Problem*

Resolves #4241

Summary*

Implementation of HashMap with open addressing and quadratic probing scheme. Since Noir requires knowing loop bounds (and recursive calls) at compile time, HashMap is of fixed capacity and no dynamic resize is accomplished with regard to load factor.

Furthermore, contribution includes implementation of PedersenHasher to be used for now. One can examine potentially better and less heavy prehash functions.

I tried to conform with best practices of engineering, however since Noir is in rapid development, there are certain things which may be optimized in future, both from the code style and performance point of view.

Additional Context

I put the PedersenHasher among the poseidon.nr and mimc.nr, so one can consider moving declaration of other pedersen-related functionality there, however that would be a breaking change.

Documentation*

Check one:

• No documentation needed.
• Documentation included in this PR.
• [Exceptional Case] Documentation to be submitted in a separate PR.

PR Checklist*

• I have tested the changes locally.
• I have formatted the changes with Prettier and/or cargo fmt on default settings.

[jfecher]

This looks like a very good hashmap implementation, thank you!

My main note that is not mentioned in my comments was that I think it may be too early to introduce a Map trait - I don't think there is need for one now when there is only one map type. Rust for example gets by without a similar trait despite having two map types so I think we can avoid the extra complexity for now.

[NikitaMasych]

Thank you for the feedback @jfecher! I will try to apply your notes and advises as soon as I can.

[zac-williamson]

Howdy. This is a really nice piece of work! I have some suggestions to how the performance of map can be improved, but please don't take this as criticism; I'm very impressed with the quality of this PR.

From what I can understand, the get method is linear-time in the size of N due to the for loop.

It is possible to create a map abstraction where accessing the map can be done in a constant number of constraints regardless of N, by using a doubly-linked-list data structure.

The idea is to store the map key/values in a size-N linked list, where the list is sorted by key value (assuming the key is a field element, or is hashed into a field element).

When get-ing a value via its key, a simple array lookup suffices if the key is present. If the key is not present, it is possible to prove that the requested key value lies between two adjacent nodes in the linked list.

A rough proof of concept is at https://gist.github.com/zac-williamson/4bee81b912471395b4e3c9b6029bad81 . It requires noir version 0.23 and is quite inefficient because of a bug in the noir compiler (#4244)

[jfecher]

Hi @zac-williamson, both approaches in Noir would have trade-offs. Quadratic (and linear) probing hashmaps typically rely on being mostly empty (typically >50%) so that their search is not linear. When this is true, it is more likely an elements first hash will lead to an empty spot and we still have amortized constant time. The lack of break in Noir however means even after finding a spot we'd still be looping doing nothing. Moreover, Noir cannot dynamically resize hashmaps like other languages so we also will most likely need to panic when the map starts to become filled over a certain percentage.

For the approach of using a list for each bucket, we can emulate list with a BoundedVec as long as we choose a maximum size for buckets, which we'll need anyway since we cannot reallocate and Noir similarly doesn't support linked lists directly. So our overall map will resemble HashMap<K, V, Len, BucketLen>. Similar to the quadratic probing approach, we'll need to panic when the map becomes filled, but this time we'll only need to panic if the item count exceeds Len or if the items in one bucket exceeds BucketLen. One issue with this method is that since the overall hashmap size is separate from the bucket size, it is less clear to users how much data they can actually put in this fixed-size hashmap since it now depends on how many hash collisions there are. E.g. if the map has a fixed size of 100 and a bucket size of just 2, we can still panic (assert(false)) if just 3 elements are inserted which happen to have the same hash (modulo 100). I think this can make the map less reliable in comparison.

[zac-williamson]

I'm not sure I follow what you're saying @jfecher . It's possible to implement linked lists in Noir with constant-time access (see the gist I linked), which means the overall map does not resemble HashMap as the access time is fixed regardless of the maximum size of the map or the number of filled elements.

Building a linked list out of arrays and using the linked list to implement a map will almost always be faster than iterating over the map in linear time.

[zac-williamson]

I'm not sure I follow what you wrote about collisions @jfecher . I was assuming we use a cryptographic hash function with collision resistance; we can assume there won't be any collisions unless the key is identical (which can be explicitly handled). Is that not your understanding as well?

[jfecher]

@zac-williamson sorry for deleting my last comment - I hadn't intended for you to reply yet. I realized I needed to stop and look at your example in more detail when I saw it used unconstrained functions and looped over the current length of the map.

With regard to a cryptographic hash function, there will always be collisions in a hash map because a hash map will take a hash and perform hash % self.capacity (to figure out which slot to insert the item into) which will lead to more collisions as the map fills regardless of the hash method used (although cryptographic hashes are still an improvement).

[zac-williamson]

Hi @zac-williamson, both approaches in Noir would have trade-offs. Quadratic (and linear) probing hashmaps typically rely on being mostly empty (typically >50%) so that their search is not linear. When this is true, it is more likely an elements first hash will lead to an empty spot and we still have amortized constant time. The lack of break in Noir however means even after finding a spot we'd still be looping doing nothing. Moreover, Noir cannot dynamically resize hashmaps like other languages so we also will most likely need to panic when the map starts to become filled over a certain percentage.

For the approach of using a list for each bucket, we can emulate list with a BoundedVec as long as we choose a maximum size for buckets, which we'll need anyway since we cannot reallocate and Noir similarly doesn't support linked lists directly. So our overall map will resemble HashMap<K, V, Len, BucketLen>. Similar to the quadratic probing approach, we'll need to panic when the map becomes filled, but this time we'll only need to panic if the item count exceeds Len or if the items in one bucket exceeds BucketLen. One issue with this method is that since the overall hashmap size is separate from the bucket size, it is less clear to users how much data they can actually put in this fixed-size hashmap since it now depends on how many hash collisions there are. E.g. if the map has a fixed size of 100 and a bucket size of just 2, we can still panic (assert(false)) if just 3 elements are inserted which happen to have the same hash (modulo 100). I think this can make the map less reliable in comparison.

I think the confusion here is perhaps my misuse of the term hashmap. The object I implemented merely emulates the behaviour of a hashmap.

I don’t derive a position in the map by taking hash % array size.

instead, each hash is linked to a specific array element.

access is still O(N) but only in the unconstrained function. The part that needs to be constrained is O(1)

[jfecher]

I wonder if we can take inspiration from @zac-williamson's proof of concept and have an unconstrained method return the final index of the key. Then in constrained code we can assert that it is correct and perform the get/insert.

[NikitaMasych]

Hi guys, sorry for the delay, apparently I've got a virus. Applied new @jfecher 's review notes, fixed incorrect call from execution_success intended to fail load factor test.

Note: I did force push to remove merge commit (rebase instead)

[NikitaMasych]

I wonder if we can take inspiration from @zac-williamson's proof of concept and have an unconstrained method return the final index of the key. Then in constrained code we can assert that it is correct and perform the get/insert.

It is indeed an interesting concept and shall be investigated further; now we have high-coupling reasoned by sequential execution in Noir, which complexifies (or makes question necessity of such) splitting index computation.

[jfecher]

Thanks!

As @zac-williamson there is room for improvement performance-wise in the future, but as long as we don't guarantee a particular implementation in Noir we should be free to swap out the internals later. I think this is a good base for now though. I'll add some more Hash impls and documentation in a separate PR.

[jfecher]

Looks like you have a test failure in CI:

error: Only sized types may be used in the entry point to a program
   ┌─ /home/runner/work/noir/noir/test_programs/execution_success/hashmap/src/main.nr:31:17
   │
31 │ fn main(input: [Entry; HASHMAP_LEN]) {
   │                 ----- Slices, references, or any type containing them may not be used in main or a contract function
   │

[jfecher]

Thanks for the fix, I just realized that is a new bug in master.

[NikitaMasych]

Rebased with regard to current master and applied small code style fix (unused zeroed), now clear from my side to be merged

[NikitaMasych]

Hi guys, as I see this PR cannot be auto-merged due to stdlib producing warnings test fail. It is indeed the responsibility of this contribution code. There are 3 entries with equal issue: unused variable self in the methods:

Two in src/collections/map.nr:

impl<K, V, N, B, H> HashMap<K, V, N, B> {
    ...
    // Get the compile-time map capacity.
    pub fn capacity(self) -> u64 {
        N
    }
    ...
    // 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
    }
    ...

And one in src/hash.nr:

trait BuildHasher<H> where H: Hasher{
    fn build_hasher(self) -> H;
}

struct BuildHasherDefault<H>;

impl<H> BuildHasher<H> for BuildHasherDefault<H>
where 
    H: Hasher + Default
{
    fn build_hasher(self) -> H{
        H::default()
    }
}

Vindication to the former is that it is required to know generic N, binded to particular instance of the HashMap (analogical to static methods in other languages, in case self is not utilized), and, regarding latter, design to use self is also because one can potentially implement BuildHasher trait with a certain state, needed to be accessed via self. syntax (think of a randomization seed for example).

Ergo I see several options on what could be done here:

1. Manually merge this PR

2. Add to the Noir attribute/macro similar to as in Rust: #[allow(unused_variables)]

3. Modify existing implementation the following way, mitigating usage of self:

   1. Remove capacity method, since it cannot be refactored in other way and is auxiliary.
   2. Update quadratic_probe as

      fn quadratic_probe(table_size: u64, hash: u64, attempt: u64) -> u64 {
          (hash + (attempt + attempt * attempt) / 2) % table_size
      }

      And call it like:

      let index = HashMap::quadratic_probe(N, hash, attempt as u64);

      Instead of the previous:

      let index = self.quadratic_probe(hash, attempt as u64);

   3. Remove self from the signature of BuildHasher trait; that is however not a very thoughtful in a long-term design solution.

What option do you think we can go along with, in order to deliver this PR's functionality?

[jfecher]

@NikitaMasych you can also rename self to _self: Self to silence the warning

[NikitaMasych]

@NikitaMasych you can also rename self to _self: Self to silence the warning

Thank you for advice! I hoped there is something like that.

[NikitaMasych]

@NikitaMasych you can also rename self to _self: Self to silence the warning

It would be valuable if added to https://noir-lang.org/docs

[jfecher]

It would be valuable if added to https://noir-lang.org/docs

@noir-lang/developerrelations commenting to log this issue for later

[NikitaMasych]

@jfecher it looks like now there are no problems with tests failing, including due to warnings - can you set auto-merge this PR or you need to put it on hold?

[jfecher]

@NikitaMasych queued to merge now 🙂