perf(rpc): cache block traces#7108
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughStateManager caches per-tipset execution traces as Vec<Arc>; callers and trace entries updated to use Arc-wrapped results; RPC trace types derive GetSize with a centralized raw_bytes_heap_size_helper; daemon warmup now invokes execution_trace for validated tipsets. ChangesExecution trace caching and Arc-wrapped results
Heap-size tracking for trace types
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
hanabi1224
requested review from
LesnyRumcajs and
sudo-shashank
and removed request for
a team
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@src/daemon/mod.rs`:
- Around line 403-407: The synchronous call to
StateManager::execution_trace(&ts) inside async fn prefill_rpc_caches_for_tipset
can block Tokio worker threads; wrap that call in tokio::task::spawn_blocking
and await its JoinHandle result, propagating any error into the existing warn
log (i.e., replace the direct call in the block with a spawn_blocking closure
that calls state_manager.execution_trace(ts.clone() or move ts) and then handle
Err(e) from the join or the inner call the same way you currently log it);
ensure you move or clone ts and state_manager into the spawn_blocking closure as
needed and preserve the existing warn message when logging failures.
In `@src/rpc/methods/state/types.rs`:
- Around line 69-70: The field execution_trace on ApiInvocResult is currently
annotated with #[get_size(ignore)], causing GetSize to under-report memory;
remove that attribute so execution_trace is included in size accounting and
ensure ExecutionTrace implements GetSize (or provide a custom GetSize impl for
ApiInvocResult that accounts for execution_trace) so cached trace memory is
measured correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a0575a7b-5bdc-4f09-ab90-e45b680aee39
📒 Files selected for processing (9)
src/daemon/mod.rssrc/rpc/methods/eth.rssrc/rpc/methods/eth/trace/parity.rssrc/rpc/methods/state/types.rssrc/shim/message.rssrc/shim/state_tree.rssrc/state_manager/execution.rssrc/state_manager/mod.rssrc/utils/get_size/mod.rs
hanabi1224
force-pushed
the
hm/cache-trace
branch
from
f89858f to
4973f64
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/utils/get_size/mod.rs (1)
62-67: ⚡ Quick winAdd rustdoc for the public heap-size helper.
Please add a doc comment on
raw_bytes_heap_size_helperto document its approximation semantics and intended usage.Suggested patch
+/// Estimates heap usage for [`fvm_ipld_encoding::RawBytes`]. +/// +/// This is a cheap approximation based on the underlying byte slice length, +/// and may undercount compared with true allocation capacity. pub fn raw_bytes_heap_size_helper(b: &fvm_ipld_encoding::RawBytes) -> usize { // Note: this is a cheap but inaccurate estimation, // the correct implementation should be `Vec<u8>.from(b.clone()).get_heap_size()`, // or `b.bytes.get_heap_size()` if `bytes` is made public. b.bytes().get_heap_size() }As per coding guidelines, "Document public functions and structs with doc comments".
🤖 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/utils/get_size/mod.rs` around lines 62 - 67, Add a Rust doc comment to the public function raw_bytes_heap_size_helper explaining that it returns a cheap but approximate heap-size estimate by calling b.bytes().get_heap_size(), describing the approximation semantics and limitations (i.e., it may be inaccurate compared to converting to Vec<u8> or using bytes.get_heap_size() if the inner bytes field were public), and stating the intended usage (quick estimate only, not for exact accounting) along with any alternative recommended approaches for precise measurement.
🤖 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.
Nitpick comments:
In `@src/utils/get_size/mod.rs`:
- Around line 62-67: Add a Rust doc comment to the public function
raw_bytes_heap_size_helper explaining that it returns a cheap but approximate
heap-size estimate by calling b.bytes().get_heap_size(), describing the
approximation semantics and limitations (i.e., it may be inaccurate compared to
converting to Vec<u8> or using bytes.get_heap_size() if the inner bytes field
were public), and stating the intended usage (quick estimate only, not for exact
accounting) along with any alternative recommended approaches for precise
measurement.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 0a1fa2be-fd42-45af-842c-94923d87f2bf
📒 Files selected for processing (9)
src/daemon/mod.rssrc/rpc/methods/eth.rssrc/rpc/methods/eth/trace/parity.rssrc/rpc/methods/state/types.rssrc/shim/message.rssrc/shim/state_tree.rssrc/state_manager/execution.rssrc/state_manager/mod.rssrc/utils/get_size/mod.rs
Codecov Report❌ Patch coverage is Additional details and impacted files
... and 8 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
LesnyRumcajs
approved these changes
May 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary of changes
Changes introduced in this pull request:
StateManager::execution_traceBenchmark
Reference issue to close (if applicable)
Closes
Other information and links
Change checklist
Outside contributions
Summary by CodeRabbit
Performance Improvements
Bug Fixes