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

Allocation free synthesis #138

Closed
wants to merge 2 commits into from
Closed

Allocation free synthesis #138

wants to merge 2 commits into from

Conversation

winston-h-zhang
Copy link
Member

@winston-h-zhang winston-h-zhang commented Nov 27, 2023

Addresses #137.

This PR introduces WitnessViewCS, a slightly modified version of WitnessCS that does not own the witness buffers. Instead, we have RecursiveSNARK own the witness buffers, and WitnessViewCS takes mutable handles into those buffers.

By allowing RecursiveSNARK to directly manage its own witness memory, we remove the extra allocations WitnessCS demands.

Benchmark results from downstream lurk-rs confirm that this improves performance: argumentcomputer/lurk-beta#922

Pasted Benchmark

Click to view benchmark
Test Base PR %
LEM Fibonacci Prove - rc = 100/fib/num-100 6.0±0.06s 5.6±0.04s -6.67%
LEM Fibonacci Prove - rc = 100/fib/num-200 12.4±0.07s 12.0±0.04s -3.23%
LEM Fibonacci Prove - rc = 600/fib/num-100 5.3±0.06s N/A N/A
LEM Fibonacci Prove - rc = 600/fib/num-200 12.1±0.04s N/A N/A

Copy link
Member

@huitseeker huitseeker left a comment

Choose a reason for hiding this comment

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

I like the direction of this, but it adds a ConstraintSystem that :

@winston-h-zhang
Copy link
Member Author

Benchmarks

Table of Contents

Benchmark Results

LEM Fibonacci Prove Fresh - rc = 100

main/fib witness-view-cs/fib new-witness-cs/fib
num-100 6.39 s (1.00x) 7.14 s (1.11x) 6.91 s (1.00x)
num-200 13.59 s (1.00x) 14.26 s (1.05x) 14.76 s (1.00x)
adthrpt 222.2 (1.00x) 224.7 (1.01x) 203.8 (0.92x)

LEM Fibonacci Prove Fresh - rc = 600

main/fib witness-view-cs/fib new-witness-cs/fib
num-100 4.54 s (1.00x) 4.46 s (0.98x) 4.45 s (1.00x)
num-200 10.58 s (1.00x) 10.38 s (0.98x) 10.40 s (1.00x)
adthrpt 298.0 (1.00x) 304.0 (1.02x) 302.5 (1.01x)

LEM Fibonacci Prove Load - rc = 100

main/fib witness-view-cs/fib new-witness-cs/fib
num-100 6.39 s (1.00x) 6.30 s (0.98x) 6.26 s (1.00x)
num-200 13.05 s (1.00x) 12.68 s (0.97x) 12.66 s (1.00x)
adthrpt 240.2 (1.00x) 250.8 (1.04x) 250.0 (1.04x)

LEM Fibonacci Prove Load - rc = 600

main/fib witness-view-cs/fib new-witness-cs/fib
num-100 5.78 s (1.00x) 5.74 s (1.00x) 5.77 s (1.00x)
num-200 15.25 s (1.00x) 13.33 s (0.87x) 13.59 s (1.00x)
adthrpt 190.1 (1.00x) 237.2 (1.25x) 230.2 (1.21x)

Made with criterion-table

@winston-h-zhang
Copy link
Member Author

I remain skeptical of using the WitnessCS API, since it still allocates large vectors on a per-prove-step basis. After running benchmarks though, it seems the difference is not significant. I've posted the results for transparency.

I'll hold off closing this for now, but a new PR with the WitnessCS API will be up, and we can merge that one.

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