Skip to content

[BugFix] Synchronize Breakable CUDA Graph after warmup before capture - #34286

Open
Phoenix3334 wants to merge 2 commits into
sgl-project:mainfrom
Phoenix3334:fix/breakable-cudagraph-post-warmup-sync
Open

Phoenix3334 wants to merge 2 commits into
sgl-project:mainfrom
Phoenix3334:fix/breakable-cudagraph-post-warmup-sync

Conversation

@Phoenix3334

@Phoenix3334 Phoenix3334 commented Aug 10, 2026

Copy link
Copy Markdown

Motivation

BreakableCudaGraphBackend.capture_one() runs two eager warmup iterations, each followed by post_warmup_hook, and then immediately constructs BreakableCUDAGraph and enters capture.

There is currently no device synchronization or TP-group rendezvous between the final warmup/hook and graph construction. That leaves a warmup-to-capture completion gap: asynchronous work triggered by the last warmup or its hook can still be in flight when capture starts, and TP ranks are not explicitly aligned at that boundary.

This is the same warmup-to-capture completion invariant that #33795 proposes to enforce for FullCudaGraphBackend; this PR pins the corresponding invariant for BreakableCudaGraphBackend.

This PR does not claim a natural BCG crash reproducer and does not claim to be the primary fix for the B300 DSpark compact-ragged producer failure. That failure class is separately localized to #32467 / #32470. The source-of-truth investigation map is #34297.

Modifications

  • Add a device synchronize() after the final BCG warmup/hook.
  • Add a TP-group barrier() immediately after the device sync and before BreakableCUDAGraph construction.
  • Add a CPU-only ordering regression for BreakableCudaGraphBackend.capture_one().

The regression records call order and requires:

final post_warmup_hook
  < synchronize
  < barrier
  < BreakableCUDAGraph construction

It also checks that capture_one() still performs two warmup forwards plus one captured forward, and that post_warmup_hook runs only for the two warmups.

Tests

The regression was first run against the unpatched production code as a negative control. It failed specifically because no synchronize() / barrier() existed between the final warmup hook and BCG construction; the forward/hook/count assertions had already passed.

After applying the production change, the same targeted test passed.

Final local validation:

runner_backend unit suite: 5 passed
pre-commit: all hooks passed
git diff --check: clean
working tree: clean

The CPU regression is registered in base-a-test-cpu and does not require CUDA or model weights.

Scope / performance

The additional sync + barrier run only during one-time graph capture, after warmup for each captured shape. There is no steady-state replay-path change.

No kernel, model-forward, or numerical logic is modified.

Root-cause map and related work

The B300/DSpark investigation separated multiple correctness layers rather than treating every illegal-memory symptom as one bug:

#31195  cross-TP verify-budget / Graph-tier consistency
#32183  verifier compressed-state rewrite window
#32467  producer-side ragged plan/index correctness
#33795  FullCudaGraphBackend capture-initialization ordering
#34286  BreakableCudaGraphBackend capture-initialization ordering
#32432  shared runtime-contract / observability RFC
#34410  DSpark PD decode handoff regression coverage

Producer-side bug: #32467 / #32470

The primary B300 producer root cause is a write-write race in plan_compress_prefill_kernel0: redundant initialization of shared warp_min / warp_max scratch can overwrite a completed per-warp reduction, making ragged input appear uniform. The wrong uniform/MTP fast path can then emit out-of-range ragged_id; a downstream kernel merely surfaces the resulting illegal access.

The final reviewer-selected #32467 implementation is the no-init formulation: remove the redundant scratch initialization instead of adding an extra barrier. Same-session A/B/C kernel regression:

Variant Init Extra barrier OOB plans
original ON OFF 13111 / 30000
barrier experiment ON ON 0 / 30000
final reviewer-selected no-init OFF OFF 0 / 30000

This is intentionally separate from the capture-ordering invariant in this PR.

2026-08-11 B300 runtime evidence

An independent TP8/DP8 B30Z runtime regression of the final #32467 no-init implementation was completed on a v0.5.16 stack with the required DSpark DP/disaggregation integration backports (#33098 merged upstream; #31513 open/unmerged at validation time).

Decode-only topology included DP Attention, DSPARK, fake disaggregation transfer, compact ragged verify, max-running-requests=1024, and per-rank CUDA-Graph max batch 128.

