Skip to content

Conversation

edg-l
Copy link
Contributor

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

According to profiling, the get_closest_nodes is mainly bound by iterating the contacts btreemap.

Replaced the contacts btreemap with indexmap. Also replaced the peers one since it also beneffits from this.

Comparing profiling info, it goes from 31% of time taken to 3%.

Hoodi synced in 7mins 32seconds more or less

image

Reasoning:

Btreemap has fast lookup and slow iteration.
indexmap has fastish lookup and vec-like iteration

IndexMap derives a couple of performance facts directly from how it is constructed, 
which is roughly:

    A raw hash table of key-value indices, and a vector of key-value pairs.

    Iteration is very fast since it is on the dense key-values.

    Removal is fast since it moves memory areas only in the table, 
and uses a single swap in the vector.

    Lookup is fast-ish because the initial 7-bit hash lookup uses SIMD, and indices are 
densely stored. Lookup also is slow-ish since the actual key-value pairs are stored separately. (Visible when cpu caches size is limiting.)

introduces a new dep: https://crates.io/crates/indexmap ( 662,138,228 downloads )

Copy link

github-actions bot commented Oct 10, 2025

Lines of code report

Total lines added: 1
Total lines removed: 0
Total lines changed: 1

Detailed view
+---------------------------------------------------+-------+------+
| File                                              | Lines | Diff |
+---------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/discv4/peer_table.rs | 993   | +1   |
+---------------------------------------------------+-------+------+

@edg-l edg-l changed the title perf(p2p): optimize get closest nodes from O(n^2) to O(n log k) perf(l1): optimize get closest nodes from O(n^2) to O(n log k) Oct 10, 2025
@github-actions github-actions bot added the L1 Ethereum client label Oct 10, 2025
@edg-l edg-l changed the title perf(l1): optimize get closest nodes from O(n^2) to O(n log k) perf(l1): optimize get closest nodes Oct 10, 2025
@edg-l edg-l force-pushed the optimize_get_closest_nodes branch from 4700ac1 to caff76b Compare October 10, 2025 05:54
Copy link

github-actions bot commented Oct 10, 2025

Benchmark Block Execution Results Comparison Against Main

Command Mean [s] Min [s] Max [s] Relative
base 88.794 ± 0.435 87.961 89.488 1.00
head 88.900 ± 0.513 88.221 89.947 1.00 ± 0.01

@edg-l edg-l marked this pull request as ready for review October 10, 2025 07:07
@edg-l edg-l requested a review from a team as a code owner October 10, 2025 07:07
@ethrex-project-sync ethrex-project-sync bot moved this to In Review in ethrex_l1 Oct 10, 2025

for contact_to_discard_id in disposable_contacts {
self.contacts.remove(&contact_to_discard_id);
self.contacts.swap_remove(&contact_to_discard_id);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does indexmap have something like Vec::retain ? We could replace this loop with that

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: In Review
Status: Todo

Development

Successfully merging this pull request may close these issues.

2 participants