refactor(trie): remove proof task manager by yongkangc · Pull Request #18934 · paradigmxyz/reth

yongkangc · 2025-10-10T10:21:50Z

Context:

As part of our performance work to reduce overhead and improve scheduling, we added worker pooling for multiproof generation.
This PR aims to perform cleanup and remove ProofTaskManager as an abstraction as we can now directly dispatch the proofs jobs to workers.

impact:

Change	Impact
Remove `run()` loop thread	-1 thread, -1 channel hop
Direct channel sends	~some time saved per task
Eliminate enum wrapping	~2 allocations saved per task

reference PRs:

- 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 <me@mediocregopher.com>

…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 <175728472+Copilot@users.noreply.github.com>

- 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.

mattsse

lgtm

left some suggestions, iirc additional changes to the worker pool are planned anyway and we could tackle those separately

mattsse · 2025-10-14T09:37:12Z

crates/trie/parallel/src/proof.rs

-        let _ = self.proof_task_handle.queue_task(ProofTaskKind::StorageProof(input, sender));
-        receiver
+        self.proof_task_handle
+            .queue_storage_proof(input)


can we rename these after we merge, because I find queue very confusing here because this only sends

thats true -> should rename it as send_task

mattsse · 2025-10-14T09:44:28Z