You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
common/hexutil: hex encode/decode in simd - #24410
Reason: reth using const-hex, neth using Core/Extensions/HexEncoder.cs (encode: AVX-512 VBMI/AVX2/SSSE3/NEON), HexConverter.cs (decode: SSSE3/NEON)
Hex encoding and decoding for RPC byte fields in assembly: AVX2 on amd64, NEON on arm64. amd64 without AVX2 and other architectures use encoding/hex. No GOEXPERIMENT needed.
The first version used simd/archsimd (up to 731ff44). It needs GOEXPERIMENT=simd, which slows all Go code on AVX-512 CPUs (golang/go#81847: struct copy +26%), so it could not ship in release builds.
ns per call (lower is better), n5 (AMD EPYC 4344P) and M4 Max; simd is the archsimd kernel:
Assembly is never async-preempted, so hex_asm.go feeds it at most 64 KiB per call. That alone is not enough: the assembler drops the stack check of a leaf function with a frame under 128 bytes, so the kernels declare a 128-byte frame, and the check is where a pending preemption stops the goroutine between chunks. Worst GC stop-the-world wait while 64 MiB goes through, GOMAXPROCS=2: 5-6 ms in one call, 0.012-0.016 ms in 64 KiB chunks.
Decoding too, by algorithm 3 of http://0x80.pl/notesen/2022-01-17-validating-hex-parse.html (as const-hex in alloy/reth): a digit maps to 0-9 and a letter of either case to 10-15 on two saturating paths, anything else to more than 15 on both, so the smaller of the two is the nibble, and one VPMADDUBSW merges each pair. A block holding a character that is not a hex digit falls to hex.Decode, which reports it with the same error.
It is 18.9% of an eth_call replaying mainnet tx 0x8653e24402c22383aaf3c92a9b08307ac4bada805eef06bd945bedb639af7393 (block 26063051, 110.8 KB of calldata), reached from hexutil.(*Bytes).UnmarshalText.
arm64 too, on NEON: encode by VTBL table lookup, 16 bytes per iteration; decode by the same algorithm, 32 characters per iteration, packed with VUZP1/VUZP2.
Both AVX2 kernels end with VZEROUPPER, as Go CL 795820 does: the Go code after them is SSE, which pays a false dependency on Intel while the upper halves of the Y registers are dirty.
The reason will be displayed to describe this comment to others. Learn more.
Kernel verified: I built the head with go1.27.1 + GOEXPERIMENT=jsonv2,simd for amd64 and ran it under Rosetta 2 (which reports AVX2): common/hexutil passes, also with -race and with GODEBUG=cpu.avx2=off; rpc/jsonstream, common and execution/types pass too. Four hand-made kernel mutants (table digit, nibble mask, shift width, dst stride) all fail TestEncodeHexMatchesStdlib. The vector loop has no bounds checks in the disassembly, as the commit says. No correctness issue.
Two things I'd like fixed before merge, one small regression, and a few notes.
1. encodeHex returns with dirty YMM upper halves (no VZEROUPPER)
go1.27.1 does not emit VZEROUPPER for archsimd code (checked with objdump), and archsimd.ClearAVXUpperBits exists for exactly this; every AVX2 routine in the Go runtime and bytealg ends with VZEROUPPER for the same reason. After encodeHex the rest of the request runs legacy-SSE code (memmove, MOVUPS copies). On Skylake and later each SSE register write pays a merge µop with a false dependency until the next VZEROUPPER, which in practice is the next memmove over 256 bytes; on Haswell/Broadwell it is a ~70-cycle state transition each way, more than the ~15 ns the kernel saves per hash. AMD has no penalty, which is why the Zen 4 numbers don't show it. Suggested shape (tested; one extra instruction after the loop, and short inputs no longer touch YMM at all):
It becomes a required ci-gate job, but skips everything setup-erigon does to keep module fetching deterministic: cache: false with no replacement, so every run downloads 32 modules (see the job log), no GOPROXY=https://proxy.golang.org|direct fallback and no retry loop. That is a new flake vector in the merge queue, which CI-GUIDELINES asks to keep free of false positives. It also lacks the "Cancel workflow run on failure" step (and the actions: write permission it needs) that the other leaves use to fast-evict a broken PR, and it runs an inline go test with no Makefile equivalent.
Suggestion: add an optional go-version input to setup-erigon (default stays the go.mod minor) and use it here, so the job inherits the mod cache, proxy fallback, retries and cache keys; register it in cache-warming.yml like test-bench; add a make test-simd target with GOEXPERIMENT=jsonv2,simd that the workflow calls. When #24411/#24418 land the package list needs extending; deriving it from grep -rl goexperiment.simd --include='*.go' would keep it in one place.
3. Default build: Encode / MarshalText are slower for small inputs
Bytes(b).AppendText(nil) goes through slices.Grow(nil, …) → growslice, while the old make([]byte, n) is stack-allocated for n ≤ 32 on Go ≥ 1.25 and cheaper than growslice above that. Default build (arm64, go1.27.1, -cpu 1, n=8), main → this PR: Encode 4/8/15 B: 21.5→35.5 ns, 23.6→38.6 ns, 28.4→45.1 ns (1→2 allocs); Encode 20/32 B: +6%; MarshalText 4–15 B: +23–28%, 20/32 B: +16%/+10%; ≥64 B unchanged. AppendText/AppendQuoted (the RPC path) are unchanged. Keeping make in MarshalText and sharing the prefix write brings both back to main or slightly better (measured):
func (bBytes) MarshalText() ([]byte, error) {
result:=make([]byte, len(HexPrefix)+2*len(b))
writeHex(result, b)
returnresult, nil
}
func (bBytes) AppendText(dst []byte) ([]byte, error) {
n, size:=len(dst), len(HexPrefix)+2*len(b)
dst=slices.Grow(dst, size)[:n+size]
writeHex(dst[n:], b)
returndst, nil
}
funcwriteHex(dst, b []byte) {
dst[0], dst[1] ='0', 'x'encodeHex(dst[2:], b)
}
funcEncode(b []byte) string {
enc, _:=Bytes(b).MarshalText()
returnstring(enc)
}
Notes, non-blocking
As merged, no shipped binary gets the kernel: the Makefile sets GOEXPERIMENT ?= jsonv2 and the Dockerfile builds through make, so the RPC numbers in the description only apply to GOEXPERIMENT=jsonv2,simd builds. Worth one sentence in the description (and the ChangeLog, if it gets an entry) so nobody expects them from a release.
TestEncodeHexMatchesStdlib passes on a runner without AVX2 without ever running the kernel. A require.True(t, hasAVX2) in encode_simd_test.go when CI is set would make that visible.
Description typo: the package is simd/archsimd, not simd/simdarch.
The self-hosted ARM64 label is the one release.yml's test-release holds
for up to two days, and ci-gate calls lint.yml on every PR and merge
group, so that job would have blocked the queue behind a release test.
Also drop two steps that covered nothing: with the feature bit cleared
these functions are hex.Encode and hex.Decode, and the cross-build is
what the two test runs and every non-simd build already do.
go.mod pins go 1.26, so the ordinary amd64 and macOS suites compiled
this test and then failed starting that toolchain with GOEXPERIMENT=simd,
which needs 1.27. It now carries the kernel's own build tag.
setup-erigon only restores caches; cleanup-erigon performs the save. This new job never runs cleanup, so the new arm64 namespace stays cold and every CI run must download and compile its dependencies again. Add the same always-running cleanup step used by the other workflow jobs.
With a second job in the workflow, zizmor reports the workflow-level
grant as excessive-permissions at high severity, which takes its exit
code past the threshold the step tolerates. Only the self-cancelling
step needs it.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Architecture-specific experimental SIMD kernels require final human review, and the new ARM job does not persist its caches.
0 open findings
Previously missed (1)
In code that hasn't changed since last review
Go cache is never saved because cleanup-erigon is missing
.github/workflows/lint.yml:178
This job restores uniquely namespaced Go caches through setup-erigon, but never runs cleanup-erigon, so no ARM cache is saved and every run must download and rebuild dependencies again. Other setup-based jobs pair the actions (for example, .github/workflows/test-integration-caplin.yml:32-57). Add the cleanup action with if: always() after the test.
🧠 Review effort: Balanced
Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.
Same algorithms as the archsimd kernels, without GOEXPERIMENT=simd, so default builds use them. A 128-byte frame keeps the stack check, which lets GC stop the goroutine between chunks. The simd CI steps go; test-all already runs common/hexutil on macos-15 (arm64).
AskAlexSharov
changed the title
common/hexutil: hex encoding by simd
common/hexutil: AVX2 and NEON hex encoding in assembly
Oct 8, 2026
AskAlexSharov
changed the title
common/hexutil: AVX2 and NEON hex encoding in assembly
common/hexutil: hex encode/decode in simd
Oct 8, 2026
AppendText(nil) grows through slices.Grow, which always allocates on the heap. MarshalText makes its buffer again, and Encode builds on it, so a short encoding stays on the stack.
@AskAlexSharov Have you considered upstreaming your encoding/hex vectorized implementation to the Go standard library? I would be happy to review the CL.
go1.27.1 does not emit VZEROUPPER for archsimd code (checked with objdump), and archsimd.ClearAVXUpperBits exists for exactly this; every AVX2 routine in the Go runtime and bytealg ends with VZEROUPPER for the same reason. After encodeHex the rest of the request runs legacy-SSE code (memmove, MOVUPS copies).
@AskAlexSharov Have you considered upstreaming your encoding/hex vectorized implementation to the Go standard library? I would be happy to review the CL.
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
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.
Reason: reth using
const-hex, neth usingCore/Extensions/HexEncoder.cs (encode: AVX-512 VBMI/AVX2/SSSE3/NEON), HexConverter.cs (decode: SSSE3/NEON)Hex encoding and decoding for RPC byte fields in assembly: AVX2 on amd64, NEON on arm64. amd64 without AVX2 and other architectures use
encoding/hex. NoGOEXPERIMENTneeded.The first version used
simd/archsimd(up to 731ff44). It needsGOEXPERIMENT=simd, which slows all Go code on AVX-512 CPUs (golang/go#81847: struct copy +26%), so it could not ship in release builds.ns per call (lower is better), n5 (AMD EPYC 4344P) and M4 Max;
simdis thearchsimdkernel:Assembly is never async-preempted, so
hex_asm.gofeeds it at most 64 KiB per call. That alone is not enough: the assembler drops the stack check of a leaf function with a frame under 128 bytes, so the kernels declare a 128-byte frame, and the check is where a pending preemption stops the goroutine between chunks. Worst GC stop-the-world wait while 64 MiB goes through,GOMAXPROCS=2: 5-6 ms in one call, 0.012-0.016 ms in 64 KiB chunks.Decoding too, by algorithm 3 of http://0x80.pl/notesen/2022-01-17-validating-hex-parse.html (as const-hex in alloy/reth): a digit maps to 0-9 and a letter of either case to 10-15 on two saturating paths, anything else to more than 15 on both, so the smaller of the two is the nibble, and one
VPMADDUBSWmerges each pair. A block holding a character that is not a hex digit falls tohex.Decode, which reports it with the same error.It is 18.9% of an
eth_callreplaying mainnet tx0x8653e24402c22383aaf3c92a9b08307ac4bada805eef06bd945bedb639af7393(block 26063051, 110.8 KB of calldata), reached fromhexutil.(*Bytes).UnmarshalText.arm64 too, on NEON: encode by
VTBLtable lookup, 16 bytes per iteration; decode by the same algorithm, 32 characters per iteration, packed withVUZP1/VUZP2.Both AVX2 kernels end with
VZEROUPPER, as Go CL 795820 does: the Go code after them is SSE, which pays a false dependency on Intel while the upper halves of the Y registers are dirty.