fix(cudagraph): synchronize auxiliary warmup streams - #617
Conversation
Complete every uncaptured warmup forward before CUDA graph capture begins. Expose the graph-preparation state so model-specific auxiliary streams can distinguish uncaptured warmup from regular serving and FULL capture. This prevents asynchronous warmup allocations from racing allocator reuse during capture. The focused lifecycle regression covers the warmup-forward, device-synchronization, capture-begin, and capture-forward order. Signed-off-by: Martin Vit <martin@voipmonitor.org>
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe change adds a CUDA graph capture state helper and synchronizes the accelerator after warmup execution. A new test verifies that synchronization occurs before graph capture. ChangesCUDA graph capture flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This change synchronizes uncaptured warmup work before CUDA graph capture, preventing auxiliary-stream work from crossing the capture boundary. The covered ordering change introduces no remaining merge-blocking risk. Sequence Diagram(s)sequenceDiagram
participant CudaGraphManager
participant forward_fn
participant torch.accelerator
participant graph_capture
CudaGraphManager->>forward_fn: Run warmup forward
forward_fn-->>CudaGraphManager: Warmup completes
CudaGraphManager->>torch.accelerator: Synchronize streams
torch.accelerator-->>CudaGraphManager: Synchronization completes
CudaGraphManager->>graph_capture: Begin capture
graph_capture->>forward_fn: Run capture forward
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
|
Independent integration qualificationStatus: qualified at the PR head embedded in The focused regression verified the required graph-setup order: uncaptured The published launcher defaults to The synchronization is confined to graph construction; serving and graph |
10d7112
into
local-inference-lab:dev/jovian-judgement
Disable the L2-prefetch side stream throughout breakable CUDA graph capture and during uncaptured graph warmup forwards. Regular FULL capture and serving retain decode-weight overlap. This branch depends on the generic auxiliary-stream warmup synchronization in #617. Focused helper tests cover breakable capture, uncaptured warmup, regular FULL capture, and serving. Signed-off-by: Martin Vit <martin@voipmonitor.org>
Disable the KDA gate projection side stream throughout breakable CUDA graph capture and during uncaptured graph warmups. Record the intermediate projection on its consumer stream so allocator reuse cannot race asynchronous reads. Regular FULL capture and serving retain the overlap. The original side-stream implementation remains authored by MadeBy561. This branch depends on the generic auxiliary-stream warmup synchronization in local-inference-lab#617. Signed-off-by: Martin Vit <martin@voipmonitor.org>
Disable the KDA gate projection side stream throughout breakable CUDA graph capture and during uncaptured graph warmups. Record the intermediate projection on its consumer stream so allocator reuse cannot race asynchronous reads. Regular FULL capture and serving retain the overlap. The original side-stream implementation remains authored by MadeBy561. This branch depends on the generic auxiliary-stream warmup synchronization in #617. Signed-off-by: Martin Vit <martin@voipmonitor.org>
Resulting behavior
CUDA graph setup synchronizes the accelerator after each uncaptured warmup forward and before
torch.cuda.graphbegins. The compilation monitor exposes whether graph preparation is active so model code can disable auxiliary overlap during uncaptured warmups while retaining overlap during serving and regular FULL capture.Technical reason
A model forward may fork kernels onto auxiliary streams and enqueue only event waits on the compute stream. Stream ordering is sufficient for normal execution, but CUDA graph capture must not begin while an uncaptured auxiliary kernel still owns temporary allocator storage. Starting capture at that boundary can race allocator reuse and surface as an illegal memory access in a later kernel.
Compatibility
The additional synchronization runs only during graph construction. It does not add synchronization to graph replay or serving. Models that do not use auxiliary streams retain the same captured graph and runtime execution.
Validation
warmup forward -> accelerator synchronization -> capture begin -> capture forwardordering.OpenAI Codex assisted with the failure isolation, implementation, and validation. The submitter reviewed the resulting source and evidence.
Summary by CodeRabbit
Bug Fixes
Tests