From 9509768b62788951973b5e68f84b7635f27fe8a4 Mon Sep 17 00:00:00 2001 From: danigomez Date: Sat, 5 Jun 2021 02:06:44 -0300 Subject: [PATCH 1/7] Add map to keep track of hashes arleady requested. Add logic to add node to the map and to remove node from the map while pruning --- lib/blocktree/blocktree.go | 34 +++++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/lib/blocktree/blocktree.go b/lib/blocktree/blocktree.go index 9f5cc2d5e8..c1bf1b2d0b 100644 --- a/lib/blocktree/blocktree.go +++ b/lib/blocktree/blocktree.go @@ -38,14 +38,16 @@ type BlockTree struct { leaves *leafMap db database.Database sync.RWMutex + blockIndex map[Hash]*node } // NewEmptyBlockTree creates a BlockTree with a nil head func NewEmptyBlockTree(db database.Database) *BlockTree { return &BlockTree{ - head: nil, - leaves: newEmptyLeafMap(), - db: db, + head: nil, + leaves: newEmptyLeafMap(), + db: db, + blockIndex: make(map[Hash]*node), } } @@ -61,9 +63,10 @@ func NewBlockTreeFromRoot(root *types.Header, db database.Database) *BlockTree { } return &BlockTree{ - head: head, - leaves: newLeafMap(head), - db: db, + head: head, + leaves: newLeafMap(head), + db: db, + blockIndex: make(map[Hash]*node), } } @@ -150,7 +153,19 @@ func (bt *BlockTree) GetAllBlocksAtDepth(hash common.Hash) []common.Hash { } // getNode finds and returns a node based on its Hash. Returns nil if not found. -func (bt *BlockTree) getNode(h Hash) *node { +func (bt *BlockTree) getNode(h Hash) (ret *node) { + defer func() { + if ret != nil { + if _, ok := bt.blockIndex[ret.hash]; !ok { + bt.blockIndex[ret.hash] = ret + } + } + }() + + if b, ok := bt.blockIndex[h]; ok { + return b + } + if bt.head.hash == h { return bt.head } @@ -175,6 +190,11 @@ func (bt *BlockTree) getNode(h Hash) *node { func (bt *BlockTree) Prune(finalised Hash) (pruned []Hash) { bt.Lock() defer bt.Unlock() + defer func() { + for _, hash := range pruned { + delete(bt.blockIndex, hash) + } + }() if finalised == bt.head.hash { return pruned From 43c216f53edebc02ad1f861c80b60eb95cb2b4f8 Mon Sep 17 00:00:00 2001 From: danigomez Date: Sat, 5 Jun 2021 02:21:55 -0300 Subject: [PATCH 2/7] Add blockIndex in deepCopy --- lib/blocktree/blocktree.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/blocktree/blocktree.go b/lib/blocktree/blocktree.go index c1bf1b2d0b..e60f716d60 100644 --- a/lib/blocktree/blocktree.go +++ b/lib/blocktree/blocktree.go @@ -370,7 +370,8 @@ func (bt *BlockTree) DeepCopy() *BlockTree { defer bt.RUnlock() btCopy := &BlockTree{ - db: bt.db, + db: bt.db, + blockIndex: bt.blockIndex, } if bt.head == nil { From 984e3a3c987f1b02fed0f4c245f52e5dc459dc73 Mon Sep 17 00:00:00 2001 From: danigomez Date: Thu, 10 Jun 2021 00:08:08 -0300 Subject: [PATCH 3/7] Test for cache cleanup on Prune, rename to blockCache --- lib/blocktree/blocktree.go | 16 +++++------ lib/blocktree/blocktree_test.go | 48 +++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 8 deletions(-) diff --git a/lib/blocktree/blocktree.go b/lib/blocktree/blocktree.go index e60f716d60..bce589d099 100644 --- a/lib/blocktree/blocktree.go +++ b/lib/blocktree/blocktree.go @@ -38,7 +38,7 @@ type BlockTree struct { leaves *leafMap db database.Database sync.RWMutex - blockIndex map[Hash]*node + blockCache map[Hash]*node } // NewEmptyBlockTree creates a BlockTree with a nil head @@ -47,7 +47,7 @@ func NewEmptyBlockTree(db database.Database) *BlockTree { head: nil, leaves: newEmptyLeafMap(), db: db, - blockIndex: make(map[Hash]*node), + blockCache: make(map[Hash]*node), } } @@ -66,7 +66,7 @@ func NewBlockTreeFromRoot(root *types.Header, db database.Database) *BlockTree { head: head, leaves: newLeafMap(head), db: db, - blockIndex: make(map[Hash]*node), + blockCache: make(map[Hash]*node), } } @@ -156,13 +156,13 @@ func (bt *BlockTree) GetAllBlocksAtDepth(hash common.Hash) []common.Hash { func (bt *BlockTree) getNode(h Hash) (ret *node) { defer func() { if ret != nil { - if _, ok := bt.blockIndex[ret.hash]; !ok { - bt.blockIndex[ret.hash] = ret + if _, ok := bt.blockCache[ret.hash]; !ok { + bt.blockCache[ret.hash] = ret } } }() - if b, ok := bt.blockIndex[h]; ok { + if b, ok := bt.blockCache[h]; ok { return b } @@ -192,7 +192,7 @@ func (bt *BlockTree) Prune(finalised Hash) (pruned []Hash) { defer bt.Unlock() defer func() { for _, hash := range pruned { - delete(bt.blockIndex, hash) + delete(bt.blockCache, hash) } }() @@ -371,7 +371,7 @@ func (bt *BlockTree) DeepCopy() *BlockTree { btCopy := &BlockTree{ db: bt.db, - blockIndex: bt.blockIndex, + blockCache: bt.blockCache, } if bt.head == nil { diff --git a/lib/blocktree/blocktree_test.go b/lib/blocktree/blocktree_test.go index 2e0b905613..3c3a14fc4e 100644 --- a/lib/blocktree/blocktree_test.go +++ b/lib/blocktree/blocktree_test.go @@ -229,6 +229,29 @@ func TestBlockTree_GetNode(t *testing.T) { } } +func TestBlockTree_GetNodeCache(t *testing.T) { + bt, branches := createTestBlockTree(testHeader, 16, nil) + + for _, branch := range branches { + header := &types.Header{ + ParentHash: branch.hash, + Number: branch.depth, + StateRoot: Hash{0x1}, + } + + err := bt.AddBlock(header, 0) + require.Nil(t, err) + } + + block := bt.getNode(branches[0].hash) + + cachedBlock, ok := bt.blockCache[block.hash] + require.True(t, ok) + require.NotNil(t, cachedBlock) + require.Equal(t, cachedBlock, block) + +} + func TestBlockTree_GetAllBlocksAtDepth(t *testing.T) { bt, _ := createTestBlockTree(testHeader, 8, nil) hashes := bt.head.getNodesWithDepth(big.NewInt(10), []common.Hash{}) @@ -386,12 +409,37 @@ func TestBlockTree_Prune(t *testing.T) { } } +func TestBlockTree_PruneCache(t *testing.T) { + var bt *BlockTree + var branches []testBranch + + for { + bt, branches = createTestBlockTree(testHeader, 5, nil) + if len(branches) > 0 && len(bt.getNode(branches[0].hash).children) > 1 { + break + } + } + + // pick some block to finalise + finalised := bt.head.children[0].children[0].children[0] + pruned := bt.Prune(finalised.hash) + + for _, prunedHash := range pruned { + block, ok := bt.blockCache[prunedHash] + + require.False(t, ok) + require.Nil(t, block) + } + +} + func TestBlockTree_DeepCopy(t *testing.T) { bt, _ := createFlatTree(t, 8) btCopy := bt.DeepCopy() require.Equal(t, bt.db, btCopy.db) + require.Equal(t, bt.blockCache, btCopy.blockCache) require.True(t, equalNodeValue(bt.head, btCopy.head), "BlockTree heads not equal") require.True(t, equalLeave(bt.leaves, btCopy.leaves), "BlockTree leaves not equal") From 85d2edbf6e536e62e6f01df564ee89f9811a761b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20G=C3=B3mez?= Date: Fri, 11 Jun 2021 17:38:29 -0300 Subject: [PATCH 4/7] Update lib/blocktree/blocktree.go Co-authored-by: noot <36753753+noot@users.noreply.github.com> --- lib/blocktree/blocktree.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/blocktree/blocktree.go b/lib/blocktree/blocktree.go index bce589d099..1d2ca3aba7 100644 --- a/lib/blocktree/blocktree.go +++ b/lib/blocktree/blocktree.go @@ -155,11 +155,13 @@ func (bt *BlockTree) GetAllBlocksAtDepth(hash common.Hash) []common.Hash { // getNode finds and returns a node based on its Hash. Returns nil if not found. func (bt *BlockTree) getNode(h Hash) (ret *node) { defer func() { - if ret != nil { - if _, ok := bt.blockCache[ret.hash]; !ok { - bt.blockCache[ret.hash] = ret - } - } + if ret == nil { + return + } + + if _, has := bt.blockCache[ret.hash]; !has { + bt.blockCache[ret.hash] = ret + } }() if b, ok := bt.blockCache[h]; ok { From abafc3538f61f1538cafd020582033c581eed75f Mon Sep 17 00:00:00 2001 From: danigomez Date: Fri, 11 Jun 2021 19:54:08 -0300 Subject: [PATCH 5/7] Change blockCache to nodeCache, Add setInCache in AddBlock, Add deepCopy of nodeCache --- lib/blocktree/blocktree.go | 52 +++++++++++++++++++-------------- lib/blocktree/blocktree_test.go | 17 +++++++++-- 2 files changed, 44 insertions(+), 25 deletions(-) diff --git a/lib/blocktree/blocktree.go b/lib/blocktree/blocktree.go index 1d2ca3aba7..bb95927ee0 100644 --- a/lib/blocktree/blocktree.go +++ b/lib/blocktree/blocktree.go @@ -38,16 +38,16 @@ type BlockTree struct { leaves *leafMap db database.Database sync.RWMutex - blockCache map[Hash]*node + nodeCache map[Hash]*node } // NewEmptyBlockTree creates a BlockTree with a nil head func NewEmptyBlockTree(db database.Database) *BlockTree { return &BlockTree{ - head: nil, - leaves: newEmptyLeafMap(), - db: db, - blockCache: make(map[Hash]*node), + head: nil, + leaves: newEmptyLeafMap(), + db: db, + nodeCache: make(map[Hash]*node), } } @@ -63,10 +63,10 @@ func NewBlockTreeFromRoot(root *types.Header, db database.Database) *BlockTree { } return &BlockTree{ - head: head, - leaves: newLeafMap(head), - db: db, - blockCache: make(map[Hash]*node), + head: head, + leaves: newLeafMap(head), + db: db, + nodeCache: make(map[Hash]*node), } } @@ -106,6 +106,7 @@ func (bt *BlockTree) AddBlock(header *types.Header, arrivalTime uint64) error { } parent.addChild(n) bt.leaves.replace(parent, n) + bt.setInCache(n) return nil } @@ -152,19 +153,21 @@ func (bt *BlockTree) GetAllBlocksAtDepth(hash common.Hash) []common.Hash { return bt.head.getNodesWithDepth(depth, hashes) } +func (bt *BlockTree) setInCache(b *node) { + if b == nil { + return + } + + if _, has := bt.nodeCache[b.hash]; !has { + bt.nodeCache[b.hash] = b + } +} + // getNode finds and returns a node based on its Hash. Returns nil if not found. func (bt *BlockTree) getNode(h Hash) (ret *node) { - defer func() { - if ret == nil { - return - } - - if _, has := bt.blockCache[ret.hash]; !has { - bt.blockCache[ret.hash] = ret - } - }() + defer func() { bt.setInCache(ret) }() - if b, ok := bt.blockCache[h]; ok { + if b, ok := bt.nodeCache[h]; ok { return b } @@ -194,7 +197,7 @@ func (bt *BlockTree) Prune(finalised Hash) (pruned []Hash) { defer bt.Unlock() defer func() { for _, hash := range pruned { - delete(bt.blockCache, hash) + delete(bt.nodeCache, hash) } }() @@ -372,8 +375,8 @@ func (bt *BlockTree) DeepCopy() *BlockTree { defer bt.RUnlock() btCopy := &BlockTree{ - db: bt.db, - blockCache: bt.blockCache, + db: bt.db, + nodeCache: make(map[Hash]*node), } if bt.head == nil { @@ -391,5 +394,10 @@ func (bt *BlockTree) DeepCopy() *BlockTree { } } + for hash := range bt.nodeCache { + b := bt.nodeCache[hash] + btCopy.nodeCache[hash] = b.deepCopy(b.parent) + } + return btCopy } diff --git a/lib/blocktree/blocktree_test.go b/lib/blocktree/blocktree_test.go index 3c3a14fc4e..434a955928 100644 --- a/lib/blocktree/blocktree_test.go +++ b/lib/blocktree/blocktree_test.go @@ -245,7 +245,9 @@ func TestBlockTree_GetNodeCache(t *testing.T) { block := bt.getNode(branches[0].hash) - cachedBlock, ok := bt.blockCache[block.hash] + cachedBlock, ok := bt.nodeCache[block.hash] + + require.True(t, len(bt.nodeCache) > 0) require.True(t, ok) require.NotNil(t, cachedBlock) require.Equal(t, cachedBlock, block) @@ -425,7 +427,7 @@ func TestBlockTree_PruneCache(t *testing.T) { pruned := bt.Prune(finalised.hash) for _, prunedHash := range pruned { - block, ok := bt.blockCache[prunedHash] + block, ok := bt.nodeCache[prunedHash] require.False(t, ok) require.Nil(t, block) @@ -439,7 +441,16 @@ func TestBlockTree_DeepCopy(t *testing.T) { btCopy := bt.DeepCopy() require.Equal(t, bt.db, btCopy.db) - require.Equal(t, bt.blockCache, btCopy.blockCache) + for hash := range bt.nodeCache { + b, ok := btCopy.nodeCache[hash] + b2 := bt.nodeCache[hash] + + require.True(t, ok) + require.True(t, b != b2) + + require.True(t, equalNodeValue(b, b2)) + + } require.True(t, equalNodeValue(bt.head, btCopy.head), "BlockTree heads not equal") require.True(t, equalLeave(bt.leaves, btCopy.leaves), "BlockTree leaves not equal") From 49c81575fb5f2a67258ab7749613ac42215f0755 Mon Sep 17 00:00:00 2001 From: danigomez Date: Fri, 11 Jun 2021 20:23:43 -0300 Subject: [PATCH 6/7] Fix copy --- lib/blocktree/blocktree.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/blocktree/blocktree.go b/lib/blocktree/blocktree.go index bb95927ee0..aff8ed51df 100644 --- a/lib/blocktree/blocktree.go +++ b/lib/blocktree/blocktree.go @@ -395,8 +395,7 @@ func (bt *BlockTree) DeepCopy() *BlockTree { } for hash := range bt.nodeCache { - b := bt.nodeCache[hash] - btCopy.nodeCache[hash] = b.deepCopy(b.parent) + btCopy.getNode(hash) } return btCopy From 5046d899f3e381c9f2ae198986f2dd16887498e8 Mon Sep 17 00:00:00 2001 From: danigomez Date: Fri, 11 Jun 2021 20:26:40 -0300 Subject: [PATCH 7/7] For replace of nodeCache so the copy is not tied to getNode --- lib/blocktree/blocktree.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/blocktree/blocktree.go b/lib/blocktree/blocktree.go index aff8ed51df..dcaeaab644 100644 --- a/lib/blocktree/blocktree.go +++ b/lib/blocktree/blocktree.go @@ -395,7 +395,7 @@ func (bt *BlockTree) DeepCopy() *BlockTree { } for hash := range bt.nodeCache { - btCopy.getNode(hash) + btCopy.nodeCache[hash] = btCopy.getNode(hash) } return btCopy