perf(glm52): launch-ahead decode — hide the ~0.7ms cuGraphLaunch under the previous step's execution#570
Merged
Merged
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 464491ad62
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+525
to
+529
| log::error!( | ||
| "GLM5.2 graph pre-capture (bucket {bucket}, full_tier {full_tier}) \ | ||
| failed on rank {rank}: {err:#}" | ||
| ); | ||
| return; |
There was a problem hiding this comment.
When any pre-capture response returns an error, this early return skips the normal shutdown broadcast at the bottom of run_dp8_coordinator. Because Glm52RankWorker::drop sends Shutdown and joins one worker at a time, a rank that has already set up DeepEP can block in its collective context drop while the remaining ranks have not been told to exit; a startup graph-capture failure then hangs until the device timeout instead of shutting down cleanly. Broadcast request_shutdown() to all workers before returning here and in the submit-error branch above.
Useful? React with 👍 / 👎.
… the ~0.7ms cuGraphLaunch The whole-step graph's cuGraphLaunch costs ~700us of host-side driver work per rank per step with the GPU idle through it (measured on jz-38; the double-replay probe showed an intra-step replay gap of 5.4us vs 810us at an unhidden boundary). This change enqueues the next step's replay while the current step still executes: - a feed kernel advances token_ids/positions/slot_mapping/seq_lens in place from the step's device argmax (rope rows re-gathered on device), - the argmax D2H lands in pinned host buffers so the copy is truly async, and the speculative launch goes out before the host blocks on it, - the coordinator grants a lease only for pure single-token decode steps with nothing queued; the rank consumes the speculation only when the next command matches exactly (bucket, slots, inputs), so any admission, EOS, or spec-decode round degrades to the full prologue path and the stale replay is a harmless overwritten recompute (rows are isolated). Byte-identical by construction on the leased path: the feed kernel writes exactly the values the host prologue would have written. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
cudarc's memcpy_dtoh copies the DESTINATION's byte count — an 8-row pinned buffer against a smaller bucket's argmax scratch read past the device allocation (CUDA_ERROR_INVALID_VALUE on the first step). The landing buffers now live in Glm52BucketState, sized exactly rows, next to the scratch they mirror. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A speculative replay is a full set of collectives, so both the decision to speculate and the decision to consume must be uniform across ranks — the first gate run desynced when the 1/8 near-tie variant hit EOS early: that rank's inputs mismatched its speculation, it fell back to the full path (two replays) while the others consumed (one), and the collective pairing slipped into the ~100s DeepEP device-timeout trap. - the coordinator now decides both flags on global data: 'lease' (next step will repeat this shape; includes the model-length headroom check — pad rows never outrun active rows) and 'consume' (this step IS the speculation; requires unchanged shapes AND no slot handoff, since a finish+admission can reuse a slot id under an identical shape), - every bucket x tier whole-step graph is pre-captured at startup while the ranks are trivially in lock-step, making captured-ness uniform and retiring the mid-serving capture stall, - all rank-side conditions became hard ensures: a rank that cannot honor a lease fails the step instead of silently skipping into a desync. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…-804us -> 43us, solo 19.5 ms/step Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…apture shutdown broadcast, testable flag decision - Glm52StepShape carries active_rows: a padding input is not value-distinguishable from an active one (single-token prompt [0] feeds (0,0)), so the sentinel classification could kill the engine via the consume desync ensure; argmax finite/vocab checks narrowed to active rows - pre-capture failures broadcast Shutdown before returning (the sequential worker Drop otherwise deadlocks in the collective destroy barrier) - lease/consume decision extracted to launch_ahead_flags() + unit tests; Step carries a Glm52StepFlags struct instead of two bare bools - launch-ahead state machine split into model/launch_ahead.rs (1k line cap) - compile-time tie of the feed kernel's 32-row cap to the batch const Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
xiaguan
force-pushed
the
feat/glm52-device-fed-decode
branch
from
464491a to
672e064
Compare
This was referenced Jul 7, 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
1 participant
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.
The whole-step decode graph's `cuGraphLaunch` costs ~700µs of host-side driver work per rank per step with the GPU idle through it (root-caused in the #568 stagger investigation; the double-replay probe measured a 5.4µs intra-step replay gap vs 810µs at an unhidden boundary). This PR enqueues the next step's replay while the current step still executes:
Measured (jz-38 8×H200, same-day baselines)
Gates (full suite on the final commit): determinism + byte-parity vs the main record PASS; 8-way identical & 8-way 128-tok concurrent (391 tok/s aggregate; 1–2/8 land on the known deterministic near-tie variant — the established cross-bucket contract); staggered-admission churn + post-churn determinism PASS; post-disconnect + teardown PASS.
Desync lesson (cost one debug round)
The first gate run desynced: the near-tie variant hit EOS early, that rank's inputs mismatched its speculation and it silently fell back (two replays) while the others consumed (one) — the collective pairing slipped into the ~100s DeepEP device-timeout trap. Fix: both decisions moved to the coordinator on global data; every rank-side condition became an invariant.
Toxic review: Request Changes → all findings fixed in the last commit: the `(0,0)` in-band padding sentinel (a single-token prompt `[0]` legally feeds `(0,0)`) replaced by an explicit `active_rows` in `Glm52StepShape`; pre-capture failure now broadcasts Shutdown before returning (serial-join deadlock); the lease/consume decision extracted to a pure `launch_ahead_flags()` with 4 unit tests; `Glm52StepFlags` struct instead of bare bools; launch-ahead state machine split into `model/launch_ahead.rs` (1k-line cap); feed-kernel row cap tied to the batch const at compile time.
🤖 Generated with Claude Code