perf(cuda-ep): make native eager decode beat ORT eager (DeepSeek +37.8%, crosses ORT +3.5%) by justinchuby · Pull Request #1383 · justinchuby/onnx-genai

justinchuby · 2026-08-19T03:43:40Z

Summary

Native eager decode was ~25% behind ORT eager on DeepSeek-V2-Lite short-ctx (66 vs ORT 86.78 tok/s) — a pure per-token host-dispatch deficit, not GPU compute (CUDA-graph capture already hid it: graph=1 ~143 tok/s). This PR closes and reverses that gap, ON by default.

Root cause

Two numerically-inert host serializers per op on the eager (!capturing) branch:

Redundant trailing per-op stream drain — every kernel ended with synchronize(). It buys nothing: the single in-order EP stream guarantees kernel to kernel ordering, and dtoh/dtod self-synchronize before their synchronous copy.
Eager-only validation D2H readbacks — rotary/gather/structural/GatherElements copy 24-byte scalars to host purely to bounds-check indices. A correct model never trips them; the captured (graph=1) path already uses a device error-latch instead.

Per-token cost (nsys): DeepSeek MoE ~1187 launches, 416 cuStreamSynchronize, 147 D2H/token; ~54% of eager wall is recoverable host overhead.

Fix (default ON; `ONNX_GENAI_DEFER_EAGER_SYNC=0` escape hatch)

runtime.rs: defer_eager_sync defaults true unless the env is explicitly falsey (0/false/off/no). Public synchronize() no-ops when deferred; new private force_synchronize() keeps the real drain for dtoh/dtod host reads (read correctness preserved).
rotary_embedding.rs/gather.rs/structural.rs/indexing.rs: skip eager validation D2H when deferred.

Default-ON makes eager consistent with the already-shipped captured path; the env var is a one-flag rollback for debugging.

Results (GPU3, greedy, medians-of-5, pinned idle GPU, flag UNSET = default ON)

Config	Native tok/s	vs ORT eager 86.78
DeepSeek eager (old path, `=0`)	66.06	-24%
DeepSeek eager (default ON)	91.64	+5.6% WINS
GLM-4-9B dense eager (off to on)	142.42 to 145.31	+2.0%
DeepSeek graph=1 (default ON)	143.12	fallbacks=0, still valid

Validation (flag UNSET, default ON)

DeepSeek 24-tok golden lock BYTE-IDENTICAL: yes (run alone, 21.18s, 1 passed).
Graph=1 still valid: yes captures=6/replays=372/fallbacks=0.
Dense qwen2.5-0.5b logprobs byte-identical vs old path: yes (md5 d2e4bc2e...).
Escape hatch =0 restores old path: yes (66.06 tok/s).

Recommendation

Ship default-ON (this PR). Remaining headroom (DeepSeek eager 91.6 vs graph 143) is the ~1187 launches/token to kernel fusion, a separately-scoped follow-up.

Findings: .squad/decisions/inbox/wallace-eager-launch-overhead.md

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

…r decode beat ORT Native eager decode was ~25% behind ORT eager (DeepSeek-V2-Lite short-ctx: 65.17 vs ORT 86.78 tok/s) purely due to per-token host-dispatch overhead, not GPU compute (CUDA-graph capture already hid it). Each eager op ended with a redundant trailing stream drain (kernel->kernel ordering is guaranteed by the single in-order EP stream; dtoh/dtod self-synchronize), and rotary/gather/ structural/GatherElements issued eager-only 24-byte validation D2H readbacks that block the pipeline. Add an env-gated (ONNX_GENAI_DEFER_EAGER_SYNC, default OFF) eager-fast path: - runtime: public synchronize() becomes a no-op when deferred; new private force_synchronize() keeps the real drain for dtoh/dtod host reads. - kernels: skip eager-only index/position validation D2H when deferred (these are numerically inert; the captured path uses the device error latch). DeepSeek eager 65.17 -> 89.80 tok/s (+37.8%), crossing ORT eager by +3.5%. GLM-4-9B dense eager 142.42 -> 145.31 (+2.0%). Graph=1 stays valid with the flag on (fallbacks=0). DeepSeek 24-tok golden lock passes BYTE-IDENTICAL with defer ON. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…atch) Flip defer_eager_sync to default ON so native eager decode is fast out of the box (65->91.6 tok/s on DeepSeek-V2-Lite, beating ORT eager 86.78) rather than only behind an opt-in flag. This makes eager consistent with the already-shipped captured (graph=1) production path, which likewise relies on the device error latch instead of per-op validation D2H. ONNX_GENAI_DEFER_EAGER_SYNC=0 (or false/off/no) remains an escape hatch that restores the old always-sync path. Re-validated with the flag UNSET (default ON): DeepSeek 24-tok golden lock BYTE-IDENTICAL; graph=1 stays valid (fallbacks=0); qwen2.5-0.5b dense logprobs byte-identical vs the old path; eager 91.64 tok/s. Escape hatch (=0) restores 66.06 tok/s old path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Records the #1383 native-eager-beats-ORT-eager default-ON win (DeepSeek 91.64 vs ORT 86.78, +5.6%; GLM +2%) and the #1379 Bug-1 regression guards in now.md. Also flags the pre-#1383 eager-vs-eager table as stale for the eager column. Docs-only. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

