Skip to content

Conversation

@Oppen
Copy link
Contributor

@Oppen Oppen commented Oct 25, 2025

Extract the block execution for payloads to an OS thread, make all its
callees regular blocking functions, and removes the reliance on Tokio.

Caveats:

  1. The worker is in the RPC, mostly because the Blockchain is not Clone;
  2. The L2 is broken from the removal of async, fee config mostly becomes
    a mess;
  3. The channel is provided by Tokio because the RPC is still async and
    shouldn't block waiting.

Motivation

Description

Closes #issue_number

Extract the block execution for payloads to an OS thread, make all its
callees regular blocking functions, and removes the reliance on Tokio.

Caveats:
1. The worker is in the RPC, mostly because the Blockchain is not Clone;
2. The L2 is broken from the removal of async, fee config mostly becomes
   a mess;
3. The channel is provided by Tokio because the RPC is still async and
   shouldn't block waiting.
@github-actions
Copy link

github-actions bot commented Oct 25, 2025

Lines of code report

Total lines added: 42
Total lines removed: 26
Total lines changed: 68

Detailed view
+-------------------------------------------------------+-------+------+
| File                                                  | Lines | Diff |
+-------------------------------------------------------+-------+------+
| ethrex/cmd/ethrex/cli.rs                              | 619   | -1   |
+-------------------------------------------------------+-------+------+
| ethrex/crates/blockchain/blockchain.rs                | 953   | -3   |
+-------------------------------------------------------+-------+------+
| ethrex/crates/blockchain/smoke_test.rs                | 232   | -9   |
+-------------------------------------------------------+-------+------+
| ethrex/crates/l2/networking/rpc/rpc.rs                | 207   | +2   |
+-------------------------------------------------------+-------+------+
| ethrex/crates/l2/sequencer/block_producer.rs          | 257   | -1   |
+-------------------------------------------------------+-------+------+
| ethrex/crates/l2/sequencer/l1_committer.rs            | 982   | -3   |
+-------------------------------------------------------+-------+------+
| ethrex/crates/networking/p2p/rlpx/l2/l2_connection.rs | 453   | -4   |
+-------------------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/engine/payload.rs        | 708   | +14  |
+-------------------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/rpc.rs                   | 835   | +24  |
+-------------------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/utils.rs                 | 384   | +2   |
+-------------------------------------------------------+-------+------+
| ethrex/crates/storage/store_db/rocksdb.rs             | 1540  | -4   |
+-------------------------------------------------------+-------+------+
| ethrex/tooling/migrations/src/cli.rs                  | 121   | -1   |
+-------------------------------------------------------+-------+------+

@MegaRedHand MegaRedHand changed the title wip: perf(l1): payload block execution in thread perf(l1): payload block execution in thread Oct 27, 2025
@github-actions github-actions bot added L1 Ethereum client performance labels Oct 27, 2025
@MegaRedHand MegaRedHand changed the title perf(l1): payload block execution in thread perf(l1): run "engine_newPayload" block execution in a worker thread Oct 27, 2025
@MegaRedHand MegaRedHand marked this pull request as ready for review October 27, 2025 18:11
@MegaRedHand MegaRedHand requested a review from a team as a code owner October 27, 2025 18:11
Copilot AI review requested due to automatic review settings October 27, 2025 18:11
@ethrex-project-sync ethrex-project-sync bot moved this to In Review in ethrex_l1 Oct 27, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors block execution in Layer 1 to run synchronously in a dedicated worker thread instead of using async operations. The main change moves engine_newPayload block execution off Tokio's runtime by converting async methods to blocking functions and introducing a thread-based worker with channel communication.

Key changes:

  • Converted add_block, store_block, execute_block, and related storage methods from async to synchronous
  • Introduced start_block_executor function that spawns an OS thread with a channel-based communication pattern
  • Removed .await calls throughout the codebase where async block operations were invoked

Reviewed Changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
crates/blockchain/blockchain.rs Converted core blockchain methods (add_block, store_block, execute_block) from async to sync
crates/storage/store.rs Made storage methods synchronous (store_block_updates, add_pending_block, apply_account_updates_batch)
crates/storage/store_db/rocksdb.rs Removed spawn_blocking wrapper and made apply_updates synchronous
crates/storage/store_db/in_memory.rs Updated in-memory store implementation to match synchronous trait
crates/storage/api.rs Updated StoreEngine trait to define synchronous method signatures
crates/networking/rpc/rpc.rs Added start_block_executor worker thread and channel infrastructure
crates/networking/rpc/engine/payload.rs Introduced add_block helper to communicate with worker thread via channel
crates/networking/rpc/utils.rs Updated test utilities to initialize block worker channel
crates/networking/rpc/lib.rs Exported start_block_executor function
crates/networking/p2p/sync.rs Removed .await from block sync operations
crates/l2/sequencer/block_producer.rs Updated L2 block production to use synchronous storage methods
crates/l2/sequencer/l1_committer.rs Updated L1 committer to use synchronous blockchain operations
crates/l2/networking/rpc/rpc.rs Initialized block worker channel for L2 RPC
crates/l2/based/block_fetcher.rs Updated block fetcher to use synchronous add_block
tooling/migrations/src/cli.rs Removed .await from migration block operations
tooling/ef_tests/state_v2/src/modules/result_check.rs Updated test to use synchronous account updates
tooling/ef_tests/state_v2/src/modules/block_runner.rs Updated test runner to use synchronous block operations
tooling/ef_tests/blockchain/test_runner.rs Updated blockchain tests to use synchronous methods
cmd/ethrex/cli.rs Updated block import to use synchronous operations
cmd/ethrex/initializers.rs Updated head state regeneration to use synchronous methods
cmd/ethrex/l2/command.rs Removed .await from account updates batch application
crates/blockchain/smoke_test.rs Updated smoke tests to use synchronous block operations
crates/blockchain/payload.rs Updated payload processing to use synchronous account updates
crates/networking/p2p/rlpx/l2/l2_connection.rs Updated L2 connection to use synchronous block operations
benches/benches/build_block_benchmark.rs Updated benchmark to use synchronous block execution

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions
Copy link

github-actions bot commented Oct 27, 2025

Benchmark Block Execution Results Comparison Against Main

Command Mean [s] Min [s] Max [s] Relative
base 76.708 ± 0.149 76.423 76.936 1.00
head 76.909 ± 0.370 76.372 77.371 1.00 ± 0.01

Comment on lines +684 to +693
ctx.block_worker_channel
.send((notify_send, block))
.map_err(|e| {
ChainError::Custom(format!(
"failed to send block execution request to worker: {e}"
))
})?;
notify_recv
.await
.map_err(|e| ChainError::Custom(format!("failed to receive block execution result: {e}")))?
Copy link
Collaborator

Choose a reason for hiding this comment

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

We'll need to check for edge cases like two competing blocks being received at the same time, in the future. That could lead to increased latency.

@jrchatruc jrchatruc added this pull request to the merge queue Oct 27, 2025
Merged via the queue into main with commit 73694e1 Oct 27, 2025
56 of 58 checks passed
@jrchatruc jrchatruc deleted the perf/block_exec_actor branch October 27, 2025 21:21
@github-project-automation github-project-automation bot moved this from Todo to Done in ethrex_performance Oct 27, 2025
@github-project-automation github-project-automation bot moved this from In Review to Done in ethrex_l1 Oct 27, 2025
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: Done
Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants