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

Perf/parallelize storage commit #7605

Merged
merged 28 commits into from
Oct 16, 2024
Merged

Conversation

asdacap
Copy link
Contributor

@asdacap asdacap commented Oct 15, 2024

  • Parallelize storage commit by committing multiple storage at the same time.
  • This need an interface change due to the locking of the dirty nodes.
    • The interface change also remove the need to know if its a state or storage trie and passing of block number to commit.
  • Also reduce the threshold at which it start parallel commit.
  • This improve block processing time by 5%.
  • Graph is before/after/before/after.
    Screenshot from 2024-10-15 14-57-40
  • Note that before, the storage time include the time it is waiting for a lock while memory pruning is happening. The time just for storage is the storage commit time minus pruning time. The times are all prometheus's rate.

Types of changes

What types of changes does your code introduce?

  • Optimization

Testing

Requires testing

  • Yes
  • No

If yes, did you write tests?

  • Yes
  • No

Notes on testing

  • Mainnet catchup
  • Mainnet sync
  • Mainnet archive node
  • Mainnet forward sync.

Copy link
Member

@LukaszRozmej LukaszRozmej left a comment

Choose a reason for hiding this comment

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

Overall great, but due to large refactor I think code can be made more readable and there are quite a few possibilities to do that.

It is now confusing what is the difference, usage and purpose of TrieStore.GetTrieStore and the difference between ITrieStore.BeginBlockCommit and IScopedTrieStore.BeginCommit. We either need better naming, or some documenting comments or both.

Also adjust doc comments where method parameters changed.

src/Nethermind/Nethermind.Trie.Test/TrieTests.cs Outdated Show resolved Hide resolved
src/Nethermind/Nethermind.Trie/Pruning/ITrieStore.cs Outdated Show resolved Hide resolved
src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs Outdated Show resolved Hide resolved
src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs Outdated Show resolved Hide resolved
src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs Outdated Show resolved Hide resolved
src/Nethermind/Nethermind.Trie/Pruning/TrieStore.cs Outdated Show resolved Hide resolved
@asdacap asdacap marked this pull request as ready for review October 16, 2024 00:23
@asdacap asdacap merged commit b0e8c2c into master Oct 16, 2024
73 checks passed
@asdacap asdacap deleted the perf/parallelize-storage-commit branch October 16, 2024 15:35
flcl42 added a commit that referenced this pull request Nov 7, 2024
@asdacap asdacap mentioned this pull request Nov 25, 2024
5 tasks
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