Skip to content

perf(core): make TrieDb use NodeHash as key #2517

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

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Conversation

edg-l
Copy link
Member

@edg-l edg-l commented Apr 22, 2025

Motivation

Followup on #2516 using the fact that NodeHash is Copy to use it as the key for the trie db instead of a Vec

Description

Changes the trait TrieDb to use a NodeHash as key instead of a generic vec, allowing less expensive clones when passing around keys since NodeHash is copy and doesn't do any allocation.

@edg-l edg-l changed the base branch from main to nodehash_inline April 22, 2025 08:51
Copy link

github-actions bot commented Apr 22, 2025

Lines of code report

Total lines added: 18
Total lines removed: 2
Total lines changed: 20

Detailed view
+--------------------------------------------------+-------+------+
| File                                             | Lines | Diff |
+--------------------------------------------------+-------+------+
| ethrex/crates/common/trie/node_hash.rs           | 129   | +12  |
+--------------------------------------------------+-------+------+
| ethrex/crates/common/trie/trie.rs                | 810   | -1   |
+--------------------------------------------------+-------+------+
| ethrex/crates/storage/trie_db/libmdbx.rs         | 172   | +3   |
+--------------------------------------------------+-------+------+
| ethrex/crates/storage/trie_db/libmdbx_dupsort.rs | 87    | +2   |
+--------------------------------------------------+-------+------+
| ethrex/crates/storage/trie_db/test_utils.rs      | 24    | +1   |
+--------------------------------------------------+-------+------+
| ethrex/crates/storage/trie_db/utils.rs           | 10    | -1   |
+--------------------------------------------------+-------+------+

Copy link

Benchmark for 2e3a2f5

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 41.1±2.56ms 33.5±2.78ms -18.49%
Trie/cita-trie insert 1k 3.7±0.04ms 3.6±0.04ms -2.70%
Trie/ethrex-trie insert 10k 140.9±2.07ms 138.3±5.70ms -1.85%
Trie/ethrex-trie insert 1k 11.8±0.18ms 11.3±0.32ms -4.24%

Copy link

Benchmark for 7152460

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 38.0±1.45ms 35.4±0.95ms -6.84%
Trie/cita-trie insert 1k 3.6±0.05ms 3.6±0.02ms 0.00%
Trie/ethrex-trie insert 10k 133.7±2.59ms 130.0±5.44ms -2.77%
Trie/ethrex-trie insert 1k 11.3±0.16ms 11.0±0.38ms -2.65%

@edg-l edg-l force-pushed the nodehash_inline branch from 14b6dd3 to d484413 Compare April 22, 2025 15:09
@edg-l edg-l force-pushed the nodehash_inline branch from d484413 to 6e724b5 Compare April 23, 2025 10:04
Copy link

Benchmark for 2175d70

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 37.1±1.52ms 41.2±2.48ms +11.05%
Trie/cita-trie insert 1k 3.7±0.05ms 3.7±0.03ms 0.00%
Trie/ethrex-trie insert 10k 134.1±2.74ms 133.3±4.33ms -0.60%
Trie/ethrex-trie insert 1k 11.2±0.14ms 11.0±0.11ms -1.79%

Copy link

Benchmark for 9cd8507

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 41.7±2.37ms 35.0±1.86ms -16.07%
Trie/cita-trie insert 1k 3.6±0.06ms 3.7±0.07ms +2.78%
Trie/ethrex-trie insert 10k 138.5±2.58ms 134.1±2.80ms -3.18%
Trie/ethrex-trie insert 1k 11.3±0.13ms 11.6±0.72ms +2.65%

@edg-l edg-l force-pushed the edgl_nodehash_db branch from 03fc03b to 21c67f4 Compare April 23, 2025 10:21
@edg-l edg-l marked this pull request as ready for review April 23, 2025 10:21
@edg-l edg-l requested a review from a team as a code owner April 23, 2025 10:21
@edg-l edg-l force-pushed the edgl_nodehash_db branch from 5a30bc6 to e92b11d Compare April 23, 2025 10:24
Copy link

Benchmark for 6a27f32

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 33.2±2.34ms 35.6±2.16ms +7.23%
Trie/cita-trie insert 1k 3.5±0.03ms 3.7±0.12ms +5.71%
Trie/ethrex-trie insert 10k 138.4±2.49ms 136.5±3.24ms -1.37%
Trie/ethrex-trie insert 1k 11.5±0.45ms 11.5±0.68ms 0.00%

Copy link

Benchmark for db5c22b

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 35.0±1.09ms 34.4±1.44ms -1.71%
Trie/cita-trie insert 1k 3.6±0.04ms 3.7±0.04ms +2.78%
Trie/ethrex-trie insert 10k 141.7±1.69ms 137.2±2.85ms -3.18%
Trie/ethrex-trie insert 1k 11.5±0.24ms 11.5±0.71ms 0.00%

Copy link

Benchmark for b6e9237

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 37.7±2.33ms 40.2±1.75ms +6.63%
Trie/cita-trie insert 1k 3.6±0.03ms 3.8±0.04ms +5.56%
Trie/ethrex-trie insert 10k 134.2±3.04ms 135.1±2.81ms +0.67%
Trie/ethrex-trie insert 1k 11.3±0.11ms 11.2±0.08ms -0.88%

Base automatically changed from nodehash_inline to main April 23, 2025 10:35
Copy link

github-actions bot commented Apr 23, 2025

Benchmark Block Execution Results Comparison Against Main

Command Mean [s] Min [s] Max [s] Relative
base 183.512 ± 1.011 181.818 185.064 1.00 ± 0.01
head 182.906 ± 1.042 181.128 184.607 1.00

Copy link

Benchmark for 80b48c4

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 36.2±1.87ms 36.3±0.88ms +0.28%
Trie/cita-trie insert 1k 3.6±0.14ms 3.7±0.03ms +2.78%
Trie/ethrex-trie insert 10k 131.4±3.11ms 129.5±5.00ms -1.45%
Trie/ethrex-trie insert 1k 11.1±0.13ms 11.2±0.41ms +0.90%

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants