Skip to content

Conversation

@iovoid
Copy link
Contributor

@iovoid iovoid commented Nov 12, 2025

Motivation

Currently we use get_cf for reading codes, which internally calls get_pinned_cf and copies the result.  This copying takes up 6% of code reading time.

Description

Since data needs to be deserialized first, we could use a borrowed value and avoid this copy by calling get_pinned_cf directly.

@github-actions github-actions bot added L1 Ethereum client performance Block execution throughput and performance in general labels Nov 12, 2025
@iovoid iovoid changed the title perf(l1): avoid copy when reading codes perf(l1): avoid copy when reading account code Nov 12, 2025
@github-actions
Copy link

Lines of code report

Total lines added: 3
Total lines removed: 0
Total lines changed: 3

Detailed view
+-------------------------------------------+-------+------+
| File                                      | Lines | Diff |
+-------------------------------------------+-------+------+
| ethrex/crates/storage/store_db/rocksdb.rs | 1606  | +3   |
+-------------------------------------------+-------+------+

@iovoid iovoid marked this pull request as ready for review November 12, 2025 13:07
@iovoid iovoid requested a review from a team as a code owner November 12, 2025 13:07
@ethrex-project-sync ethrex-project-sync bot moved this to In Review in ethrex_l1 Nov 12, 2025
@github-actions
Copy link

github-actions bot commented Nov 12, 2025

Benchmark Block Execution Results Comparison Against Main

Command Mean [s] Min [s] Max [s] Relative
base 60.126 ± 0.263 59.739 60.603 1.01 ± 0.01
head 59.780 ± 0.426 59.261 60.459 1.00

let Some(bytes) = self
.db
.get_pinned_cf(&cf, code_hash.as_bytes())
.map_err(|e| StoreError::Custom(format!("RocksDB read error: {}", e)))?
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: is there no StoreError::ReadError or similar already?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a ReadError but it's error message is "Could not open DB for reading" which implies a different kind of error, and doesn't pass-through the message.

@Oppen Oppen added this pull request to the merge queue Nov 12, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Nov 12, 2025
@jrchatruc jrchatruc enabled auto-merge November 12, 2025 14:56
@jrchatruc jrchatruc added this pull request to the merge queue Nov 12, 2025
Merged via the queue into main with commit 43a743f Nov 12, 2025
39 checks passed
@jrchatruc jrchatruc deleted the pinned_get_code branch November 12, 2025 16:33
@github-project-automation github-project-automation bot moved this from In Review to Done in ethrex_l1 Nov 12, 2025
@github-project-automation github-project-automation bot moved this from Todo to Done in ethrex_performance Nov 12, 2025
tomip01 pushed a commit that referenced this pull request Nov 12, 2025
**Motivation**

Currently we use `get_cf` for reading codes, which internally calls
`get_pinned_cf` and copies the result.  This copying takes up 6% of code
reading time.

**Description**

Since data needs to be deserialized first, we could use a borrowed value
and avoid this copy by calling `get_pinned_cf` directly.
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

Projects

Status: Done
Archived in project

Development

Successfully merging this pull request may close these issues.

6 participants