`cargo fmt --all -- --check` also fails on main (f72700a): Diff in crates/onnx-runtime-ep-cuda/src/runtime.rs:455 The `defer_eager_sync` initialiser added in #1383 (4e2a8b2) is one char over rustfmt's budget. Attributed with `git blame -L 450,458`. Folded into this PR rather than shipped separately because fmt and the cross-compile guard are two steps of the *same* required `Rust quality` job: a PR fixing only one of them still leaves that lane red, so neither could reach a green run on its own. This is the mechanical `cargo fmt --all` output, one file, one hunk, no behaviour change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…de (+35% eager tok/s) (#1427) ## Summary Raises the native **eager** decode "launch-bound floor" by removing the dominant per-token blocking scalar D2H. On clean dense GQA exports native eager sat at ~300 tok/s vs ORT eager 475; this lever lifts it to ~403 tok/s short-ctx (**+35.1%**) / ~367 deep-ctx (**+43.6%**), byte-identical. ## Root cause The floor is **not** the GQA kernel readback (that is already skipped in steady-state eager by the kernel's `warmed_signature` self-warming). The real cost is the **executor's dynamic-shape resolver**: `resolve_node_outputs` → `dynamic_output_shapes` reads GroupQueryAttention inputs 5 (`seqlens_k`) and 6 (`total_sequence_length`) back to host **per layer** (~56 blocking 4-byte D2H/token) to size the present-KV outputs. Each forces a host stall on the prior kernel. ## Fix (executor-only, 2 files) - **`dynamic_shapes.rs`** (GQA branch): `present_sequence` falls back to `past_key[2]` when `total_sequence_length` is absent. Native binds present-KV aliased in-place to past-KV at fixed physical capacity (`kernel_input_uses_physical_capacity`), so `present == max(past_key[2], total) == past_key[2]` always — host-known from the input shape. - **`dispatch.rs`** (`resolve_node_outputs`): skip materializing GQA integer shape-inputs **only when** the present-KV output (`outputs[1]`) is externally capacity-bound (`external.outputs`). A growing `past ⧺ current` GQA keeps the readback → fail-closed, correctness + generality preserved. `seqlens_k` is never used in GQA shape inference (pure waste). ## Generality Keys ONLY on structural shape conditions (GQA op + capacity-bound present output). **No model-family / head-size / head-count branch.** Lifts qwen / llama / glm / phi / deepseek eager decode uniformly (RULES.md §2). ## Escape hatch `ONNX_GENAI_GQA_SHAPE_ONDEVICE` (default-on; disable via `0`/`false`/`off`/`no`), mirroring #1383's `ONNX_GENAI_DEFER_EAGER_SYNC`. ## Measurements Export `deepseek-r1-distill-qwen-1.5b-int4-cuda` (1536h/28L GQA), H200 pinned idle, `ONNX_GENAI_CUDA_GRAPH=0` eager, greedy, medians-of-5, `profile_native --steady`: | ctx | ON (new default) | OFF (`=0`, old path) | delta | shape D2H/step | |-----|-----------------:|---------------------:|------:|---------------:| | short | **402.85 tok/s** | 298.12 | **+35.1%** | 1392→104 (13×) | | deep ~2600 | **367.31 tok/s** | 255.83 | **+43.6%** | — | vs ORT eager 475 (matched io-binding onnxruntime-genai harness, same export): native **0.85×** short — **did NOT cross 475** (honest gap −15% short / −23% deep). Residual is per-launch host dispatch: `cuLaunchKernel` unchanged at ~219/token — closing it needs kernel fusion / launch-batching, a separate larger lever. ## Correctness — golden locks (byte-identical to pre-change, default-ON, `--test-threads=1`) - DeepSeek-V2-Lite 24-tok lock (run ALONE, dodge #804): **PASS** (50.6s) - qwen3-0.6b Foundry golden: **PASS** (14.6s) - GLM-4-9B golden (block-128 GQA): **PASS** (22.2s) - Escape hatch `ONNX_GENAI_GQA_SHAPE_ONDEVICE=0` — qwen3 lock still **PASS** (old readback path restored) ## Notes ⚠️ Do **not** self-merge — coordinator validation requested. Recommend coordinator independently re-run the DeepSeek golden lock before admin-merge. Read-only otherwise: no kernel edits, no unrelated files. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Recovers coordinator `now.md` campaign-brain entries destroyed when an agent ran `git reset --hard origin/main` in the main checkout (uncommitted working-tree edits lost). Records the merged arc since #1383: **#1435** grid-fill narrow-N GEMV, **#1438** MAX_HEAD_DIM 256→512 (general), **#1442** gemma4-e2b dual head-size (256+512) e2e golden lock, **#1444** qwen3.5-2b-text context-scaling moat lock (3.03× deep), **#1445** capability-driven RMSNorm-fold gate + banked GLM structural verdict. Also updates the moat table (qwen3.5-2b recharacterized as a context-SCALING graph-block moat, not a fixed 1.65×) and the GLM verdict (structurally ORT-ahead at depth; stop forcing GLM levers). Doc-only. Co-authored-by: Copilot. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

