Skip to content

feat(l1, l2): storage commit change #3043

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 23 commits into
base: main
Choose a base branch
from
Open

Conversation

pablodeymo
Copy link
Contributor

Motivation

This PR modifies the insert strategy to make the commits at the end of the block instead of doing them on every transaction.

Description

Closes #issue_number

Copy link

github-actions bot commented Jun 4, 2025

Lines of code report

Total lines added: 175
Total lines removed: 11
Total lines changed: 186

Detailed view
+-------------------------------------------------+-------+------+
| File                                            | Lines | Diff |
+-------------------------------------------------+-------+------+
| ethrex/cmd/ef_tests/state/runner/levm_runner.rs | 382   | +1   |
+-------------------------------------------------+-------+------+
| ethrex/cmd/ethrex/l2/command.rs                 | 358   | +8   |
+-------------------------------------------------+-------+------+
| ethrex/crates/blockchain/blockchain.rs          | 491   | -9   |
+-------------------------------------------------+-------+------+
| ethrex/crates/blockchain/payload.rs             | 565   | +1   |
+-------------------------------------------------+-------+------+
| ethrex/crates/common/trie/trie.rs               | 878   | +13  |
+-------------------------------------------------+-------+------+
| ethrex/crates/storage/api.rs                    | 268   | -2   |
+-------------------------------------------------+-------+------+
| ethrex/crates/storage/error.rs                  | 53    | +2   |
+-------------------------------------------------+-------+------+
| ethrex/crates/storage/store.rs                  | 1313  | +26  |
+-------------------------------------------------+-------+------+
| ethrex/crates/storage/store_db/in_memory.rs     | 661   | +37  |
+-------------------------------------------------+-------+------+
| ethrex/crates/storage/store_db/libmdbx.rs       | 1452  | +46  |
+-------------------------------------------------+-------+------+
| ethrex/crates/storage/store_db/redb.rs          | 1291  | +41  |
+-------------------------------------------------+-------+------+

Copy link

github-actions bot commented Jun 4, 2025

Benchmark for 9f8b7ae

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 37.1±2.52ms 37.9±2.35ms +2.16%
Trie/cita-trie insert 1k 3.6±0.05ms 3.7±0.14ms +2.78%
Trie/ethrex-trie insert 10k 55.8±0.45ms 58.8±2.71ms +5.38%
Trie/ethrex-trie insert 1k 6.6±0.05ms 6.6±0.09ms 0.00%

Copy link

github-actions bot commented Jun 4, 2025

Benchmark for 32ea549

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 37.2±1.37ms 35.6±2.63ms -4.30%
Trie/cita-trie insert 1k 3.7±0.06ms 3.7±0.10ms 0.00%
Trie/ethrex-trie insert 10k 59.1±1.30ms 59.6±1.81ms +0.85%
Trie/ethrex-trie insert 1k 6.6±0.09ms 6.7±0.12ms +1.52%

Copy link

github-actions bot commented Jun 4, 2025

Benchmark for 5ff126b

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 35.3±0.55ms 36.3±1.68ms +2.83%
Trie/cita-trie insert 1k 3.7±0.13ms 3.7±0.08ms 0.00%
Trie/ethrex-trie insert 10k 56.9±1.36ms 57.0±1.56ms +0.18%
Trie/ethrex-trie insert 1k 6.5±0.06ms 6.6±0.15ms +1.54%

Copy link

github-actions bot commented Jun 4, 2025

Benchmark for 8eb61a4

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 36.2±1.31ms 35.2±0.84ms -2.76%
Trie/cita-trie insert 1k 3.6±0.03ms 3.6±0.14ms 0.00%
Trie/ethrex-trie insert 10k 56.6±0.93ms 56.8±1.66ms +0.35%
Trie/ethrex-trie insert 1k 6.6±0.03ms 6.6±0.05ms 0.00%

.storage
.apply_account_updates_batch(block.header.parent_hash, account_updates)
.await?
.ok_or(ChainError::ParentStateNotFound)?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this the only error possible? I think this might be misleading

Copy link

github-actions bot commented Jun 5, 2025

Benchmark for c738ad6

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 39.1±2.86ms 34.6±2.81ms -11.51%
Trie/cita-trie insert 1k 3.6±0.04ms 3.7±0.15ms +2.78%
Trie/ethrex-trie insert 10k 60.4±1.77ms 61.1±2.31ms +1.16%
Trie/ethrex-trie insert 1k 6.9±0.17ms 6.7±0.09ms -2.90%

Copy link

github-actions bot commented Jun 5, 2025

Benchmark for 3b8d26f

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 35.3±1.74ms 35.4±1.13ms +0.28%
Trie/cita-trie insert 1k 3.6±0.06ms 3.6±0.12ms 0.00%
Trie/ethrex-trie insert 10k 57.9±1.71ms 57.5±1.65ms -0.69%
Trie/ethrex-trie insert 1k 6.7±0.26ms 6.6±0.02ms -1.49%

Oppen and others added 4 commits June 6, 2025 16:31
…ases (#3072)

**Motivation**

Original PR was missing ReDB and InMemory implementations.

**Description**

Merges the single-update method as a special case of the batch one.

Part of #3043

---------

Co-authored-by: Juan Bono <juanbono94@gmail.com>
Co-authored-by: Ivan Litteri <67517699+ilitteri@users.noreply.github.com>
Comment on lines +365 to +369
(
H256,
Vec<(NodeHash, Vec<u8>)>,
Vec<(H256, Vec<(NodeHash, Vec<u8>)>)>,
),
Copy link
Collaborator

Choose a reason for hiding this comment

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

This return type requires documentation, and I suggest declaring it outside the function, possibly as a struct.

Copy link

github-actions bot commented Jun 9, 2025

Benchmark for f8d90b4

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 38.4±2.72ms 39.6±2.66ms +3.13%
Trie/cita-trie insert 1k 3.5±0.04ms 3.7±0.31ms +5.71%
Trie/ethrex-trie insert 10k 61.9±3.06ms 60.9±2.11ms -1.62%
Trie/ethrex-trie insert 1k 6.5±0.06ms 6.6±0.09ms +1.54%

Copy link

github-actions bot commented Jun 9, 2025

Benchmark for 57197df

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 38.1±2.22ms 37.6±3.56ms -1.31%
Trie/cita-trie insert 1k 3.6±0.05ms 3.6±0.07ms 0.00%
Trie/ethrex-trie insert 10k 65.5±2.36ms 65.6±2.01ms +0.15%
Trie/ethrex-trie insert 1k 6.8±0.16ms 6.8±0.28ms 0.00%

@juanbono juanbono changed the title [DO NOT MERGE] Storage commit change Storage commit change Jun 9, 2025
@juanbono juanbono marked this pull request as ready for review June 9, 2025 22:07
@juanbono juanbono requested a review from a team as a code owner June 9, 2025 22:07
@jrchatruc jrchatruc changed the title Storage commit change feat(l1, l2): storage commit change Jun 9, 2025
Copy link

github-actions bot commented Jun 9, 2025

Benchmark for e2c8607

Click to view benchmark
Test Base PR %
Trie/cita-trie insert 10k 34.9±1.08ms 35.3±1.31ms +1.15%
Trie/cita-trie insert 1k 3.6±0.03ms 3.6±0.06ms 0.00%
Trie/ethrex-trie insert 10k 57.4±0.87ms 57.3±1.25ms -0.17%
Trie/ethrex-trie insert 1k 6.6±0.05ms 6.8±0.35ms +3.03%

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

Successfully merging this pull request may close these issues.

6 participants