Repository navigation
test(cuda): prove grouped QMoE (M>=2) CUDA-graph capture is already safe, close coverage gaps (#82) - #1800
Merged
Conversation
…afe, close coverage gaps (#82) Investigation for this cycle found grouped-path (M>=2) QMoE CUDA-graph capture eligibility already implemented and already safe in current main (no functional change to `qmoe.rs` was needed): 1. The session executor's per-shape `KernelKey` kernel-instance cache ("Chew's guarantee") never reuses a kernel instance for a shape it wasn't compiled/warmed for, so `capture_support()`'s row-count-agnostic `warmed` boolean is honest in practice for every M. 2. Every grouped-path launch grid (`launch_grouping`/`launch_gather`/ `launch_grouped_linear`/`launch_linear`) is sized from host-static values, never a device-computed per-expert count -- no launch-shape instability, no host dependency on device state. 3. `ScratchPool::ensure(.., capturing)` already rejects (loud `Err`) any capacity growth attempted while capturing, so a violated per-shape- kernel invariant cannot silently reallocate/corrupt a captured graph. 4. The only host readback in `execute()` and its trailing `synchronize()` are both already gated `if !capturing`. This commit is purely additive test/benchmark coverage that proves and documents that invariant, closes the specific gaps the taxonomy required, and extends the #1777/#1788 bandwidth probe with capture+replay timing: - `qmoe_64experts_top6_m2/m4_capture_replay_reresolves_changed_router_probs`: explicit M=2/M=4 grouped capture+replay correctness (M=8 already had coverage), changing router logits between capture and replay. - `qmoe_grouped_capture_support_denied_before_warmup_with_reason`: missing- runtime-hook / warm-up-precondition rejection reason. - `qmoe_grouped_capture_rejects_shape_growth_without_corrupting_kernel`: dynamic-allocation/growth taxonomy, direct `ScratchPool::ensure` exercise. - `qmoe_grouped_capture_never_syncs_even_with_deferral_disabled`: hidden- sync/drain regression test, forcing a REAL blocking `synchronize()` so a capture-time sync would provably corrupt/fail the recording. - `qmoe_grouped_capture_repeated_replays_and_consecutive_capture_cycles_stay_correct`: consecutive graphs, repeated replay, basic device-memory accounting. - `qmoe_grouped_eager_execute_after_capture_replay_still_correct` and `qmoe_grouped_drop_kernel_after_capture_replay_leaves_runtime_usable`: lifecycle/teardown safety after a capture/replay cycle. - `qmoe_grouped_capture_replay_bandwidth_probe`: extends the #1777/#1788 harness with capture+replay timing across grouped M={2,4,8} for both DeepSeek-V2-Lite and GLM-5.2 shapes -- separates one-time capture cost from steady-state replay cost, reports GPU event time and host enqueue time for both replay and eager dispatch (the launch-gap reduction is thus a measured, falsifiable number: ~95-96% host-dispatch reduction, reproducible across 3 independent runs), and gates correctness with both a CPU oracle and a byte-identical check against this same kernel's eager output before any timing is trusted. Also fixes a real bug found while writing these tests: five of the new tests initially allocated/uploaded device buffers *inside* an active `begin_graph_capture`/`end_graph_capture` window, which hung the CUDA driver indefinitely (not a clean error return). All were rewritten to allocate/upload strictly before capture begins, matching the established `GemvBenchSetup`/`setup_gemv_bench` idiom. An independent review additionally found four tests whose capture-window assertions could panic between `begin_graph_capture`/`end_graph_capture` without aborting the capture or freeing buffers, plus one test that leaked its `GemvBenchSetup` buffers via a bare `drop`. Added a `CaptureGuard` RAII helper (aborts capture if dropped while still armed) and a `GemvBenchSetup::free_without_readback` method (mirrors the existing `PendingExecution::free_without_readback`) to fix both. Validated on an idle A100 (`CUDA_VISIBLE_DEVICES=1`): all 46 tests in `qmoe_gpu.rs` pass, `cargo clippy`/`cargo fmt` clean on the changed file (scoped `--test qmoe_gpu`; full-workspace clippy hits a pre-existing, unrelated compile break in `weight_paging.rs`). No CI wait per this cycle's directive; local validation is the evidence of record. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1800 +/- ##
==========================================
+ Coverage 80.26% 80.30% +0.04%
==========================================
Files 411 411
Lines 199718 200334 +616
Branches 199718 200334 +616
==========================================
+ Hits 160296 160877 +581
- Misses 33966 33994 +28
- Partials 5456 5463 +7
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
justinchuby
added a commit
that referenced
this pull request
Aug 24, 2026
…ndary window (#1810 Slice 7A) (#1922) ## Slice 7A — inert, default-OFF, producer-only expert-route telemetry (QMoE/BQMoE) Closes #1810 (Slice 7A). **Draft. Do not merge.** Crate-internal / test-only / default-OFF. > **Cycle-22 revision by Sebastian (Performance Engineer), revision owner.** > The original author is under reviewer-protocol lockout and did not advise, pair, or contribute to this revision. Reworked independently from the PR diff, the approved Slice-6 design doc, and Roy's Cycle-22 review. ### What changed vs the rejected HEAD (`8ec9d8230`) Roy's Cycle-22 NO-GO was correct and is **addressed at the design level**: the rejected design launched a **separate reset/epoch kernel on every execute/replay**, which contradicts the approved Slice-6 **coarse-boundary window** contract (`docs/memory/EXPERT_ROUTE_TELEMETRY_SLICE6_DESIGN.md` §2.3/§3 — the epoch is bumped and the record consumed at a *coarse safe boundary*, **not per replay**). That per-call reset is why the fused path measured **2.526 us / 2.18 % -> NO-GO**. This revision makes the fused route kernels **accumulate-only** and moves reset/epoch-advance to an explicit, host-ordered **coarse boundary**. The earlier NO-GO was scoped to the wrong (per-call reset) design point; the boundary-window producer now measures **GO** (see below), independently reproduced. ### Invariant / API (producer-only boundary-window contract) - **`arm(request, device, experts)`** - validates identity, allocates the stable-VA record via the existing runtime allocator, opens **window 1** (stamps identity, `epoch = 1`, zeroes bitmap/counters). **No kernel is compiled or launched.** - **execute / replay** - the fused `route_telemetry_mark_row` inside `qmoe_route` / `bqmoe_route` **only accumulates** into the stable record: `atomicOr` the routed bit into the bitmap, `atomicAdd` the in-range count, `atomicOr` the sticky **poison** bit on an out-of-range id, `atomicOr` the sticky **overflow** bit on count saturation. The **epoch is fixed for the whole window**; every eager call and captured replay in the window accumulates the routed-expert **union + count**. **No reset/epoch kernel in the captured graph; no host sync/alloc/drain/VMM on this path.** - **`reset_route_telemetry_boundary()`** - the **only** place the window advances. **Rejected while the EP stream is capturing/replaying** (fail closed, checked *before* any drain); otherwise drains prior stream work via the existing `drain_for_unmap` authority, bumps the **host-side** epoch, and re-stamps identity + zeroes the record so the next window starts empty with **no stale carryover**. Allocates nothing, moves no pointer, touches no PMM/VMM/cache/global coordinator. - **Fail-closed** - overflow **saturates** into the sticky overflow bit (never wraps into a smaller "success"); out-of-range routes **poison** without touching the bitmap; snapshot/validate run on the host at a boundary against an already-copied record. - **Isolation / stability** - multi-request / multi-device / multiple kernel instances use distinct records; the bitmap **VA is stable** across windows and captures. Epoch is a host counter, so the record footprint **drops the former 4-byte device epoch buffer** (`footprint = 4*ceil(experts/32) + 24`). - **Reuses only existing authorities** - `alloc_raw`/`free_raw`, `htod`/`dtoh`, `is_capturing`, `drain_for_unmap`, `ordinal`. No new coordinator/allocator/cache/PMM/VMM and **no host sync in steady-state replay**. Scope unchanged: **default-OFF, crate-internal, test-only**. Ordinary inference is **byte-identical** and has **zero overhead when disarmed**. No lifecycle/policy/consume wiring in this slice. ### Tests (rewritten to the new semantics, same commit) QMoE (`tests/qmoe_gpu.rs`) and BQMoE (`tests/block_quantized_moe_gpu.rs`) `mod route_telemetry`: - off/on output **byte-identity**; - eager calls **accumulate the CPU-oracle union + count** within a window at a **fixed epoch**, then a boundary reset opens an empty **epoch-2** window; - **>=3 graph replays** with different routes accumulate the union at a **fixed epoch** and a **stable VA** (QMoE - proves no per-replay reset); - boundary reset **increments the epoch and starts an empty window**; - boundary reset **rejected during an active capture** (epoch unchanged); - capacity / device mismatch stays **inert** and never fails inference; - multi-instance **request/device isolation**, footprint, teardown/accounting; - **QMoE and BQMoE routes for M in {1, 2, 4, 8}** match the oracle. Host unit tests cover **poison/overflow fail-closed** on the validator; the #1884 probe harness continues to cover device-level poison/overflow. **Results** (idle A100, GPU 5, `--test-threads=1`): QMoE `route_telemetry` 10 passed; BQMoE 8 passed; telemetry host unit tests 7 passed; #1884 probe functional regressions 6 passed; **full QMoE suite 56 passed**, **full BQMoE suite 13 passed** (no regressions to #1788/#1800/#1884/#1854). ### Measurement - G1 gate (`microbench_fused_route_telemetry_g1_gate`) Idle **pinned A100**, serialized off/on back-to-back, whole QMoE layer **CUDA-graph captured** and measured via `replay_graph` (host-enqueue gaps excluded), **BATCH = 512, RUNS = 5**, ~8 s clock ramp + first-shape recheck, GPU-event and host-enqueue reported separately. Gate denominator is a **realistic** DeepSeek-V2-Lite decode layer (64 experts, top_k 6, hidden 2048); tiny synthetic is informational only. **n = 3 independent runs:** | Run | route+layer overhead (GPU) | % of layer | host-enqueue delta | epoch | count | output | |-----|---------------------------|-----------|----------------|-------|-------|--------| | 1 | 0.976 us | 0.84 % | -0.016 us | fixed = 1 | 30726 | identical | | 2 | 0.948 us | 0.82 % | -0.006 us | fixed = 1 | 30726 | identical | | 3 | 1.054 us | 0.91 % | -0.008 us | fixed = 1 | 30726 | identical | **Range: 0.948-1.054 us, 0.82-0.91 %. Clock drift < 0.5 %.** The epoch stays **fixed at 1** across the whole BATCH x RUNS replay window (count accumulates to 30726) - a moving epoch would have proven a forbidden per-replay reset survived. **G1 GATE (<= 2 us AND <= 2 %): GO.** Independently reproduced vs Roy's boundary-only experiment (0.884 us / 0.76 %); same order, same verdict - not a reused number. Telemetry remains **default-OFF** regardless of the GO. ### Review Independent review (excluding the locked-out author and the revision owner): **APPROVE** - no blocking or substantive findings; confirmed accumulate-only execute path, reject-before-drain boundary reset, fail-closed overflow/poison, stable VA, isolation, footprint, and that the tests assert the *new* semantics (they would fail under the rejected per-replay-reset design). One MINOR note: the device-side poison branch of the fused mark is defensive and unreachable-by-construction from the production route kernel (Roy's prior approved framing) - covered by the host validator unit tests and the #1884 probe harness. **Roy's explicit final re-review is requested** before this leaves draft. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
justinchuby
added a commit
that referenced
this pull request
Aug 24, 2026
… coarse residency (#1810 Slice 7B) (#1971) ## #1810 Slice 7B — boundary-time route-telemetry **consumer** **Status: DRAFT — awaiting independent review. Do not merge.** Closes the loop opened by the merged Slice-6/7A **producer** (expert-route telemetry, PR #1922 `e1ec495ee`) and the merged Slice-4/5 **coarse-boundary residency lifecycle** (PR #1854). This is the smallest production seam the Slice-6 design §8 specifies: > expose a boundary-time consumer that produces a per-expert desired-set, and feed that set to the **existing** Slice 4/5 coarse-boundary plan application (`coarse_residency.rs`) as its policy input — with **no** new allocator, **no** id→slot rewrite, and every mapping change still owned by PMM/VMM. ### What it does New module `crates/onnx-runtime-ep-cuda/src/route_residency.rs`: `consume_route_window_at_boundary(...)`: 1. **Gate** — default-off via the existing `COARSE_RESIDENCY_ENABLE_ENV` (`coarse_residency_profile_enabled()`). When off (shipped default) it returns `Disabled` *before* reading the snapshot or touching any allocator. 2. **Safe boundary** — re-reads the existing `CudaWeightResidency::resize_safe_point` and fails closed with `RejectedNotSafeBoundary { reason }` if a graph is capturing/replaying, an admission is in flight, a deferred release has not settled, execution is multi-device, or a routed-residency guard is live. Coarse-boundary only — never a per-token remap. 3. **Validate** — runs the producer's own `consume_and_validate` on the already-completed window snapshot; fail-closed (→ `WholeBank { reason }`) on poison / overflow / stale epoch / foreign request / foreign device, or when no in-range experts were recorded. 4. **Plan** — turns the routed-expert union into a desired **hot set** and asks the already-validated `StaticProfileResidencyPolicy` to shape a `ResidencyPlan` (the design's "record → desired set" `RouteObserverPolicy` role — **reused, not duplicated**, so exactly one validated policy emits `PerExpertCandidate`). 5. **Apply** — hands the plan to the existing `CudaWeightResidency::apply_coarse_residency_plan`, which remains the **sole** authority that maps / unmaps / accounts / quarantines / rolls back through PMM/VMM. `RouteWindowConsumeOutcome` carries the exact reason on every non-applied path — **no silent fallback**. ### Invariants held by construction - **Allocates nothing**, opens no stream, owns no VA, copies no device bytes — pure host glue between two existing authorities. - **No remap during capture/replay**; coarse-boundary only, never per-token. - **No new host sync** in steady state (only the producer's already-taken snapshot and the existing transition primitive's drain). - **Default-off & byte-identical** when disarmed (two independent default-off switches: telemetry disarmed *and* this gate off). - **Same-device fail-closed** and **PMM/VMM remains the sole mapping/accounting/quarantine/rollback authority**. ### Tests — `tests/route_residency_consume_gpu.rs` (6 GPU tests, all passing on an idle A100) - `disabled_gate_is_structural_no_op` — off path is a structural no-op (byte-identical). - `route_window_hot_set_transitions_cold_experts` — routed hot-set stays resident, cold set tiers to host, bytes identical. - `expert_group_transitions_atomically_from_window` — atomic expert-group transition driven from a window. - `active_capture_and_multi_device_reject_consume` — active-capture and multi-device boundaries reject the consume (fail-closed). - `foreign_identity_and_defective_windows_fail_closed` — foreign request/device and poison/overflow/stale windows fail closed to whole-bank. - `injected_fault_rolls_back_consumer_transition` — injected driver fault (`fail_nth(Unmap, 3)` over an isolated + merged cold range) rolls back range-precisely and quarantines; `rollback_count == 1`, `values_touched == 0`, `committed_values` empty, content bit-identical (mirrors the proven `coarse_residency_plan_gpu.rs` rollback fixture). Run: ``` env -u ONNX_GENAI_WEIGHT_OFFLOAD_COARSE_RESIDENCY_ENABLE CUDA_VISIBLE_DEVICES=<idle> ONNX_GENAI_CUDA_DEVICE=0 \ cargo test -p onnx-runtime-ep-cuda --features cuda,cuda-13000,gpu-tests --release \ --test route_residency_consume_gpu -- --ignored --test-threads=1 ``` ### Honest scope Like `coarse_residency::apply_residency_plan_at_boundary` when it shipped (Slice 5), this consumer has **no live decode-loop call site yet** — wiring it into a running session's request boundary is the next slice. It ships here as the production seam: reachable, default-off, and proven by the GPU tests. `cargo fmt` applied; `cargo clippy` on the crate lib is clean for the new file (pre-existing unrelated warnings/errors in `optimizer.rs` / `standard_attention.rs` are out of scope). ### Constraints honored Did not touch PagedAttention or IQ1 fusion files. Branch `squad/1810-slice7b-telemetry-residency-consume` based on latest `main` (`011fbb284`, includes #1922 `e1ec495ee`). #1788/#1800/#1884/#1854 lifecycle regressions preserved (reused, not forked). Refs #1810. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
Summary
Working as Sebastian (Performance Engineer). Follow-up to #1777/#1788 for issue #82. This cycle's task was to determine whether grouped QMoE (M>=2) CUDA-graph capture eligibility could be made safe, if and only if a property-level invariant can be proven — no M/model allowlists, no copying the unrelated MatMulNBits fix.
Finding: it already is. Grouped-path (M>=2) QMoE CUDA-graph capture is already implemented and already safe in current main. No functional change to
qmoe.rswas needed. This PR is purely additive test/benchmark coverage that proves and documents the invariant, closes specific test-coverage gaps, and extends the existing bandwidth benchmark with capture+replay timing.The capability invariant
KernelKeykernel-instance cache ("Chew's guarantee") never reuses a kernel instance for a shape it wasn't compiled/warmed for, socapture_support()'s row-count-agnosticwarmedboolean is honest in practice for every M.launch_grouping/launch_gather/launch_grouped_linear/launch_linear) is sized from host-static values, never a device-computed per-expert count — no launch-shape instability, no host dependency on device state.ScratchPool::ensure(.., capturing)already rejects (loudErr) any capacity growth attempted while capturing — a violated per-shape-kernel invariant cannot silently reallocate/corrupt a captured graph.execute()and its trailingsynchronize()are both already gatedif !capturing.No new global synchronization/allocation authority is introduced; all of the above are existing #1777/#1788-era mechanisms.
What this PR adds
qmoe_64experts_top6_m2/m4_capture_replay_reresolves_changed_router_probs: explicit M=2/M=4 grouped capture+replay correctness (M=8 already had coverage), changing router logits between capture and replay.qmoe_grouped_capture_support_denied_before_warmup_with_reason: missing-runtime-hook / warm-up-precondition taxonomy.qmoe_grouped_capture_rejects_shape_growth_without_corrupting_kernel: dynamic-allocation/growth taxonomy, directScratchPool::ensureexercise.qmoe_grouped_capture_never_syncs_even_with_deferral_disabled: hidden-sync/drain regression test — forces a REAL blockingsynchronize()so a capture-time sync would provably corrupt/fail the recording.qmoe_grouped_capture_repeated_replays_and_consecutive_capture_cycles_stay_correct: consecutive graphs, repeated replay, basic device-memory accounting stability.qmoe_grouped_eager_execute_after_capture_replay_still_correct/qmoe_grouped_drop_kernel_after_capture_replay_leaves_runtime_usable: lifecycle/teardown safety.qmoe_grouped_capture_replay_bandwidth_probe: extends the bench(cuda): sound QMoE expert-GEMV bandwidth probe (issue #82 baseline) #1777/fix(cuda): replace QMoE's incidental trailing sync with explicit scratch/teardown drains #1788 bandwidth harness with capture+replay timing across grouped M={2,4,8} for both DeepSeek-V2-Lite and GLM-5.2 shapes.Measurements (idle A100,
CUDA_VISIBLE_DEVICES=1, 3 independent process runs, reps=25, batch=16)Replay GPU-event time is within noise of eager time at every configuration (this workload is memory-bandwidth-bound, not launch-bound, at these M values) — capture's measured benefit here is host dispatch time, cut ~95-96% (from ~60-67us to ~2-3us per launch), not GPU throughput. Variance across 3 independent process runs is <0.3% on
replay_median_us. Correctness gated by CPU oracle + byte-identical-vs-eager check on a reduced-expert-count proxy case before any timing is trusted, at every (shape, M).Full 46-test suite (
cargo test -p onnx-runtime-ep-cuda --release --features gpu-tests --test qmoe_gpu) passes, 0 failures, ~455-465s wall on this A100.cargo fmt/cargo clippyclean on the changed file (scoped to--test qmoe_gpu; full-workspace clippy hits a pre-existing, unrelated compile break inweight_paging.rs, not touched by this PR).Review
An independent code-review agent found:
begin_graph_capture/end_graph_capturewithout aborting the capture or freeing buffers on that path.GemvBenchSetupbuffers via a baredrop.Both fixed in this PR: added a
CaptureGuardRAII helper (aborts capture if dropped while still armed) and aGemvBenchSetup::free_without_readbackmethod (mirrors the existingPendingExecution::free_without_readback). Full suite re-verified passing after the fix.Scope
Test/benchmark-only — no production code in
qmoe.rs/runtime.rs/capture.rschanged. No paging/residency, exporter, or BlockQuantizedMoE changes (per instruction).Next gate for #82
Per the ordering established in the prior cycle's follow-up: BlockQuantizedMoE paging+prefetch wiring is next — the sound benchmark (#1777/#1788) and the capture-safety proof (this PR) are both now in place as prerequisites/instrumentation for that work. This PR does not close #82; it removes one more item from that issue's dependency chain.
Per user directive: no CI wait — this PR is validated by local A100 tests + independent review; CI is asynchronous supplementary evidence. Merging on local gates.
🤖 Generated by Sebastian (Performance Engineer), Squad autonomous cycle.
Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com