Skip to content
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

IntermediateRoot: add flag for threshold to update concurrently #453

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Francesco4203
Copy link
Contributor

Divide the root updating of stateObjects into goroutines if number of stateObjects is at least the threshold

@Francesco4203 Francesco4203 force-pushed the fix/concurrent-update-2 branch from 70ae86c to b0f24b0 Compare May 21, 2024 04:03
cmd/utils/flags.go Outdated Show resolved Hide resolved
Divide the root updating of stateObjects into goroutines if number of stateObjects is at least the threshold
-> results in 1.55 times faster
statedb_test.go/TestIntermediateUpdateConcurrently: add test to check if the states after processed with both options are identical
statedb/intermediateRoot: update options for concurrent: get changes that are not made yet to run concurrently
state_object/updateTrie: add option for concurrency: store and return data waiting for update
@Francesco4203 Francesco4203 force-pushed the fix/concurrent-update-2 branch from b0f24b0 to 08fd7c3 Compare May 22, 2024 11:01
@@ -319,7 +319,7 @@ func (s *stateObject) finalise(prefetch bool) {

// updateTrie writes cached storage modifications into the object's storage trie.
// It will return nil if the trie has not been loaded and no changes have been made
func (s *stateObject) updateTrie(db Database) Trie {
func (s *stateObject) updateTrie(db Database, concurrent bool) Trie {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think change this variable to skipTrieUpdate is more reasonable


if (value == common.Hash{}) {
s.setError(tr.TryDelete(key[:]))
s.db.StorageDeleted += 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is shared resource, not safe to update concurrently. Moreover, this is updated in updateTrie already

} else {
v, _ = rlp.EncodeToBytes(common.TrimLeftZeroes(value[:]))
s.setError(tr.TryUpdate(key[:], v))
s.db.StorageUpdated += 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above comment

@@ -855,11 +863,52 @@ func (s *StateDB) IntermediateRoot(deleteEmptyObjects bool) common.Hash {
// the account prefetcher. Instead, let's process all the storage updates
// first, giving the account prefeches just a few more milliseconds of time
// to pull useful data from disk.

// ---------------------------------------------- DEBUGGING -------------------------------------------------------------------------------------------
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please clean this up

for addr := range s.stateObjectsPending {
if obj := s.stateObjects[addr]; !obj.deleted {
obj.updateRoot(s.db)
updateObjs = append(updateObjs, obj)
obj.updateTrie(s.db, true)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use ConcurrentUpdateThreshold config to determine if we need to update trie concurrently or not

}
}
min := func(a, b int) int {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the new way of calculating number of objects per routine, this function is only called once, so I think removing this function and using if for that one is better

@@ -332,26 +332,33 @@ func (s *stateObject) updateTrie(db Database) Trie {
// The snapshot storage map for the object
var storage map[common.Hash][]byte
// Insert all the pending updates into the trie
tr := s.getTrie(db)
var tr Trie = nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: don't need to initialize to nil here, the default value is nil already

statedb.go/StateDB struct: remove lock since it will not be used for concurrent update
statedb.go/IntermediateRoot: check ConcurrentUpdateThreshold for choosing between sequential and concurrent update
state_object.go: change "concurrent" variable to a more reasonable name "skipTrieUpdate"
flags.go/MakeChain: add flag when creating a chain
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants