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

feat: Improve proving speed by caching zkey curve #124

Merged
merged 1 commit into from
Jun 12, 2022

Conversation

nalinbhardwaj
Copy link
Contributor

@nalinbhardwaj nalinbhardwaj commented Jan 24, 2022

Speeds up proving by caching zkey getCurve call when reading headers and avoiding double calls in grothProve. On a smallish circuit (https://github.com/nalinbhardwaj/snarky-sudoku) this amounts to a ~30% reduction, which is pretty significant IMO:

Before:
image
~4 seconds runtime

After:
Screenshot 2022-01-23 at 8 35 10 PM
~3 seconds runtime

Branched on #121

@nalinbhardwaj
Copy link
Contributor Author

Rebased and tested with the snarky sudoku repo again. Ready for merge.

@nalinbhardwaj
Copy link
Contributor Author

Rebased on latest master and ready for review again. Tested on this repo branch that's deployed here if someone else wants to test.

@phated
Copy link
Contributor

phated commented Jun 12, 2022

@nalinbhardwaj can you rebase this now that I got CI all green?? Thanks!

@nalinbhardwaj
Copy link
Contributor Author

@phated done, all green ✅

@phated
Copy link
Contributor

phated commented Jun 12, 2022

Thanks! One question, and also, please remove the built files from the PR. They get built by jordi during release (there are actually 4 files that need to be built).

@nalinbhardwaj
Copy link
Contributor Author

please remove the built files from the PR

done

@phated phated changed the title Speedup proving by caching zkey curve feat: Improve proving speed by caching zkey curve Jun 12, 2022
@phated phated merged commit 227d151 into iden3:master Jun 12, 2022
@phated
Copy link
Contributor

phated commented Jun 12, 2022

Thanks @nalinbhardwaj 🎉 Sorry it has taken so long to get this landed. It'll be included in the next release!

@nalinbhardwaj
Copy link
Contributor Author

@phated no worries, thank you for taking the time to merge it! <3

@nalinbhardwaj nalinbhardwaj deleted the curve-speed branch June 12, 2022 19:17
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.

2 participants