justinchuby · 2026-08-19T10:54:22Z

Flagging a side effect of this change that surfaced while investigating an unrelated crash. Information only — the eager-decode win here is not in question, and I have not touched anything in this PR.

defer_eager_sync makes CudaRuntime::synchronize() a no-op, and five sites in crates/onnx-runtime-ep-cuda/src/weight_paging.rs (1404, 2950, 3232, 3347, 3449) were relying on that same synchronize() as their pre-cuMemUnmap barrier. Under the eager path those five stop draining the compute stream, so eviction can unmap a weight granule while a decode kernel is still reading it — use-after-unmap, surfacing as CUDA_ERROR_ILLEGAL_ADDRESS.

Those call sites never asserted the requirement explicitly; they inherited it from the general-purpose primitive. So this reads to me as a shared-primitive contract change reaching a distant caller, not a defect in the optimisation itself.

Evidence (RTX 4060 Laptop 8 GB, CUDA 13.1, qwen2.5-14b-onnx int4, VRAM_LIMIT=6GiB):

config	`synchronize()`	result
default (deferral on)	no-op	crashes
`ONNX_GENAI_DEFER_EAGER_SYNC=0`	real drain	12 tokens, byte-identical to known-good
`CUDA_LAUNCH_BLOCKING=1`	full serialize	identical tokens

The fault also moves between calls across runs — including cuEventCreate, which takes no pointer arguments — which is what identified it as an earlier async fault poisoning the context rather than a bad pointer at the reporting call.

