EspressoSystems / jellyfish

A Rust Implementation of the PLONK ZKP System and Extensions
https://jellyfish.docs.espressosys.com
MIT License
408 stars 106 forks source link

perf: VID ADVZ verify_share use parallelism over multiplicity #650

Closed ggutoski closed 3 months ago

ggutoski commented 3 months ago

closes: #646

This PR does / key places to review:

How to test this PR:

From inside the vid/ directory run

RAYON_NUM_THREADS=N cargo bench --bench=advz_multiplicity --features="test-srs" 

On my local machine I observe significant improvement when I increase N from 1 to 6:

N=1:

advz_verify_payload_256KB_multiplicity/1
                        time:   [28.000 ms 29.097 ms 30.711 ms]
advz_verify_payload_256KB_multiplicity/256
                        time:   [364.81 ms 367.66 ms 371.66 ms]

N=6:

advz_verify_payload_256KB_multiplicity/1
                        time:   [27.515 ms 27.925 ms 28.442 ms]
                        change: [-5.5630% -2.1819% +0.6662%] (p = 0.24 > 0.05)
                        No change in performance detected.
advz_verify_payload_256KB_multiplicity/256
                        time:   [87.886 ms 89.704 ms 92.870 ms]
                        change: [-75.541% -74.950% -74.290%] (p = 0.00 < 0.05)
                        Performance has improved.

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.

akonring commented 3 months ago

Will let @mrain (or others) approve this since I don't have access to GPU. Otherwise, we could setup an ephemeral GPU server to test this.

akonring commented 3 months ago

And what's up with all the clippy warnings(?)

ggutoski commented 3 months ago

nit: 2 VID tests in advz::bytes_to_field are not working. Try cargo test -p jf-vid --features test-srs --release -- --ignored for details.

These doctests are ignored intentionally due to a Rust limitation that disallows access to private identifiers in doctest: Doctest of private functions - help - The Rust Programming Language Forum 😕

ggutoski commented 3 months ago

And what's up with all the clippy warnings(?)

Github CI is using a newer version of Rust than our Nix environment. We need to update Nix flake again. We'll do that in another PR.