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

Performance: parallelising kimchi prover more #2969

Draft
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

volhovm
Copy link
Member

@volhovm volhovm commented Jan 27, 2025

A bunch of performance improvements to the kimchi prover aiming to utilise more parallelism where available.

As of now, the save-up seems to be as follows:
screen_2025-01-29_001

TODOs:

  • Check how much faster it is on mina side. Adjust the benchmark if necessary (more columns? lookups?)
    • There is a 7% improvement as opposed to the 22% on just prover side. Maybe the benchmark circuit is too simplistic (not enough lookups?). Maybe it's because mina runs some other threads in parallel (like two provers in parallel?).
  • Make sure there are no regressions for the verifier side.
    • Parallelising constraints seems to add 3% to the verifier for 1024 gates (and nothing when there are 16k gates?)?? generally this is hard to verify: even running every verifier benchmark for 5 minutes I still get sometimes 0.5% noise. Really hard to tell.
    • Conclusion:
      • Measuring verification accurately seems to be hard for some reason I do not understand. I checked every single test parameter -- each verification bench runs for about 8 minutes (that's 5-6k iterations in total), and it's averaged over multiple SRS setups, over multiple proofs, and the benchmark type is "linear" which according to the criterion docs is the best way to do it. While in theory this seems more than enough to give reproducible results, I still see average noise around 0.5% (and sometimes up to 2%!!! out of nowhere) on both my laptop and on my server. What's even weirder is that in these cases the distribution has low variance still. I suspect this might be of some system-level threading: maybe due to the highly parallelised verification even 1-2 threads used by some system process causes this noise? I suspect this should be better on a "pristine clean" machine, or if we didn't rely heavily on parallelisation.
      • I think that there was a tiny regression (1-2%: 89ms instead of 86ms) for small circuits because of constraint parallelisation. Not important in any case, but I removed parallelisation for smaller (<4k) circuits. I do not observe any reproducible difference right now.
  • Check if parallel evaluations can be faster -- do we need to clone the vector and then interpolate?
    • Does not matter much. Interpolating directly is only 1% faster, not worth the change.
  • Check that perm_aggreg optimisation preserves correctness
  • Last step: run the threading profiler once again to make sure I didn't miss anything

@volhovm volhovm force-pushed the volhovm/profiling-kimchi-thread-utilisation branch 26 times, most recently from fe029f4 to f8ddd85 Compare January 29, 2025 14:53
Copy link

codecov bot commented Jan 29, 2025

Codecov Report

Attention: Patch coverage is 97.11934% with 7 lines in your changes missing coverage. Please review.

Project coverage is 76.87%. Comparing base (22a9862) to head (5472fed).
Report is 77 commits behind head on master.

Files with missing lines Patch % Lines
kimchi/src/prover.rs 84.78% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2969      +/-   ##
==========================================
- Coverage   76.95%   76.87%   -0.08%     
==========================================
  Files         261      261              
  Lines       61457    62092     +635     
==========================================
+ Hits        47293    47736     +443     
- Misses      14164    14356     +192     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@volhovm volhovm force-pushed the volhovm/profiling-kimchi-thread-utilisation branch 3 times, most recently from 5472fed to 25d64a0 Compare February 5, 2025 15:42
@volhovm volhovm force-pushed the volhovm/profiling-kimchi-thread-utilisation branch from d12e88e to a979221 Compare February 7, 2025 17:30
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.

1 participant