PR #6791 sped up hex conversion for TraceID and SpanID. It wrote out all the positions by hand in the encoders (hexBytes), but left the decoders (TraceIDFromHex, SpanIDFromHex in trace/trace.go) as loops.
The loop has a hidden cost. Go inserts a safety check every time you index into a string or slice. Inside a loop the compiler cannot prove the index stays in range, so it keeps every check. The compiler will list them:
$ cd trace
$ go build -gcflags='-d=ssa/check_bce/debug=1' . 2>&1 | grep trace.go
./trace.go:140:4: Found IsInBounds
./trace.go:140:21: Found IsInBounds
./trace.go:140:42: Found IsInBounds
./trace.go:141:4: Found IsInBounds
./trace.go:141:23: Found IsInBounds
./trace.go:141:46: Found IsInBounds
./trace.go:166:4: Found IsInBounds
./trace.go:166:21: Found IsInBounds
./trace.go:166:42: Found IsInBounds
./trace.go:167:4: Found IsInBounds
./trace.go:167:23: Found IsInBounds
./trace.go:167:46: Found IsInBounds
Lines 140 and 141 are the TraceIDFromHex loop body, 166 and 167 are SpanIDFromHex. That is 6 checks per pass. TraceIDFromHex loops 8 times, so it pays 48 checks per call, even though the input is always exactly 32 characters.
Proposed Solution
Write out all 32 positions by hand in the decoders, matching what the encoders in the same file already do. Once every index is a constant, the compiler can prove they are safe and removes all the checks.
Alternatives
Change the loop condition from i < len(h) to i < 32. That is a one word change and removes 4 of the 6 checks per pass, but it only recovers about a third of the speedup. Reasonable fallback if the fully written out version reads as too long.
Prior Art
PR #6791 (issue #6721) did exactly this for the encoders and was merged. This is the other half of that work.
Additional Context
Measured on the trace package. Go 1.25.0, linux/amd64, i7-13620H, 15 runs:
| case |
now |
unrolled |
change |
| valid id |
58.88n ± 1% |
41.27n ± 1% |
-29.91% (p=0.000) |
| bad character |
50.46n ± 1% |
34.53n ± 1% |
-31.57% (p=0.000) |
| all zeros |
51.33n ± 2% |
35.04n ± 2% |
-31.74% (p=0.000) |
| wrong length |
3.841n ± 1% |
3.863n ± 0% |
no change (returns early) |
| geomean |
27.67n |
20.96n |
-24.25% |
No change to memory: 0 B/op and 0 allocs/op before and after. This is purely about doing less work per call, not a repeat of the allocation win in #6791.
The tradeoff is code size. The compiled function grows from 114 to 248 instructions. Neither version is small enough to be inlined today, so inlining behaviour does not change.
SpanIDFromHex has the same problem (lines 166 and 167 above) and should be fixed at the same time.
Nothing else in the repo needs this. trace/internal/telemetry/id.go and propagation/trace_context.go both use the standard library encoding/hex.
If this looks like something worth fixing, I’ve already got a draft implementation and can open a PR anytime.
The benchmarks added by #6791 can confirm the result:
cd sdk
go test -run='xxxxMatchNothingxxxx' -bench='FromHex' -benchmem -count=10 ./trace/
PR #6791 sped up hex conversion for
TraceIDandSpanID. It wrote out all the positions by hand in the encoders (hexBytes), but left the decoders (TraceIDFromHex,SpanIDFromHexintrace/trace.go) as loops.The loop has a hidden cost. Go inserts a safety check every time you index into a string or slice. Inside a loop the compiler cannot prove the index stays in range, so it keeps every check. The compiler will list them:
Lines 140 and 141 are the
TraceIDFromHexloop body, 166 and 167 areSpanIDFromHex. That is 6 checks per pass.TraceIDFromHexloops 8 times, so it pays 48 checks per call, even though the input is always exactly 32 characters.Proposed Solution
Write out all 32 positions by hand in the decoders, matching what the encoders in the same file already do. Once every index is a constant, the compiler can prove they are safe and removes all the checks.
Alternatives
Change the loop condition from
i < len(h)toi < 32. That is a one word change and removes 4 of the 6 checks per pass, but it only recovers about a third of the speedup. Reasonable fallback if the fully written out version reads as too long.Prior Art
PR #6791 (issue #6721) did exactly this for the encoders and was merged. This is the other half of that work.
Additional Context
Measured on the
tracepackage. Go 1.25.0, linux/amd64, i7-13620H, 15 runs:No change to memory: 0 B/op and 0 allocs/op before and after. This is purely about doing less work per call, not a repeat of the allocation win in #6791.
The tradeoff is code size. The compiled function grows from 114 to 248 instructions. Neither version is small enough to be inlined today, so inlining behaviour does not change.
SpanIDFromHexhas the same problem (lines 166 and 167 above) and should be fixed at the same time.Nothing else in the repo needs this.
trace/internal/telemetry/id.goandpropagation/trace_context.goboth use the standard libraryencoding/hex.If this looks like something worth fixing, I’ve already got a draft implementation and can open a PR anytime.
The benchmarks added by #6791 can confirm the result: