From 62a151e6883a7ea0dd4d2477aec948dd0ef98b74 Mon Sep 17 00:00:00 2001 From: Gary Rong Date: Mon, 28 Jul 2025 15:05:59 +0800 Subject: [PATCH] trie: separate opTracer and prevalueTracer --- trie/committer.go | 7 +- trie/proof.go | 7 +- trie/tracer.go | 156 +++++++++++++++++++++++++++++--------------- trie/tracer_test.go | 14 ++-- trie/trie.go | 81 +++++++++++++++-------- trie/trie_test.go | 8 +-- 6 files changed, 177 insertions(+), 96 deletions(-) diff --git a/trie/committer.go b/trie/committer.go index 0939a07abb..9e2bbd1325 100644 --- a/trie/committer.go +++ b/trie/committer.go @@ -29,12 +29,12 @@ import ( // insertion order. type committer struct { nodes *trienode.NodeSet - tracer *tracer + tracer *prevalueTracer collectLeaf bool } // newCommitter creates a new committer or picks one from the pool. -func newCommitter(nodeset *trienode.NodeSet, tracer *tracer, collectLeaf bool) *committer { +func newCommitter(nodeset *trienode.NodeSet, tracer *prevalueTracer, collectLeaf bool) *committer { return &committer{ nodes: nodeset, tracer: tracer, @@ -140,8 +140,7 @@ func (c *committer) store(path []byte, n node) node { // The node is embedded in its parent, in other words, this node // will not be stored in the database independently, mark it as // deleted only if the node was existent in database before. - _, ok := c.tracer.accessList[string(path)] - if ok { + if c.tracer.has(path) { c.nodes.AddNode(path, trienode.NewDeleted()) } return n diff --git a/trie/proof.go b/trie/proof.go index 751d6f620f..cba59825ab 100644 --- a/trie/proof.go +++ b/trie/proof.go @@ -573,7 +573,12 @@ func VerifyRangeProof(rootHash common.Hash, firstKey []byte, keys [][]byte, valu } // Rebuild the trie with the leaf stream, the shape of trie // should be same with the original one. - tr := &Trie{root: root, reader: newEmptyReader(), tracer: newTracer()} + tr := &Trie{ + root: root, + reader: newEmptyReader(), + opTracer: newOpTracer(), + prevalueTracer: newPrevalueTracer(), + } if empty { tr.root = nil } diff --git a/trie/tracer.go b/trie/tracer.go index 90b9666f0b..4af99c5b7f 100644 --- a/trie/tracer.go +++ b/trie/tracer.go @@ -18,14 +18,14 @@ package trie import ( "maps" - - "github.com/ethereum/go-ethereum/common" + "slices" + "sync" ) -// tracer tracks the changes of trie nodes. During the trie operations, +// opTracer tracks the changes of trie nodes. During the trie operations, // some nodes can be deleted from the trie, while these deleted nodes // won't be captured by trie.Hasher or trie.Committer. Thus, these deleted -// nodes won't be removed from the disk at all. Tracer is an auxiliary tool +// nodes won't be removed from the disk at all. opTracer is an auxiliary tool // used to track all insert and delete operations of trie and capture all // deleted nodes eventually. // @@ -35,38 +35,25 @@ import ( // This tool can track all of them no matter the node is embedded in its // parent or not, but valueNode is never tracked. // -// Besides, it's also used for recording the original value of the nodes -// when they are resolved from the disk. The pre-value of the nodes will -// be used to construct trie history in the future. -// -// Note tracer is not thread-safe, callers should be responsible for handling +// Note opTracer is not thread-safe, callers should be responsible for handling // the concurrency issues by themselves. -type tracer struct { - inserts map[string]struct{} - deletes map[string]struct{} - accessList map[string][]byte +type opTracer struct { + inserts map[string]struct{} + deletes map[string]struct{} } -// newTracer initializes the tracer for capturing trie changes. -func newTracer() *tracer { - return &tracer{ - inserts: make(map[string]struct{}), - deletes: make(map[string]struct{}), - accessList: make(map[string][]byte), +// newOpTracer initializes the tracer for capturing trie changes. +func newOpTracer() *opTracer { + return &opTracer{ + inserts: make(map[string]struct{}), + deletes: make(map[string]struct{}), } } -// onRead tracks the newly loaded trie node and caches the rlp-encoded -// blob internally. Don't change the value outside of function since -// it's not deep-copied. -func (t *tracer) onRead(path []byte, val []byte) { - t.accessList[string(path)] = val -} - // onInsert tracks the newly inserted trie node. If it's already // in the deletion set (resurrected node), then just wipe it from // the deletion set as it's "untouched". -func (t *tracer) onInsert(path []byte) { +func (t *opTracer) onInsert(path []byte) { if _, present := t.deletes[string(path)]; present { delete(t.deletes, string(path)) return @@ -77,7 +64,7 @@ func (t *tracer) onInsert(path []byte) { // onDelete tracks the newly deleted trie node. If it's already // in the addition set, then just wipe it from the addition set // as it's untouched. -func (t *tracer) onDelete(path []byte) { +func (t *opTracer) onDelete(path []byte) { if _, present := t.inserts[string(path)]; present { delete(t.inserts, string(path)) return @@ -86,37 +73,102 @@ func (t *tracer) onDelete(path []byte) { } // reset clears the content tracked by tracer. -func (t *tracer) reset() { - t.inserts = make(map[string]struct{}) - t.deletes = make(map[string]struct{}) - t.accessList = make(map[string][]byte) +func (t *opTracer) reset() { + clear(t.inserts) + clear(t.deletes) } // copy returns a deep copied tracer instance. -func (t *tracer) copy() *tracer { - accessList := make(map[string][]byte, len(t.accessList)) - for path, blob := range t.accessList { - accessList[path] = common.CopyBytes(blob) - } - return &tracer{ - inserts: maps.Clone(t.inserts), - deletes: maps.Clone(t.deletes), - accessList: accessList, +func (t *opTracer) copy() *opTracer { + return &opTracer{ + inserts: maps.Clone(t.inserts), + deletes: maps.Clone(t.deletes), } } -// deletedNodes returns a list of node paths which are deleted from the trie. -func (t *tracer) deletedNodes() []string { - var paths []string +// deletedList returns a list of node paths which are deleted from the trie. +func (t *opTracer) deletedList() [][]byte { + paths := make([][]byte, 0, len(t.deletes)) for path := range t.deletes { - // It's possible a few deleted nodes were embedded - // in their parent before, the deletions can be no - // effect by deleting nothing, filter them out. - _, ok := t.accessList[path] - if !ok { - continue - } - paths = append(paths, path) + paths = append(paths, []byte(path)) } return paths } + +// prevalueTracer tracks the original values of resolved trie nodes. Cached trie +// node values are expected to be immutable. +// +// prevalueTracer is protected by a lock to ensure concurrency safety, as +// concurrent map writes may occur during concurrent trie read operations. +type prevalueTracer struct { + data map[string][]byte + lock sync.RWMutex +} + +// newPrevalueTracer initializes the tracer for capturing resolved trie nodes. +func newPrevalueTracer() *prevalueTracer { + return &prevalueTracer{ + data: make(map[string][]byte), + } +} + +// put tracks the newly loaded trie node and caches its RLP-encoded +// blob internally. Do not modify the value outside this function, +// as it is not deep-copied. +func (t *prevalueTracer) put(path []byte, val []byte) { + t.lock.Lock() + defer t.lock.Unlock() + + t.data[string(path)] = val +} + +// has returns a flag indicating whether the corresponding trie node specified +// by the path exists in the trie. +func (t *prevalueTracer) has(path []byte) bool { + t.lock.RLock() + defer t.lock.RUnlock() + + _, ok := t.data[string(path)] + return ok +} + +// hasList returns a list of flag indicating whether the corresponding trie nodes +// specified by the path exists in the trie. +func (t *prevalueTracer) hasList(list [][]byte) []bool { + t.lock.RLock() + defer t.lock.RUnlock() + + exists := make([]bool, 0, len(list)) + for _, path := range list { + _, ok := t.data[string(path)] + exists = append(exists, ok) + } + return exists +} + +// values returns a list of values of the cached trie nodes. +func (t *prevalueTracer) values() [][]byte { + t.lock.RLock() + defer t.lock.RUnlock() + + return slices.Collect(maps.Values(t.data)) +} + +// reset resets the cached content in the prevalueTracer. +func (t *prevalueTracer) reset() { + t.lock.Lock() + defer t.lock.Unlock() + + clear(t.data) +} + +// copy returns a copied prevalueTracer instance. +func (t *prevalueTracer) copy() *prevalueTracer { + t.lock.RLock() + defer t.lock.RUnlock() + + // Shadow clone is used, as the cached trie node values are immutable + return &prevalueTracer{ + data: maps.Clone(t.data), + } +} diff --git a/trie/tracer_test.go b/trie/tracer_test.go index 852a706021..1e8d3e922e 100644 --- a/trie/tracer_test.go +++ b/trie/tracer_test.go @@ -68,8 +68,8 @@ func testTrieTracer(t *testing.T, vals []struct{ k, v string }) { for _, val := range vals { trie.MustUpdate([]byte(val.k), []byte(val.v)) } - insertSet := copySet(trie.tracer.inserts) // copy before commit - deleteSet := copySet(trie.tracer.deletes) // copy before commit + insertSet := copySet(trie.opTracer.inserts) // copy before commit + deleteSet := copySet(trie.opTracer.deletes) // copy before commit root, nodes := trie.Commit(false) db.Update(root, types.EmptyRootHash, trienode.NewWithNodeSet(nodes)) @@ -86,7 +86,7 @@ func testTrieTracer(t *testing.T, vals []struct{ k, v string }) { for _, val := range vals { trie.MustDelete([]byte(val.k)) } - insertSet, deleteSet = copySet(trie.tracer.inserts), copySet(trie.tracer.deletes) + insertSet, deleteSet = copySet(trie.opTracer.inserts), copySet(trie.opTracer.deletes) if !compareSet(insertSet, nil) { t.Fatal("Unexpected insertion set") } @@ -112,10 +112,10 @@ func testTrieTracerNoop(t *testing.T, vals []struct{ k, v string }) { for _, val := range vals { trie.MustDelete([]byte(val.k)) } - if len(trie.tracer.inserts) != 0 { + if len(trie.opTracer.inserts) != 0 { t.Fatal("Unexpected insertion set") } - if len(trie.tracer.deletes) != 0 { + if len(trie.opTracer.deletes) != 0 { t.Fatal("Unexpected deletion set") } } @@ -249,9 +249,9 @@ func TestAccessListLeak(t *testing.T) { } for _, c := range cases { trie, _ = New(TrieID(root), db) - n1 := len(trie.tracer.accessList) + n1 := len(trie.prevalueTracer.data) c.op(trie) - n2 := len(trie.tracer.accessList) + n2 := len(trie.prevalueTracer.data) if n1 != n2 { t.Fatalf("AccessList is leaked, prev %d after %d", n1, n2) diff --git a/trie/trie.go b/trie/trie.go index fdb4da9be4..1562d81a26 100644 --- a/trie/trie.go +++ b/trie/trie.go @@ -55,8 +55,9 @@ type Trie struct { // reader is the handler trie can retrieve nodes from. reader *trieReader - // tracer is the tool to track the trie changes. - tracer *tracer + // Various tracers for capturing the modification to trie + opTracer *opTracer + prevalueTracer *prevalueTracer } // newFlag returns the cache flag value for a newly created node. @@ -67,13 +68,14 @@ func (t *Trie) newFlag() nodeFlag { // Copy returns a copy of Trie. func (t *Trie) Copy() *Trie { return &Trie{ - root: copyNode(t.root), - owner: t.owner, - committed: t.committed, - unhashed: t.unhashed, - uncommitted: t.uncommitted, - reader: t.reader, - tracer: t.tracer.copy(), + root: copyNode(t.root), + owner: t.owner, + committed: t.committed, + unhashed: t.unhashed, + uncommitted: t.uncommitted, + reader: t.reader, + opTracer: t.opTracer.copy(), + prevalueTracer: t.prevalueTracer.copy(), } } @@ -89,9 +91,10 @@ func New(id *ID, db database.NodeDatabase) (*Trie, error) { return nil, err } trie := &Trie{ - owner: id.Owner, - reader: reader, - tracer: newTracer(), + owner: id.Owner, + reader: reader, + opTracer: newOpTracer(), + prevalueTracer: newPrevalueTracer(), } if id.Root != (common.Hash{}) && id.Root != types.EmptyRootHash { rootnode, err := trie.resolveAndTrack(id.Root[:], nil) @@ -361,7 +364,7 @@ func (t *Trie) insert(n node, prefix, key []byte, value node) (bool, node, error // New branch node is created as a child of the original short node. // Track the newly inserted node in the tracer. The node identifier // passed is the path from the root node. - t.tracer.onInsert(append(prefix, key[:matchlen]...)) + t.opTracer.onInsert(append(prefix, key[:matchlen]...)) // Replace it with a short node leading up to the branch. return true, &shortNode{key[:matchlen], branch, t.newFlag()}, nil @@ -379,7 +382,7 @@ func (t *Trie) insert(n node, prefix, key []byte, value node) (bool, node, error // New short node is created and track it in the tracer. The node identifier // passed is the path from the root node. Note the valueNode won't be tracked // since it's always embedded in its parent. - t.tracer.onInsert(prefix) + t.opTracer.onInsert(prefix) return true, &shortNode{key, value, t.newFlag()}, nil @@ -444,7 +447,7 @@ func (t *Trie) delete(n node, prefix, key []byte) (bool, node, error) { // The matched short node is deleted entirely and track // it in the deletion set. The same the valueNode doesn't // need to be tracked at all since it's always embedded. - t.tracer.onDelete(prefix) + t.opTracer.onDelete(prefix) return true, nil, nil // remove n entirely for whole matches } @@ -460,7 +463,7 @@ func (t *Trie) delete(n node, prefix, key []byte) (bool, node, error) { case *shortNode: // The child shortNode is merged into its parent, track // is deleted as well. - t.tracer.onDelete(append(prefix, n.Key...)) + t.opTracer.onDelete(append(prefix, n.Key...)) // Deleting from the subtrie reduced it to another // short node. Merge the nodes to avoid creating a @@ -525,7 +528,7 @@ func (t *Trie) delete(n node, prefix, key []byte) (bool, node, error) { // Replace the entire full node with the short node. // Mark the original short node as deleted since the // value is embedded into the parent now. - t.tracer.onDelete(append(prefix, byte(pos))) + t.opTracer.onDelete(append(prefix, byte(pos))) k := append([]byte{byte(pos)}, cnode.Key...) return true, &shortNode{k, cnode.Val, t.newFlag()}, nil @@ -616,13 +619,33 @@ func (t *Trie) resolveAndTrack(n hashNode, prefix []byte) (node, error) { if err != nil { return nil, err } - t.tracer.onRead(prefix, blob) + t.prevalueTracer.put(prefix, blob) // The returned node blob won't be changed afterward. No need to // deep-copy the slice. return decodeNodeUnsafe(n, blob) } +// deletedNodes returns a list of node paths, referring the nodes being deleted +// from the trie. +func (t *Trie) deletedNodes() [][]byte { + list := t.opTracer.deletedList() + + // It's possible a few deleted nodes were embedded in their parent before, + // the deletions can be no effect by deleting nothing, filter them out. + var ( + pos int + flags = t.prevalueTracer.hasList(list) + ) + for i := 0; i < len(list); i++ { + if flags[i] { // exist, keep it + list[pos] = list[i] + pos++ + } + } + return list[:pos] // trim to the new length +} + // Hash returns the root hash of the trie. It does not write to the // database and can be used even if the trie doesn't have one. func (t *Trie) Hash() common.Hash { @@ -644,13 +667,13 @@ func (t *Trie) Commit(collectLeaf bool) (common.Hash, *trienode.NodeSet) { // (b) The trie was non-empty and all nodes are dropped => return // the node set includes all deleted nodes if t.root == nil { - paths := t.tracer.deletedNodes() + paths := t.deletedNodes() if len(paths) == 0 { return types.EmptyRootHash, nil // case (a) } nodes := trienode.NewNodeSet(t.owner) for _, path := range paths { - nodes.AddNode([]byte(path), trienode.NewDeleted()) + nodes.AddNode(path, trienode.NewDeleted()) } return types.EmptyRootHash, nodes // case (b) } @@ -667,11 +690,11 @@ func (t *Trie) Commit(collectLeaf bool) (common.Hash, *trienode.NodeSet) { return rootHash, nil } nodes := trienode.NewNodeSet(t.owner) - for _, path := range t.tracer.deletedNodes() { - nodes.AddNode([]byte(path), trienode.NewDeleted()) + for _, path := range t.deletedNodes() { + nodes.AddNode(path, trienode.NewDeleted()) } // If the number of changes is below 100, we let one thread handle it - t.root = newCommitter(nodes, t.tracer, collectLeaf).Commit(t.root, t.uncommitted > 100) + t.root = newCommitter(nodes, t.prevalueTracer, collectLeaf).Commit(t.root, t.uncommitted > 100) t.uncommitted = 0 return rootHash, nodes } @@ -692,12 +715,13 @@ func (t *Trie) hashRoot() node { // Witness returns a set containing all trie nodes that have been accessed. func (t *Trie) Witness() map[string]struct{} { - if len(t.tracer.accessList) == 0 { + values := t.prevalueTracer.values() + if len(values) == 0 { return nil } - witness := make(map[string]struct{}, len(t.tracer.accessList)) - for _, node := range t.tracer.accessList { - witness[string(node)] = struct{}{} + witness := make(map[string]struct{}, len(values)) + for _, val := range values { + witness[string(val)] = struct{}{} } return witness } @@ -708,6 +732,7 @@ func (t *Trie) Reset() { t.owner = common.Hash{} t.unhashed = 0 t.uncommitted = 0 - t.tracer.reset() + t.opTracer.reset() + t.prevalueTracer.reset() t.committed = false } diff --git a/trie/trie_test.go b/trie/trie_test.go index b806ae6b0c..0cee84362b 100644 --- a/trie/trie_test.go +++ b/trie/trie_test.go @@ -595,18 +595,18 @@ func runRandTest(rt randTest) error { deleteExp[path] = struct{}{} } } - if len(insertExp) != len(tr.tracer.inserts) { + if len(insertExp) != len(tr.opTracer.inserts) { rt[i].err = errors.New("insert set mismatch") } - if len(deleteExp) != len(tr.tracer.deletes) { + if len(deleteExp) != len(tr.opTracer.deletes) { rt[i].err = errors.New("delete set mismatch") } - for insert := range tr.tracer.inserts { + for insert := range tr.opTracer.inserts { if _, present := insertExp[insert]; !present { rt[i].err = errors.New("missing inserted node") } } - for del := range tr.tracer.deletes { + for del := range tr.opTracer.deletes { if _, present := deleteExp[del]; !present { rt[i].err = errors.New("missing deleted node") }