Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Better implementation of Num::add_assign. #13

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

alex-ozdemir
Copy link

The original implementation of Num::add_assign had a few inefficiencies:

  • It instantiated the standard hashmap with small keys.
  • It put a bunch of items into an empty hashmap, without setting the
    intial hashmap capacity.
  • It used multiple hash lookups in an insert-or-modify pattern.

This commit:

  • Uses the Fnv hash function instead of the default: SipHash.
  • Initializes the hashmap with appropriate capacity.
  • Uses the entry API.

In my microbenchmarks, this sped up Poseidon evaluation by 1.7x.

The original implementation of Num::add_assign had a few inefficiencies:

   * It instantiated the standard hashmap with small keys.
   * It put a bunch of items into an empty hashmap, without setting the
     intial hashmap capacity.
   * It used multiple hash lookups in an insert-or-modify pattern.

This commit:

   * Uses the Fnv hash function instead of the default: SipHash.
   * Initializes the hashmap with appropriate capacity.
   * Uses the entry API.

In my microbenchmarks, this sped up Poseidon evaluation by 1.7x.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant