Skip to content

perf(trie): parallel rlp node updates in sparse trie #13251

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

Closed
wants to merge 14 commits into from

Conversation

shekhirin
Copy link
Collaborator

@shekhirin shekhirin commented Dec 9, 2024

Makes the RevealedSparseTrie::rlp_node take a non-mutable reference to self and return the list of updates that are need to be applied to the trie nodes. This allows us to run multiple rlp_node in parallel in the RevealedSparseTrie::update_rlp_node_level, because we gather the nodes at the provided level and calculate their hashes independently from each other.

@shekhirin shekhirin added C-perf A change motivated by improving speed, memory usage or disk footprint A-trie Related to Merkle Patricia Trie implementation labels Dec 13, 2024
@shekhirin shekhirin force-pushed the alexey/sparse-trie-parallel branch from 75d8c87 to a50b561 Compare December 15, 2024 13:04
@shekhirin shekhirin marked this pull request as ready for review December 15, 2024 13:04
@shekhirin shekhirin marked this pull request as draft December 16, 2024 07:05
Comment on lines +1144 to +1145
targets
.into_par_iter()
Copy link
Member

Choose a reason for hiding this comment

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

feature gate behind rayon or parallel or smth pls

@shekhirin shekhirin marked this pull request as ready for review January 6, 2025 12:03
@shekhirin shekhirin marked this pull request as draft January 27, 2025 12:11
@shekhirin shekhirin force-pushed the alexey/sparse-trie-parallel branch from bee10a4 to 2b81abe Compare January 29, 2025 12:17
Copy link

codspeed-hq bot commented Jan 29, 2025

CodSpeed Performance Report

Merging #13251 will degrade performances by 11.01%

Comparing alexey/sparse-trie-parallel (0f07a91) with main (905fd37)

Summary

❌ 4 regressions
✅ 73 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark BASE HEAD Change
size 100000 | updated 1% | depth 5 10.2 ms 11.4 ms -10.07%
sparse trie[init size 1000 | update size 100 | num updates 1] 1.3 ms 1.4 ms -10.97%
sparse trie[init size 1000 | update size 1000 | num updates 1] 5.7 ms 6.4 ms -11.01%
sparse trie[init size 10000 | update size 1000 | num updates 1] 12.7 ms 14.2 ms -10.52%

&self,
prefix_set: &mut PrefixSet,
depth: usize,
) -> (Vec<(usize, Nibbles)>, PrefixSetMut) {
Copy link
Member

Choose a reason for hiding this comment

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

with these changes the type returned doesn't match the method name. It is also quite complex, looking at the type itself it is not obvious for instance what the usize is, wdyt?

@shekhirin shekhirin force-pushed the alexey/sparse-trie-parallel branch from 52bc052 to 513eab7 Compare February 5, 2025 17:04
@jenpaff jenpaff added the S-needs-triage This issue needs to be labelled label Feb 24, 2025
@jenpaff jenpaff linked an issue Feb 24, 2025 that may be closed by this pull request
@jenpaff jenpaff removed a link to an issue Feb 24, 2025
@github-actions github-actions bot added the S-stale This issue/PR is stale and will close with no further activity label Mar 18, 2025
@github-actions github-actions bot removed the S-stale This issue/PR is stale and will close with no further activity label Mar 21, 2025
@shekhirin
Copy link
Collaborator Author

This PR is outdated, will be revised later.

@shekhirin shekhirin closed this Mar 25, 2025
@github-project-automation github-project-automation bot moved this from In Progress to Done in Reth Tracker Mar 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-trie Related to Merkle Patricia Trie implementation C-perf A change motivated by improving speed, memory usage or disk footprint S-needs-triage This issue needs to be labelled
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants