use goroutines manager for trie commit #6611
Conversation
…actor-trie-commit # Conflicts: # common/interface.go # trie/branchNode_test.go # trie/extensionNode_test.go # trie/patriciaMerkleTrie.go
…actor-trie-commit # Conflicts: # trie/branchNode.go # trie/extensionNode_test.go # trie/patriciaMerkleTrie.go # trie/rootManager.go
…actor-trie-commit
| @@ -353,23 +354,30 @@ func (tr *patriciaMerkleTrie) Commit() error { | |||
| return nil | |||
| } | |||
|
|
|||
| oldRootHash := tr.GetOldRootHash() | |||
| if log.GetLevel() == logger.LogTrace { | |||
There was a problem hiding this comment.
It was helpful for some debugging scenarios. I would leave it like this for now.
trie/branchNode.go
Outdated
| encNode, err := bn.getEncodedNode() | ||
| if err != nil { | ||
| return err | ||
| goRoutinesManager.SetError(err) | ||
| return | ||
| } | ||
| hash := bn.hasher.Compute(string(encNode)) | ||
| bn.hash = hash | ||
| hashesCollector.AddDirtyHash(hash) | ||
|
|
||
| err = targetDb.Put(hash, encNode) | ||
| if err != nil { | ||
| goRoutinesManager.SetError(err) | ||
| return | ||
| } |
There was a problem hiding this comment.
can we have like a common function for this code?
There was a problem hiding this comment.
it seems similar with extention and leaf node code
There was a problem hiding this comment.
Indeed, extracted in a separate function.
| @@ -1,6 +1,7 @@ | |||
| package syncer | |||
|
|
|||
| import ( | |||
| "github.com/multiversx/mx-chain-go/state/hashesCollector" | |||
trie/depthFirstSync_test.go
Outdated
| @@ -4,6 +4,7 @@ import ( | |||
| "bytes" | |||
| "context" | |||
| "fmt" | |||
| "github.com/multiversx/mx-chain-go/state/hashesCollector" | |||
update/genesis/import.go
Outdated
| @@ -5,6 +5,7 @@ import ( | |||
| "encoding/hex" | |||
| "encoding/json" | |||
| "fmt" | |||
| "github.com/multiversx/mx-chain-go/state/hashesCollector" | |||
|
|
||
| // NewDataTrieHashesCollector creates a new instance of dataTrieHashesCollector. | ||
| // This collector is used to collect hashes related to the data trie. | ||
| func NewDataTrieHashesCollector() *dataTrieHashesCollector { |
There was a problem hiding this comment.
You make a new one for this every single time ? and that is why you do not have delete function ?
I think adding some capacity on the creation of the oldHashes / newHashes can make processing faster.
There was a problem hiding this comment.
Yes. Added a capacity of 10k. That means 10000 hashes => 320kB
state/accountsDB.go
Outdated
| oldHashes := make(common.ModifiedHashes) | ||
| newHashes := make(common.ModifiedHashes) | ||
| hc := hashesCollector.NewDisabledHashesCollector() | ||
| if adb.mainTrie.GetStorageManager().IsPruningEnabled() { |
There was a problem hiding this comment.
make a separate function for this 3 lines -> and could we have without NEW at every single commit? like you could have a cleanup function here -> might be faster.
There was a problem hiding this comment.
for this if you could have a factory - which creates the good structure by default.
There was a problem hiding this comment.
Added a cleanup function. There is no need for a factory after the changes since we create the component on the constructor.
state/accountsDB.go
Outdated
| } | ||
| } | ||
|
|
||
| func createTheHashesCollector(mainTrie common.Trie) common.TrieHashesCollector { |
There was a problem hiding this comment.
| func createTheHashesCollector(mainTrie common.Trie) common.TrieHashesCollector { | |
| func createTrieHashesCollector(mainTrie common.Trie) common.TrieHashesCollector { |
?
BeniaminDrasovean
dismissed
ssd04’s stale review
The base branch was changed.
Reasoning behind the pull request
GetObsoleteHashes,GetDirtyHashes,GetOldRoot. These can be extracted in a new componentProposed changes
hashesCollectorwhich gathers all the necessary data for aCommitTesting procedure
Pre-requisites
Based on the Contributing Guidelines the PR author and the reviewers must check the following requirements are met:
featbranch created?featbranch merging, do all satellite projects have a proper tag insidego.mod?