perf: split keccak benchmark into narrow and wide variants - #2195
Open
gbotrel wants to merge 1 commit into
Open
Conversation
Two implementations of the same optimised keccak, benchmarked side by
side until the prover cost of wide call lookups is settled; the loser
gets deleted later.
Both variants: fully fused round (theta/rho/pi/chi/iota in one
straight-line pass), constant rotations as ::split/rejoin instead of
runtime shifts, and no integer division (the absorb loop guard replaces
/136, a u5::u3 split replaces /%8).
- keccak_narrow: 25 state lanes in keccak_state memory between rounds;
narrow call signatures, 50 memory accesses per round.
- keccak_wide: lanes threaded through keccak_f/keccak_round as
parameters/returns; the permutation touches no memory, keccak_state
shrinks to a 17-lane scratch for the final padded block, at the cost
of two 50-value call signatures.
KOALABEAR_16, 4,236-byte message (32 permutations), fast mode on 1 MB:
cells complexity fast-mode
old 35,924,894 195,360 2,639 ms
narrow 7,811,096 128,676 801 ms
wide 6,143,716 255,040 564 ms
ParallelTracing is dropped from the keccak tests: sharded tracing of
read-write memories violates active_monotony (see the
util.Config.ParallelTracing docstring) and already failed on the
unmodified benchmark whenever sampling picked the 4,236-byte input.
Signed-off-by: Gautam Botrel <gautam.botrel@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR replaces the single Keccak benchmark implementation with two side-by-side variants (narrow vs wide call signatures) to compare prover/runtime tradeoffs while keeping the underlying optimized round logic equivalent.
Changes:
- Split the Keccak benchmark into
keccak_narrow(state stored inkeccak_statememory between rounds) andkeccak_wide(state threaded through calls; permutation is memory-free). - Simplify the Keccak scratch-memory layout to match the new benchmark variants (remove now-unused buffers/offsets).
- Update ZkC bench tests to run both variants and drop
ParallelTracingdue to the known read-write-memory sharding issue.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| testdata/zkc/bench/keccak/memory.zkc | Shrinks/repurposes keccak_state layout to just rows + PIB scratch used by the new variants. |
| testdata/zkc/bench/keccak/impl.zkc | Removes the previous single Keccak implementation (superseded by narrow/wide variants). |
| testdata/zkc/bench/keccak/impl_narrow.zkc | Adds the memory-backed (“narrow-call”) Keccak implementation. |
| testdata/zkc/bench/keccak/impl_wide.zkc | Adds the register-threaded (“wide-call”) Keccak implementation with pure permutation calls. |
| testdata/zkc/bench/keccak/constants.zkc | Removes rotation/mod helpers that are no longer used by the fused-round implementations. |
| testdata/zkc/bench/keccak_narrow.zkc | New benchmark entrypoint wiring inputs/outputs for the narrow variant. |
| testdata/zkc/bench/keccak_narrow.accepts | Adds acceptance vectors for the narrow benchmark. |
| testdata/zkc/bench/keccak_wide.zkc | Switches benchmark entrypoint to include impl_wide.zkc. |
| testdata/zkc/bench/keccak_wide.accepts | Adds acceptance vectors for the wide benchmark. |
| pkg/test/zkc_bench_test.go | Splits the benchmark test into Narrow/Wide cases and removes ParallelTracing for stability. |
Suppressed comments (2)
testdata/zkc/bench/keccak/impl_wide.zkc:177
print_big_endianprintsb0..b7(least-significant byte first), so it is not big-endian. Updating the comment avoids confusion when comparing against expected digest byte order.
// print_big_endian prints a u64 in BIG-ENDIAN convention
testdata/zkc/bench/keccak/impl_narrow.zkc:62
print_big_endianprintsb0..b7(least-significant byte first), so it is not big-endian. Updating the comment avoids confusion about endianness.
// print_big_endian prints a u64 in BIG-ENDIAN convention
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| print_hash(s00, s10, s20, s30) | ||
| } | ||
|
|
||
| // print_hash prints the hash in BIG-ENDIAN convention |
| print_hash() | ||
| } | ||
|
|
||
| // print_hash prints the hash in BIG-ENDIAN convention |
Contributor
|
Additional stats: OriginalNarrowWide |
Contributor
|
The cell reduction for both wide and narrow implementations looks great, but they are not attacking the problem of columns. Overall, I am a little surprised the This is introducing a lot of temporary columns, instead of using a separate |
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
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.
Replaces the keccak benchmark with two implementations of the same optimised algorithm, kept side by side until the prover cost of wide call lookups is settled. Then we delete the loser.
Shared by both: fully fused round (θ ρ π χ ι in one straight-line pass), constant rotations as
::split/rejoin instead of runtime shifts, zero integer divisions (the absorb loop guard replaces/136; au5::u3split replaces/%8).keccak_narrow— 25 state lanes live inkeccak_statememory between rounds. Narrow call signatures, 50 memory accesses per round.keccak_wide— lanes threaded throughkeccak_f/keccak_roundas parameters/returns. The permutation touches no memory;keccak_stateshrinks to a 17-lane scratch for the final padded block. Cost: two 50-value call signatures.KOALABEAR_16, 4,236-byte message (32 permutations); fast mode on 1 MB:
wide wins cells and execution; narrow wins the complexity metric. The entire gap is the two 50-lane call-lookup edges — whichever way the lookup argument prices those, that variant stays.
ParallelTracingis dropped from the keccak tests: sharded tracing of read-write memories violatesactive_monotony(see theutil.Config.ParallelTracingdocstring). Pre-existing — reproducible on unmodified main withzkc trace --check --sharding keccak_f:10on the 4,236-byte input; the old test was flaky whenever sampling picked that line.Verified: 8/8 sponge-boundary hashes (lengths 0, 1, 7, 8, 135, 136, 137, 4236) +
trace --checkclean, both variants;Test_ZkcBench_Keccak{Narrow,Wide}green ×3;make zkc-lintclean.