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

fix(ssa refactor): Speedup acir-gen #1793

Merged
merged 2 commits into from
Jun 22, 2023
Merged

Conversation

jfecher
Copy link
Contributor

@jfecher jfecher commented Jun 22, 2023

Description

Problem*

Resolves #1750

Summary*

After a flamegraph profiling I found out that most time in acir-gen for the eddsa test was spent in mul_var and sub_var, and most time in those methods was spent in TwoWayMap::insert where to insert a new AcirVar, AcirVarData pair we were calling FieldElement::eq repeatedly which uses to_bytes_be internally.

To fix this, I changed the TwoWayMap to a single-way map. The downside of this fix is if an identical AcirVarData is ever tried to be inserted, it will be assigned a new id instead of an existing one. This may slow down proving later on.

Documentation

  • This PR requires documentation updates when merged.

    • I will submit a noir-lang/docs PR.
    • I will request for and support Dev Rel's help in documenting this PR.

Additional Context

PR Checklist*

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

@jfecher
Copy link
Contributor Author

jfecher commented Jun 22, 2023

It is worth noting that nargo prove --experimental-ssa still takes 62s on my machine for eddsa in release mode. Most of the time is spent on proving. Only the first second is spent on the rest of the compiler.

@jfecher jfecher added this pull request to the merge queue Jun 22, 2023
Merged via the queue into master with commit 1e75f0e Jun 22, 2023
@jfecher jfecher deleted the jf/fix-acir-gen-slowdown branch June 22, 2023 17:38
This was referenced Jun 22, 2023
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.

ACIR gen extremely slow for eddsa
2 participants