perf(utxo): eliminate encode-side allocations in the store Create path (#1002)#1011
Merged
Merged
Conversation
Contributor
|
🤖 Claude Code Review Status: Complete No issues found. The PR implements a clean performance optimization with excellent test coverage and proper safety mechanisms. History:
|
Contributor
Benchmark Comparison ReportBaseline: Current: Summary
All benchmark results (sec/op)
Threshold: >10% with p < 0.05 | Generated: 2026-06-01 20:52 UTC |
|
ordishs
approved these changes
Jun 2, 2026
ordishs
left a comment
Collaborator
There was a problem hiding this comment.
Approved. Verified byte-equivalence of all hand-rolled serializers against go-bt v2.6.4 source (appendOutputInto ≡ Output.Bytes, appendInputExtendedInto ≡ Input.Bytes + extended suffix, extendedTxSize ≡ len(ExtendedBytes) including the −32 nil-txid correction, VarInt.AppendTo ≡ Bytes, UTXOHashInto preimage unchanged).
Arena lifetime is safe: Arena.Alloc's grow path orphans the old slab (copies, doesn't overwrite), so earlier bins stay valid if the arena grows mid-batch; deferred putCreateArena fires only after BatchOperate returns; multi-record and RECORD_TOO_BIG escape paths rebuild with nil arena before handing bins to goroutines.
Local checks green: go build, go vet, gci diff (clean), go test -race on concurrent arena reuse, and the new encode/arena/hash unit tests.
Non-blocking: consider a one-line comment at the appendInputExtendedInto size calc noting the deliberate 32-byte over-allocation for nil previousTxIDHash (mirrors the note in extendedTxSize). Please ensure the full Aerospike testcontainer CI suite runs green before merge.
freemans13
approved these changes
Jun 2, 2026
freemans13
added a commit
to freemans13/teranode
that referenced
this pull request
Jun 2, 2026
Upstream bsv-blockchain#1011 (perf: eliminate encode-side allocations) added an arena *bt.Arena parameter to GetBinsToStore. The batch-create path has no per-batch arena to reuse, so it passes nil (heap-backed bins), matching upstream's non-hot-path callers.
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
3 participants
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.
What
Cuts avoidable heap allocations in the Aerospike utxo-store
Createpath — the live source of the encode-side churn from #1002. The repeatedTxIDChainHashre-serialization the issue describes is already neutralised on the legacy path (createTxMap→SetTxHash); the remaining churn is per-tx/per-output/per-input serialization insideGetBinsToStore.teranode-only, no go-bt change (v2.6.4 already ships the zero-alloc primitives). No stored byte format,
ExtendedSizebin value, or hashing semantics change — output is byte-identical.Changes
util.UTXOHashInto(scratch, ...)— scratch-reusing, zero-alloc UTXO hash.UTXOHash/UTXOHashFromInput/UTXOHashFromOutputbecome thin wrappers (signatures unchanged).GetUtxoHashes/GetFeesAndUtxoHashes— allocations now O(1) in output count (contiguous[]chainhash.Hashbacking + reused scratch) instead of O(N). Signatures unchanged.output.Size()instead oflen(output.Bytes()); arithmeticextendedTxSize(tx)instead oflen(tx.ExtendedBytes())at both measure-only sites — removes two full-tx serializations done purely to read a length.appendOutputInto/appendInputExtendedInto— zero-alloc serialization into a pooled per-batchbt.Arena(mirrors the fix(blockvalidation): arena-backed tx decode to eliminate catch-up OOM #929subtreevalidation/arena_pool.goconvention), reset afterBatchOperate. The multi-record / external escape paths rebuild bins heap-owned so the arena reset can't corrupt the fire-and-forget goroutines that outlivesendStoreBatch.Verification
appendOutputInto == Output.Bytes(),appendInputExtendedInto ==prior manual layout,extendedTxSize == len(ExtendedBytes())(incl. >252B scripts, coinbase, decoded-tx shape),UTXOHashInto == UTXOHash, and arena-vs-nil bins byte-identical across coinbase / multi-output / OP_RETURN / no-input shapes.UTXOHashInto0 allocs/op with scratch;GetUtxoHashesflat in output count (195→4 allocs for 64 outputs);BenchmarkAppendOutputInto_Arena0 allocs/op.-racetest on concurrent arena reuse + the escape-rebuild path. Arena bytes are only referenced untilBatchOperate(NewBytesValue does not copy); reset is deferred to after it returns.go build ./...clean; touched-package tests +go vet+staticcheckgreen.Test plan
appendTo+toBytesHelper+Output.Bytesshould drop from ~50% to <20% of cumulativealloc_space(the go-bt encode-side allocations still 50% of legacy alloc churn after #929 (decode arena) #1002 acceptance criterion — mainnet IBD isn't reproducible locally, so the alloc-gate benchmarks stand in for it pre-merge).Addresses #1002.