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

refactor, perf: prover/crypto/sis improvements #554

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
Open

Conversation

gbotrel
Copy link
Contributor

@gbotrel gbotrel commented Jan 14, 2025

Sister PR in gnark-crypto.

This pull request includes significant refactoring and cleanup of the ringsis package within the prover/crypto directory. The changes mainly focus on removing unnecessary code and simplifying the implementation.

Essentially this PR uses the latest implementation from gnark-crypto, and concentrate the core logic of sis in gnark-crypto. Moreover, it reworked the TransversalHash to improve its performance (currently bls12377 only; koalabear to come).

Parameters for SIS are optimized for degree==64 and log2(bound) a multiple of 8 (byte size).

Allocations are reduced through the use of a LimbIterator . Removed buffers, locks. (the sis instance is now stateless, as it should be).

Removed all generated code for TransversalHash.

Benchmarks for transversal hash (matrix of size 1024*1024 on hpc7a )

ignoring BenchmarkTransversalHash/testcase-1100-192: before has 3 instances, after has 0
benchmark                                      old ns/op     new ns/op     delta
BenchmarkTransversalHash/testcase-0-192        3487853       3034115       -13.01%
BenchmarkTransversalHash/testcase-0-192        3549104       3317616       -6.52%
BenchmarkTransversalHash/testcase-0-192        3633424       3418450       -5.92%
BenchmarkTransversalHash/testcase-1-192        23254182      15093093      -35.10%
BenchmarkTransversalHash/testcase-1-192        23818238      15126497      -36.49%
BenchmarkTransversalHash/testcase-1-192        23526251      15075473      -35.92%
BenchmarkTransversalHash/testcase-10-192       25649222      16470154      -35.79%
BenchmarkTransversalHash/testcase-10-192       25710068      15966326      -37.90%
BenchmarkTransversalHash/testcase-10-192       25398553      16401198      -35.42%
BenchmarkTransversalHash/testcase-11-192       25284658      15625441      -38.20%
BenchmarkTransversalHash/testcase-11-192       25273225      15657162      -38.05%
BenchmarkTransversalHash/testcase-11-192       25041925      16189546      -35.35%
BenchmarkTransversalHash/testcase-100-192      24898676      15922206      -36.05%
BenchmarkTransversalHash/testcase-100-192      24709647      15831611      -35.93%
BenchmarkTransversalHash/testcase-100-192      24356574      16014427      -34.25%
BenchmarkTransversalHash/testcase-101-192      23785252      15019279      -36.85%
BenchmarkTransversalHash/testcase-101-192      23497154      14869629      -36.72%
BenchmarkTransversalHash/testcase-101-192      23961674      15057460      -37.16%
BenchmarkTransversalHash/testcase-110-192      26541592      16300219      -38.59%
BenchmarkTransversalHash/testcase-110-192      26483187      16229312      -38.72%
BenchmarkTransversalHash/testcase-110-192      26093940      16172361      -38.02%
BenchmarkTransversalHash/testcase-111-192      25964799      16054350      -38.17%
BenchmarkTransversalHash/testcase-111-192      25901920      16257190      -37.24%
BenchmarkTransversalHash/testcase-111-192      26087142      16434559      -37.00%
BenchmarkTransversalHash/testcase-1000-192     26856945      17358226      -35.37%
BenchmarkTransversalHash/testcase-1000-192     26985485      17223062      -36.18%
BenchmarkTransversalHash/testcase-1000-192     26834124      17065547      -36.40%
BenchmarkTransversalHash/testcase-1001-192     26720896      16411102      -38.58%
BenchmarkTransversalHash/testcase-1001-192     26578605      16115618      -39.37%
BenchmarkTransversalHash/testcase-1001-192     26379708      16212102      -38.54%
BenchmarkTransversalHash/testcase-1010-192     26365410      16191832      -38.59%
BenchmarkTransversalHash/testcase-1010-192     25969353      16190636      -37.65%
BenchmarkTransversalHash/testcase-1010-192     25966581      16437708      -36.70%
BenchmarkTransversalHash/testcase-1011-192     26235959      16006237      -38.99%
BenchmarkTransversalHash/testcase-1011-192     26321122      16089955      -38.87%
BenchmarkTransversalHash/testcase-1011-192     26284375      15963178      -39.27%

