Skip to content

Conversation

Oppen
Copy link
Contributor

@Oppen Oppen commented Sep 20, 2025

During snap sync, specifically when downloading storages, we were accidentally dumping all account addresses in every file, even when that file had no storages for most of them.
By filtering, we reduce from 40GiB to 27GiB the auxiliary files in Sepolia, and the sync goes from 3h10' to 3h2' in ethrex-sync-3, and for mainnet in ethrex-sync-2 we got from 185GiB to 40GiB and from 8h6' to 7h7'.

Copy link

Lines of code report

Total lines added: 2
Total lines removed: 0
Total lines changed: 2

Detailed view
+----------------------------------------------+-------+------+
| File                                         | Lines | Diff |
+----------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/peer_handler.rs | 1670  | +2   |
+----------------------------------------------+-------+------+

@Oppen Oppen marked this pull request as ready for review September 23, 2025 23:11
@Oppen Oppen requested a review from a team as a code owner September 23, 2025 23:11
@Copilot Copilot AI review requested due to automatic review settings September 23, 2025 23:11
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR optimizes L1 network performance by filtering out empty storage entries from snapshots before encoding them. The change reduces the size of encoded snapshots by excluding accounts with no storage data.

  • Adds filtering to remove empty storage collections from account snapshots
  • Applied consistently across two snapshot creation locations in the peer handler

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@ethrex-project-sync ethrex-project-sync bot moved this to In Review in ethrex_l1 Sep 23, 2025
Copy link

github-actions bot commented Sep 24, 2025

Benchmark Block Execution Results Comparison Against Main

Command Mean [s] Min [s] Max [s] Relative
base 87.351 ± 0.230 87.054 87.837 1.00 ± 0.00
head 87.242 ± 0.146 87.098 87.546 1.00

Copy link
Collaborator

@MegaRedHand MegaRedHand left a comment

Choose a reason for hiding this comment

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

LGTM. Left a small nit


### 2025-09-24

- Avoid dumping empty storage accounts to disk [#4590](https://github.com/lambdaclass/ethrex/pull/4590)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: this needs more context

Suggested change
- Avoid dumping empty storage accounts to disk [#4590](https://github.com/lambdaclass/ethrex/pull/4590)
- Avoid dumping empty storage accounts to disk during snap sync [#4590](https://github.com/lambdaclass/ethrex/pull/4590)

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

Copy link
Contributor

@damiramirez damiramirez left a comment

Choose a reason for hiding this comment

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

🚀

@ilitteri ilitteri added this pull request to the merge queue Sep 24, 2025
@ilitteri ilitteri removed this pull request from the merge queue due to a manual request Sep 24, 2025
@ilitteri
Copy link
Contributor

Removed from the merge queue until it is tested enough.

@jrchatruc jrchatruc added this pull request to the merge queue Sep 25, 2025
Merged via the queue into main with commit dc20e3e Sep 25, 2025
41 checks passed
@jrchatruc jrchatruc deleted the perf/filter_empty_storages branch September 25, 2025 19:48
@github-project-automation github-project-automation bot moved this from Todo to Done in ethrex_performance Sep 25, 2025
@github-project-automation github-project-automation bot moved this from In Review to Done in ethrex_l1 Sep 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
L1 Ethereum client performance
Projects
Status: Done
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants