perf: MSM/FFT prover optimizations, build flags, C API use-after-free fix - #49
Open
OBrezhniev wants to merge 10 commits into
Open
perf: MSM/FFT prover optimizations, build flags, C API use-after-free fix#49OBrezhniev wants to merge 10 commits into
OBrezhniev wants to merge 10 commits into
Conversation
groth16_prover_create_zkey_file loaded the zkey into a local FileLoader, passed its mmap'd buffer to groth16_prover_create, then returned -- running the FileLoader destructor and munmap-ing the buffer. BinFile stores the pointer without copying (addr = fileData), so every zkey section pointer held by the Prover dangled, causing a segfault on the first prove() call. Add a file-path constructor to Groth16Prover that builds its BinFile from the path directly. BinFile's filename ctor owns its FileLoader member for the object's lifetime, so the zkey stays mapped across every prove(). The buffer and file-path ctors now share a common init() helper. Route groth16_prover_create_zkey_file through the new ctor with matching null-arg and exception handling. Verified: create_zkey_file + two proves + snarkjs verify OK (previously crashed with SIGSEGV on the first prove). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add portable build-optimization knobs (all opt-in; default `make host` and direct cmake builds are unchanged). Measured on a 1.1GB sha256 zkey (C-API steady-state per-prove), identical RSS across configs and proofs verifying OK: host_lto -5.7% portable host_march_lto -8.3% host-CPU-locked host_pgo +2% vs march_lto (worse on this ASM-heavy workload) - CMake: USE_MARCH_NATIVE / USE_LTO options (default OFF) + PGO=generate|use two-phase support. -march=native bakes in host CPU features (SIGILL risk on older CPUs) so it is opt-in; LTO is portable. - Makefile: host_march / host_lto / host_march_lto / host_pgo targets, plus *_lto variants for android / android_x86_64 / ios. Expanded `clean` to cover the new build/package dirs. - PGO is wired but measured a ~2% regression here (the compute hot path is hand-written NASM, leaving little branchy C++ for PGO); kept as documented opt-in. Mobile LTO targets are config-validated for the cross toolchain; on-device build/benchmark still pending (no NDK/Xcode in this environment). - binfile_utils.hpp: add #include <cstdint> (GCC 16 build fix; the header uses uint32_t/uint64_t without including it). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Bumps ffiasm to the scalar-size partitioned MSM with wall-clock-aware window selection, and adds test_msm: correctness of multiMulByScalarMSM against the independent ParallelMultiexp across witness-like scalar distributions and boundary values, plus a benchmark mode (test_msm bench). Measured G1 n=2^20 on 20 cores: uniform -14%, iden3-realistic witness -14%, all-binary -87%. End to end: sha256_test proof 2.41s -> 1.85s (-23%); small identity circuits unchanged (dominated by zkey load). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JpMJeCdh75Shs3hBRFFLkq
Bumps ffiasm to the task-based MSM and, on pools of 12+ threads, prepares the A, B1, B2 and C multiexponentiations up front and runs all their bucket tasks in a single parallel region (G1 and G2 tasks mixed, G2 first as the heaviest), so no MSM's straggler tail leaves cores idle. Below 12 threads the sequential path is kept: the batch holds every MSM's scratch at once, a poor trade on mobile. test_msm gains a batch correctness test mirroring the prover phase. Measured on 20 cores, interleaved, thermally controlled: identity circuits (authV3, sigV2, mtpV2, OnChain variants, V3) -5..-9% proof time end to end; sha256 unchanged on top of the previous -23%. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JpMJeCdh75Shs3hBRFFLkq
Bumps ffiasm to batch-affine bucket accumulation and adapts the prover's batched MSM phase and test harness to the byte-based task arena. test_msm gains infinity-base and duplicate-point-pool tests covering the new accumulator's special-case paths. Measured on 20 cores, interleaved: sha256 proof 1.89 -> 1.59 s (-16%), OnChain identity circuits -9..-10%, others -6%. Cumulative over the three MSM waves: sha256 2.41 -> 1.59 s, identity circuits -10..-18%. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JpMJeCdh75Shs3hBRFFLkq
Replaces the ifft/shift/fft trio with DIF-iFFT -> one fused coset pass -> DIT-FFT. The coset table (omega_2n^BR(i) with 1/n folded in, bit-reverse indexed) is precomputed once in the Prover constructor and reused by every proof. The coset stays the odd powers of omega_2n that snarkjs bakes into the zkey's H points, so zkey compatibility is untouched and the evaluations return in natural order for the H MSM. Eliminates six bit-reversal permutation passes and three reindex/scale passes per proof. test_msm gains a unit test asserting the new pipeline reproduces ifft+shift+fft exactly. Measured interleaved on 20 cores: sha256 -8.6%, sigV2/v3/authV2 -6%. Cumulative over the four optimization waves: sha256 2.41 -> 1.38 s (-43%), identity circuits -11..-18%. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JpMJeCdh75Shs3hBRFFLkq
Fixes the iOS simulator Debug dylib link on CI (undefined BATCH_SIZE for arm64): the MSM class constants are now a typed enum, never ODR-used. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JpMJeCdh75Shs3hBRFFLkq
Bumps ffiasm to the int16-digit / indexed-partition / halved-roots MSM+FFT, and: - the batched A/B1/B2/C phase uses one shared bucket arena for both curves (a thread runs one task at a time; a common stride keeps the per-thread rows disjoint) instead of separate G1 and G2 arenas; - the Prover builds FFT(domainSize) instead of FFT(domainSize*2), deriving omega_2n via FFT::higherRootOfUnity and computing the bit-reverse-indexed coset table from a chunked power scan. Measured interleaved on 20 cores: peak RSS -15..-25% on the identity circuits (authV2 84 -> 63 MB), -9.7% on sha256 (1444 -> 1304 MB); proof times unchanged or slightly better (sha256 -1.4%, OnChain circuits -3.4/-3.8%). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JpMJeCdh75Shs3hBRFFLkq
The bit-reversed coset table was built in two passes: scan the powers of omega_2n into a full-size scratch array, then permute into bit-reversed order while folding in 1/n. That costs two multiplies per element, and it keeps a second domainSize table alive alongside cosetBR -- 64 MB each at 2^21, on a library that also ships to phones. Walking the exponent j upward lets each power chain from the previous one, and folding 1/n into the per-thread seed leaves a single multiply per element. Storing at BR(j) rather than j is what removes the second pass: BR is an involution, so sequential j fills exactly the bit-reversed table the h pipeline reads, and it is a bijection, so threads owning disjoint j ranges write disjoint slots -- no synchronisation needed. Measured on i9-13900H, 20 threads, 11 interleaved rounds, package gated below 58 C before each configuration: domain construct peak RSS (construction only) 2^21 30.0 -> 17.1 ms 196.6 -> 132.4 MB (-64.2 MB) 2^16 1.10 -> 1.36 ms 10.5 -> 8.2 MB ( -2.3 MB) 2^16 is a small regression, about +0.26 ms, because the scattered writes cost more than the saved pass once the whole table fits in cache. That is ~0.15% of a cAQV3 proof, against 1.4-1.8% for the whole of construction, so it is not worth branching on domain size to avoid. A cache-oblivious blocked bit-reversal would fix both ends and is not worth the complexity for a table built once per prover. Two things this deliberately does not do. It does not lower the peak RSS of a proof -- the proof itself peaks near 1.13 GB at 2^21, far above construction, and a full-run measurement is unchanged (1160380 vs 1160424 KB). What it saves is transient allocation, which matters when several provers are constructed concurrently. And the roots table cannot get the same treatment: rootInv(s, j) reads omega^(n-j) out of its upper half, so the full domainSize array is load-bearing and halving it would need a separate inverse table of the same size. Proofs verified on cAQV3, authV3 and sha256_test (2^21); public signals byte-identical to the pre-change build. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q4sRt9NSzrTmWyMRkka2ed
…ffiasm bump) The FFT roots table and the bit-reversed coset table depend on nothing but the scalar field and the domain size -- not the circuit, the points, or the witness. Every Groth16Prover nevertheless built its own pair, 128 MB per instance at 2^21, so a service holding one prover per circuit paid that per circuit even when the circuits share a domain size -- as the Polygon ID set does (cAQV3 and authV3 are both 2^16). Both tables move into a DomainTables object handed out by a registry of weak references keyed on domain size, one registry per Engine. Provers hold a shared_ptr; the last one out frees the tables, so a lone prover behaves exactly as before -- build on construction, free on destruction -- and nothing persists past the provers using it. Construction runs under the registry lock so two provers of one size cannot both build; that briefly serialises construction of different sizes too, paid once per domain size per process against a build of tens of milliseconds. Immutability of the shared tables is compiler-enforced, not conventional. DomainTables keeps its pointers private and hands out only a const FrElement* and an FFT whose table accessors (root/rootInv/nInv -- const-qualified in the accompanying ffiasm bump) return const references; the registry vends shared_ptr<const DomainTables>, and Prover's borrowed cosetBR pointer is pointer-to-const. Verified negatively: writing a coset entry, writing a root through the FFT, and assigning the table to a mutable pointer each fail to compile, while the unmodified control compiles. Sharing across concurrent proofs is safe because the transforms hold no per-proof state: they read the tables only through the const accessors and write solely to the caller's array. Verified by racing proofs on two provers sharing tables, plus a lifecycle check at 2^21 with no proofs in between muddying RSS: first prover +129.0 MB tables built second prover +0.4 MB shared destroy 2nd -0.4 MB refcount 2->1, nothing freed destroy 1st -128.4 MB last ref gone, tables freed new prover +128.4 MB rebuilt, not resurrected Proofs verified on cAQV3, authV3 (both 2^16, different circuits, shared tables) and sha256_test (2^21); public signals byte-identical to the previous commit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q4sRt9NSzrTmWyMRkka2ed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Groth16 prover performance work plus two smaller items that were already on this branch.
Prover optimizations (bumps ffiasm to iden3/ffiasm#7)
Four waves, each committed and benchmarked separately (interleaved old/new binaries, thermally controlled, 20-core x86-64):
ee122b8) — witness scalars classified by bit-length; 0/1 wires cost ~one addition instead of ~16 window additions. Addstest_msm, a correctness/benchmark harness (ctest target) comparing against the independentParallelMultiexpimplementation.184a656) — on pools of 12+ threads, the A/B1/B2/C multiexponentiations execute as one task set in a single parallel region (G2 first), removing four sequential barriers and straggler tails. Below 12 threads the sequential path is kept (mobile memory).0f2412b) — ~5M+1S per bucket addition instead of 8M+2S, batches of ≤512 additions per field inversion.98edc5c) — DIF-iFFT → fused coset pass → DIT-FFT with a per-prover precomputed bit-reverse-indexed coset table; eliminates six permutation passes and three scaling passes per proof. The coset stays the odd powers of ω_2n baked into snarkjs zkeys, so zkey compatibility is unchanged (unit test asserts exact equivalence with the old ifft/shift/fft).Measured end-to-end (median of interleaved rounds)
Every wave was verified by proving and verifying on nine production circuits plus the
test_msmsuite (scalar distributions, boundary values, infinity bases, duplicate points, mixed-curve batching, FFT equivalence).Also on this branch
4af8b85— opt-in-march=native/LTO/PGO build targets and mobile-LTO targets212bff8— fix use-after-free ingroth16_prover_create_zkey_file🤖 Generated with Claude Code
https://claude.ai/code/session_01JpMJeCdh75Shs3hBRFFLkq