Skip to content

perf(rpc): add progressive block trace caching - #4021

Open
danielntmd wants to merge 3 commits into
mainfrom
danielntmd/progressive-trace-cache
Open

perf(rpc): add progressive block trace caching#4021
danielntmd wants to merge 3 commits into
mainfrom
danielntmd/progressive-trace-cache

Conversation

@danielntmd

@danielntmd danielntmd commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

User description

Summary

TraceTransaction currently executes every transaction in a finalized block on a cache miss, even when the requested transaction appears near the beginning.

This PR caches successful tracing progress from the beginning of each block (a prefix). Later requests reconstruct the state at the cached point and execute only the remaining transactions through the requested target (the missing suffix). Only one request extends a block at a time, while different blocks can be traced concurrently.

Successful execution publishes a new cache record while existing cached traces remain available to readers.

Workflow

lookup(block, target)
├─ target cached → return
├─ extension active → wait or cancel, then retry lookup
└─ target missing → reconstruct state and execute missing suffix
                     ├─ success → append, publish, wake waiters
                     └─ failure → preserve record, wake waiters

Cache Invariants

  • A cache record is a contiguous, append-only prefix of successful transaction traces.
  • At most one flight extends a block; concurrent callers wait and recheck the record.
  • A successful extension is published atomically, while failure or panic leaves the record unchanged.
  • Empty blocks are not cached.

Tracing Behavior

  • TraceTransaction executes only through the requested transaction instead of tracing the complete block.
  • TraceBlockTransactions completes any existing prefix and returns the full block.
  • Requesting initial reads replays the full block if they are missing from the cache.

Benchmarks

Cold starknet_traceTransaction at 100 VUs

Broad corpus sample size: 1,000
Everything else: 200

Block corpus Target Throughput Δ Baseline avg PR avg Latency Δ CPU/request Δ
Broad Beginning +637.40% 7,460.6 ms 972.0 ms -86.97% -86.83%
Broad Middle +85.73% 7,469.8 ms 4,007.6 ms -46.35% -46.40%
Broad End +0.50% 7,478.6 ms 7,438.8 ms -0.53% -0.55%
1 transaction Beginning +0.18% 700.8 ms 704.0 ms +0.46% +1.73%
1 transaction Middle +4.08% 694.9 ms 685.8 ms -1.31% -1.03%
1 transaction End -0.60% 698.7 ms 666.3 ms -4.63% -2.62%
10 transactions Beginning +620.60% 7,132.4 ms 801.6 ms -88.76% -86.85%
10 transactions Middle +81.34% 6,887.2 ms 3,780.5 ms -45.11% -45.31%
10 transactions End -0.19% 7,121.4 ms 7,122.2 ms +0.01% +0.24%
>20 transactions Beginning +1,631.16% 14,871.4 ms 726.8 ms -95.11% -94.82%
>20 transactions Middle +82.75% 14,804.1 ms 7,953.5 ms -46.27% -45.45%
>20 transactions End +0.21% 14,864.8 ms 14,926.2 ms +0.41% +0.78%

Interleaved traceTransaction -> traceBlockTransactions

Sample size: 200

Block corpus VUs Throughput Δ Baseline avg PR avg Latency Δ CPU/pair Δ
Broad 1 -4.69% 448.6 ms 470.7 ms +4.92% +5.83%
Broad 50 -4.56% 3,249.3 ms 3,437.4 ms +5.79% +3.87%
1 transaction 1 +1.33% 47.4 ms 46.7 ms -1.31% +0.39%
1 transaction 50 -4.73% 389.3 ms 401.3 ms +3.09% +1.00%
10 transactions 1 -0.91% 593.5 ms 598.9 ms +0.92% +1.39%
10 transactions 50 -2.90% 4,074.9 ms 4,334.4 ms +6.37% +3.30%
>20 transactions 1 -0.43% 1,093.1 ms 1,097.8 ms +0.43% +0.58%
>20 transactions 50 +1.21% 8,533.2 ms 8,756.5 ms +2.62% -0.11%

PR Type

Enhancement, Tests


Description

  • Add progressive caching for block transaction traces

  • Optimize trace requests with prefix-based reconstruction

  • Coordinate in-flight block trace extensions concurrently

  • Replay blocks to fetch missing initial reads


