Skip to content

perf(l1): improve block headers vec handling in syncer#4771

Merged
edg-l merged 7 commits into
mainfrom
improve_block_headers_snap
Oct 7, 2025
Merged

perf(l1): improve block headers vec handling in syncer#4771
edg-l merged 7 commits into
mainfrom
improve_block_headers_snap

Conversation

@edg-l

@edg-l edg-l commented Oct 6, 2025

Copy link
Copy Markdown
Contributor

Motivation

Currently, in snap and full sync we remove the first header from the vec because we already have it, this is quite wasteful specially on a Vec where removing the first element is the most expensive.

Description

Converted process_incoming_headers to take an owned iterator instead, this way we avoid any copy, also in full sync changed the clone and clear into a single mem take, which both clears and returns the owned vec to be used as an iterator.

Also avoided some clones when passing block hashes around in store block

@edg-l edg-l added the syncing Snap sync, Full sync label Oct 6, 2025
@github-actions github-actions Bot added L1 Ethereum client performance Block execution throughput and performance in general labels Oct 6, 2025
@edg-l
edg-l marked this pull request as ready for review October 6, 2025 09:19
@edg-l
edg-l requested a review from a team as a code owner October 6, 2025 09:19
@edg-l edg-l moved this from Todo to In review in ethrex_performance Oct 6, 2025
@edg-l edg-l moved this to In Review in ethrex_l1 Oct 6, 2025
@edg-l edg-l changed the title perf(l1): improve block headers vec handling in sync perf(l1): improve block headers vec handling in syncer Oct 6, 2025
@github-actions

github-actions Bot commented Oct 6, 2025

Copy link
Copy Markdown

Lines of code report

Total lines added: 7
Total lines removed: 4
Total lines changed: 11

Detailed view
+----------------------------------------------+-------+------+
| File                                         | Lines | Diff |
+----------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/peer_handler.rs | 1689  | -4   |
+----------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/sync.rs         | 1224  | +7   |
+----------------------------------------------+-------+------+

@github-actions

github-actions Bot commented Oct 6, 2025

Copy link
Copy Markdown

Benchmark Block Execution Results Comparison Against Main

Command Mean [s] Min [s] Max [s] Relative
base 88.386 ± 0.165 88.147 88.668 1.00 ± 0.00
head 88.364 ± 0.184 88.106 88.723 1.00

@Arkenan Arkenan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@edg-l
edg-l added this pull request to the merge queue Oct 7, 2025
Merged via the queue into main with commit 0dc57dd Oct 7, 2025
35 checks passed
@edg-l
edg-l deleted the improve_block_headers_snap branch October 7, 2025 12:07
@github-project-automation github-project-automation Bot moved this from In Review to Done in ethrex_l1 Oct 7, 2025
@github-project-automation github-project-automation Bot moved this from In review to Done in ethrex_performance Oct 7, 2025
Peponks9 pushed a commit to Peponks9/ethrex that referenced this pull request Oct 7, 2025
)

**Motivation**

Currently, in snap and full sync we remove the first header from the vec
because we already have it, this is quite wasteful specially on a Vec
where removing the first element is the most expensive.

**Description**

Converted process_incoming_headers to take an owned iterator instead,
this way we avoid any copy, also in full sync changed the clone and
clear into a single mem take, which both clears and returns the owned
vec to be used as an iterator.

Also avoided some clones when passing block hashes around in store block
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

L1 Ethereum client performance Block execution throughput and performance in general syncing Snap sync, Full sync

Projects

Status: Done
Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants