From e09d1d64afaa86a33d1e74cee4bdbd3a121c607e Mon Sep 17 00:00:00 2001 From: awskii Date: Tue, 11 Aug 2026 19:07:19 +0700 Subject: [PATCH 1/5] execution/commitment: arena-allocate prefixNode extensions prefixTrie.Insert allocated a fresh []byte for every new leaf's extension. prefixNodes themselves already come from a slab arena that survives across batches; their extension bytes did not, so a block's trie build paid one heap allocation per new key. Add a byte-chunk arena alongside the node slabs: 64KB chunks, a new chunk swapped in on overflow rather than grown, so outstanding extensions keep their backing. resetArena now truncates and reuses the first chunk, matching the slab policy, so a steady-state batch allocates no extension bytes. Benchmark_Commitment_DirectVsParallel cannot see this: runParallelBench calls WrapKeyUpdates, which drives every Insert, before b.StartTimer(), so the whole trie build is outside the timer. Measured on the build phase itself (-benchtime=3x -count=8): sec/op 208.2m -> 198.3m -4.73% (p=0.000) allocs/op 1.825M -> 1.326M -27.36% (p=0.000) A -memprofilerate=1 profile over the same corpus confirms it: Insert's flat allocation objects fall 2,462,526 -> 962,526 across 3 iterations, the 1.5M per-leaf allocations replaced by 1,431 chunk allocations. --- execution/commitment/prefix_trie.go | 45 +++++++++++--- execution/commitment/prefix_trie_test.go | 75 ++++++++++++++++++++++++ 2 files changed, 113 insertions(+), 7 deletions(-) diff --git a/execution/commitment/prefix_trie.go b/execution/commitment/prefix_trie.go index 1e6ea3ed3df..c54a6f41ab3 100644 --- a/execution/commitment/prefix_trie.go +++ b/execution/commitment/prefix_trie.go @@ -19,6 +19,7 @@ package commitment import "math/bits" const prefixSlabSize = 16384 +const prefixExtChunkSize = 65536 // prefixNode is a path-compressed prefix-trie node keyed on nibbles (each ext byte is one nibble 0x00..0x0F). // children is dense: len == popcount(bitmap). subtreeCount is the number of distinct keys in the @@ -37,15 +38,20 @@ type prefixSlab struct { nodes [prefixSlabSize]prefixNode } -// prefixArena bump-allocates prefixNodes from a list of slabs. +// prefixArena bump-allocates prefixNodes from a list of slabs and leaf extensions from a list of byte chunks. type prefixArena struct { - slabs []*prefixSlab - slabIdx int - nextIdx int + slabs []*prefixSlab + slabIdx int + nextIdx int + extChunks [][]byte + extChunkIdx int } func newPrefixArena() *prefixArena { - return &prefixArena{slabs: []*prefixSlab{new(prefixSlab)}} + return &prefixArena{ + slabs: []*prefixSlab{new(prefixSlab)}, + extChunks: [][]byte{make([]byte, 0, prefixExtChunkSize)}, + } } func (a *prefixArena) allocNode() *prefixNode { @@ -62,6 +68,26 @@ func (a *prefixArena) allocNode() *prefixNode { return n } +// allocExt copies b into the current chunk, swapping in a fresh chunk instead of growing this one +// so sub-slices already handed out keep their backing array. +func (a *prefixArena) allocExt(b []byte) []byte { + if len(b) == 0 { + return nil + } + chunk := a.extChunks[a.extChunkIdx] + if cap(chunk)-len(chunk) < len(b) { + a.extChunkIdx++ + if a.extChunkIdx >= len(a.extChunks) { + a.extChunks = append(a.extChunks, make([]byte, 0, prefixExtChunkSize)) + } + chunk = a.extChunks[a.extChunkIdx] + } + off := len(chunk) + chunk = append(chunk, b...) + a.extChunks[a.extChunkIdx] = chunk + return chunk[off:len(chunk):len(chunk)] +} + // resetArena clears touched nodes for reuse, keeping the first slab and releasing the rest. func (a *prefixArena) resetArena() { for i := 0; i <= a.slabIdx && i < len(a.slabs); i++ { @@ -76,6 +102,11 @@ func (a *prefixArena) resetArena() { a.slabs = a.slabs[:1] a.slabIdx = 0 a.nextIdx = 0 + + clear(a.extChunks[1:]) + a.extChunks = a.extChunks[:1] + a.extChunks[0] = a.extChunks[0][:0] + a.extChunkIdx = 0 } func (a *prefixArena) nodeCount() int { @@ -169,7 +200,7 @@ func (t *prefixTrie) Insert(hashedKey, plainKey []byte, update *Update) (isNew b newLeaf := t.arena.allocNode() newNib := remain[m] - newLeaf.ext = append([]byte(nil), remain[m+1:]...) + newLeaf.ext = t.arena.allocExt(remain[m+1:]) newLeaf.subtreeCount = 1 newLeaf.plainKey = plainKey newLeaf.update = update @@ -210,7 +241,7 @@ func (t *prefixTrie) Insert(hashedKey, plainKey []byte, update *Update) (isNew b idx, ok := childIndex(node, nib) if !ok { newLeaf := t.arena.allocNode() - newLeaf.ext = append([]byte(nil), hashedKey[keyOffset+1:]...) + newLeaf.ext = t.arena.allocExt(hashedKey[keyOffset+1:]) newLeaf.subtreeCount = 1 newLeaf.plainKey = plainKey newLeaf.update = update diff --git a/execution/commitment/prefix_trie_test.go b/execution/commitment/prefix_trie_test.go index cbc237aa50c..88d125d3337 100644 --- a/execution/commitment/prefix_trie_test.go +++ b/execution/commitment/prefix_trie_test.go @@ -225,6 +225,81 @@ func TestPrefixTrieArenaReuse(t *testing.T) { assert.Equal(t, first, tr.arena.nodeCount()) } +func TestPrefixArenaAllocExt(t *testing.T) { + t.Run("capEqualsLen", func(t *testing.T) { + a := newPrefixArena() + ext := a.allocExt([]byte{1, 2, 3, 4, 5}) + assert.Equal(t, []byte{1, 2, 3, 4, 5}, ext) + assert.Equal(t, len(ext), cap(ext), "allocExt must not hand out spare capacity") + }) + + t.Run("emptyInputReturnsNil", func(t *testing.T) { + a := newPrefixArena() + assert.Nil(t, a.allocExt(nil)) + assert.Nil(t, a.allocExt([]byte{})) + }) + + t.Run("appendPastCapDoesNotAliasNextExtension", func(t *testing.T) { + a := newPrefixArena() + first := a.allocExt([]byte{1, 2, 3}) + second := a.allocExt([]byte{4, 5, 6}) + + first = append(first, 0xFF, 0xFF, 0xFF, 0xFF) + + assert.Equal(t, []byte{4, 5, 6}, second, "growing one extension past its cap must not corrupt the next") + assert.Equal(t, []byte{1, 2, 3, 0xFF, 0xFF, 0xFF, 0xFF}, first) + }) +} + +func TestPrefixTrieExtSurvivesChunkBoundary(t *testing.T) { + tr := newPrefixTrie() + + // keys share nibbles [4:keyLen) so leaf extensions stay long; nibbles [0:4) alone already + // make every key distinct, which is what drives enough allocExt traffic to cross a chunk. + const keyLen = 32 + const total = 6000 + want := make(map[string]bool, total) + for i := range total { + k := make([]byte, keyLen) + v := i + for j := range 4 { + k[j] = byte(v % 16) + v /= 16 + } + want[string(k)] = true + tr.Insert(k, nil, nil) + } + require.Greater(t, len(tr.arena.extChunks), 1, "test must actually cross a chunk boundary") + + got := make(map[string]bool, total) + for _, e := range collectWalk(tr) { + if len(e.prefix) == keyLen { + got[string(e.prefix)] = true + } + } + assert.Equal(t, want, got, "leaf extensions must reproduce their original key bytes after crossing a chunk boundary") +} + +func TestPrefixTrieArenaReusesExtChunkBacking(t *testing.T) { + tr := newPrefixTrie() + tr.Insert(nibs(0x01, 0x02, 0x03, 0x04), nil, nil) + + chunk := tr.arena.extChunks[0] + full := chunk[:cap(chunk)] + + tr.Reset() + + require.Len(t, tr.arena.extChunks, 1, "Reset must trim trailing chunks") + assert.Empty(t, tr.arena.extChunks[0], "Reset must truncate the reused chunk's length") + assert.Equal(t, cap(chunk), cap(tr.arena.extChunks[0]), "Reset must keep the chunk's capacity") + + tr.Insert(nibs(0x05, 0x06, 0x07, 0x08), nil, nil) + require.Len(t, tr.root.children, 1) + newExt := tr.root.children[0].ext + assert.Equal(t, nibs(0x06, 0x07, 0x08), newExt) + assert.Equal(t, newExt, full[:len(newExt)], "post-reset extension must land in the same backing array as before Reset") +} + func TestPrefixTrieArenaSpansMultipleSlabs(t *testing.T) { tr := newPrefixTrie() // Allocate directly to cross the slab boundary; reaching it via inserts needs >prefixSlabSize keys. From a537b72164b47e93429fc378290031e11393988f Mon Sep 17 00:00:00 2001 From: awskii Date: Tue, 11 Aug 2026 20:28:00 +0700 Subject: [PATCH 2/5] execution/commitment: address review on the prefixNode extension arena - resetArena kept only the first chunk, so any batch needing more than 64KB of extensions reallocated every chunk above the first on every reset. Truncate all chunks in place instead, matching Updates.Reset's arena-ring policy. This is what makes "a steady-state batch allocates no extension bytes" actually true rather than true only for sub-chunk batches. - allocExt had no guard for an extension larger than a chunk: it moved to a fresh chunk and let append reallocate it, contradicting the swap-don't-grow contract in its own doc. Oversize extensions now get their own exact-capacity allocation, matching plainKeyArena.intern's fallback. - The split path handed out node.ext = oldExt[:m], keeping capacity over bytes oldChild.ext owns. Harmless when every extension was its own allocation; with arena backing an append there would reach into a sibling's nibbles. Capped to oldExt[:m:m], which caps oldChild.ext too. - Documented that ext is arena-backed and only valid until Reset. - resetArena's doc no longer omits the extension chunks. - The chunk-boundary test derived its key count from the chunk size instead of a hand-tuned 6000. extChunkIdx stays: it is derivable from len(extChunks) only under the old keep-one-chunk reset. Now that reset retains the grown chunks, the index is what lets the next batch refill them from the start instead of appending past them. Benchmark_PrefixTrieBuildAcrossResets (20k keys, insert+Reset per iteration), Apple M5 Max, -benchtime=200x -count=8: sec/op 1.677m -> 1.448m -13.69% B/op 4.365Mi -> 3.149Mi -27.85% allocs/op 39.99k -> 19.99k -50.01% All p=0.000, n=8. --- execution/commitment/prefix_trie.go | 21 ++++++++---- execution/commitment/prefix_trie_test.go | 41 +++++++++++++++++++++++- 2 files changed, 55 insertions(+), 7 deletions(-) diff --git a/execution/commitment/prefix_trie.go b/execution/commitment/prefix_trie.go index c54a6f41ab3..901b0de8478 100644 --- a/execution/commitment/prefix_trie.go +++ b/execution/commitment/prefix_trie.go @@ -25,6 +25,8 @@ const prefixExtChunkSize = 65536 // children is dense: len == popcount(bitmap). subtreeCount is the number of distinct keys in the // subtree; re-inserting an existing key merges its update without bumping it. type prefixNode struct { + // ext is arena-backed: it stays valid only until the owning trie's Reset, which + // recycles the chunk in place. A reader that must outlive the batch copies it. ext []byte children []*prefixNode plainKey []byte // set only where a key terminates @@ -69,11 +71,17 @@ func (a *prefixArena) allocNode() *prefixNode { } // allocExt copies b into the current chunk, swapping in a fresh chunk instead of growing this one -// so sub-slices already handed out keep their backing array. +// so sub-slices already handed out keep their backing array. An extension larger than a chunk gets +// its own allocation rather than forcing a chunk to grow under live sub-slices. func (a *prefixArena) allocExt(b []byte) []byte { if len(b) == 0 { return nil } + if len(b) > prefixExtChunkSize { + own := make([]byte, len(b)) + copy(own, b) + return own + } chunk := a.extChunks[a.extChunkIdx] if cap(chunk)-len(chunk) < len(b) { a.extChunkIdx++ @@ -88,7 +96,8 @@ func (a *prefixArena) allocExt(b []byte) []byte { return chunk[off:len(chunk):len(chunk)] } -// resetArena clears touched nodes for reuse, keeping the first slab and releasing the rest. +// resetArena clears touched nodes for reuse, keeping the first slab and releasing the rest, and +// truncates the extension chunks in place so the next batch refills them without reallocating. func (a *prefixArena) resetArena() { for i := 0; i <= a.slabIdx && i < len(a.slabs); i++ { limit := prefixSlabSize @@ -103,9 +112,9 @@ func (a *prefixArena) resetArena() { a.slabIdx = 0 a.nextIdx = 0 - clear(a.extChunks[1:]) - a.extChunks = a.extChunks[:1] - a.extChunks[0] = a.extChunks[0][:0] + for i := range a.extChunks { + a.extChunks[i] = a.extChunks[i][:0] + } a.extChunkIdx = 0 } @@ -186,7 +195,7 @@ func (t *prefixTrie) Insert(hashedKey, plainKey []byte, update *Update) (isNew b node.plainKey = nil node.update = nil - node.ext = oldExt[:m] + node.ext = oldExt[:m:m] if m == len(remain) { // Key ends inside the old extension: one child, no new sibling. diff --git a/execution/commitment/prefix_trie_test.go b/execution/commitment/prefix_trie_test.go index 88d125d3337..08e8b2d6341 100644 --- a/execution/commitment/prefix_trie_test.go +++ b/execution/commitment/prefix_trie_test.go @@ -257,7 +257,7 @@ func TestPrefixTrieExtSurvivesChunkBoundary(t *testing.T) { // keys share nibbles [4:keyLen) so leaf extensions stay long; nibbles [0:4) alone already // make every key distinct, which is what drives enough allocExt traffic to cross a chunk. const keyLen = 32 - const total = 6000 + const total = 2 * prefixExtChunkSize / keyLen want := make(map[string]bool, total) for i := range total { k := make([]byte, keyLen) @@ -506,3 +506,42 @@ func TestPrefixTrieInsertDuplicateMerges(t *testing.T) { // Merge is copy-on-write: a concurrent fold snapshot may still hold the prior update pointer. assert.Equal(t, BalanceUpdate, first.Flags, "merge must not mutate the previously stored update") } + +func TestPrefixArenaAllocExt_OversizeAndReuse(t *testing.T) { + t.Run("extension larger than a chunk gets its own backing", func(t *testing.T) { + a := newPrefixArena() + big := bytes.Repeat([]byte{0x7}, prefixExtChunkSize+1) + got := a.allocExt(big) + require.Equal(t, big, got) + require.Equal(t, len(got), cap(got)) + require.Len(t, a.extChunks, 1, "an oversize extension must not consume a chunk") + require.Empty(t, a.extChunks[0], "the current chunk must be untouched") + }) + + t.Run("reset refills existing chunks instead of reallocating", func(t *testing.T) { + a := newPrefixArena() + block := make([]byte, prefixExtChunkSize/4) + for range 12 { + a.allocExt(block) + } + require.Greater(t, len(a.extChunks), 1, "test must cross a chunk boundary") + grown := len(a.extChunks) + backing := make([]*byte, grown) + for i := range a.extChunks { + backing[i] = &a.extChunks[i][:1][0] + } + + a.resetArena() + require.Len(t, a.extChunks, grown, "reset must keep the grown chunks") + for i := range a.extChunks { + require.Empty(t, a.extChunks[i]) + } + for range 12 { + a.allocExt(block) + } + require.Len(t, a.extChunks, grown, "refill must reuse chunks, not allocate new ones") + for i := range a.extChunks { + require.Same(t, backing[i], &a.extChunks[i][:1][0], "chunk backing array must be reused") + } + }) +} From fa2f888545f5e9872fefd9afabecb6a06d7f3608 Mon Sep 17 00:00:00 2001 From: awskii Date: Tue, 11 Aug 2026 20:29:56 +0700 Subject: [PATCH 3/5] execution/commitment: spell prefixExtChunkSize as 64 * 1024, matching the sibling arenas --- execution/commitment/prefix_trie.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/execution/commitment/prefix_trie.go b/execution/commitment/prefix_trie.go index 901b0de8478..d09dfa2f797 100644 --- a/execution/commitment/prefix_trie.go +++ b/execution/commitment/prefix_trie.go @@ -19,7 +19,7 @@ package commitment import "math/bits" const prefixSlabSize = 16384 -const prefixExtChunkSize = 65536 +const prefixExtChunkSize = 64 * 1024 // prefixNode is a path-compressed prefix-trie node keyed on nibbles (each ext byte is one nibble 0x00..0x0F). // children is dense: len == popcount(bitmap). subtreeCount is the number of distinct keys in the From b97a0d23c8c2a5a6d2cc0292da8725403aa327b8 Mon Sep 17 00:00:00 2001 From: awskii Date: Tue, 11 Aug 2026 21:06:34 +0700 Subject: [PATCH 4/5] execution/commitment: commit the build-phase benchmark for the extension arena No committed benchmark covered the change: runParallelBench calls WrapKeyUpdates, which drives every Insert, outside its timed region, so the whole prefix-trie build sits outside the timer in every Benchmark_Commitment_*. The allocation win was measured through a throwaway benchmark and defended by nothing in CI, so a later regression here would be invisible. Benchmark_PrefixTrieBuildAcrossResets times the build itself across Reset cycles, which is also what exercises the arena's per-batch chunk reuse. It is the benchmark the PR description's numbers come from. --- .../commitment/prefix_trie_bench_test.go | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 execution/commitment/prefix_trie_bench_test.go diff --git a/execution/commitment/prefix_trie_bench_test.go b/execution/commitment/prefix_trie_bench_test.go new file mode 100644 index 00000000000..50df3d2154b --- /dev/null +++ b/execution/commitment/prefix_trie_bench_test.go @@ -0,0 +1,53 @@ +// Copyright 2026 The Erigon Authors +// This file is part of Erigon. +// +// Erigon is free software: you can redistribute it and/or modify +// it under the terms of the GNU Lesser General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Erigon is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Lesser General Public License for more details. +// +// You should have received a copy of the GNU Lesser General Public License +// along with Erigon. If not, see . + +package commitment + +import ( + "fmt" + "testing" +) + +// The Benchmark_Commitment_* family cannot measure the trie build: runParallelBench +// calls WrapKeyUpdates, which drives every Insert, outside the timed region. This +// times the build itself across Reset cycles, so the arena's per-batch reuse is +// what the numbers reflect. +func Benchmark_PrefixTrieBuildAcrossResets(b *testing.B) { + for _, keys := range []int{5_000, 20_000} { + b.Run(fmt.Sprintf("%dk-keys", keys/1000), func(b *testing.B) { + const keyLen = 64 + corpus := make([][]byte, keys) + for i := range corpus { + k := make([]byte, keyLen) + v := i + for j := range keyLen { + k[j] = byte(v % 16) + v /= 3 + } + corpus[i] = k + } + tr := newPrefixTrie() + b.ReportAllocs() + b.ResetTimer() + for range b.N { + for _, k := range corpus { + tr.Insert(k, k, nil) + } + tr.Reset() + } + }) + } +} From 87cc4aa3cd3a740d7766afb5968f60ebaef0d41b Mon Sep 17 00:00:00 2001 From: Alexey Sharov Date: Wed, 12 Aug 2026 13:37:53 +0700 Subject: [PATCH 5/5] kick ci