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

Use Vec instead of BTreeMap for lc_map and lc_assignment_cache #381

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

winderica
Copy link

@winderica winderica commented Dec 3, 2024

Description

It seems that using BTreeMap to store linear combinations is pretty expensive, and to improve the efficiency of constraint synthesis, this PR instead uses plain vectors for lc_map and lc_assignment_cache.

Below is some data sampled by running the augmented circuit for sha256 from the sonobe library, and this PR can reduce the generate_constraints time from about 90ms to about 40ms:

  • Without this PR:
generate_constraints: 91.372512ms
Nova::prove_step 0: 323.217956ms
generate_constraints: 88.990764ms
Nova::prove_step 1: 331.140575ms
generate_constraints: 89.081936ms
Nova::prove_step 2: 427.015151ms
generate_constraints: 90.216658ms
Nova::prove_step 3: 427.42358ms
generate_constraints: 91.529767ms
Nova::prove_step 4: 466.352438ms
generate_constraints: 90.638139ms
Nova::prove_step 5: 423.848768ms
generate_constraints: 91.094708ms
Nova::prove_step 6: 368.598278ms
generate_constraints: 88.381463ms
Nova::prove_step 7: 413.475741ms
generate_constraints: 90.22178ms
Nova::prove_step 8: 456.199133ms
generate_constraints: 88.873222ms
Nova::prove_step 9: 421.229509ms
  • With this PR:
generate_constraints: 51.273273ms
Nova::prove_step 0: 235.242424ms
generate_constraints: 50.845459ms
Nova::prove_step 1: 328.031474ms
generate_constraints: 54.128816ms
Nova::prove_step 2: 351.362552ms
generate_constraints: 51.012215ms
Nova::prove_step 3: 330.077002ms
generate_constraints: 53.165829ms
Nova::prove_step 4: 339.404506ms
generate_constraints: 48.017148ms
Nova::prove_step 5: 375.576104ms
generate_constraints: 48.47549ms
Nova::prove_step 6: 320.267184ms
generate_constraints: 46.634053ms
Nova::prove_step 7: 378.505293ms
generate_constraints: 47.959163ms
Nova::prove_step 8: 377.381112ms
generate_constraints: 47.119074ms
Nova::prove_step 9: 288.827563ms

Note that for lc_assignment_cache, HashMap might be a better choice than Vec, as it does not cache entries that are never read and thus requires less memory, but it may take more time. Let me know which one you prefer!


Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Targeted PR against correct branch (master)
  • Linked to Github issue with discussion and accepted design OR have an explanation in the PR that describes this work.
  • Wrote unit tests (N/A)
  • Updated relevant documentation in the code (N/A)
  • Added a relevant changelog entry to the Pending section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

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