Add runtime-selected LtHash backend with AVX-512 Blake3 XOF kernel - #4151
Conversation
🤖 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).
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4151 +/- ##
==========================================
- Coverage 66.60% 65.48% -1.13%
==========================================
Files 2196 2090 -106
Lines 169188 157791 -11397
==========================================
- Hits 112692 103325 -9367
+ Misses 56355 54325 -2030
Partials 141 141
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
LtHash default vs SIMD (
|
LtHash default vs SIMD (
|
PR SummaryMedium Risk Overview The default backend keeps the prior pooled A new GitHub Actions workflow runs Reviewed by Cursor Bugbot for commit 356d8e3. Bugbot is set up for automated code reviews on this repo. Configure here. |
![seidroid[bot]](https://avatars.githubusercontent.com/in/2745135?s=60&v=4){kind=small}
There was a problem hiding this comment.
The backend indirection is clean and the generated 16-lane BLAKE3 root-XOF kernel traces correctly against the spec (flags, chunk-vs-output counter, message permutation, finalize, and the little-endian limb aliasing all check out), with a solid differential test against zeebo/blake3 across block and chunk boundaries. No blocking defects; the notes below are about how the new consensus-critical second implementation is validated in CI and whether it reaches production builds at all.
Findings: 0 blocking | 4 non-blocking | 2 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- [suggestion] No production build path enables the SIMD backend: no Makefile target, Dockerfile, or release workflow in the tree sets
GOEXPERIMENT=simd, so shippedseidbinaries compilebackend_nosimd.goand always run the default path. The speedups in the description are not realized by any current build. Worth stating explicitly in the PR description (or a follow-up) so the change isn't assumed to be live on validators. - [suggestion] Nothing logs
ActiveBackend()— it is exported but referenced only frombackend_test.go. Since two kernels now feed a consensus-relevant hash, a node should record which one it selected at startup (e.g. where the flatkv hash engine is constructed); otherwise, if a divergence ever did occur, there would be no way to tell from logs which nodes ran which kernel.SEI_LTHASH_BACKENDis also undocumented outside the const's godoc. - 2 suggestion(s)/nit(s) flagged inline on specific lines.
| - name: Test without GOEXPERIMENT=simd | ||
| run: go test -count=1 -race ${{ env.LTHASH_PKG }} | ||
|
|
||
| - name: Test with GOEXPERIMENT=simd |
There was a problem hiding this comment.
[suggestion] This step passes vacuously when the runner CPU lacks AVX-512F/VBMI2. simdBackend() returns false, so availableBackends() yields only default, and TestBackendsAgreeWithReference / TestBackendsAgreeOnMix degrade to comparing zeebo/blake3 against itself — green, with no signal that the SIMD kernel was never exercised. The ::warning:: at line 98 only covers the benchmark step, and go test here runs without -v, so the missing subtest isn't visible in the log either.
Since the main go-test.yml never sets GOEXPERIMENT=simd, this workflow is the only place the kernel is validated at all, and a consensus-critical hash implementation can reach main untested if neither matrix leg has AVX-512. Consider asserting coverage — e.g. run this step with -v and grep -q 'TestBackendsAgreeWithReference/simd', or emit the same ::warning:: (or fail a dedicated job) when no matrix leg saw simd.
|
|
||
| // selectBackend picks the backend named by pin, or the fastest available one | ||
| // when pin is empty or unknown. | ||
| func selectBackend(pin string) backend { |
There was a problem hiding this comment.
[suggestion] An unrecognized SEI_LTHASH_BACKEND value silently falls through to automatic selection. The knob's main operational use is pinning the portable path during a suspected divergence, and a typo (defualt) — or pinning simd on a host without AVX-512 — produces exactly the opposite of what the operator asked for, with no log line and no error to notice it by. Emitting a warning when pin != "" and the name isn't in availableBackends() would make the failure self-reporting.
…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`.
…protocol#4157) 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/sha256` already uses single-lane SHA-NI, so the achievable win is smaller than for LtHash (sei-protocol#4151) and had to be measured rather than assumed. This adds `sei-tendermint/crypto/tmhash` with the same runtime-selected backend pattern as sei-protocol#4151. `SumBatch(prefix, msgs, out)` is served by a default backend (a reused `sha256.New()`, always compiled) or, under `GOEXPERIMENT=simd` on a CPU with AVX-512F/VBMI/VBMI2, by a generated 16-lane `archsimd.Uint32x16` SHA-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=default` pins the portable path. `HashFromByteSlices` keeps 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 6962 `getSplitPoint` shape. A differential test checks totals 1 to 130 against the recursive implementation, and the tmhash tests check every backend against `crypto/sha256` across block and padding boundaries, so the output is byte-identical. Two Go 1.27 findings are handled here. The compiler never emits `VZEROUPPER` after `archsimd` code, so the legacy-SSE SHA-NI path that followed ran several times slower with dirty ZMM state; the SIMD backend calls a one-instruction assembly `vzeroupper` before handing off. Separately, with `GOEXPERIMENT=simd` on an AVX-512 machine the runtime's async preemption restores the ZMM registers without `VZEROUPPER`, 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 a `GOEXPERIMENT=simd` binary. 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 backends` workflow runs both packages' tests with and without the experiment and posts the benchstat table as a job summary and PR comment.
2 participants
Giga's flatkv LtHash spends most of a block's hashing time in the Blake3 XOF that expands each serialized key/value into 2048 bytes, followed by the scalar MixIn/MixOut over 1024 uint16 limbs. Profiling put Blake3 compression at roughly 56% of hashChunk and the two mixes at another 31%. The 32 XOF output blocks of one hash are independent compressions of the same chaining value with different counters, which maps directly onto a 16-lane AVX-512 kernel without any cross-mutation batching, and the limb arithmetic is a plain wrapping add/sub over 32 lanes of uint16.
This change moves the expand/add/sub steps behind a small backend struct selected once at init. The default backend is the existing pooled zeebo/blake3 XOF plus scalar mixing and always builds. A second backend, compiled only under
goexperiment.simd && amd64and enabled at runtime only whenarchsimd.X86.AVX512()andAVX512VBMI2()report support, runs a generated, fully unrolled 16-lane Blake3 compression usingsimd/archsimd(VPSHRDD for the rotates, pre-broadcast input rows loaded as vectors to avoid the legacy-SSE cost ofBroadcast*) and vectorised limb mixing. Inputs longer than one Blake3 chunk fall back to the default expand. Output is byte-identical to the reference;SEI_LTHASH_BACKEND=defaultpins the portable path. A new workflow builds and tests the package both with and without the experiment, benchmarks every backend the runner CPU can execute, and writes abenchstat -col /backendcomparison to the step summary.Locally on a Xeon 8559C (AVX-512 + VBMI2), benchstat over 4 runs: Expand 2.61 µs → 1.04 µs, MixIn 213 ns → 18 ns, HashKV 3.13 µs → 1.08 µs, hashChunk (1000 mutations) 5.40 ms → 2.37 ms. Differential tests compare every backend against zeebo/blake3 across block and chunk boundaries (1..5000 bytes), existing lthash tests pass unchanged under both builds with
-race, andgolangci-lint runis clean with and withoutGOEXPERIMENT=simd.