core/state: move metrics out of state objects #29665
Merged
Conversation
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
|
Ran a benchmark for 1+ hours, seemed mostly the same, ever so slight difference which is probably due to machine specs. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR is more of a prep thing and maaaaybe (?) a very light performance optimization.
Currently we're trying to measure things very accurately when it comes to state object updates, hashes and commits. The only problem is that there are a lot of these, and many will have insignificant runtimes (maybe update 1 slot?). The overhead of retriving system timers and updating metrics might be more than teh actual computation of 1 slot. Due to that, this PR moves the metering a bit to the outside. It does mean we lose the ability to differentiate between storage update and storage hash, but I doubt it's really relevant given that we can only change so many slots in a block.
On the other hand, moving these metrics out does a few useful cleanups: state objects don't start poking at statedb fields directly. It was a fairly bad design (yeah, there's plenty left). The other cleanup is that if we want to make any of the above operations (update, hash, commit) concurrent across state objects, then measuring them individually becomes noise; we want the real time across all of them, not the sum of times elapsed concurrently. This PR also does the prep work for that.