Conversation
Adds sei-tendermint/crypto/tmhash with a default crypto/sha256 backend and a 16-lane AVX-512 kernel built under goexperiment.simd, and routes merkle.HashFromByteSlices through level-batched hashing when a multi-lane backend is active. Output is byte-identical to the recursive tree.
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
LtHash default vs SIMD (
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4157 +/- ##
==========================================
- Coverage 66.68% 65.40% -1.28%
==========================================
Files 2201 2082 -119
Lines 169523 157308 -12215
==========================================
- Hits 113039 102891 -10148
+ Misses 56343 54276 -2067
Partials 141 141
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
tmhash / merkle default vs SIMD (
|
LtHash default vs SIMD (
|
tmhash / merkle default vs SIMD (
|
…sh-simd-merkle # Conflicts: # .github/workflows/lthash-bench.yml
…otocol#4164) Follow-up to sei-protocol#4151. The Go compiler emits no `VZEROUPPER` after `archsimd` AVX-512 code, so the LtHash SIMD kernels returned to their callers with dirty upper ZMM halves. Any legacy-SSE code that runs next (`memmove`, SHA-NI, encoding helpers) then pays an upper-state merge penalty until the registers are cleared; a throwaway benchmark of `Expand` followed by `sha256.Sum256` over the 2 KiB serialisation goes 4.36 µs → 3.69 µs (-15%) on an Intel Xeon 8559C once the guard is in place, while the package's own benchmarks are neutral (MixIn +~1 ns, the cost of the instruction itself). This adds a package-local `vzeroupper()` asm stub, the same shape as the one in sei-protocol#4157, and calls it from thin wrappers registered in `simdBackend()`, the one place every SIMD entry point passes through, so a kernel added later cannot skip it. Hash output is unchanged and the differential tests pass under both builds; the stub is only built under `goexperiment.simd && amd64`.
PR SummaryHigh Risk Overview
CI renames the workflow to SIMD hash backends, widens path triggers, and adds a Reviewed by Cursor Bugbot for commit 38fbc9c. Bugbot is set up for automated code reviews on this repo. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 2a05ac8. Configure here.
![seidroid[bot]](https://avatars.githubusercontent.com/in/2745135?s=60&v=4){kind=small}
There was a problem hiding this comment.
Adds a runtime-selected 16-lane AVX-512 SHA-256 backend for batched Merkle hashing plus a level-batched HashFromByteSlices; the tree shape, padding, index bucketing and pooled-scratch reuse all check out, and the production default path is unchanged outside GOEXPERIMENT=simd builds. Two non-blocking items: the new benchmark job pools two different builds into one default baseline, and the mixed-size differential test never exercises the over-simdMaxBlocks fallback alongside filled SIMD lanes.
Findings: 0 blocking | 2 non-blocking | 2 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- None at the file/PR level.
- 2 suggestion(s)/nit(s) flagged inline on specific lines.
1 participant
Tendermint's Merkle hashing (
merkle.HashFromByteSlices: tx hashes, part sets, commit signatures, results, validator sets) hashes every leaf and every tree level as independent SHA-256 calls, which is the batch shape a multi-lane kernel wants.crypto/sha256already uses single-lane SHA-NI, so the achievable win is smaller than for LtHash (#4151) and had to be measured rather than assumed.This adds
sei-tendermint/crypto/tmhashwith the same runtime-selected backend pattern as #4151.SumBatch(prefix, msgs, out)is served by a default backend (a reusedsha256.New(), always compiled) or, underGOEXPERIMENT=simdon a CPU with AVX-512F/VBMI/VBMI2, by a generated 16-lanearchsimd.Uint32x16SHA-256 kernel that loads, prefixes, pads and transposes sixteen messages in-register and falls back to scalar for remainders and mixed lengths;SEI_TMHASH_BACKEND=defaultpins the portable path.HashFromByteSliceskeeps its signature and, when a multi-lane backend is active and there are at least sixteen leaves, builds the tree level by level, pairing adjacent nodes and carrying an odd trailing node up, which is the RFC 6962getSplitPointshape. A differential test checks totals 1 to 130 against the recursive implementation, and the tmhash tests check every backend againstcrypto/sha256across block and padding boundaries, so the output is byte-identical.Two Go 1.27 findings are handled here. The compiler never emits
VZEROUPPERafterarchsimdcode, so the legacy-SSE SHA-NI path that followed ran several times slower with dirty ZMM state; the SIMD backend calls a one-instruction assemblyvzeroupperbefore handing off. Separately, withGOEXPERIMENT=simdon an AVX-512 machine the runtime's async preemption restores the ZMM registers withoutVZEROUPPER, which slowed all SHA-NI code in the same binary 2 to 4x; the CI job therefore takes the default column from a plain build and the SIMD column from the experiment build. That second effect applies to every legacy-SSE path in the process and should weigh on any decision to ship aGOEXPERIMENT=simdbinary.On an Intel Xeon Platinum 8559C (benchstat, n=8) the kernel is 2.0x faster than SHA-NI on 1024 inner nodes (124 µs to 61 µs), 1.6x on 256-byte leaves and 1.3x on 1 KiB leaves; the whole 1024 x 32-byte-leaf tree goes from 223 µs to 122 µs. This is a per-block cost of a few thousand hashes, so the node-level effect is modest. The
SIMD hash backendsworkflow runs both packages' tests with and without the experiment and posts the benchstat table as a job summary and PR comment.