Repository navigation
fix: share setup-prefix coefficients and inner rows - #261
Conversation
|
Warning This PR has more than 500 changed lines and does not include a spec. Large features and architectural changes benefit from a short spec before implementation review. If this PR is a bug fix, small cleanup, or otherwise does not warrant a spec, feel free to ignore this message. |
Documentation blast radius (advisory)These regions may need doc/spec/book updates based on changed paths. Changed files in this PR: 19
|
e61b1c0 to
12fcf74
Compare
PCS Profile Benchmark
13 of 13 profiles passed. Times are medians of Each sample verifies the same proof first with the configured multi-threaded pool and then with one thread. Both timings reuse the same verifier setup. Merge-base comparisons are available for Benchmark shards
Public opening statements
One-hot profiles generate deterministic witnesses with one Direct evaluates the public setup contribution during Stage 2. Recursive carries the same check through a Stage 3 setup-product sumcheck. Both modes execute the complete fold schedule and terminal verification. The chunked profiles Generated profiles may select different A, B, and D ring dimensions at different fold levels. The short profile names omit those dimensions. Each sample generates deterministic witnesses and opening points, prepares setup, commits, proves, serializes the proof, checks its size, prepares verifier setup, and verifies the claimed openings. It does not test malformed proofs. Phase time
Proof size and protocol shape
Grinding retries are rejected attempts at each fold, listed in measured-run order. Zero means the first sampled nonce was accepted. Each multi-group profile has two precommitted nv16 singleton polynomials and two polynomials in the displayed final group. Native proof accounting
Memory and setup size
Deltas are shown only for profiles with a matching merge-base case. Negative is smaller or faster. The uploaded |
CI test timing
Run summary
Wall time spans 2 parallel nextest slice shards. Slowest tests
Regressions vs main
New slow testsNo new tests ≥30s vs main baseline. |
|
/ai-review |
|
AI review attempt 1/3 reserved. Failed or incomplete attempts also count toward this PR's limit. |
There was a problem hiding this comment.
Usefulness: benefit supported. The memory-saving premise is supported: setup-prefix sources replace per-prefix coefficient copies with bounded Arc-backed views, and equal inner rings share storage after coefficient-by-coefficient comparison. Lazy source initialization also avoids retaining polynomial sources during export. These benefits justify the storage changes, although the new streaming arithmetic adds maintenance surface and validation currently introduces avoidable full-image copies.
Approval not recommended: new P0-P3 findings in this review.
| let mut total_coefficients = 0usize; | ||
| for rows in &self.inner_rows { | ||
| for rows in self | ||
| .inner_rows() |
There was a problem hiding this comment.
[P2] For shared hints, inner_rows() reconstructs every coefficient in a fresh buffer, so this shape check now copies the entire A image just to inspect its dimensions. Registry insertion and checking reach this through hint.check(), which then materializes the image a second time; serialization likewise materializes once here and again to write coefficients. Even a warm import of an exported registry or serialization directly to a file therefore needs an extra full-image allocation and repeated copies. Can we expose shape metadata and coefficient iteration on InnerRows, validate and serialize the shared blocks directly, and reserve materialization for consumers that actually require contiguous storage?
|
/ai-review |
|
AI review attempt 2/3 reserved. Failed or incomplete attempts also count toward this PR's limit. |
There was a problem hiding this comment.
Usefulness: benefit supported. The memory-saving benefit is supported by the code: bounded Arc-backed setup views replace per-prefix coefficient copies, equality-checked interning shares inner rings across hints and imports, and export no longer eagerly retains polynomial sources. The added pooling and tiled-arithmetic complexity serves those savings while reusing existing sampling, decomposition, NTT, and compression primitives. The latest revision also removes the previously identified full-image allocations from validation and serialization, with regression assertions covering byte equality and zero materializations.
Recommended for approval: no unresolved findings in this automated review.
Summary
Share setup-prefix coefficients through bounded views of the expanded setup. Store equal inner ring commitments once across prefix hints, including clones and imports.
Motivation
Verifier preprocessing retains duplicate source coefficients and inner commitments across prefixes. Shared storage reduces memory use while keeping setup ownership within each backend.
Testing
Tests cover source lifetime, bounds, zero padding, shared rows, hash collisions, concurrent access, imports, and serialized byte equality.
Security Considerations
Verifier acceptance, transcripts, serialization, and decoding limits remain unchanged. Hashes select sharing candidates only; every coefficient must match before storage is shared.
No dependencies or unsafe code are added.
Breaking Changes
None.