File Walkthrough

Relevant files
Refactoring
2 files
adapt_trace.go
Remove nil check from adaptVMInitialReads                               
+1/-9     
handlers.go
Replace generic trace cache with custom blockTraceCache   
+4/-9     
Enhancement
4 files
simulation.go
Add emptyInitialReads helper function                                       
+9/-0     
trace.go
Integrate progressive caching and trace targets in block tracing
+91/-127
trace_cache.go
Implement concurrent block trace cache with flight coordination
+164/-0 
trace_progressive.go
Add logic for progressive trace execution and state checkpoints
+208/-0 
Tests
3 files
trace_cache_test.go
Add unit tests for the block trace cache                                 
+103/-0 
trace_progressive_test.go
Add unit tests for progressive trace execution logic         
+441/-0 
trace_test.go
Update and expand trace handler tests for cache coherence
+157/-84

@codecov

codecov Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.39301% with 22 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.24%. Comparing base (e85a15e) to head (0c2c855).
⚠️ Report is 17 commits behind head on main.

Files with missing lines Patch % Lines
rpc/v10/trace_progressive.go 83.78% 18 Missing ⚠️
rpc/v10/trace.go 92.85% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4021      +/-   ##
==========================================
+ Coverage   79.19%   79.24%   +0.05%     
==========================================
  Files         464      467       +3     
  Lines       35741    35947     +206     
==========================================
+ Hits        28305    28487     +182     
- Misses       7427     7451      +24     
  Partials        9        9              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread rpc/v10/handlers.go Outdated
@danielntmd
danielntmd force-pushed the danielntmd/progressive-trace-cache branch from 58d79fc to 6ab7d60 Compare September 3, 2026 11:36
@danielntmd
danielntmd marked this pull request as ready for review September 3, 2026 11:41
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Claude finished @danielntmd's task in 6m 39s —— View job


Review complete

  • Gather context (diff, changed files, CLAUDE.md)
  • Review rpc/v10/trace_cache.go (new cache structure, concurrency)
  • Review rpc/v10/trace_progressive.go (progressive execution logic)
  • Review rpc/v10/trace.go (handler changes)
  • Review rpc/v10/trace_cache_initial_reads.go
  • Review rpc/v10/handlers.go / adapt_trace.go
  • Review tests for coverage/flakiness
  • Post inline comments for issues found
  • Post final summary

I focused on concurrency correctness in the new blockTraceCache/traceFlight design and the state-checkpoint reconstruction in trace_progressive.go, since that's where a bug would be most consequential (returning a wrong trace or corrupting a cached record).

Findings: no blockers or bugs. The single-flight-per-block design, panic-safe waiter release (defer work.abort()), the append-only record with capacity-limited slice views (preventing torn reads via shared backing arrays), and the "record survives LRU eviction until re-published" behavior are all correctly implemented, and each of these edge cases has a dedicated test (TestProgressiveTraceCacheWaiterCancellationDoesNotCancelExtension, TestProgressiveTraceCachePanicClearsInflight, TestProgressiveTraceCacheAllowsRecordEvictionDuringFlight, TestProgressiveTraceCacheFailureRetainsPublishedPrefix). Lock ordering between the cache-level mutex and per-record RWMutex is consistent (no nested-lock cycle), and pending.State/checkpoint reconstruction (checkpointFromTraces + checkpointClasses) correctly rebuilds intermediate state from cached per-transaction state diffs, verified in TestProgressiveTraceCacheExtendsPrefix via a gomock.Cond on the reconstructed state.

Two non-blocking nits posted inline:

  • rpc/v10/trace.go:417-428 — the new single-flight invariant only covers the local progressive-tracing path; concurrent requests for the same uncached feeder-gateway-sourced block still race and can trigger redundant gateway fetches (pre-existing gap, not introduced by this PR).
  • rpc/v10/trace_progressive.go:100-108checkpointFromTraces/checkpointClasses fully re-derive the merged state diff from scratch on every extension rather than incrementally, which is O(prefix length) per call (fine for typical usage, worth a comment or follow-up if adversarial one-at-a-time extension patterns show up).

