From 8d42e115b1cae4f09fd02b71c06ec9c85f22ad4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Fri, 26 Apr 2024 15:24:40 +0300 Subject: [PATCH 1/2] core/state: revert pending storage updates if they revert to original (#29661) --- core/state/state_object.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/core/state/state_object.go b/core/state/state_object.go index d3d20c3dc4..14a1bd389f 100644 --- a/core/state/state_object.go +++ b/core/state/state_object.go @@ -27,6 +27,7 @@ import ( "github.com/ethereum/go-ethereum/core/tracing" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/crypto" + "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/rlp" "github.com/ethereum/go-ethereum/trie/trienode" "github.com/holiman/uint256" @@ -255,9 +256,16 @@ func (s *stateObject) setState(key common.Hash, value *common.Hash) { func (s *stateObject) finalise(prefetch bool) { slotsToPrefetch := make([][]byte, 0, len(s.dirtyStorage)) for key, value := range s.dirtyStorage { - s.pendingStorage[key] = value + // If the slot is different from its original value, move it into the + // pending area to be committed at the end of the block (and prefetch + // the pathways). if value != s.originStorage[key] { + s.pendingStorage[key] = value slotsToPrefetch = append(slotsToPrefetch, common.CopyBytes(key[:])) // Copy needed for closure + } else { + // Otherwise, the slot was reverted to its original value, remove it + // from the pending area to avoid thrashing the data strutures. + delete(s.pendingStorage, key) } } if s.db.prefetcher != nil && prefetch && len(slotsToPrefetch) > 0 && s.data.Root != types.EmptyRootHash { @@ -371,6 +379,11 @@ func (s *stateObject) updateTrie() (Trie, error) { } s.db.StorageDeleted += 1 } + // If no slots were touched, issue a warning as we shouldn't have done all + // the above work in the first place + if len(usedStorage) == 0 { + log.Error("State object update was noop", "addr", s.address, "slots", len(s.pendingStorage)) + } if s.db.prefetcher != nil { s.db.prefetcher.used(s.addrHash, s.data.Root, usedStorage) } From 4253030ef67a2af2e59bbd1fd90a4c1e75939b9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Fri, 26 Apr 2024 18:35:52 +0300 Subject: [PATCH 2/2] core/state: move metrics out of state objects (#29665) --- core/blockchain.go | 4 +--- core/state/state_object.go | 9 --------- core/state/statedb.go | 9 +++++++-- 3 files changed, 8 insertions(+), 14 deletions(-) diff --git a/core/blockchain.go b/core/blockchain.go index b45cd92e52..e4c8966824 100644 --- a/core/blockchain.go +++ b/core/blockchain.go @@ -68,7 +68,6 @@ var ( accountCommitTimer = metrics.NewRegisteredResettingTimer("chain/account/commits", nil) storageReadTimer = metrics.NewRegisteredResettingTimer("chain/storage/reads", nil) - storageHashTimer = metrics.NewRegisteredResettingTimer("chain/storage/hashes", nil) storageUpdateTimer = metrics.NewRegisteredResettingTimer("chain/storage/updates", nil) storageCommitTimer = metrics.NewRegisteredResettingTimer("chain/storage/commits", nil) @@ -1937,8 +1936,7 @@ func (bc *BlockChain) processBlock(block *types.Block, statedb *state.StateDB, s accountUpdateTimer.Update(statedb.AccountUpdates) // Account updates are complete(in validation) storageUpdateTimer.Update(statedb.StorageUpdates) // Storage updates are complete(in validation) accountHashTimer.Update(statedb.AccountHashes) // Account hashes are complete(in validation) - storageHashTimer.Update(statedb.StorageHashes) // Storage hashes are complete(in validation) - triehash := statedb.AccountHashes + statedb.StorageHashes // The time spent on tries hashing + triehash := statedb.AccountHashes // The time spent on tries hashing trieUpdate := statedb.AccountUpdates + statedb.StorageUpdates // The time spent on tries update trieRead := statedb.SnapshotAccountReads + statedb.AccountReads // The time spent on account read trieRead += statedb.SnapshotStorageReads + statedb.StorageReads // The time spent on storage read diff --git a/core/state/state_object.go b/core/state/state_object.go index 14a1bd389f..1454f7a459 100644 --- a/core/state/state_object.go +++ b/core/state/state_object.go @@ -294,9 +294,6 @@ func (s *stateObject) updateTrie() (Trie, error) { if len(s.pendingStorage) == 0 { return s.trie, nil } - // Track the amount of time wasted on updating the storage trie - defer func(start time.Time) { s.db.StorageUpdates += time.Since(start) }(time.Now()) - // The snapshot storage map for the object var ( storage map[common.Hash][]byte @@ -400,9 +397,6 @@ func (s *stateObject) updateRoot() { if err != nil || tr == nil { return } - // Track the amount of time wasted on hashing the storage trie - defer func(start time.Time) { s.db.StorageHashes += time.Since(start) }(time.Now()) - s.data.Root = tr.Hash() } @@ -415,9 +409,6 @@ func (s *stateObject) commit() (*trienode.NodeSet, error) { s.origin = s.data.Copy() return nil, nil } - // Track the amount of time wasted on committing the storage trie - defer func(start time.Time) { s.db.StorageCommits += time.Since(start) }(time.Now()) - // The trie is currently in an open state and could potentially contain // cached mutations. Call commit to acquire a set of nodes that have been // modified, the set can be nil if nothing to commit. diff --git a/core/state/statedb.go b/core/state/statedb.go index 4a934fe82c..6d9cc907e0 100644 --- a/core/state/statedb.go +++ b/core/state/statedb.go @@ -151,7 +151,6 @@ type StateDB struct { AccountUpdates time.Duration AccountCommits time.Duration StorageReads time.Duration - StorageHashes time.Duration StorageUpdates time.Duration StorageCommits time.Duration SnapshotAccountReads time.Duration @@ -856,6 +855,7 @@ func (s *StateDB) IntermediateRoot(deleteEmptyObjects bool) common.Hash { // the account prefetcher. Instead, let's process all the storage updates // first, giving the account prefetches just a few more milliseconds of time // to pull useful data from disk. + start := time.Now() for addr, op := range s.mutations { if op.applied { continue @@ -865,6 +865,8 @@ func (s *StateDB) IntermediateRoot(deleteEmptyObjects bool) common.Hash { } s.stateObjects[addr].updateRoot() } + s.StorageUpdates += time.Since(start) + // Now we're about to start to write changes to the trie. The trie is so far // _untouched_. We can check with the prefetcher, if it can give us a trie // which has the same root, but also has some content loaded into it. @@ -1151,6 +1153,7 @@ func (s *StateDB) Commit(block uint64, deleteEmptyObjects bool) (common.Hash, er return common.Hash{}, err } // Handle all state updates afterwards + start := time.Now() for addr, op := range s.mutations { if op.isDelete() { continue @@ -1179,13 +1182,15 @@ func (s *StateDB) Commit(block uint64, deleteEmptyObjects bool) (common.Hash, er storageTrieNodesDeleted += deleted } } + s.StorageCommits += time.Since(start) + if codeWriter.ValueSize() > 0 { if err := codeWriter.Write(); err != nil { log.Crit("Failed to commit dirty codes", "error", err) } } // Write the account trie changes, measuring the amount of wasted time - start := time.Now() + start = time.Now() root, set, err := s.trie.Commit(true) if err != nil {