perf(tree): worker pooling for account proofs #18901

yongkangc · 2025-10-08T08:09:22Z

This PR builds on top of #18887 by adding worker pooling to account and blinded account proofs, which allows us to reduce overhead from spawning the worker.

Changes:

Move Account to pooling logic
Move Blinded Account Nodes to pooling logic. Removed underlying structure and functions (pending tasks)

Core logic:

Determine workers based on parallelism
Spawn x workers each with its own tx
Queue storage proofs via crossbeam channels to the workers
Await Storage proofs to build account proofs

Caller
    ↓ (requests account multiproof)
ProofTaskManager
    ↓ (routes to account worker pool)
Account Worker
    ↓ (calls collect_storage_proofs)
    ↓ (queues all storage proofs to storage worker pool)
Storage Workers (parallel computation)
    ↓ (return storage proofs)
Account Worker
    ↓ (calls build_account_multiproof_with_storage_roots)
    ↓ (walks account trie, looks up pre-computed storage roots)
Return to Caller (final account multiproof)

Issues closed:

References:

Overall POC: perf(trie): proofmanager optimisation WIP #18829

Next Steps:

Removing ProofTaskManager + other cleanups

- Added configuration for maximum and minimum storage proof workers. - Implemented a worker pool for processing storage proof tasks, improving efficiency by reusing transactions. - Updated `ProofTaskManager` to handle storage proof tasks via a dedicated channel. - Enhanced metrics to track storage proof requests and fallback scenarios. - Adjusted existing tests to accommodate the new storage worker functionality.

- Enhanced documentation for `StorageProofJob` to clarify its current unused status and potential for future type-safe design. - Updated comments in `ProofTaskManager` regarding the handling of on-demand tasks and the possibility of refactoring to a more type-safe enum. - Improved logging for worker pool disconnection scenarios, emphasizing fallback to on-demand execution.

…Metrics and ProofTaskTrieMetrics

…clarity - Updated comments in `ProofTaskManager` to enhance clarity regarding on-demand transaction handling and queue management. - Renamed `pending_on_demand` to `on_demand_queue` for better understanding of its purpose. - Adjusted the `new` function documentation to reflect the correct allocation of concurrency budget between storage workers and on-demand transactions. - Improved the `queue_proof_task` method to use the new queue name.

…ement - Removed the unused `OnDemandTask` enum and updated comments in `ProofTaskManager` to clarify the distinction between storage worker pool and on-demand execution. - Enhanced documentation to better describe the public interface and task submission process. - Improved clarity regarding transaction handling and execution paths for proof requests.

- Eliminated the `storage_proof_workers` field and related constants from `TreeConfig`. - Updated the default implementation and related methods to reflect the removal, streamlining the configuration structure.

- Improved comments in `ProofTaskManager` and related functions for better clarity on task management and processing. - Updated queue capacity calculation to use 4x buffering, reducing fallback to slower on-demand execution during burst loads. - Removed redundant variable assignments to streamline the code.

Co-authored-by: Brian Picciano <[email protected]>

…ursor factories - Introduced pre-created cursor factories in `storage_worker_loop` to reduce overhead during proof computation. - Updated `compute_storage_proof` to accept cursor factories as parameters, enhancing efficiency and clarity. - Improved logging to provide better insights into proof task calculations.

- not change the logic for pending_tasks and proof_tasks_txs (on-demand proofs) and just continue using it for the BlindedAccountNode requests, but start using dedicated storage workers for StorageProof and BlindedStorageNode requests

…roof

Co-authored-by: Copilot <[email protected]>

- Added a function to determine the default number of storage worker threads based on available parallelism. - Updated TreeConfig to include a storage_worker_count field, initialized with the default value. - Modified payload processor to utilize the new storage_worker_count instead of a hardcoded value.

yongkangc · 2025-10-10T09:11:15Z

Latest bench on reth4 mainnet

Mean percent difference ~4%

Copilot

Pull Request Overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 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 8 out of 8 changed files in this pull request and generated 6 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/trie/parallel/src/proof.rs

Co-authored-by: Copilot <[email protected]>

yongkangc · 2025-10-13T01:59:56Z

@Rjected @shekhirin @mediocregopher ready for a final review 🙏🏻

yongkangc · 2025-10-13T05:13:12Z

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

+    storage_proof_task_handle: ProofTaskManagerHandle,
+    /// Handle to the proof task manager used for account multiproofs.
+    account_proof_task_handle: ProofTaskManagerHandle,


this isaddressed later on in the pr for cleanup where we use proof_task_handle instead

…18933)

mattsse

haven't looked too close into the actual proof logic

other than that lgtm

pending @Rjected

shekhirin · 2025-10-14T12:44:11Z

crates/trie/parallel/src/proof.rs

+        let storage_root_targets_len = StorageRootTargets::count(
+            &prefix_sets.account_prefix_set,
+            &prefix_sets.storage_prefix_sets,
        );


as a small optimization for future PRs, this calculation can be skipped if trace logs below won't be emitted

Thanks for the note

shekhirin

LGTM, my only suggestion is the same as Dan's — about free functions instead of structs with methods that keep the state.

Rjected

lgtm with the one suggestions about putting the worker state into structs

Rjected · 2025-10-14T16:54:19Z

crates/trie/parallel/src/proof_task.rs

 }

-impl<Factory> ProofTaskManager<Factory>
+// TODO: Refactor this with storage_worker_loop. ProofTaskManager should be removed in the following


do you mean that we should keep the ProofTaskManager?

no, I just mean that a struct, and a method on the struct called run or something similar, makes more sense to me than having _loop functions, since they are tasks with state

ie something like

let worker = AccountProofWorker::new(...); executor.spawn_blocking(move || { worker.run(); });

makes more sense to me than how we currently spawn

yongkangc · 2025-10-15T00:18:02Z

@shekhirin @Rjected will address Ur comments about structs in another pr

yongkangc and others added 30 commits October 7, 2025 06:35

fmt, clippy

fbeec50

fix comments

13891ad

refactor(metrics): remove unused storage proof metrics from ProofTask…

d4e0adb

…Metrics and ProofTaskTrieMetrics

refactor(config): remove storage proof worker configuration

29d48d4

- Eliminated the `storage_proof_workers` field and related constants from `TreeConfig`. - Updated the default implementation and related methods to reflect the removal, streamlining the configuration structure.

disable max concurrency

5779b86

nits

0e33837

Update crates/trie/parallel/src/proof_task.rs

3bcbc71

Co-authored-by: Brian Picciano <[email protected]>

Update crates/trie/parallel/src/proof_task.rs

4a67076

Co-authored-by: Brian Picciano <[email protected]>

using unbounded queue

b2d5bcc

rm comment

8f4e3a1

propogate error up

6282d2e

reduce scope of pr - exclude all accs

838dc67

- not change the logic for pending_tasks and proof_tasks_txs (on-demand proofs) and just continue using it for the BlindedAccountNode requests, but start using dedicated storage workers for StorageProof and BlindedStorageNode requests

fmt, clippy

5897945

fmt

6b5de7c

refactor(proof_task): consolidate blinded storage node with storage p…

05e0eb8

…roof

rm comment

4829de9

simplify worker concurrency

6472cfe

bump to error!

61ecc9a

Update crates/engine/tree/src/tree/payload_processor/mod.rs

30f6fda

Co-authored-by: Copilot <[email protected]>

handle sending error back

4680336

fmt

58d6f8b

fix fmt

59b0353

update message

1954502

Merge branch 'main' into yk/worker_pool_acc

870938f

yongkangc added 2 commits October 10, 2025 09:11

fmt

14def07

made count same as storage worker

44bca43

yongkangc mentioned this pull request Oct 10, 2025

refactor(trie): remove proof task manager #18934

Merged

yongkangc requested a review from Copilot October 10, 2025 10:34

Copilot AI reviewed Oct 10, 2025

View reviewed changes

shekhirin reviewed Oct 10, 2025

View reviewed changes

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

yongkangc requested a review from Copilot October 10, 2025 10:55

Copilot AI reviewed Oct 10, 2025

View reviewed changes

yongkangc and others added 2 commits October 10, 2025 18:58

Update crates/trie/parallel/src/proof_task.rs

d1eb0ec

Co-authored-by: Copilot <[email protected]>

merge

4e00944

yongkangc requested a review from shekhirin October 10, 2025 12:35

yongkangc commented Oct 13, 2025

View reviewed changes

yongkangc added 2 commits October 13, 2025 13:13

refactor(tree): remove unused Factory generic from multiproof system (#…

3e957a5

…18933)

fix clippy

18ff58d

mediocregopher approved these changes Oct 13, 2025

View reviewed changes

fmt

f692d75

mattsse approved these changes Oct 14, 2025

View reviewed changes

shekhirin reviewed Oct 14, 2025

View reviewed changes

shekhirin approved these changes Oct 14, 2025

View reviewed changes

Rjected approved these changes Oct 14, 2025

View reviewed changes

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

Merged via the queue into main with commit e0b7a86 Oct 15, 2025
40 of 41 checks passed

yongkangc deleted the yk/worker_pool_acc branch October 15, 2025 00:41

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

yongkangc mentioned this pull request Oct 15, 2025

refactor: add struct to loop and ProofTaskMangerHandle #19007

Open

perf(tree): worker pooling for account proofs #18901

perf(tree): worker pooling for account proofs #18901

Uh oh!

Conversation

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

Uh oh!

Uh oh!

yongkangc commented Oct 10, 2025

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!

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!

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

yongkangc Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

mattsse left a comment

Choose a reason for hiding this comment

Uh oh!

shekhirin Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

yongkangc Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

shekhirin left a comment

Choose a reason for hiding this comment

Uh oh!

Rjected left a comment

Choose a reason for hiding this comment

Uh oh!

Rjected Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

yongkangc commented Oct 15, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

yongkangc commented Oct 8, 2025 •

edited

Loading

yongkangc commented Oct 13, 2025 •

edited

Loading