forked from bnb-chain/bsc
-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: updated by review tips #15
Merged
Merged
Changes from 6 commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
6cb2322
chore: updated by review tips
5d5b309
chore: refine log/metrics
e2b366f
fix: fix disklayer pointer
0d229af
chore: refine logs
a38c161
fix: refine cache cleanup workflow
958b3bf
chore: refine some log
25e0c9a
chore: check repeated difflayer
00e8f9d
chore: add origin disklayer to difflayer
8e6e681
chore: refine some code
7cce143
chore: adjust some log
8606534
chore: refine log print
625f31a
fix: update origin workflow
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,12 +26,17 @@ import ( | |
"github.com/ethereum/go-ethereum/trie/triestate" | ||
) | ||
|
||
type HashIndex struct { | ||
type RefTrieNode struct { | ||
refCount uint32 | ||
node *trienode.Node | ||
} | ||
|
||
type HashNodeCache struct { | ||
lock sync.RWMutex | ||
cache map[common.Hash]*trienode.Node | ||
cache map[common.Hash]*RefTrieNode | ||
} | ||
|
||
func (h *HashIndex) length() int { | ||
func (h *HashNodeCache) length() int { | ||
if h == nil { | ||
return 0 | ||
} | ||
|
@@ -40,54 +45,68 @@ func (h *HashIndex) length() int { | |
return len(h.cache) | ||
} | ||
|
||
func (h *HashIndex) set(hash common.Hash, node *trienode.Node) { | ||
func (h *HashNodeCache) set(hash common.Hash, node *trienode.Node) { | ||
if h == nil { | ||
return | ||
} | ||
h.lock.Lock() | ||
defer h.lock.Unlock() | ||
h.cache[hash] = node | ||
if n, ok := h.cache[hash]; ok { | ||
n.refCount++ | ||
} else { | ||
h.cache[hash] = &RefTrieNode{1, node} | ||
} | ||
} | ||
|
||
func (h *HashIndex) Get(hash common.Hash) *trienode.Node { | ||
func (h *HashNodeCache) Get(hash common.Hash) *trienode.Node { | ||
if h == nil { | ||
return nil | ||
} | ||
h.lock.RLock() | ||
defer h.lock.RUnlock() | ||
if n, ok := h.cache[hash]; ok { | ||
return n | ||
return n.node | ||
} | ||
return nil | ||
} | ||
|
||
func (h *HashIndex) del(hash common.Hash) { | ||
func (h *HashNodeCache) del(hash common.Hash) { | ||
if h == nil { | ||
return | ||
} | ||
h.lock.Lock() | ||
defer h.lock.Unlock() | ||
delete(h.cache, hash) | ||
n, ok := h.cache[hash] | ||
if !ok { | ||
return | ||
} | ||
if n.refCount > 0 { | ||
n.refCount-- | ||
} | ||
if n.refCount == 0 { | ||
delete(h.cache, hash) | ||
} | ||
} | ||
|
||
func (h *HashIndex) Add(ly layer) { | ||
func (h *HashNodeCache) Add(ly layer) { | ||
if h == nil { | ||
return | ||
} | ||
dl, ok := ly.(*diffLayer) | ||
if !ok { | ||
return | ||
} | ||
beforeAdd := h.length() | ||
for _, subset := range dl.nodes { | ||
for _, node := range subset { | ||
h.set(node.Hash, node) | ||
} | ||
} | ||
diffHashCacheLengthGauge.Update(int64(h.length())) | ||
log.Debug("Add difflayer to hash map", "root", ly.rootHash(), "map_len", h.length()) | ||
log.Info("Add difflayer to hash map", "root", ly.rootHash(), "block_number", dl.block, "map_len", h.length(), "add_delta", h.length()-beforeAdd) | ||
} | ||
|
||
func (h *HashIndex) Remove(ly layer) { | ||
func (h *HashNodeCache) Remove(ly layer) { | ||
if h == nil { | ||
return | ||
} | ||
|
@@ -96,13 +115,14 @@ func (h *HashIndex) Remove(ly layer) { | |
return | ||
} | ||
go func() { | ||
beforeDel := h.length() | ||
for _, subset := range dl.nodes { | ||
for _, node := range subset { | ||
h.del(node.Hash) | ||
} | ||
} | ||
diffHashCacheLengthGauge.Update(int64(h.length())) | ||
log.Debug("Remove difflayer from hash map", "root", ly.rootHash(), "map_len", h.length()) | ||
log.Info("Remove difflayer from hash map", "root", ly.rootHash(), "block_number", dl.block, "map_len", h.length(), "del_delta", beforeDel-h.length()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. debug. |
||
}() | ||
} | ||
|
||
|
@@ -119,7 +139,7 @@ type diffLayer struct { | |
nodes map[common.Hash]map[string]*trienode.Node // Cached trie nodes indexed by owner and path | ||
states *triestate.Set // Associated state change set for building history | ||
memory uint64 // Approximate guess as to how much memory we use | ||
cache *HashIndex // trienode cache by hash key. | ||
cache *HashNodeCache // trienode cache by hash key. | ||
|
||
parent layer // Parent layer modified by this one, never nil, **can be changed** | ||
lock sync.RWMutex // Lock used to protect parent | ||
|
@@ -139,13 +159,15 @@ func newDiffLayer(parent layer, root common.Hash, id uint64, block uint64, nodes | |
states: states, | ||
parent: parent, | ||
} | ||
|
||
if pdl, ok := parent.(*diffLayer); ok && pdl.cache != nil { | ||
dl.cache = pdl.cache | ||
} else { | ||
dl.cache = &HashIndex{ | ||
cache: make(map[common.Hash]*trienode.Node), | ||
dl.cache = &HashNodeCache{ | ||
cache: make(map[common.Hash]*RefTrieNode), | ||
} | ||
} | ||
|
||
for _, subset := range nodes { | ||
for path, n := range subset { | ||
dl.memory += uint64(n.Size() + len(path)) | ||
|
@@ -233,13 +255,14 @@ func (dl *diffLayer) Node(owner common.Hash, path []byte, hash common.Hash) ([]b | |
parent := dl.parent | ||
for { | ||
if disk, ok := parent.(*diskLayer); ok { | ||
blob, err := disk.NodeByLogger(owner, path, hash, log.Debug) | ||
blob, err := disk.Node(owner, path, hash) | ||
if err != nil { | ||
// This is a bad case with a very low probability. The same trienode exists | ||
// in different difflayers and can be cleared from the map in advance. In | ||
// this case, the 128-layer difflayer is queried again. | ||
// This is a bad case with a very low probability. | ||
// Reading the difflayer cache and reading the disklayer are not in the same lock, | ||
// so in extreme cases, both reading the difflayer cache and reading the disklayer may fail. | ||
// In this case, fallback to the original 128-layer recursive difflayer query path. | ||
diffHashCacheSlowPathMeter.Mark(1) | ||
log.Debug("Hash map and disklayer mismatch, retry difflayer", "owner", owner, "path", path, "hash", hash.String()) | ||
log.Debug("Hash map and disklayer mismatch, retry difflayer", "owner", owner, "path", path, "hash", hash.String(), "error", err) | ||
return dl.node(owner, path, hash, 0) | ||
} else { | ||
return blob, nil | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -51,9 +51,20 @@ func (tree *layerTree) reset(head layer) { | |
tree.lock.Lock() | ||
defer tree.lock.Unlock() | ||
|
||
for _, ly := range tree.layers { | ||
if dl, ok := ly.(*diffLayer); ok { | ||
// Clean up the hash cache of the child difflayer corresponding to difflayer. | ||
dl.cache.Remove(dl) | ||
} | ||
} | ||
|
||
var layers = make(map[common.Hash]layer) | ||
for head != nil { | ||
layers[head.rootHash()] = head | ||
if dl, ok := head.(*diffLayer); ok { | ||
// Add the hash cache of the child difflayer corresponding to difflayer. | ||
dl.cache.Add(dl) | ||
} | ||
head = head.parentLayer() | ||
} | ||
tree.layers = layers | ||
|
@@ -129,14 +140,32 @@ func (tree *layerTree) cap(root common.Hash, layers int) error { | |
tree.lock.Lock() | ||
defer tree.lock.Unlock() | ||
|
||
defer func() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove. |
||
log.Info("Succeed to cap", "root", root.String(), "layers", layers, "layer_tree_len", len(tree.layers)) | ||
for _, ly := range tree.layers { | ||
if dl, ok := ly.(*diffLayer); ok { | ||
log.Info("Print cache", "cache_length", dl.cache.length()) | ||
break | ||
} | ||
} | ||
}() | ||
|
||
// If full commit was requested, flatten the diffs and merge onto disk | ||
if layers == 0 { | ||
base, err := diff.persist(true) | ||
if err != nil { | ||
return err | ||
} | ||
for _, ly := range tree.layers { | ||
if dl, ok := ly.(*diffLayer); ok { | ||
// Clean up difflayer hash cache. | ||
dl.cache.Remove(dl) | ||
log.Info("Cleanup difflayer hash cache", "diff_root", dl.root.String(), "diff_block_number", dl.block) | ||
} | ||
} | ||
// Replace the entire layer tree with the flat base | ||
tree.layers = map[common.Hash]layer{base.rootHash(): base} | ||
log.Info("Cap all difflayers to disklayer", "disk_root", base.rootHash().String()) | ||
return nil | ||
} | ||
// Dive until we run out of layers or reach the persistent database | ||
|
@@ -186,7 +215,9 @@ func (tree *layerTree) cap(root common.Hash, layers int) error { | |
if df, exist := tree.layers[root]; exist { | ||
if dl, ok := df.(*diffLayer); ok { | ||
// Clean up the hash cache of the child difflayer corresponding to the stale parent. | ||
// include re-org case. | ||
dl.cache.Remove(dl) | ||
log.Info("Cleanup difflayer hash cache due to reorg", "diff_root", dl.root.String(), "diff_block_number", dl.block) | ||
} | ||
} | ||
delete(tree.layers, root) | ||
|
@@ -198,6 +229,7 @@ func (tree *layerTree) cap(root common.Hash, layers int) error { | |
for root, layer := range tree.layers { | ||
if dl, ok := layer.(*diskLayer); ok && dl.isStale() { | ||
remove(root) | ||
log.Info("Remove stale the disklayer", "disk_root", dl.root.String()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. debug. |
||
} | ||
} | ||
return nil | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
debug.