Worth knowing beyond the crash: with stable-slot weights (#716) the VA is retained and physical granules are remapped under it, so a late kernel reading that VA after a remap would read the successor weight's bytes rather than faulting — wrong numbers, no crash. That path is inferred from the mapping mechanism, not directly observed as a wrong-number event, but it means weight-lending measurements taken with deferral on may be unreliable. (I audited our own benchmark docs against the build boundary; none of ours are affected — they predate 4e2a8b2e.)

Full chain, including the exoneration of #1325 which was the first suspect: #1439. A candidate fix is proposed there — draining unconditionally at the five pre-unmap sites via a dedicated drain_for_unmap(), leaving defer_eager_sync and this PR's optimisation untouched — offered for whoever owns this area to accept, modify or reject.

justinchuby · 2026-08-19T11:34:16Z

Correction to my earlier comment — I overstated the severity and the follow-up measurement refutes it.

I wrote that a late kernel reading a retained VA after a remap "would read the successor weight's bytes rather than faulting — wrong numbers, no crash", labelled as inferred from the #716 mapping mechanism. That inference is wrong, and a deliberate attempt to reproduce it failed.

Measured (RTX 4060, qwen2.5-14b-onnx int4, VRAM_LIMIT=6GiB, weight offload, unfixed binary, deterministic greedy workload):

11/11 runs crash, 0 diverge, 0 clean. Size-blind (default) arm 8/8 CUDA_ERROR_ILLEGAL_ADDRESS; the byte-aware arm that Does decode correctness depend on eviction ORDER? Byte-aware residency corrupted output (#886) — is that the change, or a latent assumption in the offload path? #888 flags as remap-prone 3/3 crash.
With the candidate fix, both arms produce byte-identical output — including a run where the Does decode correctness depend on eviction ORDER? Byte-aware residency corrupted output (#886) — is that the change, or a latent assumption in the offload path? #888 slotted-key-bypass actually fired (key=26, 8259 evictions).

Why the silent path does not exist here, from the code rather than from the run: stable slots are keyed by weight key, a slot's VA is reused only for the same key, and where two weights would collide on one key the admission path refuses rather than remaps — verbatim, "would corrupt the baked-pointer contract. Refuse rather than remap." So a stale read through a stable VA hits either decommitted physical (fault) or the same weight's byte-identical bytes. A different weight's bytes never occupy that VA. The 11/11 crash rate is consistent with the fault always landing in decommitted space; an out-of-bounds landing in live memory would have shown up as a non-crash divergence, and none did.

So the accurate severity is: a use-after-unmap that crashes loudly, with a byte-identical workaround — not "results on this path may be silently wrong". That is materially less alarming than what I said, and the correction is on #1439 as well.

The bound on this conclusion, stated honestly: two shipped configs were exercised (size-blind and byte-aware); every default-off diagnostic knob was not swept, so it holds for reachable shipped behaviour rather than universally.

Nothing else changes — #1455 remains available if you want it, ONNX_GENAI_DEFER_EAGER_SYNC=0 remains the workaround, and the eager-decode win in this PR is not in question.

github-actions · 2026-08-19T12:00:38Z

🔴 Benchmark Regression Detected

Comparison of criterion micro-benchmarks: PR head vs merge-base, measured on the same runner in the same job (base first → PR second).

ℹ️ Absolute times are informational only — they vary with runner load. The % change column is the reliable signal because both sides ran under identical conditions.

Status	Scenario	Base	PR	Change
🔴	`add/medium_f16_threads=1-internal/262144`	98.79 µs	171.99 µs	+74.1%
🔴	`add/medium_bf16_threads=1-internal/262144`	105.23 µs	138.37 µs	+31.5%
⚠️	`add/medium_f32_threads=1-internal/262144`	26.99 µs	31.69 µs	+17.4%
✅	`add/small_bf16_threads=1-internal/1024`	412.5 ns	470.6 ns	+14.1%
✅	`qwen3_sampling_processors/top_p_full_sort_after_top_k_baseline`	3.36 ms	3.83 ms	+14.0%
✅	`qwen3_sampling_processors/top_p_fast_after_top_k`	482.05 µs	535.53 µs	+11.1%
✅	`add/large_f32_threads=1-internal/4194304`	666.04 µs	734.22 µs	+10.2%
✅	`add/small_f32_threads=1-internal/1024`	200.6 ns	216.3 ns	+7.8%
✅	`qwen3_sampling_processors/top_k_partial_selection`	130.01 µs	138.67 µs	+6.7%
✅	`gather/medium_f32_threads=1-internal/32768`	3.90 µs	4.13 µs	+5.9%
✅	`block_quantized_moe_cached_dense/mxfp4_uncached_expert_dequant_each_call/rows=1,H=256,I=256,E=4,top_k=1`	348.62 µs	363.81 µs	+4.4%
✅	`block_quantized_matmul_cached_dense/mxfp4_cached_dense_repeated_call/1x1024x1024`	40.39 µs	41.93 µs	+3.8%
✅	`sampling_latency/greedy_per_token`	2.98 µs	3.04 µs	+1.9%
✅	`tokenization/decode_tokens_per_second`	5.66 ms	5.75 ms	+1.5%
✅	`block_quantized_matmul_cached_dense/mxfp4_uncached_dequant_each_call/1x1024x1024`	505.45 µs	512.83 µs	+1.5%
✅	`tokenization/encode_tokens_per_second`	350.69 µs	354.62 µs	+1.1%
✅	`add/small_f16_threads=1-internal/1024`	456.2 ns	460.2 ns	+0.9%
✅	`qwen3_sampling_processors/top_k_top_p_fast`	633.94 µs	638.31 µs	+0.7%
✅	`matmul/large_generic_bf16_threads=8/32x1024x1024`	1.25 ms	1.26 ms	+0.5%
✅	`gather/small_f32_threads=1-internal/4096`	670.7 ns	672.5 ns	+0.3%
✅	`qwen3_sampling_processors/top_k_top_p_full_sort_baseline`	5.24 ms	5.25 ms	+0.2%
✅	`sampling_latency/top_p_per_token`	350.46 µs	349.57 µs	-0.3%
✅	`sampling_latency/min_p_per_token`	194.57 µs	193.97 µs	-0.3%
✅	`matmul/medium_generic_f32_threads=8/32x512x512`	1.13 ms	1.12 ms	-0.3%
✅	`kv_cache/alloc_dealloc_pages`	36.43 µs	36.20 µs	-0.6%
✅	`grammar_masking/llguidance_compute_mask/32`	71.79 µs	70.39 µs	-1.9%
✅	`qwen3_sampling_processors/top_k_full_sort_baseline`	2.00 ms	1.95 ms	-2.3%
✅	`gather/small_bf16_threads=1-internal/4096`	506.5 ns	484.9 ns	-4.3%
✅	`block_quantized_moe_cached_dense/mxfp4_cached_dense_expert_repeated_call/rows=1,H=256,I=256,E=4,top_k=1`	53.96 µs	51.42 µs	-4.7%
✅	`sampling_latency/top_k_per_token`	51.56 µs	48.85 µs	-5.3%
✅	`matmul/medium_generic_bf16_threads=1/32x512x512`	539.26 µs	509.91 µs	-5.4%
✅	`gather/small_f16_threads=1-internal/4096`	517.9 ns	488.1 ns	-5.7%
✅	`reduce_mean/large_f32_threads=1-internal/262144`	1.11 ms	1.04 ms	-6.6%
✅	`block_quantized_matmul_cached_dense/mxfp4_preexpanded_dense_oncelock_like_proxy/1x1024x1024`	44.97 µs	41.48 µs	-7.8%
✅	`add/large_f16_threads=1-internal/4194304`	1.93 ms	1.78 ms	-7.9%
✅	`matmul/large_generic_bf16_threads=1/32x1024x1024`	2.05 ms	1.88 ms	-8.3%
✅	`gather/medium_bf16_threads=1-internal/32768`	2.44 µs	2.22 µs	-8.8%
✅	`logit_processing/seven_processor_chain_per_step`	341.83 µs	310.08 µs	-9.3%
✅	`reduce_mean/medium_f32_threads=1-internal/65536`	271.57 µs	241.87 µs	-10.9%
✅	`matmul/medium_generic_f16_threads=8/32x512x512`	30.85 µs	27.44 µs	-11.1%
✅	`matmul/large_generic_f32_threads=1/32x1024x1024`	10.89 ms	9.67 ms	-11.2%
✅	`matmul/small_generic_f32_threads=1/1x256x256`	40.00 µs	35.38 µs	-11.5%
🟢	`matmul/medium_generic_f32_threads=1/32x512x512`	2.66 ms	2.22 ms	-16.4%
🟢	`matmul/large_generic_f32_threads=8/32x1024x1024`	5.55 ms	4.59 ms	-17.2%
🟢	`matmul/large_generic_f16_threads=1/32x1024x1024`	90.96 µs	73.86 µs	-18.8%
🟢	`gather/medium_f16_threads=1-internal/32768`	2.78 µs	2.25 µs	-19.3%
🟢	`matmul/small_generic_bf16_threads=1/1x256x256`	42.91 µs	32.95 µs	-23.2%
🟢	`matmul/large_generic_f16_threads=8/32x1024x1024`	101.66 µs	77.49 µs	-23.8%
🟢	`matmul/medium_generic_f16_threads=1/32x512x512`	36.90 µs	28.03 µs	-24.0%
🟢	`add/large_bf16_threads=1-internal/4194304`	2.40 ms	1.80 ms	-25.1%
🟢	`reduce_mean/small_f32_threads=1-internal/4096`	20.69 µs	14.85 µs	-28.2%
🟢	`matmul/small_generic_f16_threads=1/1x256x256`	40.44 µs	28.37 µs	-29.9%
🟢	`matmul/small_generic_bf16_threads=8/1x256x256`	42.15 µs	28.90 µs	-31.4%
🟢	`matmul/small_generic_f32_threads=8/1x256x256`	53.76 µs	34.51 µs	-35.8%
🟢	`matmul/medium_generic_bf16_threads=8/32x512x512`	583.55 µs	360.06 µs	-38.3%
🟢	`gather/large_f16_threads=1-internal/131072`	17.90 µs	10.72 µs	-40.1%
🟢	`gather/large_bf16_threads=1-internal/131072`	16.45 µs	9.74 µs	-40.8%
🟢	`gather/large_f32_threads=1-internal/131072`	44.06 µs	24.16 µs	-45.2%
🟢	`matmul/small_generic_f16_threads=8/1x256x256`	59.54 µs	29.53 µs	-50.4%

Visual flags: ⚠️ ≥ 15% slower, 🔴 ≥ 30% slower — calibrated against measured runner noise (~27% worst-case on multi-threaded matmul)

Host info

CPU: Apple M1 (Virtual)
Cores: 3
OS: Darwin 25.5.0 arm64
Rust: rustc 1.97.1 (8bab26f4f 2026-07-14)
Load avg: { 3.17 3.23 4.94 }

What this cannot catch

Regressions in code paths not covered by these benchmarks (e.g., end-to-end decode with a real model)
Sub-threshold regressions that compound over multiple PRs
Performance changes that only manifest under GPU execution
Latency changes in the ORT integration path (these benchmarks exercise the native Rust kernels)

… path was falsified The doc-comment stated as fact that under stable-slot reuse (#716) a late read "silently returns the successor weight's bytes rather than faulting". That is the claim our 11/11 experiment refuted, and it was corrected on #1439/#1383. Leaving it in a comment would outlive the discussion and be read as settled fact. Restate what was actually established: - eliding the pre-unmap drain is a use-after-unmap, observed as CUDA_ERROR_ILLEGAL_ADDRESS (11/11 runs on the offload repro); - the silent successor-weight variant was hypothesised and FALSIFIED — stable slots are per-key and admission refuses rather than remaps on collision, so a stale read faults or returns the same weight's byte-identical bytes, never a different weight (11/11 crashed, 0 diverged); - reference #1439 for the full chain. Comment-only change; no behaviour change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Records Wallace's dense eager-vs-ORT-eager generality pass. Defer-sync lever generalizes; pure-eager mixed on dense; production auto-capture path wins everywhere. Docs-only. Co-authored-by: Copilot. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

… crash/silent corruption (#1439) (#1455) ## Summary Fixes the reproducible `CUDA_ERROR_ILLEGAL_ADDRESS` crash in the VMM weight-lending page-in path (#1439), and the silent weight-corruption it can cause. **Root cause.** Three shipped-path barriers in `weight_paging.rs` — the VMM release/evict free (`release_allocation`), admission eviction, and `admit()` eviction — drain the compute stream via `CudaRuntime::synchronize()` immediately before `cuMemUnmap`ing an evicted weight granule, so no in-flight decode kernel is still reading it. **#1383 (`4e2a8b2e`) changed that primitive's contract**: `synchronize()` became a **no-op** whenever `defer_eager_sync` is set, which is **on by default**. The pre-unmap drains are therefore silently elided, and eviction unmaps a granule while a decode kernel is still reading it — a **use-after-unmap**. Because the fault is an *asynchronous* out-of-bounds access, it surfaces later, at the first synchronising call — which is why the crash appears at varying, pointer-free sites such as `cuEventCreate(start)`. Worse, under stable-slot VA remapping (#716) the late read can land inside the *successor* weight's freshly-mapped physical granule and **silently return the wrong bytes instead of faulting**. **Why the fix is here and not in #1383.** #1383's optimisation is correct — eliding a *trailing per-op* eager sync is safe on a single in-order stream. What was wrong is that five call sites relied on `synchronize()` as an *unconditional pre-unmap barrier*, an assumption they never asserted. This PR makes that requirement explicit and local: it does **not** touch `defer_eager_sync` or anything in #1383. ## Change - Add `CudaRuntime::drain_for_unmap()` — an unconditional compute-stream drain that ignores the eager-sync deferral, documented as a correctness barrier. - Replace every explicit `self.runtime.synchronize()` barrier in `weight_paging.rs` with `drain_for_unmap()` (3 shipped-path pre-free sites + 3 default-off diagnostics: #945 pin-refill, #888 sync-before-fill, #945 pin-checksum). The 4 `copy_stream().synchronize()` calls are left untouched — they go direct and were never deferred. `+24 / -6` across 2 files. ## Verification (RTX 4060 8 GB, `qwen2.5-14b-onnx`, `ONNX_GENAI_VRAM_LIMIT=6GiB`, weight offload, `defer_eager_sync` **default-on**, same source base) Both directions, built from this exact source with only the fix stashed/unstashed between them: | Binary | Result | |---|---| | **unfixed control** | `CUDA_ERROR_ILLEGAL_ADDRESS` at `cuEventCreate(start)`, EXIT 1, ~90 s | | **fixed (this PR)** | 12 tokens, **byte-identical** to known-good `[96347, 3375, 724, 11, 358, 2776, 14589, 311, 6723, 429, 498, 3003]`, EXIT 0 | ## Cost The added drains fire **only when the page cache evicts/frees**. A model that fits in its VRAM budget evicts nothing and adds **zero** drains, and #1383's per-op eager win is untouched by construction (no decode-path per-op sync was changed). In the deliberately pathological offload-stress config above (4615 evictions, 6010 page-ins, 59.8 GB H2D over 12 tokens), total barrier time was `admit_sync 98.8 ms + vram_free_sync 7941 ms ≈ 8.0 s` over a **1085 s** run — **~0.7%**, dwarfed by the paging it protects. (`vram_free_sync_ms` is the pre-`cuMemUnmap` drain bracket, separate from the bookkeeping-dominated `vram_free_ms`; see #1439 / the vram-free attribution note.) ## Merge intent **Do not merge on my authority** — #1383 is not my change. This is a ready, verified, reversible fix posted so whoever owns #1383 can merge in one click or decide on an alternative. Until it lands, the workaround on #1439 is `ONNX_GENAI_DEFER_EAGER_SYNC=0` (byte-identical to known-good). Fixes #1439. --------- Co-authored-by: justinchuby <223556219+Copilot@users.noreply.github.com>

Sibling to §40, which I added earlier today. Where §40 covers **thresholds** that were correct in the regime they were calibrated in, this covers a **shared primitive changing contract** while distant callers break — because they depended on the old behaviour without ever asserting it. Two instances landed within a day of each other and produced five downstream failures between them. **#1383 made `CudaRuntime::synchronize()` a no-op** under `defer_eager_sync`. That is correct for its purpose: eliding a *trailing per-op* eager sync is safe, since the single in-order EP stream already preserves kernel-to-kernel ordering. But two other sets of callers used the same function for something else: - **Six pre-`cuMemUnmap` barriers in `weight_paging.rs`** needed a real drain. Without it, a granule is unmapped while an in-flight decode kernel still references its VA — use-after-unmap, `CUDA_ERROR_ILLEGAL_ADDRESS` on 11/11 runs of the weight-offload repro (#1439). Fixed in #1455 by giving those sites a dedicated `drain_for_unmap()` that *states* the requirement rather than inheriting it. - **Two `graph::tests` capture tests** assert `synchronize()` **errors** inside an active capture. With the no-op they get `Ok`, so they are red on default `main` and pass with `ONNX_GENAI_DEFER_EAGER_SYNC=0` — stale assertions against a changed contract, not a code defect. **Marlin M>1 default-on** changed what the M>1 decode path *is*, staling the `m == 1` capture-safe invariant (#1405) and two bit-exactness assertions that pass only with `ONNX_GENAI_MARLIN_M_GT_1=0`. **Neither change was wrong.** Both were measured, deliberate, and beneficial where their author was working. The cost came from **implicit dependence** — `weight_paging.rs` never said "I need a real drain here", it called the function that happened to provide one. And the failures surface in a different crate from the change, which is why every one of these was found while investigating something else: a crash in a paging path does not look like a decode optimisation. **Practice suggested**: name the guarantee at the call site when you depend on a specific behaviour of a general-purpose primitive (#1455's `drain_for_unmap()` is the shape); grep call sites across crates when changing one; and mark assertions that encode a *contract* rather than a *result*, so a stale expectation stays distinguishable from a real regression. That last distinction matters — updating a stale assertion is correct, weakening a live one to get green is how a real bug ships. All claims verified against `main` before writing: six `drain_for_unmap` call sites in `weight_paging.rs`, the two `synchronize().is_err()` assertions at `graph.rs:566` and `:843`, and `marlin_m_gt_1_enabled()` defaulting on. Docs-only. Co-authored-by: justinchuby <223556219+Copilot@users.noreply.github.com> Copilot-Session: d60eb808-7cc6-4abc-b48d-2a6dd3841624

…tch/teardown drains (#1788) ## Summary Working as Sebastian (Performance Engineer). Follow-up to #1777 for issue #82: investigates and resolves `QMoEKernel::execute()`'s unconditional eager-path `runtime.synchronize()` call. **Headline correction to the record**: `CudaRuntime::synchronize()` has been a deferred no-op by default since #1383 (merged before #1777 was authored), gated by `defer_eager_sync`. This means the trailing sync in `execute()` was **already inert in production** before this PR. An A/B test (remove the call vs. keep it) shows **zero measurable performance difference** — confirming this was never a live blocking point in the default configuration. #1777's causal claim that this call caused `host_us≈median_us` convergence for grouped M>1 shapes is falsified by this A/B test. **The real invariants** that call accidentally protected (only in configurations where the debug flag forces synchronous behavior, or in any future default change) are now handled explicitly with the runtime's existing unconditional-barrier primitive (`drain_for_unmap`, already used by `elementwise.rs`'s `BroadcastMetadataCache`) rather than a second synchronization authority: - `ScratchPool::ensure()`: drains before freeing a growing slot's old pointer (order fixed so a drain failure can't leak the fresh allocation — mirrors the `elementwise.rs` precedent). - `Drop for QMoEKernel`: best-effort drain before freeing scratch at teardown. `self.runtime.synchronize()` itself is **kept, not deleted**, at the end of `execute()`: it's a no-op by default, but keeping it preserves the `ONNX_GENAI_DEFER_EAGER_SYNC=0` debug escape hatch for QMoE, consistent with every other kernel in this EP (e.g. `MatMulNBitsKernel::run`). **A second, related bug** was found and fixed in the #1777 benchmark harness itself (`qmoe_gpu.rs`): its own inter-rep/post-warmup `runtime.synchronize()` calls were *also* no-ops, letting consecutive reps' async-enqueued kernels pile up in the CUDA launch queue until `cuLaunchKernel` itself blocked on a full queue. This queue-saturation effect — not a QMoE sync defect — is the actual mechanism behind #1777's `host_us≈median_us` observation for grouped shapes. Fixed by using `drain_for_unmap` (a real barrier) between reps/after warm-up. Stale doc comments describing the original (falsified) theory are corrected. ## Measurements (idle A100, `CUDA_VISIBLE_DEVICES=1`, merged #1777 probe, 25 reps) | shape | M | median_us (GPU) before | median_us after | host_us before | host_us after | |---|---|---|---|---|---| | deepseek-v2-lite | 2 | 674.6 | 674.9/674.6 | ~672 | 60–66 | | deepseek-v2-lite | 8 | 3308–3318 | unchanged | ~3300 | 60–66 | | glm-5.2 | 8 | 7150–7157 | unchanged | ~7150 | 60–67 | 3 independent "after" runs are consistent within noise; `median_us` (true GPU event time) is unchanged across all runs — confirming this is a pure measurement-methodology fix to the harness, not a compute-time change. No tok/s claims are made. ## Tests New regression tests in `qmoe_gpu.rs` (in the same commit): - `qmoe_scratch_pool_regrows_and_shrinks_across_calls_matches_cpu`: one long-lived kernel instance cycled through growing/shrinking row counts (`[1,8,8,2,8,4,16,1,16,2]`), with consecutive calls enqueued back-to-back (no intervening drain) so a missing `drain_for_unmap` in the growth path has a genuine chance to corrupt data; every step checked against the CPU oracle. - `qmoe_execute_does_not_block_host_for_device_completion`: asserts host-side `execute()` return time stays a small fraction of measured GPU completion time; fails if an unconditional blocking sync is reintroduced. - `qmoe_drop_after_inflight_launch_leaves_runtime_usable`: drops a kernel immediately after an async call with no readback, then verifies the runtime/stream remain healthy for independent subsequent work (both a fresh QMoE kernel and a plain htod/dtoh round trip). **Verified on idle A100 (device 1)**: `cargo test -p onnx-runtime-ep-cuda --release --features gpu-tests --test qmoe_gpu` → **37/37 passed** (34 existing + 3 new). `cargo clippy -p onnx-runtime-ep-cuda --release --features gpu-tests --test qmoe_gpu -- -D warnings` and `cargo fmt --check` are clean. ## Review An independent review pass (code-review agent) found 3 substantive issues in an earlier revision of this diff, all fixed and confirmed resolved by a follow-up review pass before this commit: 1. Two of the three new regression tests forced a full stream drain (via `dtoh`) immediately after every call, making them structurally unable to expose the bugs they claimed to guard against — fixed by splitting "enqueue" from "read back" (`PendingExecution`/`enqueue_with_existing_kernel`) so consecutive calls genuinely overlap. 2. A potential device-memory leak on the `drain_for_unmap()` error path in `ScratchPool::ensure()` (alloc happened before the fallible drain) — fixed by reordering to drain-before-alloc, mirroring `elementwise.rs`'s existing precedent. 3. A doc comment inaccurately claimed parity with `MatMulNBitsKernel` while actually diverging from it (having fully removed the debug-escape-hatch-respecting sync call) — fixed by reinstating `self.runtime.synchronize()` and correcting the comment. ## Scope No paging/residency, per-expert-identity, or exporter changes are included in this PR (explicitly out of scope per issue #82's follow-up plan). ## Next gate for #82 With this merged, the remaining #82 dependency chain is: (1) this PR — QMoE `execute()` async-safety + benchmark methodology fix [this PR], (2) the batch-M≥2 capture-eligibility fix (still open — QMoE grouped paths remain capture-ineligible; the "repeat-launch + CUDA-event" pattern used by the #1777 probe is the correct proxy until captured replay is supported for M>1), then (3) wiring `BlockQuantizedMoE` paging+prefetch (explicitly deferred, not touched here). This PR does not change capture eligibility; it only fixes the async-safety and measurement-methodology issues blocking trustworthy baselines for that next step. Per explicit user directive, this PR proceeds on local A100 validation (34+3 targeted tests, 3 independent benchmark runs, clippy/fmt, independent review) without waiting on remote CI, which remains supplementary asynchronous evidence. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

justinchuby and others added 2 commits August 19, 2026 03:48

justinchuby force-pushed the squad/native-eager-launch-overhead branch from 2166207 to 48cf0ab Compare August 19, 2026 03:58

justinchuby merged commit 4e2a8b2 into main Aug 19, 2026
6 checks passed

justinchuby deleted the squad/native-eager-launch-overhead branch August 19, 2026 04:05

justinchuby mentioned this pull request Aug 19, 2026

docs(now): record #1383 eager-beats-ORT-eager win + #1379 guards #1390

Merged

justinchuby mentioned this pull request Aug 19, 2026

style(cuda-ep): restore rustfmt compliance in CudaRuntime::new #1396

Closed

This was referenced Aug 19, 2026

VMM weight-lending page-in use-after-unmap crash (CUDA_ERROR_ILLEGAL_ADDRESS) — regression from #1383 (defer_eager_sync); WORKAROUND: ONNX_GENAI_DEFER_EAGER_SYNC=0 #1439

Closed

docs(now): reconstruct arc #1435→#1445 campaign-brain entries #1447

Merged

justinchuby mentioned this pull request Aug 19, 2026

fix(cuda): drain compute stream before weight unmap — use-after-unmap crash/silent corruption (#1439) #1455

Merged

justinchuby mentioned this pull request Aug 19, 2026

docs(benchmarks): record the implicit-contract-breakage pattern #1469

Merged

justinchuby mentioned this pull request Aug 23, 2026

fix(cuda): replace QMoE's incidental trailing sync with explicit scratch/teardown drains #1788

Merged

Navigation Menu

Repository navigation

perf(cuda-ep): make native eager decode beat ORT eager (DeepSeek +37.8%, crosses ORT +3.5%) - #1383

perf(cuda-ep): make native eager decode beat ORT eager (DeepSeek +37.8%, crosses ORT +3.5%)#1383
justinchuby merged 2 commits into
mainfrom
squad/native-eager-launch-overhead

justinchuby commented Aug 19, 2026 •

edited

Loading

Uh oh!

Uh oh!

justinchuby commented Aug 19, 2026

Uh oh!

justinchuby commented Aug 19, 2026

Uh oh!

github-actions Bot commented Aug 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

justinchuby commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Root cause

Fix (default ON; ONNX_GENAI_DEFER_EAGER_SYNC=0 escape hatch)

Results (GPU3, greedy, medians-of-5, pinned idle GPU, flag UNSET = default ON)

Validation (flag UNSET, default ON)

Recommendation

Uh oh!

Uh oh!

justinchuby commented Aug 19, 2026

Uh oh!

justinchuby commented Aug 19, 2026

Uh oh!

github-actions Bot commented Aug 19, 2026

🔴 Benchmark Regression Detected

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

justinchuby commented Aug 19, 2026 •

edited

Loading

Fix (default ON; `ONNX_GENAI_DEFER_EAGER_SYNC=0` escape hatch)