perf: Eliminate spawn_blocking in multiproof manager #19203

yongkangc · 2025-10-22T04:34:52Z

Closes: #19183

This PR refactors the multiproof pipeline to improve performance and simplify its design by eliminating a spawn_blocking hop for handling proof results.

Previously, the MultiproofManager would spawn a blocking task to await the result of each proof calculation, which introduced unnecessary overhead and complexity.

The new implementation streamlines this process:

Proof workers now send their results (ProofResultMessage) directly to the MultiProofTask event loop via a dedicated crossbeam channel.
The MultiProofTask uses crossbeam::select! to concurrently handle both incoming control messages (like state updates) and the proof results from workers.

This change removes the intermediate blocking task, resulting in a more direct, efficient, and easier-to-understand data flow.

The updated message flow is as follows:

MultiProofTask -> MultiproofManager -> ProofWorkerHandle -> Storage/Account Worker
       ^                                          |
       |                                          v
ProofResultMessage <-------- ProofResultSender ---

yongkangc · 2025-10-22T04:36:20Z

crates/engine/tree/src/tree/payload_processor/multiproof.rs

-/// Message about completion of proof calculation for a specific state update
-#[derive(Debug)]
-pub(super) struct ProofCalculated {
-    /// The index of this proof in the sequence of state updates
-    sequence_number: u64,
-    /// Sparse trie update
-    update: SparseTrieUpdate,
-    /// The time taken to calculate the proof.
-    elapsed: Duration,
-}
-


We can remove ProofCalculated entirely because the task loop no longer
forwards worker results throughProofCalculated.

Results now arrive as ProofResultMessage over proof_result_rx, and
the loop converts them straight into SparseTrieUpdates inline—no
intermediate wrapper needed.

Copilot

Pull Request Overview

This PR eliminates spawn_blocking calls in the multiproof manager by restructuring proof result delivery. Instead of spawning blocking tasks that wait for proof completion, workers now send results directly through a dedicated crossbeam channel to the MultiProofTask event loop.

Key changes:

Workers deliver proof results directly via ProofResultMessage through a crossbeam channel
MultiProofTask uses crossbeam_channel::select! to handle both state updates and proof results concurrently
Removed intermediate blocking tasks and message types (ProofCalculated, ProofCalculationError)

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
crates/trie/parallel/src/proof_task.rs	Adds `ProofResultMessage` type and `ProofResultContext` for direct result delivery; workers send results via channel instead of returning through receivers
crates/trie/parallel/src/proof.rs	Updated to use crossbeam channel for receiving proof results directly from workers
crates/engine/tree/src/tree/payload_processor/multiproof.rs	Removed spawn_blocking; workers send results to shared channel; event loop uses select! to handle state updates and proof results
crates/engine/tree/src/tree/payload_processor/prewarm.rs	Updated channel type from std mpsc to crossbeam for multiproof messages
crates/engine/tree/src/tree/payload_processor/mod.rs	Updated channel types to use crossbeam channels for multiproof messages
crates/engine/tree/Cargo.toml	Added crossbeam-channel dependency

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

crates/engine/tree/src/tree/payload_processor/multiproof.rs

Copilot

Pull Request Overview

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

Comments suppressed due to low confidence (1)

crates/engine/tree/src/tree/payload_processor/multiproof.rs:463

The elapsed variable calculation on line 422 was removed but is still used on line 485. This will cause a compilation error as elapsed is undefined at the point of use in the storage multiproof result message.

            let elapsed = start.elapsed();

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

crates/trie/parallel/src/proof_task.rs

crates/engine/tree/src/tree/payload_processor/multiproof.rs

mattsse · 2025-10-22T10:07:46Z

this is looking pretty good already

crates/trie/parallel/src/proof_task.rs

Copilot

Pull Request Overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

crates/trie/parallel/src/proof_task.rs

crates/engine/tree/src/tree/payload_processor/multiproof.rs

Copilot

Pull Request Overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

crates/engine/tree/src/tree/payload_processor/multiproof.rs

shekhirin

Will review properly in a bit, but looks good directionally

shekhirin · 2025-10-23T14:08:48Z

crates/trie/parallel/src/proof.rs

            collect_branch_node_masks: self.collect_branch_node_masks,
            multi_added_removed_keys: self.multi_added_removed_keys.clone(),
            missed_leaves_storage_roots: self.missed_leaves_storage_roots.clone(),
+            proof_result_sender: (


this is getting harder to read, let's introduce a separate struct?

good suggestion! addressed in 5aa0087

shekhirin · 2025-10-23T14:11:56Z

crates/engine/tree/src/tree/payload_processor/multiproof.rs

-                            prefetch_proofs_requested,
-                            updates_finished,
-                        ) {
+            crossbeam_channel::select! {


I think it's a very nice change: we now have a channel that we receive the state updates on, and a separate channel that we receive the proof results on. Much cleaner imo, we can maybe separate the handling of messages into separate methods so it's easier to navigate, but not a blocker

this makes sense, let me configure and see how to clean that up

we can maybe separate the handling of messages into separate methods so it's easier to navigate, but not a blocker

@shekhirin just did a prototype, i think its easier to review in a separate pr :) noted down in #19182

mattsse

I like this a lot especially because this removes a totally redundant spawn

imo there's room for cleanup (not in this pr tho) see also #19182

mattsse · 2025-10-23T17:17:41Z

crates/engine/tree/src/tree/payload_processor/multiproof.rs

    }

    /// Spawns a single storage proof calculation task.
    fn spawn_storage_proof(&mut self, storage_multiproof_input: StorageMultiproofInput) {


I'd like to rename this now to just send_ removing spawn_ from the fn names because misleading

i changed to the hierarchy and naming to dispatch (felt more fitting than send) :

dispatch() // Entry point - checks if empty, then sends
~~→ send_multiproof_task() // removed shallow function~~
→ dispatch_storage_proof() // dispatch to storage workers
→ dispatch_multiproof() // dispatch to account workers

what do you think @mattsse ?

crates/engine/tree/src/tree/payload_processor/multiproof.rs

mattsse · 2025-10-23T17:23:47Z

crates/engine/tree/src/tree/payload_processor/multiproof.rs

-                            prefetch_proofs_requested,
-                            updates_finished,
-                        ) {
+            crossbeam_channel::select! {


all of this looks good to me

mattsse · 2025-10-23T17:24:44Z

crates/engine/tree/src/tree/payload_processor/multiproof.rs

+                            }
+                        }
+                        Err(_) => {
+                            error!(target: "engine::tree::payload_processor::multiproof", "Proof result channel closed unexpectedly");


this should be unreachable because always keep the channel open via the

MultiproofManager.proof_result_tx

good pt, changed to unreachable! 01f1db8

mattsse · 2025-10-23T17:29:07Z

crates/engine/tree/src/tree/payload_processor/multiproof.rs

+/// 4. `MultiProofTask` consumes the message from the same channel and sequences it with
+///    `ProofSequencer`.
 #[derive(Debug)]
 pub struct MultiproofManager {


this type now simply acts as container to groups certain fields within the MultiProofTask

I personally find this introduces more complexity because now you also need to know what this entity does even if this now only does simple dispatch, similar situation with the ProofSequencer

all of this could be squashed into the
MultiProofTask

but we can decide this later

thanks for the note, noted this down as a cleanup in
#19182

receiving the ProofCalculated in multiproof manager
This is incorrect. we should get rid of them

ProofResultVariant references

…lt handling

…r closed proof result channel

Switch back to start.elapsed() to capture full request latency instead of just calculation time, fixing under-reported metrics.

shekhirin

Very nice, LGTM

shekhirin · 2025-10-24T09:11:18Z

crates/engine/tree/src/tree/payload_processor/multiproof.rs

            proof_sequence_number,
-            state_root_message_sender,
            multi_added_removed_keys,
+            ..


nit, to be the same as in dispatch_storage_proof

Suggested change

..

state_root_message_sender: _,

makes sense! resolved

shekhirin · 2025-10-24T09:14:30Z

crates/engine/tree/src/tree/payload_processor/multiproof.rs

+                                    break
+                                }
+                            }
+                            MultiProofMessage::EmptyProof { sequence_number, state } => {


something is going on with indentation here, rustfmt gave up I guess

@yongkangc I think we may need to fix this manually, idk why rustfmt doesn't do it for us

shekhirin · 2025-10-24T09:17:48Z

crates/engine/tree/src/tree/payload_processor/multiproof.rs

    proof_targets: MultiProofTargets,
    proof_sequence_number: u64,
-    state_root_message_sender: Sender<MultiProofMessage>,
+    state_root_message_sender: CrossbeamSender<MultiProofMessage>,


now it's only used for EmptyProof message, I wonder if we can remove it altogether and just send the empty proof to proof_result_tx? No need to address it in this PR, just a thought cc @mattsse

makes sense

mattsse

one last question otherwise lgtm

mattsse · 2025-10-24T10:27:21Z

crates/engine/tree/src/tree/payload_processor/multiproof.rs

            proof_sequence_number,
-            state_root_message_sender,
            multi_added_removed_keys,
+            state_root_message_sender: _,


yeah this feels a bit weird because we first clone stuff from self into the

StorageMultiproofInput only to drop it here

imo this entire type StorageMultiproofInput is redundant

mattsse · 2025-10-24T10:34:34Z

crates/engine/tree/src/tree/payload_processor/multiproof.rs

+                            // SAFETY: This is unreachable because `self.multiproof_manager` owns
+                            // `proof_result_tx` (the sender), which keeps the channel open for
+                            // the entire lifetime of this task. The channel cannot close while
+                            // `self` exists.
+                            unreachable!("proof result channel closed while multiproof manager still exists")


this checks out because select does return RecvError and not TryRecvError so this is only reachable if the channel is disconnected.

tho maybe we should just return the an error here

mattsse · 2025-10-24T10:39:58Z

crates/trie/parallel/src/proof_task.rs

+                        // Extract storage proof from the multiproof wrapper
+                        let (mut multiproof, _stats) = proof_msg.result?;
+                        let proof =
+                            multiproof.storages.remove(&hashed_address).ok_or_else(|| {
+                                ParallelStateRootError::Other(format!(
+                                    "storage proof not found in multiproof for {hashed_address}"
+                                ))
+                            })?;


why do we need to do this now?

this is added a few times and unclear what changed here?

mattsse · 2025-10-24T10:40:07Z

crates/trie/parallel/src/proof_task.rs

+        if let Ok(proof_msg) = receiver.recv() {
+            // Extract storage proof from the multiproof wrapper
+            if let Ok((mut multiproof, _stats)) = proof_msg.result &&
+                let Some(proof) = multiproof.storages.remove(&hashed_address)


shekhirin · 2025-10-24T11:11:22Z

crates/trie/parallel/src/proof_task.rs

+                    Err(e) => Err(e),
+                };
+
+                // Send result directly to MultiProofTask


let's remove this comment, because this codepath is also used from ParallelProof::storage_proof

@shekhirin removed comment

github-project-automation bot added this to Reth Tracker Oct 22, 2025

github-project-automation bot moved this to Backlog in Reth Tracker Oct 22, 2025

yongkangc self-assigned this Oct 22, 2025

yongkangc added C-perf A change motivated by improving speed, memory usage or disk footprint S-needs-benchmark This set of changes needs performance benchmarking to double-check that they help labels Oct 22, 2025

yongkangc commented Oct 22, 2025

View reviewed changes

yongkangc requested a review from Copilot October 22, 2025 08:59

Copilot AI reviewed Oct 22, 2025

View reviewed changes

crates/engine/tree/src/tree/payload_processor/multiproof.rs Outdated Show resolved Hide resolved

yongkangc requested a review from Copilot October 22, 2025 09:16

Copilot AI reviewed Oct 22, 2025

View reviewed changes

yongkangc moved this from Backlog to In Progress in Reth Tracker Oct 23, 2025

yongkangc commented Oct 23, 2025

View reviewed changes

crates/trie/parallel/src/proof_task.rs Show resolved Hide resolved

yongkangc added the A-engine Related to the engine implementation label Oct 23, 2025

yongkangc force-pushed the yk/mpm_wiring branch from 0edadef to c9494dd Compare October 23, 2025 08:06

yongkangc requested a review from Copilot October 23, 2025 10:56

Copilot AI reviewed Oct 23, 2025

View reviewed changes

crates/trie/parallel/src/proof_task.rs Outdated Show resolved Hide resolved

crates/engine/tree/src/tree/payload_processor/multiproof.rs Show resolved Hide resolved

yongkangc marked this pull request as ready for review October 23, 2025 11:25

yongkangc requested review from Rjected, fgimenez, mattsse, mediocregopher and shekhirin as code owners October 23, 2025 11:25

yongkangc requested a review from Copilot October 23, 2025 11:25

Copilot AI reviewed Oct 23, 2025

View reviewed changes

crates/engine/tree/src/tree/payload_processor/multiproof.rs Outdated Show resolved Hide resolved

shekhirin reviewed Oct 23, 2025

View reviewed changes

mattsse requested changes Oct 23, 2025

View reviewed changes

yongkangc moved this from In Progress to In Review in Reth Tracker Oct 24, 2025

yongkangc mentioned this pull request Oct 24, 2025

refactor: reorder types in multiproof.rs #19182

Open

yongkangc requested a review from shekhirin October 24, 2025 07:08

yongkangc added 14 commits October 24, 2025 08:04

fmt

8278f42

fix comment

7922b1e

chnage comment

fc27841

update targets

9c98edf

fix: rename spawn_* methods to dispatch_* and fix

8106583

ProofResultVariant references

refactor: replace tuple with ProofResultContext struct for proof resu…

895179c

…lt handling

reorder

c0a2495

rename

43e5148

changed

98f90c8

fix: update error handling in MultiProofTask to use unreachable!() fo…

8d94683

…r closed proof result channel

fix docs

ffec87e

fix: improve documentation for MultiProofTask message flow

1e5f4fe

metrics: Record total proof latency

4b86b2c

Switch back to start.elapsed() to capture full request latency instead of just calculation time, fixing under-reported metrics.

fmt

105008f

yongkangc force-pushed the yk/mpm_wiring branch from 077e5b5 to 105008f Compare October 24, 2025 08:04

shekhirin approved these changes Oct 24, 2025

View reviewed changes

fix fmt

aeff510

mattsse reviewed Oct 24, 2025

View reviewed changes

mattsse approved these changes Oct 24, 2025

View reviewed changes

github-project-automation bot moved this from In Review to In Progress in Reth Tracker Oct 24, 2025

shekhirin reviewed Oct 24, 2025

View reviewed changes

yongkangc added 3 commits October 24, 2025 11:17

removed comment

5157392

added error

c395873

Improve logging in MultiProofTask message processing

6550884

yongkangc enabled auto-merge October 24, 2025 11:53

yongkangc added this pull request to the merge queue Oct 24, 2025

Merged via the queue into main with commit f29f4ca Oct 24, 2025
39 of 40 checks passed

yongkangc deleted the yk/mpm_wiring branch October 24, 2025 12:13

github-project-automation bot moved this from In Progress to Done in Reth Tracker Oct 24, 2025

perf: Eliminate spawn_blocking in multiproof manager #19203

perf: Eliminate spawn_blocking in multiproof manager #19203

Conversation

yongkangc commented Oct 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mattsse commented Oct 22, 2025

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

shekhirin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yongkangc Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mattsse left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yongkangc Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yongkangc Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shekhirin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yongkangc commented Oct 22, 2025 •

edited

Loading

yongkangc Oct 24, 2025 •

edited

Loading

yongkangc Oct 24, 2025 •

edited

Loading

yongkangc Oct 24, 2025 •

edited

Loading