From 76e499605ab35830a96c4e260d2436f21e1a1c3a Mon Sep 17 00:00:00 2001 From: Martin Holst Swende Date: Mon, 6 Jan 2020 21:16:16 +0100 Subject: [PATCH] trie: review feedback, mainly docs + minor changes --- trie/pure_committer.go | 21 ++++++++++++++++++--- trie/pure_hasher.go | 6 ++++-- trie/trie.go | 30 +++++++++--------------------- 3 files changed, 31 insertions(+), 26 deletions(-) diff --git a/trie/pure_committer.go b/trie/pure_committer.go index d74fc0d4c8..80c83af27b 100644 --- a/trie/pure_committer.go +++ b/trie/pure_committer.go @@ -26,6 +26,10 @@ import ( "golang.org/x/crypto/sha3" ) +// LeafChanSize is the size of the leafCh. It's a pretty arbitrary number, to allow +// some paralellism but not incur too much memory overhead. +const LeafChanSize = 200 + // Leaf represents a trie leaf value type Leaf struct { size int // size of the rlp data (estimate) @@ -34,6 +38,12 @@ type Leaf struct { vnodes bool // set to true if the node (possibly) contains a valueNode } +// committer is a type used for the trie Commit operation. A committer has some +// internal preallocated temp space, and also a callback that is invoked when +// leaves are committed. The leafs are passed through the `leafCh`, to allow +// some level of paralellism. +// By 'some level' of paralellism, it's still the case that all leaves will be +// processed sequentially - onleaf will never be called in paralell or out of order. type committer struct { tmp sliceBuffer sha keccakState @@ -52,11 +62,17 @@ var committerPool = sync.Pool{ }, } +// newCommitter creates a new committer or picks one from the pool, and +// initializes the leafCh, if needed. +// In case no onleaf-callback is provided, the committer does not +// use a channel-based commit, but inlined. +// Typically, the account trie is committed with a channel-based leaf-commit, +// whereas storage tries are committed 'inline'. func newCommitter(onleaf LeafCallback) *committer { h := committerPool.Get().(*committer) h.onleaf = onleaf if onleaf != nil { - h.leafCh = make(chan *Leaf, 200) // arbitrary number + h.leafCh = make(chan *Leaf, LeafChanSize) } return h } @@ -200,8 +216,7 @@ func (h *committer) store(n node, db *Database, force bool, hasVnodeChildren boo } // commitLoop does the actual insert + leaf callback for nodes -func (h *committer) commitLoop(db *Database, wg *sync.WaitGroup) { - defer wg.Done() +func (h *committer) commitLoop(db *Database) { for item := range h.leafCh { var ( hash = item.hash diff --git a/trie/pure_hasher.go b/trie/pure_hasher.go index f13d2607ba..f6d536db5c 100644 --- a/trie/pure_hasher.go +++ b/trie/pure_hasher.go @@ -23,6 +23,8 @@ import ( "golang.org/x/crypto/sha3" ) +// pureHasher is a type used for the trie Hash operation. A pureHasher has some +// internal preallocated temp space type pureHasher struct { sha keccakState @@ -30,7 +32,7 @@ type pureHasher struct { tmpKey []byte } -// hashers live in a global db. +// pureHasherPool holds pureHashers var pureHasherPool = sync.Pool{ New: func() interface{} { return &pureHasher{ @@ -139,7 +141,7 @@ func (h *pureHasher) shortnodeToHash(n *shortNode, force bool) node { func (h *pureHasher) fullnodeToHash(n *fullNode, force bool) node { h.tmp.Reset() // Generate the RLP encoding of the node - if err := rlp.Encode(&h.tmp, n); err != nil { + if err := n.EncodeRLP(&h.tmp); err != nil { panic("encode error: " + err.Error()) } diff --git a/trie/trie.go b/trie/trie.go index 74666fbdda..78e7d724f5 100644 --- a/trie/trie.go +++ b/trie/trie.go @@ -410,27 +410,8 @@ func (t *Trie) Hash() common.Hash { return common.BytesToHash(hash.(hashNode)) } -// oldCommit is the old implementation of Commit, which uses the -// regular hasher. -// It writes all nodes to the trie's memory database, tracking the internal +// Commit writes all nodes to the trie's memory database, tracking the internal // and external (for account tries) references. -func (t *Trie) oldCommit(onleaf LeafCallback) (root common.Hash, err error) { - if t.db == nil { - panic("commit called on trie with nil database") - } - if t.root == nil { - return emptyRoot, nil - } - h := newHasher(onleaf) - defer returnHasherToPool(h) - hash, cached, err := h.hash(t.root, t.db, true) - if err != nil { - return common.Hash{}, err - } - t.root = cached - return common.BytesToHash(hash.(hashNode)), nil -} - func (t *Trie) Commit(onleaf LeafCallback) (root common.Hash, err error) { if t.db == nil { panic("commit called on trie with nil database") @@ -450,10 +431,17 @@ func (t *Trie) Commit(onleaf LeafCallback) (root common.Hash, err error) { var wg sync.WaitGroup if onleaf != nil { wg.Add(1) - go h.commitLoop(t.db, &wg) + go func() { + defer wg.Done() + h.commitLoop(t.db) + }() } _, err = h.commit(t.root, t.db, true) if onleaf != nil { + // The leafch is created in newCommitter if there was an onleaf callback + // provided. The commitLoop only _reads_ from it, and the commit + // operation was the sole writer. Therefore, it's safe to close this + // channel here. close(h.leafCh) wg.Wait() }