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

Root hash thread pool, new trie benches. #404

Merged
merged 4 commits into from
Feb 5, 2025
Merged

Conversation

dvush
Copy link
Contributor

@dvush dvush commented Feb 5, 2025

📝 Summary

  • adds a new, configurable thread pool used for root hash with custom thread names
  • refactors sparse trie benches and tests to use gzip files
  • adds more benches for real tries with focus on p99 latency

💡 Motivation and Context

I was experimenting with thread affinity and improved eth sparse trie along the way. Affinity does not appear to give boost in real builder so its not here. We should revisit it in the future.

✅ I have completed the following steps:

  • Run make lint
  • Run make test
  • Added tests (if applicable)

* adds a new, configurable thread pool used for root hash with custom
thread names
* refactors sparse trie benches and tests to use gzip files
* adds more benches for real tries with focus on p99 latency
@dvush dvush requested a review from efbig February 5, 2025 11:21
Copy link

github-actions bot commented Feb 5, 2025

Benchmark results for 6b7a62c

Report: https://flashbots-rbuilder-ci-stats.s3.us-east-2.amazonaws.com/benchmark/6b7a62c-eb2784e/report/index.html

Date (UTC) 2025-02-05T16:27:35+00:00
Commit 6b7a62c17834b2b2e549507d44eb8f5c73d19187
Base SHA eb2784e73a428b02b4c8a46577ddf7bfe397a1c4

Significant changes

Benchmark Mean Status
gather_nodes_clone 17.38% Performance has degraded.
MEV-Boost SubmitBlock serialization/JSON encoding -30.90% Performance has improved.
reference_trie_insert_and_hash_3000 -4.88% Performance has improved.

Copy link
Contributor

@ZanCorDX ZanCorDX left a comment

Choose a reason for hiding this comment

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

StateProviderFactory::root_hasher should not have "thread_pool: Option", since it's for a specific implementation.
It should be in StateProviderFactoryFromProviderFactory/StateProviderFactoryFromRethProvider the same way we hide the root_hash_config from the roothash users (maybe StateProviderFactoryFromRethProvider/StateProviderFactoryFromProviderFactory need a refactoring?).

@dvush
Copy link
Contributor Author

dvush commented Feb 5, 2025

good point, I moved it to root hash config (now called context) since its begins from config file and its being copied from there.

@dvush dvush requested a review from ZanCorDX February 5, 2025 16:18
@dvush dvush merged commit c77646b into develop Feb 5, 2025
5 checks passed
@dvush dvush deleted the root_hash_thread_pool branch February 5, 2025 16:40
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