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

Improved Neptune Witness Generation via Bellpepper #177

Merged
merged 7 commits into from
Aug 8, 2023

Conversation

porcuquine
Copy link
Contributor

@porcuquine porcuquine commented Jun 5, 2023

This PR adds support for optimized witness generation and will automatically include the neptune optimization. Note: this PR depends on the neptune and bellperson PRs so should not be merged yet.

I did run the recursive-snark benchmark to compare with existing performance. However, I noticed that (for whatever reason) this benchmark does not have very stable results. I saw noticeable fluctuations in performance between multiple runs on the same branch. The strongest conclusion I could draw is that the present work does not obviously improve performance on Nova benchmarks. [Performance improvements in the final revision are noticeable, consistent, and reported below.]

However, inclusion of these changes does allow significant performance improvements in downstream usage in lurk-rs, where the step circuit performs significant amounts of poseidon hashing. This suggests that future Nova work (SuperNova, HyperNova) which includes more explicit hashing will benefit more.

The main thrust of this work is that it allows a constraint system to specify that it can function as a 'witness generator'. In this mode, a block of witness values can be 'allocated' at once, and an optimized witness-generation function can simply write witness values into this pre-allocated block. This structure also facilitates parallelization of such witness-generation functions. Both aspects are used in the downstream work, and with adaptation, both could also be applied to Nova itself.

We leave tighter integration with Nova for future work and here aim mainly to support consumers of Nova who want to either passively take advantage of the baseline neptune optimizations — or to more actively structure application-level parallelism. Although not implemented yet, this class of optimizations could be extended beyond poseidon hashing to include the entire verifier circuit, for example.

Finally, as a matter of principle, this work removes all superfluous namespace-related bookkeeping from the ShapeCS used for Nova proving. The previous behavior is retained in a new TestShapeCS, and the latter is now used in Nova tests. This preserves the status quo of development-time diagnostic information, while removing unnecessary work from the hot path of actual proving.

By the same principle, this PR removes the vestigial DensityTrackers from the SatisfyingAssignment constraint system. These are not needed for Nova folding/proving.

@porcuquine
Copy link
Contributor Author

porcuquine commented Jun 20, 2023

The reason the benchmarks weren't showing any improvement is that the neptune changes were not automatically providing the first-order witness-generation optimization for the sponge API (SAFE). I've pushed a commit that does so [UPDATE: I mean that I've pushed a commit to the neptune branch — no change is required in Nova to consume this change for free.], and the expected benefit can now be observed:

➜  nova git:(witness-gen) cargo criterion --bench recursive-snark
   Compiling nova-snark v0.21.0 (/Users/clwk/fil/nova)
    Finished bench [optimized] target(s) in 8.98s
Gnuplot not found, using plotters backend
Benchmarking RecursiveSNARK-StepCircuitSize-0/Prove: Collecting 10 samples in estimated 6.9203 s (16                                                                                                    RecursiveSNARK-StepCircuitSize-0/Prove                        
                        time:   [41.520 ms 41.660 ms 41.828 ms]
                        change: [-34.137% -32.420% -31.034%] (p = 0.00 < 0.05)
                        Performance has improved.

The apparent benefit decreases as circuit size increases, which is expected, since the verifier circuit accounts for proportionally less synthesis time.

@porcuquine porcuquine changed the title [Do not merge until Git dependencies removed] Witness gen Improved Neptune Witness Generation via Bellpepper Aug 8, 2023
@huitseeker
Copy link
Contributor

huitseeker commented Aug 8, 2023

A new bench run on the rebased PR confirms the performance gains reported above:

RecursiveSNARK-StepCircuitSize-0/Prove
                        time:   [143.21 ms 150.55 ms 159.23 ms]
                        change: [-34.207% -28.388% -21.268%] (p = 0.00 < 0.05)
                        Performance has improved.

Benchmarking RecursiveSNARK-StepCircuitSize-6565/Prove: Warming up for 3.0000 s
Warning: Unable to complete 10 samples in 5.0s. You may wish to increase target time to 8.0s or enable flat sampling.
RecursiveSNARK-StepCircuitSize-6565/Prove
                        time:   [138.75 ms 147.80 ms 153.51 ms]
                        change: [-35.655% -28.749% -20.903%] (p = 0.00 < 0.05)
                        Performance has improved.

Benchmarking RecursiveSNARK-StepCircuitSize-22949/Prove: Warming up for 3.0000 s
Warning: Unable to complete 10 samples in 5.0s. You may wish to increase target time to 8.4s or enable flat sampling.
RecursiveSNARK-StepCircuitSize-22949/Prove
                        time:   [134.90 ms 141.55 ms 149.25 ms]
                        change: [-37.690% -31.988% -25.830%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 2 outliers among 10 measurements (20.00%)
  1 (10.00%) low mild
  1 (10.00%) high mild

Benchmarking RecursiveSNARK-StepCircuitSize-55717/Prove: Warming up for 3.0000 s
Warning: Unable to complete 10 samples in 5.0s. You may wish to increase target time to 9.0s or enable flat sampling.
RecursiveSNARK-StepCircuitSize-55717/Prove
                        time:   [160.77 ms 170.46 ms 183.03 ms]
                        change: [-22.173% -14.530% -5.6067%] (p = 0.01 < 0.05)
                        Performance has improved.
Found 1 outliers among 10 measurements (10.00%)
  1 (10.00%) high mild

RecursiveSNARK-StepCircuitSize-121253/Prove
                        time:   [231.61 ms 240.88 ms 251.30 ms]
                        change: [-18.023% -12.650% -7.3458%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 2 outliers among 10 measurements (20.00%)
  2 (20.00%) high mild

RecursiveSNARK-StepCircuitSize-252325/Prove
                        time:   [422.82 ms 437.72 ms 454.09 ms]
                        change: [-16.538% -11.070% -5.1106%] (p = 0.00 < 0.05)
                        Performance has improved.

Full bench logs at https://gist.github.com/huitseeker/b99e1803e9605703746d5745ab05002d

@porcuquine porcuquine marked this pull request as ready for review August 8, 2023 15:49
@porcuquine
Copy link
Contributor Author

porcuquine commented Aug 8, 2023

@srinathsetty At long last, this is completely ready to go. As you can see above, the witness-generation improvements carry through automatically to Nova itself. The smaller the circuit, the more effect they have, since poseidon hashing is proportionally more of the work. Note, however, that any downstream users of neptune will also see automatic performance improvements. Basically, this improves neptune hashing synthesis across the board. In order to accomplish that, we had to add support for a witness-generation constraint-system, and for simplicity we took the opportunity to finally create the bellpepper crate. This removes Groth16 support, and a further artifact of that can be seen in some simplifications to the adapter in Nova (removing vestigial support for density trackers, which Nova doesn't need).

@srinathsetty srinathsetty merged commit ee07c9e into microsoft:main Aug 8, 2023
2 checks passed
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.

3 participants