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

chore: Replace hashers of hashmaps in dfg with fxhashes #2490

Merged
merged 3 commits into from
Sep 5, 2023
Merged

Conversation

Ethan-000
Copy link
Contributor

Description

Problem*

Resolves #2487

Summary*

very small speedup #2487 (comment)

Additional Context

PR Checklist*

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

Copy link
Contributor

@jfecher jfecher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you replace the HashMaps used in map.rs as well? E.g. DenseMap. How much more would that speed things up?

Edit: Nevermind, forgot that DenseMaps are just Vecs internally 😅

@jfecher
Copy link
Contributor

jfecher commented Aug 30, 2023

Might as well replace HashMaps all throughout SSA I suppose. Just don't replace BTreeMaps, we use those where the ordering needs to be deterministic.

@Ethan-000 Ethan-000 marked this pull request as draft August 30, 2023 17:01
@jfecher
Copy link
Contributor

jfecher commented Aug 30, 2023

bit shift test failing after changing the hasher

bit_shifts_comptime is failing on master as well. CI only tests on release mode and it is only failing in debug so it wasn't caught before

@Ethan-000
Copy link
Contributor Author

bit shift test failing after changing the hasher

bit_shifts_comptime is failing on master as well. CI only tests on release mode and it is only failing in debug so it wasn't caught before

ah thought i did something wrong

@Ethan-000 Ethan-000 marked this pull request as ready for review August 30, 2023 17:15
@Ethan-000
Copy link
Contributor Author

dk if i can give actually numbers for this at the moment

tried with criterion - been running for a day

with iai the program just crushes

@Ethan-000
Copy link
Contributor Author

got some numbers on this branch after master is merged dfd234d v. master 9fe4cfd

don't know if it is accurate

this branch:

this branch

master:

master

Copy link
Contributor

@jfecher jfecher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@jfecher jfecher added this pull request to the merge queue Sep 5, 2023
Merged via the queue into master with commit 255febd Sep 5, 2023
@jfecher jfecher deleted the e/fxmaps branch September 5, 2023 16:46
TomAFrench added a commit that referenced this pull request Sep 5, 2023
* master:
  chore: Replace hashers of hashmaps in dfg with fxhashes (#2490)
  chore: remove duplicate span from FunctionReturnType (#2546)
  feat: Add support for brillig call stacks in runtime errors (#2549)
  feat: add `noirc_abi_wasm` crate for ABI encoding in JS (#1945)
  chore: move CRS files into their own directory (#2558)
  chore: Cleanup `rebuild.sh` script (#2470)
  chore(ci): add mocked backend binary to improve `compile_success_empty` tests (#2554)
  chore: add noir-source-resolver (#2485)
  chore: fix double verify proof (#2556)
  feat: add `nargo backend ls` and `nargo backend use` command to switch between backends (#2552)
  chore(ci): bump checkout action to v4 (#2551)
  feat: Support for optional assertion messages (#2491)
  fix: allow usage of decimal string encoding for fields larger than a `i128` (#2547)
  feat(nargo): add hidden option to produce JSON output from `nargo info` (#2542)
  chore(stdlib)!: Rename `fixed_base_scalar_mul` to be more descriptive (#2488)
  chore: Document requirement for range opcode on `r_witness` in  `GeneratedAcir::euclidean_division` (#2437)
  chore!: ACVM 0.24 (#2504)
  fix(aztec_noir): generalise loop to not always inject a hasher instance (#2529)
  chore: create helper functions for writing programs and contracts to file (#2526)
TomAFrench added a commit that referenced this pull request Sep 5, 2023
* master:
  fix(aztec): fix compilation of `aztec_library.rs` (#2567)
  feat(ssa): Replace values which have previously been constrained with simplified value (#2483)
  fix: Black box func slice handling (#2562)
  chore: Temporarily disable `noir_wasm` test (#2566)
  fix: Make def collector ordering more deterministic (#2515)
  chore: refactor `constant_folding` pass (#2533)
  chore: Cleanup mem2reg pass (#2531)
  chore: Replace hashers of hashmaps in dfg with fxhashes (#2490)
  chore: remove duplicate span from FunctionReturnType (#2546)
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.

Replace Hashmaps with FxHashmaps or another data structure
2 participants