From f52e3b7dff021785b7dbe7bd8aefdb27c05a3cee Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Tue, 12 Nov 2024 10:04:35 +0100 Subject: [PATCH] core/types, trie: reduce allocs in derivesha --- core/types/hashing.go | 14 +++++++------- core/types/hashing_test.go | 23 ++++++++++++++++------- internal/blocktest/test_hash.go | 6 ++++++ trie/stacktrie.go | 26 ++++++++++++++++++++++++-- trie/trie.go | 6 ++++++ 5 files changed, 59 insertions(+), 16 deletions(-) diff --git a/core/types/hashing.go b/core/types/hashing.go index 224d7a87ea..5e465edc08 100644 --- a/core/types/hashing.go +++ b/core/types/hashing.go @@ -81,6 +81,9 @@ func prefixedRlpHash(prefix byte, x interface{}) (h common.Hash) { type TrieHasher interface { Reset() Update([]byte, []byte) error + // UpdateSafe is identical to Update, except that this method will copy the + // value slice. The caller is free to modify the value bytes after this method returns. + UpdateSafe([]byte, []byte) error Hash() common.Hash } @@ -95,10 +98,7 @@ type DerivableList interface { func encodeForDerive(list DerivableList, i int, buf *bytes.Buffer) []byte { buf.Reset() list.EncodeIndex(i, buf) - // It's really unfortunate that we need to perform this copy. - // StackTrie holds onto the values until Hash is called, so the values - // written to it must not alias. - return common.CopyBytes(buf.Bytes()) + return buf.Bytes() } // DeriveSha creates the tree hashes of transactions, receipts, and withdrawals in a block header. @@ -118,17 +118,17 @@ func DeriveSha(list DerivableList, hasher TrieHasher) common.Hash { for i := 1; i < list.Len() && i <= 0x7f; i++ { indexBuf = rlp.AppendUint64(indexBuf[:0], uint64(i)) value := encodeForDerive(list, i, valueBuf) - hasher.Update(indexBuf, value) + hasher.UpdateSafe(indexBuf, value) } if list.Len() > 0 { indexBuf = rlp.AppendUint64(indexBuf[:0], 0) value := encodeForDerive(list, 0, valueBuf) - hasher.Update(indexBuf, value) + hasher.UpdateSafe(indexBuf, value) } for i := 0x80; i < list.Len(); i++ { indexBuf = rlp.AppendUint64(indexBuf[:0], uint64(i)) value := encodeForDerive(list, i, valueBuf) - hasher.Update(indexBuf, value) + hasher.UpdateSafe(indexBuf, value) } return hasher.Hash() } diff --git a/core/types/hashing_test.go b/core/types/hashing_test.go index a6949414f3..d51b60e76c 100644 --- a/core/types/hashing_test.go +++ b/core/types/hashing_test.go @@ -81,13 +81,16 @@ func BenchmarkDeriveSha200(b *testing.B) { if err != nil { b.Fatal(err) } - var exp common.Hash - var got common.Hash + want := types.DeriveSha(txs, trie.NewEmpty(triedb.NewDatabase(rawdb.NewMemoryDatabase(), nil))) + var have common.Hash b.Run("std_trie", func(b *testing.B) { b.ResetTimer() b.ReportAllocs() for i := 0; i < b.N; i++ { - exp = types.DeriveSha(txs, trie.NewEmpty(triedb.NewDatabase(rawdb.NewMemoryDatabase(), nil))) + have = types.DeriveSha(txs, trie.NewEmpty(triedb.NewDatabase(rawdb.NewMemoryDatabase(), nil))) + } + if have != want { + b.Errorf("have %x want %x", have, want) } }) @@ -95,12 +98,12 @@ func BenchmarkDeriveSha200(b *testing.B) { b.ResetTimer() b.ReportAllocs() for i := 0; i < b.N; i++ { - got = types.DeriveSha(txs, trie.NewStackTrie(nil)) + have = types.DeriveSha(txs, trie.NewStackTrie(nil)) + } + if have != want { + b.Errorf("have %x want %x", have, want) } }) - if got != exp { - b.Errorf("got %x exp %x", got, exp) - } } func TestFuzzDeriveSha(t *testing.T) { @@ -226,6 +229,12 @@ func (d *hashToHumanReadable) Update(i []byte, i2 []byte) error { return nil } +// UpdateSafe is identical to Update, except that this method will copy the +// value slice. The caller is free to modify the value bytes after this method returns. +func (d *hashToHumanReadable) UpdateSafe(key, value []byte) error { + return d.Update(key, common.CopyBytes(value)) +} + func (d *hashToHumanReadable) Hash() common.Hash { return common.Hash{} } diff --git a/internal/blocktest/test_hash.go b/internal/blocktest/test_hash.go index 4d2b077e89..05608e5721 100644 --- a/internal/blocktest/test_hash.go +++ b/internal/blocktest/test_hash.go @@ -53,6 +53,12 @@ func (h *testHasher) Update(key, val []byte) error { return nil } +// UpdateSafe is identical to Update, except that this method will copy the +// value slice. The caller is free to modify the value bytes after this method returns. +func (h *testHasher) UpdateSafe(key, value []byte) error { + return h.Update(key, common.CopyBytes(value)) +} + // Hash returns the hash value. func (h *testHasher) Hash() common.Hash { return common.BytesToHash(h.hasher.Sum(nil)) diff --git a/trie/stacktrie.go b/trie/stacktrie.go index 99e782a78f..05bc05e153 100644 --- a/trie/stacktrie.go +++ b/trie/stacktrie.go @@ -50,6 +50,7 @@ type StackTrie struct { onTrieNode OnTrieNode kBuf []byte // buf space used for hex-key during insertions pBuf []byte // buf space used for path during insertions + vPool *bytesPool } // NewStackTrie allocates and initializes an empty trie. The committed nodes @@ -61,10 +62,20 @@ func NewStackTrie(onTrieNode OnTrieNode) *StackTrie { onTrieNode: onTrieNode, kBuf: make([]byte, 0, 64), pBuf: make([]byte, 0, 32), + vPool: newBytesPool(300, 20), } } +// UpdateSafe is identical to Update, except that this method will copy the +// value slice. The caller is free to modify the value bytes after this method returns. +func (t *StackTrie) UpdateSafe(key, value []byte) error { + // The stacktrie always copies the value (is already safe). + return t.Update(key, value) +} + // Update inserts a (key, value) pair into the stack trie. +// The value is copied, and the caller is free to modify the value after this +// method returns. func (t *StackTrie) Update(key, value []byte) error { if len(value) == 0 { return errors.New("trying to insert empty (deletion)") @@ -87,7 +98,14 @@ func (t *StackTrie) Update(key, value []byte) error { } else { t.last = append(t.last[:0], k...) // reuse key slice } - t.insert(t.root, k, value, t.pBuf[:0]) + vBuf := t.vPool.Get() + if cap(vBuf) < len(value) { + vBuf = common.CopyBytes(value) + } else { + vBuf = vBuf[:len(value)] + copy(vBuf, value) + } + t.insert(t.root, k, vBuf, t.pBuf[:0]) return nil } @@ -392,7 +410,11 @@ func (t *StackTrie) hash(st *stNode, path []byte) { st.typ = hashedNode st.key = st.key[:0] - st.val = nil // Release reference to potentially externally held slice. + // Release reference to (potentially externally held) value-slice. + if cap(st.val) > 0 && t.vPool != nil { + t.vPool.Put(st.val) + } + st.val = nil // Skip committing the non-root node if the size is smaller than 32 bytes // as tiny nodes are always embedded in their parent except root node. diff --git a/trie/trie.go b/trie/trie.go index ae2a7b21a2..5bea868e04 100644 --- a/trie/trie.go +++ b/trie/trie.go @@ -308,6 +308,12 @@ func (t *Trie) Update(key, value []byte) error { return t.update(key, value) } +// UpdateSafe is identical to Update, except that this method will copy the +// value slice. The caller is free to modify the value bytes after this method returns. +func (t *Trie) UpdateSafe(key, value []byte) error { + return t.Update(key, common.CopyBytes(value)) +} + func (t *Trie) update(key, value []byte) error { t.unhashed++ t.uncommitted++