Benchmarks for transversal hash (matrix of size 1024102492 on hpc7a )

❯ benchcmp ringsis_64_16/ref_92x_hpc7a.txt new_92x_hpc7a8.txt
benchcmp is deprecated in favor of benchstat: https://pkg.go.dev/golang.org/x/perf/cmd/benchstat
benchmark                                     old ns/op     new ns/op     delta
BenchmarkTransversalHash/testcase-0-192       77954298      45216921      -42.00%
BenchmarkTransversalHash/testcase-1-192       645227476     570904781     -11.52%
BenchmarkTransversalHash/testcase-10-192      730085426     676721836     -7.31%
BenchmarkTransversalHash/testcase-11-192      716476925     650540575     -9.20%
BenchmarkTransversalHash/testcase-100-192     710198530     653845027     -7.93%
BenchmarkTransversalHash/testcase-101-192     715168735     657276892     -8.09%

benchmark                                     old allocs     new allocs     delta
BenchmarkTransversalHash/testcase-0-192       206417         130949         -36.56%
BenchmarkTransversalHash/testcase-1-192       206455         131116         -36.49%
BenchmarkTransversalHash/testcase-10-192      206434         131043         -36.52%
BenchmarkTransversalHash/testcase-11-192      206426         131081         -36.50%
BenchmarkTransversalHash/testcase-100-192     206371         131017         -36.51%
BenchmarkTransversalHash/testcase-101-192     206410         131042         -36.51%

benchmark                                     old bytes     new bytes     delta
BenchmarkTransversalHash/testcase-0-192       404228104     224151528     -44.55%
BenchmarkTransversalHash/testcase-1-192       404243848     224223568     -44.53%
BenchmarkTransversalHash/testcase-10-192      404235544     224193904     -44.54%
BenchmarkTransversalHash/testcase-11-192      404264456     224210528     -44.54%
BenchmarkTransversalHash/testcase-100-192     404207320     224182032     -44.54%
BenchmarkTransversalHash/testcase-101-192     404222856     224193104     -44.54%

Copy link

cla-assistant bot commented Jan 14, 2025

CLA assistant check
All committers have signed the CLA.

Copy link

cla-assistant bot commented Jan 14, 2025

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@codecov-commenter
Copy link

codecov-commenter commented Jan 14, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.44%. Comparing base (728487b) to head (26b5930).

Additional details and impacted files
@@             Coverage Diff              @@
##               main     #554      +/-   ##
============================================
- Coverage     68.89%   68.44%   -0.45%     
+ Complexity     1186     1139      -47     
============================================
  Files           327      326       -1     
  Lines         13128    12876     -252     
  Branches       1317     1309       -8     
============================================
- Hits           9044     8813     -231     
+ Misses         3535     3515      -20     
+ Partials        549      548       -1     
Flag Coverage Δ *Carryforward flag
hardhat 98.74% <ø> (ø)
kotlin 66.00% <ø> (-0.54%) ⬇️ Carriedforward from 54322d9

*This pull request uses carry forward flags. Click here to find out more.

see 21 files with indirect coverage changes

@AlexandreBelling AlexandreBelling added the Prover Tag to use for all work impacting the prover label Jan 15, 2025
Copy link
Contributor

@AlexandreBelling AlexandreBelling left a comment

Choose a reason for hiding this comment

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

Approved but there are a couple of issues on the CI

@gbotrel gbotrel had a problem deploying to docker-build-and-e2e January 15, 2025 15:23 — with GitHub Actions Error
@gbotrel gbotrel had a problem deploying to docker-build-and-e2e January 17, 2025 15:08 — with GitHub Actions Error
@gbotrel gbotrel had a problem deploying to docker-build-and-e2e January 17, 2025 15:30 — with GitHub Actions Error
@gbotrel gbotrel had a problem deploying to docker-build-and-e2e January 17, 2025 15:35 — with GitHub Actions Error
@gbotrel gbotrel enabled auto-merge (squash) January 17, 2025 16:26
@gbotrel gbotrel requested a deployment to docker-build-and-e2e January 23, 2025 02:20 — with GitHub Actions Waiting
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Prover Tag to use for all work impacting the prover
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants