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

Parallel Trie Branch KeyGeneration #7048

Merged
merged 7 commits into from
May 20, 2024
Merged

Parallel Trie Branch KeyGeneration #7048

merged 7 commits into from
May 20, 2024

Conversation

benaadams
Copy link
Member

@benaadams benaadams commented May 19, 2024

Changes

  • For the root of the Trie calculate all the branches keys in parallel (which occurs in the GetLength path)

Before 7+% of block processing
image

After 1.5% of block processing

image

Types of changes

What types of changes does your code introduce?

  • Optimization

Testing

Requires testing

  • No

@benaadams benaadams marked this pull request as ready for review May 19, 2024 17:34
@benaadams
Copy link
Member Author

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.

Why this can only be supported for the root key?

src/Nethermind/Nethermind.Trie/TrieNode.Decoder.cs Outdated Show resolved Hide resolved
src/Nethermind/Nethermind.Trie/TrieNode.Decoder.cs Outdated Show resolved Hide resolved
src/Nethermind/Nethermind.Trie/TrieNode.Decoder.cs Outdated Show resolved Hide resolved
src/Nethermind/Nethermind.Trie/TrieNode.Decoder.cs Outdated Show resolved Hide resolved
src/Nethermind/Nethermind.Trie/TrieNode.Decoder.cs Outdated Show resolved Hide resolved
@benaadams
Copy link
Member Author

Why this can only be supported for the root key?

🤔 idea was only single level of Parallism, not sure how Parallel.For performs if recursed at each level. Will test

@Scooletz
Copy link
Contributor

🤔 idea was only single level of Parallism, not sure how Parallel.For performs if recursed at each level. Will test

Doesn't it fall into poor parallelism category? Also: how many cores one can expect?

@benaadams
Copy link
Member Author

🤔 idea was only single level of Parallism, not sure how Parallel.For performs if recursed at each level. Will test

Doesn't it fall into poor parallelism category? Also: how many cores one can expect?

In what regard? Isn't it Embarrassingly Parallel? Am more concerned about blocking threads from 16^N splits

@benaadams
Copy link
Member Author

benaadams commented May 20, 2024

95.1% is in ProcessTransaction; not sure it much gained by further parallising it? vs overheads

image

@benaadams benaadams requested a review from LukaszRozmej May 20, 2024 17:29
@benaadams benaadams merged commit 0a0a1c9 into master May 20, 2024
67 checks passed
@benaadams benaadams deleted the parallelkey branch May 20, 2024 20:39
@kamilchodola kamilchodola restored the parallelkey branch May 27, 2024 19:28
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.

3 participants