trie: roll back some changes

core/state/statedb.go

@@ -945,7 +945,7 @@ func (s *StateDB) fastDeleteStorage(snaps *snapshot.Tree, addrHash common.Hash,
 		slots = make(map[common.Hash][]byte)
 	)
 	stack := trie.NewStackTrie(func(path []byte, hash common.Hash, blob []byte) {
-		nodes.AddNode(string(path), trienode.NewDeleted())
+		nodes.AddNode(path, trienode.NewDeleted())
 	})
 	for iter.Next() {
 		slot := common.CopyBytes(iter.Slot())
@@ -991,7 +991,7 @@ func (s *StateDB) slowDeleteStorage(addr common.Address, addrHash common.Hash, r
 		if it.Hash() == (common.Hash{}) {
 			continue
 		}
-		nodes.AddNode(string(it.Path()), trienode.NewDeleted())
+		nodes.AddNode(it.Path(), trienode.NewDeleted())
 	}
 	if err := it.Error(); err != nil {
 		return nil, nil, err

trie/committer.go

@@ -35,9 +35,9 @@ type committer struct {
 }
 
 // newCommitter creates a new committer or picks one from the pool.
-func newCommitter(nodes *trienode.NodeSet, tracer *tracer, collectLeaf bool, parallel bool) *committer {
+func newCommitter(nodeset *trienode.NodeSet, tracer *tracer, collectLeaf bool, parallel bool) *committer {
 	return &committer{
-		nodes:       nodes,
+		nodes:       nodeset,
 		tracer:      tracer,
 		collectLeaf: collectLeaf,
 		parallel:    parallel,
@@ -118,20 +118,22 @@ func (c *committer) commitChildren(path []byte, n *fullNode, parallel bool) [17]
 			children[i] = c.commit(append(path, byte(i)), child, false)
 		} else {
 			wg.Add(1)
+			var nodesMu sync.Mutex
 			go func(index int) {
 				p := append(path, byte(i))
 				set := trienode.NewNodeSet(c.nodes.Owner)
 				childComitter := newCommitter(set, c.tracer, c.collectLeaf, false)
 				h := childComitter.commit(p, child, false)
 				children[index] = h
+				nodesMu.Lock()
 				c.nodes.MergeSet(set)
+				nodesMu.Unlock()
 				wg.Done()
 			}(i)
 		}
 	}
 	if parallel {
 		wg.Wait()
-
 	}
 	// For the 17th child, it's possible the type is valuenode.
 	if n.Children[16] != nil {
@@ -156,12 +158,13 @@ func (c *committer) store(path []byte, n node) node {
 		// deleted only if the node was existent in database before.
 		_, ok := c.tracer.accessList[string(path)]
 		if ok {
-			c.nodes.AddNode(path, trienode.NewDeleted()) // TODO
+			c.nodes.AddNode(path, trienode.NewDeleted())
 		}
 		return n
 	}
+	// Collect the dirty node to nodeset for return.
 	nhash := common.BytesToHash(hash)
-	c.nodes.AddNode(path, trienode.New(nhash, nodeToBytes(n))) // TODO
+	c.nodes.AddNode(path, trienode.New(nhash, nodeToBytes(n)))
 
 	// Collect the corresponding leaf node if it's required. We don't check
 	// full node since it's impossible to store value in fullNode. The key
@@ -169,7 +172,7 @@ func (c *committer) store(path []byte, n node) node {
 	if c.collectLeaf {
 		if sn, ok := n.(*shortNode); ok {
 			if val, ok := sn.Val.(valueNode); ok {
-				c.nodes.AddLeaf(nhash, val) // TODO
+				c.nodes.AddLeaf(nhash, val)
 			}
 		}
 	}

trie/trie.go

@@ -37,13 +37,25 @@ import (
 //
 // Trie is not safe for concurrent use.
 type Trie struct {
-	root      node
-	owner     common.Hash
-	committed bool        // The Flag whether the commit operation is already performed
-	reader    *trieReader // The handler trie can retrieve nodes from
-	tracer    *tracer     // The tool to track the trie changes
-	mutate    int         // The number of trie mutations that have been performed
-	hashed    int         // The number of mutations that have been hashed
+	root  node
+	owner common.Hash
+
+	// Flag whether the commit operation is already performed. If so the
+	// trie is not usable(latest states is invisible).
+	committed bool
+
+	// Keep track of the number leaves which have been inserted since the last
+	// hashing operation. This number will not directly map to the number of
+	// actually unhashed nodes.
+	unhashed int
+	// uncommitted is the number of updates since last commit.
+	uncommitted int
+
+	// reader is the handler trie can retrieve nodes from.
+	reader *trieReader
+
+	// tracer is the tool to track the trie changes.
+	tracer *tracer
 }
 
 // newFlag returns the cache flag value for a newly created node.
@@ -54,13 +66,13 @@ func (t *Trie) newFlag() nodeFlag {
 // Copy returns a copy of Trie.
 func (t *Trie) Copy() *Trie {
 	return &Trie{
-		root:      t.root,
-		owner:     t.owner,
-		committed: t.committed,
-		reader:    t.reader,
-		tracer:    t.tracer.copy(),
-		mutate:    t.mutate,
-		hashed:    t.hashed,
+		root:        t.root,
+		owner:       t.owner,
+		committed:   t.committed,
+		reader:      t.reader,
+		tracer:      t.tracer.copy(),
+		uncommitted: t.uncommitted,
+		unhashed:    t.unhashed,
 	}
 }
 
@@ -295,11 +307,12 @@ func (t *Trie) Update(key, value []byte) error {
 	if t.committed {
 		return ErrCommitted
 	}
-	t.mutate++
 	return t.update(key, value)
 }
 
 func (t *Trie) update(key, value []byte) error {
+	t.unhashed++
+	t.uncommitted++
 	k := keybytesToHex(key)
 	if len(value) != 0 {
 		_, n, err := t.insert(t.root, nil, k, valueNode(value))
@@ -413,7 +426,8 @@ func (t *Trie) Delete(key []byte) error {
 	if t.committed {
 		return ErrCommitted
 	}
-	t.mutate++
+	t.uncommitted++
+	t.unhashed++
 	k := keybytesToHex(key)
 	_, n, err := t.delete(t.root, nil, k)
 	if err != nil {
@@ -633,9 +647,9 @@ func (t *Trie) Commit(collectLeaf bool) (common.Hash, *trienode.NodeSet) {
 	for _, path := range t.tracer.deletedNodes() {
 		nodes.AddNode([]byte(path), trienode.NewDeleted())
 	}
-	// If the number of changes is below 100, we let one thread handle it
-	t.root = newCommitter(nodes, t.tracer, collectLeaf, t.mutate > 100).Commit(t.root)
-	t.mutate = 0
+	// If the number of changes is below 400, we let one thread handle it
+	t.root = newCommitter(nodes, t.tracer, collectLeaf, t.uncommitted > 400).Commit(t.root)
+	t.uncommitted = 0
 	return rootHash, nodes
 }
 
@@ -645,10 +659,10 @@ func (t *Trie) hashRoot() (node, node) {
 		return hashNode(types.EmptyRootHash.Bytes()), nil
 	}
 	// If the number of changes is below 100, we let one thread handle it
-	h := newHasher(t.mutate-t.hashed >= 100)
+	h := newHasher(t.unhashed >= 100)
 	defer func() {
 		returnHasherToPool(h)
-		t.hashed = t.mutate
+		t.unhashed = 0
 	}()
 	hashed, cached := h.hash(t.root, true)
 	return hashed, cached
@@ -670,8 +684,8 @@ func (t *Trie) Witness() map[string]struct{} {
 func (t *Trie) Reset() {
 	t.root = nil
 	t.owner = common.Hash{}
+	t.unhashed = 0
+	t.uncommitted = 0
 	t.tracer.reset()
 	t.committed = false
-	t.hashed = 0
-	t.mutate = 0
 }

trie/trie_test.go

@@ -26,6 +26,7 @@ import (
 	"math/rand"
 	"reflect"
 	"sort"
+	"strings"
 	"testing"
 	"testing/quick"
 
@@ -40,7 +41,6 @@ import (
 	"github.com/ethereum/go-ethereum/trie/trienode"
 	"github.com/holiman/uint256"
 	"golang.org/x/crypto/sha3"
-	"strings"
 )
 
 func init() {
@@ -1240,8 +1240,8 @@ func BenchmarkCommit(b *testing.B) {
 	//benchmarkCommit(b, 100)
 	//benchmarkCommit(b, 200)
 	//benchmarkCommit(b, 500)
-	//benchmarkCommit(b, 1000)
-	//benchmarkCommit(b, 2000)
+	benchmarkCommit(b, 1000)
+	benchmarkCommit(b, 2000)
 	benchmarkCommit(b, 5000)
 }
 
@@ -1265,7 +1265,7 @@ func testCommit(b *testing.B, n int, parallel bool) {
 		}
 		tries[i].Hash()
 		if !parallel {
-			tries[i].mutate = 0
+			tries[i].uncommitted = 0
 		}
 	}
 	b.ResetTimer()
@@ -1286,9 +1286,8 @@ func TestCommitCorrect(t *testing.T) {
 		refTrie.Update(common.CopyBytes(key), common.CopyBytes(val))
 	}
 	paraTrie.Hash()
-	//paraTrie.mutate = 0
 	refTrie.Hash()
-	refTrie.mutate = 0
+	refTrie.uncommitted = 0
 
 	haveRoot, haveNodes := paraTrie.Commit(true)
 	wantRoot, wantNodes := refTrie.Commit(true)
@@ -1315,7 +1314,7 @@ func printSet(set *trienode.NodeSet) string {
 	var out = new(strings.Builder)
 	fmt.Fprintf(out, "nodeset owner: %v\n", set.Owner)
 	var paths []string
-	for k, _ := range set.Nodes {
+	for k := range set.Nodes {
 		paths = append(paths, k)
 	}
 	sort.Strings(paths)

trie/trienode/node.go

@@ -22,7 +22,7 @@ import (
 	"strings"
 
 	"github.com/ethereum/go-ethereum/common"
-	"sync"
+	"golang.org/x/exp/maps"
 )
 
 // Node is a wrapper which contains the encoded blob of the trie node and its
@@ -60,8 +60,6 @@ type leaf struct {
 // NodeSet contains a set of nodes collected during the commit operation.
 // Each node is keyed by path. It's not thread-safe to use.
 type NodeSet struct {
-	mu sync.Mutex
-
 	Owner   common.Hash
 	Leaves  []*leaf
 	Nodes   map[string]*Node
@@ -102,21 +100,18 @@ func (set *NodeSet) AddNode(path []byte, n *Node) {
 	set.Nodes[string(path)] = n
 }
 
-// MergeSet mergest set with other. It assumes that the sets are disjunct, so
-// that it does not need to deduplicate data (count deletes, dedup leaves etc).
+// MergeSet merges this 'set' with 'other'. It assumes that the sets are disjoint, and
+// thus does not deduplicate data (count deletes, dedup leaves etc).
 func (set *NodeSet) MergeSet(other *NodeSet) error {
 	if set.Owner != other.Owner {
 		return fmt.Errorf("nodesets belong to different owner are not mergeable %x-%x", set.Owner, other.Owner)
 	}
-	set.mu.Lock()
-	defer set.mu.Unlock()
-	for path, node := range other.Nodes {
-		set.Nodes[path] = node
-	}
+	maps.Copy(set.Nodes, other.Nodes)
+
 	set.deletes += other.deletes
 	set.updates += other.updates
-	// Since we assume the sets are disjunct, we can safely append leaves
-	// like this without dedup.
+	// Since we assume the sets are disjoint, we can safely append leaves
+	// like this without deduplication.
 	set.Leaves = append(set.Leaves, other.Leaves...)
 	return nil
 }
@@ -126,8 +121,6 @@ func (set *NodeSet) Merge(owner common.Hash, nodes map[string]*Node) error {
 	if set.Owner != owner {
 		return fmt.Errorf("nodesets belong to different owner are not mergeable %x-%x", set.Owner, owner)
 	}
-	set.mu.Lock()
-	defer set.mu.Unlock()
 	for path, node := range nodes {
 		prev, ok := set.Nodes[path]
 		if ok {

trie/trienode/node_test.go

@@ -42,7 +42,7 @@ func benchmarkMerge(b *testing.B, count int) {
 		blob := make([]byte, 32)
 		rand.Read(blob)
 		hash := crypto.Keccak256Hash(blob)
-		s.AddNode(string(path), New(hash, blob))
+		s.AddNode(path, New(hash, blob))
 	}
 	for i := 0; i < count; i++ {
 		// Random path of 4 nibbles