One thing worth double-checking with the author rather than a code issue: the VM is now always invoked with ReturnInitialReads: true, even for unflagged single-transaction traces — intentional per the PR description, and the benchmarks show no meaningful regression for the single-transaction cases, so this looks like an accepted tradeoff rather than an oversight.

Note: I wasn't able to run go build/go test -race in this environment (no tool access to execute Go commands), so this review is based on static reading of the diff and existing tests rather than a live race-detector run.

Comment thread rpc/v10/trace_progressive.go Outdated
Comment thread rpc/v10/trace.go Outdated
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Claude finished @danielntmd's task in 4m 16s —— View job


Review complete

  • Gather context (diff since last review, new commit ff917ed53)
  • Review rpc/v10/trace.go changes (feeder gateway InitialReads wire format)
  • Review rpc/v10/trace_test.go new assertion
  • Cross-check against pre-PR (origin/main) behavior for wire-format parity
  • Build/vet rpc/v10 package
  • Post summary

This PR was synchronized with a new commit (ff917ed53, "fix(rpc): preserve feeder initial reads wire format") on top of the previously-reviewed 6ab7d603e. Only rpc/v10/trace.go and rpc/v10/trace_test.go changed; the rest of the PR (cache/single-flight design in trace_cache.go, trace_progressive.go) is unchanged from the earlier review, which found no blockers.

New commit review: correct, no issues.

  • traceFinalisedBlockSource (rpc/v10/trace.go:472-476) now builds the feeder-gateway InitialReads as &InitialReads{} (nil slices) instead of emptyInitialReads() (empty, non-nil slices). Since InitialReads's fields have no omitempty tag, this is a real wire-format difference (null vs []), and matches the pre-PR behavior on origin/main (InitialReads: &InitialReads{} at the equivalent gateway branch), so this restores backward compatibility for clients relying on the historical shape.
  • The local-execution empty-block path (rpc/v10/trace.go:439-442) intentionally still uses emptyInitialReads() (empty arrays) — that's a genuine "zero reads occurred" case rather than "the gateway doesn't tell us," so keeping it distinct from the gateway's null-shaped placeholder is correct, not an inconsistency.
  • The new test assertion in rpc/v10/trace_test.go:194-198 directly locks in the fix by asserting require.Equal(t, &rpcv10.InitialReads{}, withReads.InitialReads) against real feeder testdata fixtures — good regression coverage for exactly the field that broke.
  • storeComplete still requires non-nil InitialReads (trace_cache.go); &InitialReads{} satisfies that, so no nil-pointer risk was introduced.

Severity: none (nit/important/blocker) — no new issues found. The two nits from the earlier review round (feeder-gateway path lacking single-flight coordination, and checkpointFromTraces recomputing from scratch on each extension) were both already discussed inline and acknowledged by the author as pre-existing/out-of-scope and an accepted trade-off, respectively — no further action needed there.

Note: go build ./rpc/v10/... and go vet ./rpc/v10/... both pass. I was not able to run go test in this environment (command required interactive approval that wasn't available), so this is based on static review plus the build/vet checks above rather than a live test run.

@danielntmd
danielntmd requested a review from infrmtcs September 3, 2026 12:26
@danielntmd
danielntmd marked this pull request as draft September 7, 2026 05:53
Collect initial reads on demand, replaying the block when cached traces lack requested reads. Shape responses at the RPC boundary and share finalised trace routing.

Publish immutable cache records with per-block flight coordination. Trim test fixtures and strengthen checkpoint and prefix preservation coverage.
@danielntmd
danielntmd marked this pull request as ready for review September 8, 2026 06:38
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Review in progress

  • Gather context (diff vs origin/main, CLAUDE.md, changed files)
  • Review rpc/v10/trace_cache.go (cache/flight structure, concurrency)
  • Review rpc/v10/trace_progressive.go (progressive execution logic)
  • Review rpc/v10/trace.go changes (handler wiring)
  • Review rpc/v10/adapt_trace.go, handlers.go, simulation.go
  • Review tests (trace_cache_test.go, trace_progressive_test.go, trace_test.go)
  • Build/vet rpc/v10
  • Post inline comments for any new issues found
  • Post final summary

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants