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

Optimize/fetcher (EVM) #141

Merged
merged 8 commits into from
Feb 6, 2025
Merged

Optimize/fetcher (EVM) #141

merged 8 commits into from
Feb 6, 2025

Conversation

codyx
Copy link
Collaborator

@codyx codyx commented Jan 31, 2025

EVM only:

  • Fixes EVM headers overfetching.
  • Constructs MPT tries for receipts and transactions only once per relevant block.

@Okm165
Copy link
Collaborator

Okm165 commented Feb 3, 2025

Do u have any benches on improvements here?
Like time of fetching for certain significant size example?
Do u think flattening keys improves anything here? Dry run stage already does it imo in his DryRunKey sets.
The Receipt and Tx part is to be fixed from technical pow but idea is good, do u know how much improvement it brought?

@Okm165 Okm165 added enhancement New feature or request Rust labels Feb 3, 2025
@codyx
Copy link
Collaborator Author

codyx commented Feb 5, 2025

Do u have any benches on improvements here? Like time of fetching for certain significant size example? Do u think flattening keys improves anything here? Dry run stage already does it imo in his DryRunKey sets. The Receipt and Tx part is to be fixed from technical pow but idea is good, do u know how much improvement it brought?

Yes, key flattening is used to fetch all required headers all at once in the first stage (during fetching) which eliminates over-fetching as it used to be (i.e., headers were requested at different stages and were often re-fetched over and over again).

For receipts and transactions, with this PR we only construct the transaction and receipt Merkle tree once per queried block instead of n times with n being the number of transactions or receipts accessed within the same block.

Without optimization:
Starting 3 tests across 14 binaries (59 tests skipped)
PASS [ 23.431s] tests_modules evm::fetcher_modules::test_modules_evm_fetcher_many_keys_same_header
PASS [ 51.683s] tests_modules evm::fetcher_modules::test_modules_evm_fetcher_many_keys_same_header_10x
PASS [ 55.128s] tests_modules evm::fetcher_modules::test_modules_evm_fetcher_many_txns_same_header
Summary [ 55.130s] 3 tests run: 3 passed, 59 skipped

With optimization:
Starting 3 tests across 14 binaries (59 tests skipped)
PASS [ 18.494s] tests_modules evm::fetcher_modules::test_modules_evm_fetcher_many_keys_same_header
PASS [ 31.472s] tests_modules evm::fetcher_modules::test_modules_evm_fetcher_many_txns_same_header
PASS [ 44.286s] tests_modules evm::fetcher_modules::test_modules_evm_fetcher_many_keys_same_header_10x
Summary [ 44.287s] 3 tests run: 3 passed, 59 skipped

The optimized run reduced the total test time from 55,130 s to 44,287 s—a roughly 19,7% improvement overall. Notably, one test improved by nearly 39%, while the others had around a 20% speed-up.

Also, those optimizations are particularly effective to avoir over-fetching of Ethereum headers in several scenarios (i.e., typically when multiple keys access the same block).

Copy link
Collaborator

@Okm165 Okm165 left a comment

Choose a reason for hiding this comment

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

LGTM ready to merge

@Okm165
Copy link
Collaborator

Okm165 commented Feb 6, 2025

@codyx Can u run same benches as above and paste them here before merge?

@codyx
Copy link
Collaborator Author

codyx commented Feb 6, 2025

Pretty much identical, small delta due to network latency/randomness.

Starting 3 tests across 14 binaries (59 tests skipped)
    PASS [  20.828s] tests_modules evm::fetcher_modules::test_modules_evm_fetcher_many_keys_same_header
    PASS [  32.499s] tests_modules evm::fetcher_modules::test_modules_evm_fetcher_many_txns_same_header
    PASS [  45.416s] tests_modules evm::fetcher_modules::test_modules_evm_fetcher_many_keys_same_header_10x

 **Summary [  45.416s] 3 tests run: 3 passed, 59 skipped**

@codyx codyx merged commit 4457117 into rust-hints Feb 6, 2025
2 checks passed
@codyx codyx deleted the optimize/fetcher branch February 6, 2025 07:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Rust
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants