fix: spawn_blocking for fvm calls#7184
Conversation
WalkthroughRenames all synchronous state-computation functions to ChangesBlocking/async execution path refactor
Sequence Diagram(s)sequenceDiagram
participant Caller as Async Caller
participant AsyncFn as get_lookback_tipset_for_round
participant BlockingFn as get_lookback_tipset_for_round_blocking
participant ApplyMsgs as apply_block_messages_blocking
Caller->>AsyncFn: chain_index, chain_config, tipset, round (owned)
AsyncFn->>BlockingFn: tokio::spawn_blocking (references to owned inputs)
BlockingFn->>ApplyMsgs: compute state_root for null-blocks branch
ApplyMsgs-->>BlockingFn: ExecutedTipset { state_root }
BlockingFn-->>AsyncFn: (Tipset, Cid)
AsyncFn-->>Caller: Result<(Tipset, Cid), Error>
sequenceDiagram
participant RPCHandler as RPC Handler
participant CWG as call_with_gas (async)
participant SpawnBlocking as tokio::spawn_blocking
participant FVM as stacker::grow / FVM
RPCHandler->>CWG: message (owned), prior_messages Arc, tipset, vm_flush
CWG->>SpawnBlocking: move message + prior_messages + sm.shallow_clone()
SpawnBlocking->>FVM: apply prior_messages, adjust nonce, apply message
FVM-->>SpawnBlocking: ApplyRet + optional state root
SpawnBlocking-->>CWG: InvocResult + duration + state root
CWG-->>RPCHandler: Result<(InvocResult, ApplyRet, Duration, Option<Cid>)>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/state_manager/state_computation.rs (1)
191-191:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStale doc comment reference.
The doc comment references
apply_block_messages, but this function was renamed toapply_block_messages_blocking.📝 Suggested fix
- /// For details, see the documentation for [`apply_block_messages`]. + /// For details, see the documentation for [`apply_block_messages_blocking`].🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/state_manager/state_computation.rs` at line 191, The doc comment at the line with "For details, see the documentation for [`apply_block_messages`]" references a function that no longer exists with that name. Update the function name reference in the doc comment from `apply_block_messages` to `apply_block_messages_blocking` to match the actual current function name.
🧹 Nitpick comments (3)
src/tool/subcommands/api_cmd/api_compare_tests.rs (1)
2638-2642: ⚡ Quick winOffload chain revalidation work from the async executor.
This async function directly runs
validate_tipsets_blocking, which can monopolize Tokio workers for large ranges. Prefer atokio::task::spawn_blockingwrapper for this compute-heavy section.As per coding guidelines, use
tokio::task::spawn_blockingfor CPU-intensive work such as VM execution and cryptography.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tool/subcommands/api_cmd/api_compare_tests.rs` around lines 2638 - 2642, The validate_tipsets_blocking call is a CPU-intensive blocking operation that is currently running directly in an async context, which can monopolize Tokio worker threads. Wrap the state_manager.validate_tipsets_blocking call with tokio::task::spawn_blocking to offload this blocking work to a dedicated thread pool, allowing the async executor to continue processing other tasks. This ensures the blocking validation work does not starve the Tokio runtime.Source: Coding guidelines
src/tool/subcommands/archive_cmd.rs (1)
856-865: ⚡ Quick winMove tipset execution to
spawn_blockinginshow_tipset_diff.
apply_block_messages_blockingperforms synchronous VM computation; calling it inline in this async flow can block Tokio workers during large snapshot diffs. Running it throughtokio::task::spawn_blockingwould avoid executor contention.As per coding guidelines, use
tokio::task::spawn_blockingfor CPU-intensive work such as VM execution and cryptography.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tool/subcommands/archive_cmd.rs` around lines 856 - 865, The call to apply_block_messages_blocking is a CPU-intensive synchronous operation being executed directly in an async context, which can block Tokio runtime workers. Wrap this call in tokio::task::spawn_blocking by creating a closure that contains the entire apply_block_messages_blocking invocation with all its parameters, then await the result to ensure the VM execution runs on a dedicated blocking thread pool instead of blocking the async executor.Source: Coding guidelines
src/tool/offline_server/server.rs (1)
194-198: ⚡ Quick winRun tipset validation in
spawn_blockingon this async path.
validate_tipsets_blockingis VM-heavy synchronous work, so invoking it directly here can pin a Tokio worker during startup. Wrapping this section intokio::task::spawn_blockingwould keep async scheduling responsive.As per coding guidelines, use
tokio::task::spawn_blockingfor CPU-intensive work such as VM execution and cryptography.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tool/offline_server/server.rs` around lines 194 - 198, The call to state_manager.validate_tipsets_blocking with the chain and take_while iterator is CPU-intensive synchronous work that can block the Tokio worker thread during this async path. Wrap the entire state_manager.validate_tipsets_blocking call and its argument chain (head_ts.chain(rpc_state.db()).take_while(...)) inside tokio::task::spawn_blocking to offload this VM-heavy work to a dedicated thread pool, preventing the async runtime from being pinned and keeping async scheduling responsive.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/state_manager/state_computation.rs`:
- Line 191: The doc comment at the line with "For details, see the documentation
for [`apply_block_messages`]" references a function that no longer exists with
that name. Update the function name reference in the doc comment from
`apply_block_messages` to `apply_block_messages_blocking` to match the actual
current function name.
---
Nitpick comments:
In `@src/tool/offline_server/server.rs`:
- Around line 194-198: The call to state_manager.validate_tipsets_blocking with
the chain and take_while iterator is CPU-intensive synchronous work that can
block the Tokio worker thread during this async path. Wrap the entire
state_manager.validate_tipsets_blocking call and its argument chain
(head_ts.chain(rpc_state.db()).take_while(...)) inside
tokio::task::spawn_blocking to offload this VM-heavy work to a dedicated thread
pool, preventing the async runtime from being pinned and keeping async
scheduling responsive.
In `@src/tool/subcommands/api_cmd/api_compare_tests.rs`:
- Around line 2638-2642: The validate_tipsets_blocking call is a CPU-intensive
blocking operation that is currently running directly in an async context, which
can monopolize Tokio worker threads. Wrap the
state_manager.validate_tipsets_blocking call with tokio::task::spawn_blocking to
offload this blocking work to a dedicated thread pool, allowing the async
executor to continue processing other tasks. This ensures the blocking
validation work does not starve the Tokio runtime.
In `@src/tool/subcommands/archive_cmd.rs`:
- Around line 856-865: The call to apply_block_messages_blocking is a
CPU-intensive synchronous operation being executed directly in an async context,
which can block Tokio runtime workers. Wrap this call in
tokio::task::spawn_blocking by creating a closure that contains the entire
apply_block_messages_blocking invocation with all its parameters, then await the
result to ensure the VM execution runs on a dedicated blocking thread pool
instead of blocking the async executor.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 29279800-5370-47df-830d-35b7052387ed
📒 Files selected for processing (18)
src/chain/store/chain_store.rssrc/chain/store/errors.rssrc/chain_sync/tipset_syncer.rssrc/daemon/mod.rssrc/fil_cns/validation.rssrc/interpreter/externs.rssrc/rpc/methods/eth.rssrc/rpc/methods/gas.rssrc/rpc/methods/miner.rssrc/state_manager/execution.rssrc/state_manager/message_simulation.rssrc/state_manager/mining.rssrc/state_manager/mod.rssrc/state_manager/state_computation.rssrc/tool/offline_server/server.rssrc/tool/subcommands/api_cmd/api_compare_tests.rssrc/tool/subcommands/archive_cmd.rssrc/tool/subcommands/snapshot_cmd.rs
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
filecoin-project/lotus(manual)
Summary of changes
Fvm calls are expensive and could exhaust tokio worker threads. Call them with
spawn_blockinginstead. Also update fn namings to end with_blockingwhen they're sync ones and invoking fvm.Changes introduced in this pull request:
Reference issue to close (if applicable)
Closes
Other information and links
Change checklist
Outside contributions
Summary by CodeRabbit
Performance
Bug Fixes