The sustained sweep completed 18560/18560 requests across C64/C256/C512/C1024 with zero observed illegal-address faults, device assertions, scheduler exceptions, or client failures; /health = 200. C1024 reached per-rank running batch 128 and ~24.8k generated tok/s.

A forced compact-ragged window (dspark_force_budget_frac=0.5) produced verify lengths 1–6; 39/43 fully parsed multi-request blocks were truly ragged, with no GPU fault/NaN/budget violation observed.

A subsequent external-concurrency sweep up to C2560 also completed without request failures or observed GPU correctness faults, but max-running-requests=1024 kept the true running peak at 128 per rank. Those higher external-concurrency points are admission/queue pressure evidence, not proof of >1024 simultaneously running requests.

This runtime evidence strengthens the separation of concerns: #32467 is the primary producer-side fix for the tested B300 failure class, while #34286 remains framework hardening for the Breakable capture boundary.

Related


CI States

Latest PR Test (Base): ❌ Run #34925147699
Latest PR Test (Extra): ❌ Run #34925147675
Latest PR Test (AMD ROCm 10): ❌ Run #34925147711

BreakableCudaGraphBackend.capture_one runs two eager warmup iterations,
each followed by post_warmup_hook, and then immediately constructs the
BreakableCUDAGraph and enters capture. There was no device synchronize
and no TP barrier between the final warmup/hook and that construction,
so asynchronous work still in flight from the last warmup (or from the
hook) could straddle the capture boundary, and the TP ranks were not
aligned before one of them started capturing.

Drain the device and rendezvous the TP ranks after the last warmup,
mirroring the sync that already precedes each warmup iteration. This is
the same warmup-to-capture completion gap that sgl-project#33795 proposes to close
for FullCudaGraphBackend; BreakableCudaGraphBackend is the default
prefill graph backend on CUDA.

Add a CPU-only ordering regression that records the call sequence of
capture_one and asserts a synchronize() followed by a barrier() lands
between the final post_warmup_hook and the graph construction. The test
fails on the pre-fix code and passes with the fix.

Copy link
Copy Markdown
Author

Cross-backend provenance note for review:

#33795 independently identified the same final warmup-to-capture completion requirement in FullCudaGraphBackend, and its discussion explicitly called out the analogous gap in BreakableCudaGraphBackend. This PR is the concrete Breakable-backend implementation + regression follow-up.

The current evidence map is:

#33795 -> FullCudaGraphBackend
  H200/TP4 natural failure evidence
  final synchronize + TP barrier before CUDAGraph construction

#34286 -> BreakableCudaGraphBackend
  same structural boundary
  CPU-only deterministic ordering regression
  negative-control RED on unpatched backend -> GREEN after fix

I am intentionally not using #34286 to explain the B300 DSpark producer failure. That path is separately localized to #32467 (plan_compress_prefill_kernel0) with direct invalid-plan output and B300 startup/replay validation.

So the value of this PR is narrower: make the capture-initialization invariant consistent and testable in the second backend without adding replay-hot-path synchronization.

Copy link
Copy Markdown
Author

2026-08-11 body refresh: I updated this PR's context section to match the current root-cause map in #34297 and today's B300 evidence.

The important scope boundary is unchanged: #34286 is BreakableCudaGraphBackend capture-boundary hardening, not the primary fix for the tested B300 compact-ragged producer failure. That producer bug is #32467/#32470 (plan_compress_prefill_kernel0 write-write race; final reviewer-selected no-init patch).

Today's independent TP8/DP8 B30Z decode-only regression of the final no-init stack completed 18560/18560 requests through C1024 with no observed CUDA illegal-address/device-assert/scheduler failures and /health=200; a forced ragged window also remained clean. A later external-concurrency sweep through C2560 was clean as admission-pressure evidence, while MRR=1024 kept the true per-rank running peak at 128.

I added these data only as investigation context; they are not evidence that #34286 itself caused the B300 runtime improvement.

Copy link
Copy Markdown
Author

@merrymercy @ispobock @Fridge003 @hnyls2002 — adding the remaining requested Code Owners to the CI rerun request above. Could one of you help trigger /tag-and-rerun-ci when convenient? Thanks!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant