diff --git a/.agents/issue-index.md b/.agents/issue-index.md index 6ed3766c5..d96518154 100644 --- a/.agents/issue-index.md +++ b/.agents/issue-index.md @@ -766,6 +766,10 @@ rather than merged. `scripts/check-agent-record.py` gates both. | [#2015](https://github.com/mudler/vllm.cpp/issues/2015) | `ENG-HYBRID-PLACEMENT` | Map llama.cpp's four placement surfaces (`-ot` general, `-cmoe` and `-ncmoe N` as sugar over it, `--fit` auto) onto the `vllm_cpp` extension key of `--offload-config` instead of a bespoke flag, per developer direction 2026-08-26, and re-anchor the spec at the recorded pin `b10451` — `237ad9b96` was superseded on 2026-08-22 and none of its cited line numbers resolve | record | | [#2005](https://github.com/mudler/vllm.cpp/issues/2005) | `BACKEND-TENSTORRENT` | **No Tenstorrent sibling of `gpu_clock_state.py` existed, so every TT speed figure on record was clock-unattributed — including #2003's inversion, which could have been a clock excursion.** Landed `tools/bench/tt_clock_state.py` (+`tt_refold_busy.py`): AICLK via 430 ms `tt-smi -s` snapshots at 1 Hz, NVIDIA thresholds copied constant-for-constant (spread 5%, cross-arm median AND mean 1%, 30 busy samples, majority-busy), stop-only summaries (#1657 shape), NOT-APPLICABLE fields stated (no persistence/applications knobs; no live throttle bitmap → carried caveat), claimed-max as provenance-carrying argument, busy proxied by the leg pid holding `/dev/tenstorrent` fds checked live per interval, machine identity compared unconditionally even under a boot waiver. First wired use re-measured #2003 same-day: the P150 AICLK governor proved TWO-STATE (800 idle / pegged cap under load); raw windows refuse within-run spread 40.74% by construction of that platform, and the live-recorded busy flags enable a criterion-independent busy-slice refold in which **all six windows carry exactly one distinct value {1350}, spread 0.00%, judge PASS** — the inverted ratio is clock-attributed parity. Owed: verified claimed-max pin (1350 is class folklore today), in-process pyluwen sampler for sub-second cadence, and a policy decision on whether two-state governors deserve a spread rule scoped to busy slices inside the tool itself rather than the offline refold script | perf | | [#2032](https://github.com/mudler/vllm.cpp/issues/2032) | `ENG-HYBRID-PLACEMENT` | W3a: `MoePlacementPlan`, resolving a name-keyed `DevicePlacement` to the per-layer decision the forward can actually read, against llama.cpp's own GGUF tensor spelling — and REFUSING a partial placement, where a layer's gate, up and down do not share a device, because the MoE block runs one grouped GEMM over the three | feature | +| [#2029](https://github.com/mudler/vllm.cpp/issues/2029) | `ENG-CUDAGRAPH-BREAK` | With DFlash2 speculation OFF the engine dies at c=8 in CUDA graph capture: `cudaMalloc: operation not permitted when stream is capturing`. Located statically: `Pool(b).PreGrowForCapture(b, s.demand)` — the #1380 capture pre-grow — sits INSIDE `if (dbuf)` in both Qwen3.5 decode-graph drivers (`qwen3_5.cpp:10885/10907`, `:11439/11461`), and `dbuf = impl_->dbuf \|\| spec_step` is false on the DEFAULT server, where `VT_ASYNC_EXECUTOR` is unset and no step is speculative. So the pre-grow is exactly the "path taken only when speculation is off" the issue names, by its absence. Spec [cudagraph-pregrow-nonspec.md](specs/cudagraph-pregrow-nonspec.md) | bug | +| [#2035](https://github.com/mudler/vllm.cpp/issues/2035) | `ENG-CUDAGRAPH-BREAK` | Seven decode-graph drivers (`qwen3.cpp`, `qwen3_moe.cpp`, `deepseek_v2.cpp`, `deepseek_v4.cpp`, `voxtral.cpp`, `laguna.cpp`, `qwen3_dflash.cpp`) open a `vt::GraphCaptureScope` with no `DevicePool::PreGrowForCapture` and no demand profile at all — only `qwen3_5.cpp` uses any of the #1380 machinery. Found while fixing #2029; owed under `## Owed` in [cudagraph-pregrow-nonspec.md](specs/cudagraph-pregrow-nonspec.md) | bug | +| [#2036](https://github.com/mudler/vllm.cpp/issues/2036) | `ENG-CUDAGRAPH-BREAK` | `DenseAlignFor` (`qwen3_5.cpp:2825-2849`) allocates five blocks and calls `d.b.Synchronize(d.q)` at `:2846` on an `M` miss, and `EnsureCtmp` (`cuda_marlin_dense.cu:74-89`) grows with `cudaMallocAsync` at `:85`, both with no `cudaStreamIsCapturing` refusal — unlike the six sibling shape-keyed caches that have one. Latent today (the cold step visits the same key), found while fixing #2029; owed under `## Owed` in [cudagraph-pregrow-nonspec.md](specs/cudagraph-pregrow-nonspec.md) | bug | +| [#2037](https://github.com/mudler/vllm.cpp/issues/2037) | — | `EngineDeadError` promises "See stack trace (above)" (`include/vllm/v1/engine/core_client.h:63`) and the fatal handler prints only `e.what()` (`src/vllm/v1/engine/core_client.cpp:36-38`), so no trace is ever emitted. #1380 closed only because somebody instrumented `CudaBackend::Alloc` by hand; #2028 and #2029 both record the gap. Owed under `## Owed` in [cudagraph-pregrow-nonspec.md](specs/cudagraph-pregrow-nonspec.md) | bug | | [#2026](https://github.com/mudler/vllm.cpp/issues/2026) | `ENG-HYBRID-PLACEMENT` | W3b: route a placed layer's routed-expert compute to the placement device, with the activation round trip at the boundary — and refuse the fp4-resident arm, whose device residents are built eagerly at load so placing it would upload every expert and then compute across the bus, a defect a token gate cannot see | feature | | [#2040](https://github.com/mudler/vllm.cpp/issues/2040) | `ENG-HYBRID-PLACEMENT` | W3c: replace W3b's hand-written per-model round trip with ONE shared seam every architecture routes through, exploiting that every MoE block already has the same `(Dev, weights, params, [T,H] dh, T)` shape — and correct `docs/ENVIRONMENT.md` and `docs/FEATURES.md`, which W3b made false by leaving them saying the knobs place nothing | bug | | [#2049](https://github.com/mudler/vllm.cpp/issues/2049) | `MODEL-MM-QWEN4-EXP` | **The row spec's settled-decisions list stated the REFUTED QSA mapping**, telling a fresh W4/W5 implementer to build QSA on MiniMax-M3 and calling the DeepSeek-V4 lane "the wrong port" — the exact reverse of the correction recorded in the same file's Port map and Design section, in the matrix row, and in [#1978](https://github.com/mudler/vllm.cpp/issues/1978). Pre-existing on `main`; found while reviewing the W6a merge ([#2019](https://github.com/mudler/vllm.cpp/pull/2019)) and fixed in that same flow per AGENTS.md "Every change starts from an issue". Load-bearing rather than cosmetic: item 2 sits in the section written so an implementer does NOT re-derive it, and a top-down reader hits the stale instruction before the corrected Design section. | doc | @@ -774,6 +778,7 @@ rather than merged. `scripts/check-agent-record.py` gates both. | [#1984](https://github.com/mudler/vllm.cpp/issues/1984) | `SAMPLE-CORE` | `RandomSampleKernel` is launched `<<>>` and scans a 248,320-wide vocab on one thread per row, computing two `SplitMix64` rounds and an f64 `log` per element. Eleven lines above it the same file records that a single-block single-thread scan of a ~151k vocab cost ~7.5 ms/token, which is why greedy argmax was rewritten into `ArgmaxPartialKernel`/`ArgmaxFinalKernel`; the Gumbel draw never got that treatment. Upstream is whole-tensor (`vllm/v1/sample/ops/topk_topp_sampler.py::sample_with_exponential_noise`), so this is a mirror obligation. Reached by every non-greedy row through `ModelRunner::execute_model` -> `Sampler::forward` -> `vt::RandomSample`. Spec: [sample-gen-config-and-parallel-gumbel.md](specs/sample-gen-config-and-parallel-gumbel.md) | perf | | [#1985](https://github.com/mudler/vllm.cpp/issues/1985) | `SAMPLE-CORE` | `generation_config.json` is read for `eos_token_id` only (`hf_config.cpp::ReadGenerationConfigEosIds`), so `Qwen/Qwen3.8-27B`'s shipped `top_k: 20` / `top_p: 0.95` never reach `SamplingParams` and `to_sampling_params` resolves omitted knobs straight to the neutral OpenAI defaults, which disable both filters. vLLM applies them through `ModelConfig.get_diff_sampling_param` -> `OpenAIServing*.default_sampling_params` -> `to_sampling_params`. Since `vllm bench serve` stopped sending `--temperature`, both engines sample at temperature 1.0 and vLLM draws from 20 candidates while we draw from 248,320: different sampling on two sides of a parity benchmark. Spec: [sample-gen-config-and-parallel-gumbel.md](specs/sample-gen-config-and-parallel-gumbel.md) | bug | | [#2002](https://github.com/mudler/vllm.cpp/issues/2002) | `SAMPLE-CORE` | With `--speculative-config` set, `GPUModelRunner::sample_tokens` branches on `num_draft_tokens > 0` alone and returns the greedy-only `RejectionSampler`'s output, so `Sampler::forward` and `vt::RandomSample` are never called and a `temperature: 1.0` request decodes GREEDILY. `include/vllm/v1/spec_decode/rejection_sampler.h` states the contract it violates in its own deferral list ("a temperature > 0 request must NOT be routed here yet"); neither the runner nor `RejectionSampler::forward` enforces it. Found while writing #1984's acceptance measurement against a baseline recipe carrying `--speculative-config`, where the sampler under test would never have been launched and the null result would have read as "the change did nothing" | bug | +| [#2059](https://github.com/mudler/vllm.cpp/issues/2059) | — | `.github/workflows/ci.yml:1598` sets `VT_POOL_BYPASS: "1"` for BOTH `sanitize-cpu` lanes, so the `DevicePool` free list, size-class ladder, best-fit borrow (#1922) and capture pre-grow (#1380) are unexecuted under ASan AND TSan. The stated justification is ASan's `detect_leaks`; ThreadSanitizer has no leak detector and gains nothing. MEASURED: under `-DVLLM_CPP_SANITIZE=thread` with the pool ENABLED, `test_qwen3_5_decode_graph_seam` is 10/10, 156 assertions, exit 0, zero TSan warnings. Found while repairing the #2047 red; owed under `## Owed` in [cudagraph-pregrow-nonspec.md](specs/cudagraph-pregrow-nonspec.md) | bug | | [#2055](https://github.com/mudler/vllm.cpp/issues/2055) | `ENG-UPSTREAM-LTX2-PIN` | `tools/oracle/ltx2_oracle.py` asserts the LTX-2 oracle's revision and its resolved `ltx_core` / `ltx_pipelines` origins in the PARENT, then renders in a child started with `python -m`, which puts the CURRENT WORKING DIRECTORY on that child's `sys.path[0]`. The parent's `importlib.util.find_spec` never consults it, so a directory holding a decoy `ltx_pipelines`, made the CWD, is imported by the process that loads the weights while the process that checked identity sees nothing. MEASURED by a fresh reviewer of [#2053](https://github.com/mudler/vllm.cpp/pull/2053) on a scratch copy: the parent printed `IDENTITY_OK` naming the pinned tree and the child then printed `DECOY ti2vid_one_stage RUNNING -- the parent's assert never saw me`. This is the `.agents/specs/ltx-2-5.md` §7.0(b) decoy failure surviving in the one process that touches weights. The 2026-08-27 reference render is NOT affected and its `gateable = yes` stands: `render.sh` issues no `cd`, `/workspace/ltx2-oracle/` holds no `ltx_*` package, and the committed manifest records module origins inside the pinned clone — so the defect is an OVER-CLAIMED guarantee, not a wrong measurement. Fix: `-P` (or `PYTHONSAFEPATH=1`) on the child plus an explicit `cwd=`, with the reviewer's decoy as the red-first test. NOT fixed in flow and deliberately: the script's sha256 equals the one the worker executed and printed, and that equality is the provenance chain `.agents/oracles/ltx-2.md`'s `gateable = yes` rests on, so editing the file for a hardening that changed no result would trade a verifiable fact for a better comment. Owned by `ENG-UPSTREAM-LTX2-PIN` and listed under `## Owed` in [oracle-ltx-2-pin.md](specs/oracle-ltx-2-pin.md) | bug | | [#2056](https://github.com/mudler/vllm.cpp/issues/2056) | `MODEL-MM-QWEN4-EXP` | **`check-agent-record.py` accepts TWO claim files owning the same matrix row**, so a claim collision merges clean and silent. Measured on this branch: copying W6a's `CLAIM-MODEL-MM-QWEN4-EXP.md` beside W1's `CLAIM-MODEL-MM-QWEN4-EXP-W1.md` gives `agent record OK`, rc=0, with both files asserting ownership of `MODEL-MM-qwen4-exp-qwen4-exp-for-conditional-generation` and both marked `ACTIVE`. Git cannot conflict on it because the two sides touch different PATHS. The matrix owner cell holds exactly ONE value, so the record goes silently ambiguous. Resolved here by merge ORDER, which is an operator remembering rather than a gate. NOT fixed in flow: it changes checker semantics and owes its own row, spec and red-before test per AGENTS.md. Listed under `## Owed` in [qwen4-exp-flash-next.md](specs/qwen4-exp-flash-next.md). | bug | | [#1973](https://github.com/mudler/vllm.cpp/issues/1973) | `KV-DSV4-MULTICACHE` | **`MakeDeepseekV4KVCache` publishes ONE placeholder group for a model that needs 167 caches, and the runner would drop the real topology in silence.** W2 of [#1925](https://github.com/mudler/vllm.cpp/issues/1925), after W1 ([#1960](https://github.com/mudler/vllm.cpp/issues/1960)) landed the allocation metadata and published none of it. `deepseek_v4_registry.cpp:126-148` emits one `"mla"` group at `head_size = head_dim + qk_rope_head_dim = 576` and calls itself a STUB; upstream at the pin `5559679229bc961848b121ccdeaa8fa5d79bec98` publishes **167 entries in seven groups** across DeepSeek-V4-Flash's 43 layers -- 21 C4A + 20 C128A compressed latents and 21 indexer key caches as `MLAAttentionSpec` (`vllm/models/deepseek_v4/attention.py:631-645`, `:669-684`), 43 SWA caches (`vllm/v1/attention/backends/mla/sparse_swa.py:86-101`) and 21 + 21 + 20 compressor states (`vllm/models/deepseek_v4/compressor.py:188-200`) as `SlidingWindowMLASpec`, at block sizes 256, 64, 4 and 8. **The factory is REACHED from production** -- it is `ModelFactory::make_kv_cache` (`:120`), called from `LoadedEngine`'s constructor through `MakeKVCacheResolved`/`MakeKVCacheMaybeSpec` (`src/vllm/entrypoints/model_loader.cpp:1394-1404`, `:1681`) -- so what it publishes reaches `GPUModelRunner::initialize_kv_cache` unfiltered, and that loop (`src/vllm/v1/worker/gpu/runner.cpp:577-597`) has two arms and no `else`: `kSlidingWindowMla` matches nothing and a SECOND `kMlaAttention` group is passed over by the `full_attn_group_id_ < 0` guard, with no diagnostic either way. **Worse than one dropped group, and this is what W1 could not see from the selection loop alone:** `membership_by_name` is set only inside `if (has_mamba_group)` (`runner.cpp:820-845`), DeepSeek-V4 has no Mamba group, so the allocation loop falls into `is_full_attn = !is_gdn` and allocates ONE buffer per HIDDEN LAYER sized from the target group's page -- 43 buffers of one page for a model needing 167 of seven. A silently short KV allocation is a wrong-tokens failure, not a crash. FIXED by publishing the seven groups WITH a `VT_CHECK` refusal naming any published group the runner does not allocate, which is `AGENTS.md`'s "refuse an unimplemented arm with a message that names the missing part" rather than a new policy. **Consequence stated rather than implied: DeepSeek-V4 on the server path now refuses at engine construction instead of running.** What it loses is a full-recompute path with no decode step -- `Forward`/`ForwardDevice` discard `attn_kv` (`src/vllm/model_executor/models/deepseek_v4.cpp:2886-2887`, `:2959-2960`) -- and `examples/deepseek_v4_gen` does not go through the runner and is unaffected. Two further refusals are NOT in scope and are named so nobody takes them for this: `HybridKVCacheCoordinator`'s uniform-block-size `assert`, inert under `NDEBUG` (`src/vllm/v1/core/kv_cache_coordinator.cpp:340-346`, W4), and the missing third `ModelForwardInput` channel (`include/vllm/model_executor/models/model_registry.h:303-315`, W3). Nothing published is consumed; reachability is owed to W3 and W5 under `## Owed` in [kv-dsv4-multicache.md](specs/kv-dsv4-multicache.md) | bug | diff --git a/.agents/specs/cudagraph-pregrow-nonspec.md b/.agents/specs/cudagraph-pregrow-nonspec.md new file mode 100644 index 000000000..adc22914e --- /dev/null +++ b/.agents/specs/cudagraph-pregrow-nonspec.md @@ -0,0 +1,361 @@ +# ENG-CUDAGRAPH-BREAK — the capture pre-grow runs only when the double buffer or a speculative step turns it on, so the DEFAULT server captures against an unprepared pool ([#2029](https://github.com/mudler/vllm.cpp/issues/2029)) + +Row: `ENG-CUDAGRAPH-BREAK` ([`.agents/engine-matrix.md`](../engine-matrix.md)). +Issue: [#2029](https://github.com/mudler/vllm.cpp/issues/2029). +Parent wave: [`eng-cudagraph-break.md`](eng-cudagraph-break.md), whose #1380 fix +([`7dec1d990`](https://github.com/mudler/vllm.cpp/commit/7dec1d990)) built the +pre-grow this wave makes reachable. + +## The finding + +With DFlash2 speculation **off**, the engine dies at concurrency 8: + +``` +engine-fatal: EngineCore busy loop threw: vt cuda: cudaMalloc: operation not permitted when stream is capturing +``` + +and every later request returns 500 `[request submitted to a stopped AsyncLLM]`. +The same binary with `--speculative-config` warms and serves c=1/2/4 cleanly, so +#2029 records that "the failing allocation is on a path taken only when +speculation is off". That is exactly true, and the path is the **absence** of the +#1380 capture pre-grow. + +`Qwen3_5DenseDecodeGraph::Step` and `Qwen3_5DecodeGraph::Step` both compute + +``` +const bool dbuf = impl_->dbuf || spec_step; // qwen3_5.cpp:11315, :10764 +``` + +with `impl_->dbuf = enabled && DecodeGraphDoubleBufferEnabled()` (`:11082`, +`:10529`) and `DecodeGraphDoubleBufferEnabled()` returning false unless +`VT_ASYNC_EXECUTOR=1` (`:10302-10308`), and +`spec_step = gdn_meta.num_spec_decodes > 0` (`:11267`, `:10715`). + +The pre-grow sits **inside** `if (dbuf)`: + +| Driver | `if (dbuf)` | `PreGrowForCapture` | +|---|---|---| +| `Qwen3_5DecodeGraph` (35B MoE) | `qwen3_5.cpp:10885` | `qwen3_5.cpp:10907` | +| `Qwen3_5DenseDecodeGraph` (27B dense) | `qwen3_5.cpp:11439` | `qwen3_5.cpp:11461` | + +The 27B dense driver is the one #2029 measured. The #1574 subject is a Qwen3.8-27B +checkpoint, which declares `Qwen3_5ForConditionalGeneration` and registers onto +`kQwen3_5DenseFactory` (`src/vllm/model_executor/models/qwen3_5_dense.cpp:283`, `:296`, +`:259-266`); its decode dispatch is `Qwen3_5DenseDecodeGraph::Step` at +`qwen3_5_dense.cpp:223-232`. #1380's own repro was this same driver reporting this same +message. + +So on the **default** server — no `VT_ASYNC_EXECUTOR`, no speculation — `dbuf` +is false, the pre-grow never executes, and the driver opens +`vt::GraphCaptureScope` over a `DevicePool` nobody prepared. A `Get` miss inside +that region is `DevicePool::Get`'s `b.Alloc(key)` +(`include/vllm/model_executor/models/device_pool.h:212`), which on CUDA is a raw +`cudaMalloc` (`src/vt/cuda/cuda_backend.cu`) and produces exactly the message +#2029 reports. #1380 named the same message from the same driver. + +The dependence on batch size follows: each captured `SizeSlot` retains its +`[S, vocab]` f32 logits and its `[S, H]` bf16 hidden for the life of the process, +and `DevicePool` is keyed by SIZE CLASS rather than by tensor, so every +additional padded size a ramping server captures removes blocks from classes a +later capture needs. At c=1/2 the free list happens to be deep enough; at c=8 it +is not. That is #1380's mechanism, one axis over. + +`s.demand` — the cold step's per-class peak — is recorded **unconditionally** +(`:11607`, `:11046`), and `Pool(b).MarkStepBoundary()` runs unconditionally at +the top of both `Step`s (`:11251`, `:10697`). Only the consumer is gated. The +gate is an accident of history: the block that now holds the pre-grow was +introduced for the `VT_ASYNC_EXECUTOR` parity ring, and #1393 replaced the +single-block pre-grow in place without revisiting the guard it sat under. + +## The trigger is not the batch size, and a second measurement says so + +Measured 2026-08-26 on `dgx:gpu0` (GB10) by the operator, same binary +`3d895a202`, `--num-blocks 3744 --max-num-seqs 16 --max-model-len 8192`, 1024 in +/ 512 out, `--speculative-config` removed, **plus `--enable-prefix-caching +--scheduling-policy lpm`**: the identical fault fires at **concurrency 1**. + +``` +engine-fatal: EngineCore busy loop threw: vt cuda: cudaMalloc: operation not permitted when stream is capturing +``` + +`stream is capturing` x4, `illegal memory access` 0, `position discontinuity` 0, +`server: prefix caching enabled` present, zero `dflash` mentions. Rung +`ok=0 failed=8`. + +**This wave's finding is unchanged by it, and predicts it.** The gap is that the +pre-grow does not run when `dbuf` is false, and `dbuf` is false on EVERY +non-speculative step at EVERY padded size. Nothing about it is specific to +`S = 8`. What the two configurations differ in is only whether the free list +happens to be short when the capture opens, which is a property of the pool's +state and not of the captured shape. This wave's own gate is a **single-request, +`S = 1`** case, and it goes red. + +**The rival hypothesis — a buffer sized from a quantity that varies per step — +was checked against the code and does not hold on this driver.** + +| Candidate | Why not | +|---|---| +| `block_table_num_cols` / `max_blocks` | fixed once by `BlockTable`'s constructor from `max_model_len` (`src/vllm/v1/worker/gpu/block_table.cpp:49`) and read unchanged by `gather_block_table` (`runner.cpp:1185-1187`). It cannot move between two steps, and prefix caching does not touch it | +| FA-2 decode scratch | `DecodeShapeKey` is `{batch, hq, heads, groups, head_dim, max_blocks, page_size, num_splits}` (`src/vt/cuda/cuda_flash_attn_fa2.cu:1013`), all key-determined or constant — and it carries its OWN capture refusal at `:1021-1029` with a distinct message, so it cannot produce this one | +| `seq_lens` / `max_seq_len` | values and a host grid bound; `seq_lens` is a `[num_reqs]` buffer refreshed in place | +| per-request counts | `BuildPaddedDecode` rewrites `num_reqs = S` on the non-spec path (`qwen3_5.cpp:10233-10261`) | +| the forward's knowledge of prefix caching | it has none: `enable_prefix_caching` appears nowhere in `src/vllm/v1/worker/gpu/runner.cpp`'s forward path | + +So the quantity that varies is the pool's free-list state. **Which SIZE CLASS is +short in either run is not determined here and cannot be from a CPU box**; see +`## Owed` for the three device experiments that would settle it, and note that +the fix does not depend on the answer — an unprepared pool is unprepared whatever +empties it. + +Not the same fault as [#2042](https://github.com/mudler/vllm.cpp/issues/2042), +which is prefix caching plus DFlash2 dying at c=1 on the draft's position +invariant. With speculation ON, prefix caching fails through #2042; with +speculation OFF, through this. + +## Scope + +**In.** Make the capture pre-grow run before **every** capture in both Qwen3.5 +decode-graph drivers, and gate the guarantee it delivers — that the captured +region performs no driver allocation — on a non-speculative step with the double +buffer off. + +**Out.** [#2028](https://github.com/mudler/vllm.cpp/issues/2028) (the illegal +memory access with speculation ON), [#2007](https://github.com/mudler/vllm.cpp/issues/2007) +(two pools), the `P == 1` capture gate at `qwen3_dflash.cpp:1577`, the missing +backtrace at the fatal handler ([#2037](https://github.com/mudler/vllm.cpp/issues/2037)), +and the seven other decode drivers that have no pre-grow at all +([#2035](https://github.com/mudler/vllm.cpp/issues/2035)). Each is named under `## Owed`. + +## Upstream + +vLLM has no analogue to port. Its decode graphs come from `torch.compile` and its +activations from torch's caching allocator, which is warmed by +`capture_model()`'s dummy runs over every `cudagraph_capture_sizes` bucket before +any capture (`vllm/v1/worker/gpu_model_runner.py::_dummy_run` / +`capture_model` @ pin `5559679229`). The warm-then-capture ordering this tree's +cold step implements is that shape; the pre-grow is the part torch's allocator +does for free and ours does not, because our pool is class-keyed with no split +and no per-block best fit over the whole cache. So this is a repair of our own +seam, not a parity gap, and nothing about the pool's design changes here. + +## Design + +One statement moves in each driver: `Pool(b).PreGrowForCapture(b, s.demand);` +leaves the `if (dbuf)` block and runs immediately before it, on the `s.warm` +capture path. + +Everything else stays inside `if (dbuf)`: `b.Synchronize` drains an in-flight +replay that only the parity ring can leave behind, and the `StepDevInputs` / +`s.pin` construction is the persistent-input path the ring and the spec capture +need. On the `!dbuf` arm `DenseForwardLayers` is called with +`persistent_sdi == nullptr` (`:11531`, `:10974`), so it builds its step inputs +from the main pool **inside** the captured region — which is why that arm needs +the pre-grow at least as much as the `dbuf` arm, and why the demand profile is +exact for it: the cold step takes the identical `nullptr` branch, so the cold +step's per-class peak IS the capture's per-class demand. On the `dbuf` arm the +capture's main-pool demand is a strict subset of the cold step's, which is the +containment #1393 recorded. + +Cost on the newly covered path: `PreGrowForCapture` is idempotent and a no-op +once the free list is deep enough, so a warm server pays nothing after the first +capture at each shape. + +## Risks and decisions + +- **D1. Move the call rather than widen `dbuf`.** `dbuf` selects the parity ring + and the persistent-input path; making it true off the async lever would change + what is captured. The pre-grow is unrelated to both. +- **D2. Do not add a pre-grow to the other seven drivers here.** `qwen3.cpp`, + `qwen3_moe.cpp`, `deepseek_v2.cpp`, `deepseek_v4.cpp`, `voxtral.cpp`, + `laguna.cpp` and `qwen3_dflash.cpp` neither pre-grow nor record a demand + profile, so each needs its own `MarkStepBoundary` / `StepDemandProfile` wiring + and its own gate. Filed as + [#2035](https://github.com/mudler/vllm.cpp/issues/2035), not fixed in flow: it is a + different change with a different blast radius, and #2029's subject is the Qwen3.8-27B + target. +- **D4. Two unguarded shape-keyed caches found on the same path, filed not fixed.** + `DenseAlignFor` (`qwen3_5.cpp:2825-2849`) allocates five blocks AND calls + `d.b.Synchronize(d.q)` at `:2846` on an `M` miss, and `EnsureCtmp` + (`src/vt/cuda/cuda_marlin_dense.cu:74-89`) grows with `cudaMallocAsync` at `:85` — both + with no `cudaStreamIsCapturing` refusal, unlike the six sibling caches that have one. + Neither is #2029's cause: both are keyed by a quantity the cold step already visits at + the same shape. [#2036](https://github.com/mudler/vllm.cpp/issues/2036). +- **D3. The soft-cap hazard #1393 recorded is unchanged.** + `PreGrowForCapture` ignores `device_pool_cap_bytes` and every platform resolves + it to 0. Running the pre-grow on more paths does not reach it; it widens who + would meet it if a non-zero cap were ever set, which that platform already owes. + +## Tests + +`tests/vllm/models/test_qwen3_5_decode_graph_seam.cpp`, one new case per driver, +on the existing `decode_graph_seam_harness.h` CPU seam. + +The assertion is the guarantee, not the call: **zero driver allocations occur +between `BeginCapture` and `EndCaptureGraph`** on a step where +`num_spec_decodes == 0` and `VT_ASYNC_EXECUTOR` is unset. The harness backend +already counts `Alloc`; this wave adds the capture-window split +(`allocs_during_capture()`), which is one counter incremented under a flag the +existing `BeginCapture` / `EndCaptureGraph` overrides set. + +The pool is **drained between the cold step and the capture step** +(`vllm::Pool(b).Drain(b)`), which is what makes the case deterministic rather +than dependent on whether two of the tiny model's tensors happen to share a size +class. It is strictly stronger than the production condition — an empty free list +rather than a short one — and it is the only construction that also asserts the +premise #1393 left ungated: that the cold step's demand profile is SUFFICIENT for +the capture, not merely non-empty. + +Asserting a transcription would be the failure `.agents/verification.md` names: +`CHECK(pool.misses() == ...)` against a constant, or checking that the pre-grow +was called, both stay green when the profile is wrong. + +## The lane the gate does not apply to, and the correction that came with it + +`sanitize-cpu (thread)` refused the two new cases on the PRECONDITION, not on +the guarantee: + +``` +tests/vllm/models/test_qwen3_5_decode_graph_seam.cpp:948: + FATAL ERROR: REQUIRE( freed > 0 ) is NOT correct! values: REQUIRE( 0 > 0 ) +``` + +**It is not ThreadSanitizer and not its allocator.** +`.github/workflows/ci.yml:1598` sets `VT_POOL_BYPASS: "1"` for that job, on BOTH +the `address,undefined` and the `thread` lane. The identical `REQUIRE( 0 > 0 )` +reproduces at the same two lines on an ordinary non-sanitized Release build with +that one variable set and nothing else changed. The `address,undefined` lane on +#2047 read `pending` rather than green, so it never contradicted this; it had +not finished. + +Under bypass every `Get` is a raw `Backend::Alloc` and every `Put` a real +`Free`, `Drain` reports 0, and `PreGrowForCapture` returns before it grows +anything. The guarantee is **false by design** there, identically for the fixed +and the unfixed driver, so the cases carry `doctest::skip(PoolBypassLane())` +with the reason in the case name. The predicate mirrors `DevicePool::Bypass()` +exactly. `REQUIRE(freed > 0)` is unchanged — M3 measured that it is what stops +the pooled lane from asserting nothing. + +**A correction, because the first reading of the failure was backwards.** The +guard did NOT save a vacuous pass. Removed, the cases reach the capture +assertion and FAIL it, at **107** and **195** driver allocations inside the +capture, because under bypass every `Get` is a driver call. What the guard buys +is an inevitable failure that names the PRECONDITION instead of the SYMPTOM. +That is worth having and it is a smaller claim than the one first made for it. + +**The guarantee is therefore not exercised by `sanitize-cpu`, and that is the +job's configuration rather than a limit of the instrument.** Measured on a real +TSan build (`-DVLLM_CPP_SANITIZE=thread`, `setarch -R`): + +| Environment | Result | +|---|---| +| `VT_POOL_BYPASS=1`, as CI sets it | exit 0, 8 passed / **2 skipped**, 138 assertions, 0 TSan warnings | +| bypass unset, the pool ENABLED | exit 0, **10 passed / 0 skipped**, 156 assertions, `0` driver allocations inside both captures, **0 TSan warnings** | + +So ThreadSanitizer runs this guarantee green when the pool is on. Nothing in +this file can opt one case back in — `Bypass()` is read once into a process-wide +function-local static (`device_pool.h:480-486`), so no scope, no locally +constructed `DevicePool` and no `ActivePoolScope` reaches it — and unsetting the +variable for these cases would be worse than the gap, because the pool would +then retain blocks that the job's own `ASAN_OPTIONS=detect_leaks=1` reports as +leaks. Filed as [#2059](https://github.com/mudler/vllm.cpp/issues/2059): the +bypass is an ASan requirement applied to a lane that has no leak detector. + +## Gates + +```sh +cmake -S . -B build -DCMAKE_BUILD_TYPE=Release -DVLLM_CPP_CUDA=OFF -DVLLM_CPP_TRITON=OFF -DVLLM_CPP_SERVER=OFF +cmake --build build -j 4 --target test_qwen3_5_decode_graph_seam test_device_pool test_breakable_graph +./build/tests/test_qwen3_5_decode_graph_seam +./build/tests/test_device_pool +./build/tests/test_breakable_graph +scripts/agent-preflight.sh --staged +``` + +`cuda-fat-build` is the only thing that compiles the CUDA arm; this change edits +no `.cu`, so the CUDA half is a build verdict rather than a run verdict either +way. + +## Evidence + +Measured on `mudler-ubuntu-box` (x86-64, CPU-only), Release, +`-DVLLM_CPP_CUDA=OFF -DVLLM_CPP_TRITON=OFF -DVLLM_CPP_SERVER=OFF`, Ninja, `-j 4`. + +| Reading | Result | +|---|---| +| RED, both new cases, before the fix | exit 1, 2 cases / 0 passed / 2 failed. `45` driver allocations inside the dense capture, `42` inside the MoE one | +| GREEN, both new cases | exit 0, `0` and `0` inside the capture, `49` in the step — every one of them the pre-grow, outside the region | +| `test_qwen3_5_decode_graph_seam` whole file, three runs | exit 0 each, 10 cases / 156 assertions | +| neighbours on the shared harness and the pool | `test_qwen3_decode_graph_seam` 4/231, `test_qwen3_moe_decode_graph_seam` 3/228, `test_deepseek_v2_decode_graph_seam` 3/230, `test_voxtral_decode_graph_seam` 3/230, `test_qwen3_dflash_decode_graph_seam` 4/23, `test_qwen3_dflash2_draft` 43/449, `test_moe_async_device_ids` 6/191, `test_device_pool` 11/59, `test_breakable_graph` 30/265, `test_persistent_step_input` 10/66 — all exit 0 | +| Qwen3.5 numerics neighbours | `test_qwen35_paged_forward` 7/63, `test_qwen3_5_gdn_spec_routing` 6/52, `test_qwen3_5_fa2_class` 6/15 — all exit 0 | + +Three mutations, each compiled clean and each restored by sha256: + +| Mutation | Result | +|---|---| +| M1, the production call site: put `PreGrowForCapture` back inside `if (dbuf)` | RED, 10 cases / 8 passed / **2 failed** — both new cases, and nothing else | +| M2, the pool half: `PreGrowForCapture` returns 0 before it grows anything | RED, `test_qwen3_5_decode_graph_seam` 8/10 and `test_device_pool` 10/11 | +| M3, the case's own construction: remove the `Drain`, on the UNFIXED driver | the capture-window assertion goes **vacuously green** — `0` allocations inside the capture and `0` in the whole step — and only the `allocs() > 0` guard fires. This is the measurement that says the drain is load-bearing rather than decorative: without it the case cannot detect the defect, and without the non-vacuity guard it would report a pass while measuring nothing | +| M4, the skip predicate: `PoolBypassLane()` returns false, run under `VT_POOL_BYPASS=1` | RED, 10 cases / 8 passed / **2 failed**, **146 assertions** — a byte-for-byte reproduction of the `sanitize-cpu (thread)` reading, on a NON-sanitized build. The skip is load-bearing and not a decorative no-op | + +All four were re-run after the fixture changed, because a fixture change can +disarm a mutation proof. M1, M2 and M3 read identically before and after. + +**`.cu` is untouched, so nothing here is `REMOTE_UNVERIFIED` on the CUDA arm for a +compile reason.** What IS unverified is the device behaviour: no GPU was +available to this implementer, and the c=8 speculation-off rung has not been +re-run. + +## Stop conditions + +- If the drained-pool case does not go red before the fix, the premise is wrong + and the wave stops: it would mean the capture makes no main-pool allocation the + cold step did not already leave servable, and #2029's allocation is somewhere + else. Report the three candidates and the device experiment that discriminates + them rather than landing a speculative fix. +- If the case cannot be made green, `s.demand` does not cover the capture and the + fix is the demand profile rather than its call site. + +## Owed + +- [#2029](https://github.com/mudler/vllm.cpp/issues/2029) stays open until a + device run confirms the speculation-off rungs serve. This wave removes a + proven, non-speculative-only gap that produces exactly its message; it does not + prove that gap was the only one. **No GPU was available to this implementer**, + and nothing here is reported as device-verified. + + Three experiments settle it, in this order. Each uses the operator's own + invocation with `--speculative-config` removed and + `--enable-prefix-caching --scheduling-policy lpm` added, at c=1 — the cheapest + reproducer known. + + 1. **`VT_ASYNC_EXECUTOR=1` on the UNCHANGED `3d895a202` binary.** That flips + `impl_->dbuf` true, which turns the pre-grow on with no rebuild and no + patch. A run that SERVES proves the missing pre-grow is the cause, on the + device, against the exact binary the issue was measured on. A run that still + dies is **inconclusive rather than a refutation**, and the asymmetry has to + be stated when the result is read: the same flag also opens the parity ring, + which retains a second `[S, vocab]` logits and `[S, H]` hidden per size and + can therefore empty the free list by a route of its own. + 2. **The same configuration on a binary built from this branch**, environment + untouched. Serving says the fix closes this entry point; dying says a second + site exists. + 3. **`VLLM_CPP_QWEN3_DENSE_DECODE_GRAPH=0`, as the control.** Serving confirms + the fault is inside THIS driver's capture; dying says the capture that + allocates is somewhere else, and 1 and 2 answered the wrong question. + + If 2 still dies, the next instrument is a backtrace at `CudaBackend::Alloc`, + which is how #1380 was located and is exactly what + [#2037](https://github.com/mudler/vllm.cpp/issues/2037) exists to make + unnecessary. `VT_POOL_STATS` cannot substitute: it prints per-pool totals at + destruction, after the engine is already dead, and names no class. +- The seven decode drivers with no capture pre-grow at all — + [#2035](https://github.com/mudler/vllm.cpp/issues/2035) (D2). +- The two unguarded Marlin-path caches — + [#2036](https://github.com/mudler/vllm.cpp/issues/2036) (D4). +- `sanitize-cpu` disabling the `DevicePool` on both lanes, so neither sanitizer + executes the pool at all and this wave's gate is not exercised there — + [#2059](https://github.com/mudler/vllm.cpp/issues/2059). +- The fatal handler prints `e.what()` and no backtrace while + `include/vllm/v1/engine/core_client.h:63` promises "See stack trace (above)", + which is why #2029 and #2028 were both expensive to chase — + [#2037](https://github.com/mudler/vllm.cpp/issues/2037). diff --git a/src/vllm/model_executor/models/qwen3_5.cpp b/src/vllm/model_executor/models/qwen3_5.cpp index 8b7c889bf..d86206193 100644 --- a/src/vllm/model_executor/models/qwen3_5.cpp +++ b/src/vllm/model_executor/models/qwen3_5.cpp @@ -10866,33 +10866,58 @@ ForwardLogits Qwen3_5DecodeGraph::Step( // Warm: the pool + residency were warmed for this size by the previous (eager) // step. CAPTURE the layer region once, instantiate the graph, then launch it. if (s.warm) { + // #1380: THE POOL MUST BE ABLE TO SERVE THE WHOLE CAPTURED FORWARD, not one + // block of one tensor. This used to alloc-and-free a single [S, vocab] f32 + // block, on the reasoning that the capture RETAINS its logits while the + // other ring slot still holds its own, so the free list is one short. The + // retention half is right and the "one block of that shape" half is not: + // `DevicePool` is keyed by SIZE CLASS, so the block the capture then misses + // on need not be the logits. Measured on `thor:gpu0` (sm_110), the second + // ring slot's capture died in `dconv`, the GDN causal-conv output + // (`GdnBlockPaged`, this file), whose [T, conv_dim] lands in the SAME class + // as the retained [S, vocab] logits at the gate's shape -- and a cudaMalloc + // inside a captured region aborts the capture. An alloc-and-free grows the + // pool only when that class's free list is EMPTY, so with one block free + // and TWO needed live at once it grew nothing at all. + // + // So the driver stops naming a tensor and asks the pool for the demand the + // EAGER step at this shape actually made (`s.demand`, taken at the end of + // the cold step). Working scratch is still freed at ForwardLayers return and + // still SAFELY shared between the two graphs -- they replay sequentially on + // one stream. This runs OUTSIDE the capture, which is where a cudaMalloc is + // legal, and it is a no-op once the free list is deep enough. + // + // #2029: IT RUNS BEFORE EVERY CAPTURE, and until this line moved it ran + // before almost none of them. It sat inside the `if (dbuf)` below, and + // `dbuf` is `impl_->dbuf || spec_step` -- the `VT_ASYNC_EXECUTOR` parity + // ring, or a speculative step. The DEFAULT server sets neither, so the lane + // a user gets by omitting `--speculative-config` opened its capture over a + // pool nobody had prepared, and #1380's own failure came back at c=8 as + // `cudaMalloc: operation not permitted when stream is capturing` while the + // SAME binary with a speculative config served. The guard was never about + // the pre-grow: that block exists for the parity ring's drain and its + // persistent step inputs, and #1393 replaced the pre-grow in place without + // revisiting what it sat under. + // + // The `!dbuf` arm needs this at least as much as the `dbuf` arm. It passes + // `persistent_sdi == nullptr` to ForwardLayers, so `BuildStepDevInputs` + // and `MaybeBuildAttnCosSin` run from the MAIN pool INSIDE the captured + // region -- and that is also what makes `s.demand` exact for it, because the + // cold step takes the identical `nullptr` branch. On the `dbuf` arm the + // captured region's main-pool demand is a strict SUBSET of the cold step's, + // which is the containment #1393 recorded. + // + // Ordering: this now precedes the `b.Synchronize` below rather than + // following it. A pre-grow is `Backend::Alloc` and nothing else -- it + // enqueues no work and reads no in-flight buffer -- and the drain still + // happens before `BeginCapture`, which is the property it was added for. + Pool(b).PreGrowForCapture(b, s.demand); // dbuf: the runner may have skipped the depth-2 drain (the previous step // returned a slot view), so a prior replay can still be in flight. Capture must // begin on an idle stream — drain once here. One-time (≤2 captures per size); // steady-state replay never captures, so this never touches the overlap path. if (dbuf) { b.Synchronize(impl_->queue); - // #1380: THE POOL MUST BE ABLE TO SERVE THE WHOLE CAPTURED FORWARD, not one - // block of one tensor. This used to alloc-and-free a single [S, vocab] f32 - // block, on the reasoning that the capture RETAINS its logits while the - // other ring slot still holds its own, so the free list is one short. The - // retention half is right and the "one block of that shape" half is not: - // `DevicePool` is keyed by SIZE CLASS, so the block the capture then misses - // on need not be the logits. Measured on `thor:gpu0` (sm_110), the second - // ring slot's capture died in `dconv`, the GDN causal-conv output - // (`GdnBlockPaged`, this file), whose [T, conv_dim] lands in the SAME class - // as the retained [S, vocab] logits at the gate's shape -- and a cudaMalloc - // inside a captured region aborts the capture. An alloc-and-free grows the - // pool only when that class's free list is EMPTY, so with one block free - // and TWO needed live at once it grew nothing at all. - // - // So the driver stops naming a tensor and asks the pool for the demand the - // EAGER step at this shape actually made (`s.demand`, taken at the end of - // the cold step). Working scratch is still freed at ForwardLayers return and - // still SAFELY shared between the two graphs -- they replay sequentially on - // one stream. This runs OUTSIDE the capture, which is where a cudaMalloc is - // legal, and it is a no-op once the free list is deep enough. - Pool(b).PreGrowForCapture(b, s.demand); // Option A: build this slot's PERSISTENT device inputs + pinned staging OUTSIDE // the capture. BuildStepDevInputs fills s.dev from the refreshed host vectors so // the capture step reads correct inputs; the persistent cos|sin (fused-preamble @@ -11420,33 +11445,58 @@ ForwardLogits Qwen3_5DenseDecodeGraph::Step( // Warm: the pool + residency were warmed for this size by the previous (eager) // step. CAPTURE the dense layer region once, instantiate the graph, launch it. if (s.warm) { + // #1380: THE POOL MUST BE ABLE TO SERVE THE WHOLE CAPTURED FORWARD, not one + // block of one tensor. This used to alloc-and-free a single [S, vocab] f32 + // block, on the reasoning that the capture RETAINS its logits while the + // other ring slot still holds its own, so the free list is one short. The + // retention half is right and the "one block of that shape" half is not: + // `DevicePool` is keyed by SIZE CLASS, so the block the capture then misses + // on need not be the logits. Measured on `thor:gpu0` (sm_110), the second + // ring slot's capture died in `dconv`, the GDN causal-conv output + // (`GdnBlockPaged`, this file), whose [T, conv_dim] lands in the SAME class + // as the retained [S, vocab] logits at the gate's shape -- and a cudaMalloc + // inside a captured region aborts the capture. An alloc-and-free grows the + // pool only when that class's free list is EMPTY, so with one block free + // and TWO needed live at once it grew nothing at all. + // + // So the driver stops naming a tensor and asks the pool for the demand the + // EAGER step at this shape actually made (`s.demand`, taken at the end of + // the cold step). Working scratch is still freed at DenseForwardLayers return and + // still SAFELY shared between the two graphs -- they replay sequentially on + // one stream. This runs OUTSIDE the capture, which is where a cudaMalloc is + // legal, and it is a no-op once the free list is deep enough. + // + // #2029: IT RUNS BEFORE EVERY CAPTURE, and until this line moved it ran + // before almost none of them. It sat inside the `if (dbuf)` below, and + // `dbuf` is `impl_->dbuf || spec_step` -- the `VT_ASYNC_EXECUTOR` parity + // ring, or a speculative step. The DEFAULT server sets neither, so the lane + // a user gets by omitting `--speculative-config` opened its capture over a + // pool nobody had prepared, and #1380's own failure came back at c=8 as + // `cudaMalloc: operation not permitted when stream is capturing` while the + // SAME binary with a speculative config served. The guard was never about + // the pre-grow: that block exists for the parity ring's drain and its + // persistent step inputs, and #1393 replaced the pre-grow in place without + // revisiting what it sat under. + // + // The `!dbuf` arm needs this at least as much as the `dbuf` arm. It passes + // `persistent_sdi == nullptr` to DenseForwardLayers, so `BuildStepDevInputs` + // and `MaybeBuildAttnCosSin` run from the MAIN pool INSIDE the captured + // region -- and that is also what makes `s.demand` exact for it, because the + // cold step takes the identical `nullptr` branch. On the `dbuf` arm the + // captured region's main-pool demand is a strict SUBSET of the cold step's, + // which is the containment #1393 recorded. + // + // Ordering: this now precedes the `b.Synchronize` below rather than + // following it. A pre-grow is `Backend::Alloc` and nothing else -- it + // enqueues no work and reads no in-flight buffer -- and the drain still + // happens before `BeginCapture`, which is the property it was added for. + Pool(b).PreGrowForCapture(b, s.demand); // dbuf: the runner may have skipped the depth-2 drain (the previous step // returned a slot view), so a prior replay can still be in flight. Capture must // begin on an idle stream — drain once here. One-time (≤2 captures per size); // steady-state replay never captures, so this never touches the overlap path. if (dbuf) { b.Synchronize(impl_->queue); - // #1380: THE POOL MUST BE ABLE TO SERVE THE WHOLE CAPTURED FORWARD, not one - // block of one tensor. This used to alloc-and-free a single [S, vocab] f32 - // block, on the reasoning that the capture RETAINS its logits while the - // other ring slot still holds its own, so the free list is one short. The - // retention half is right and the "one block of that shape" half is not: - // `DevicePool` is keyed by SIZE CLASS, so the block the capture then misses - // on need not be the logits. Measured on `thor:gpu0` (sm_110), the second - // ring slot's capture died in `dconv`, the GDN causal-conv output - // (`GdnBlockPaged`, this file), whose [T, conv_dim] lands in the SAME class - // as the retained [S, vocab] logits at the gate's shape -- and a cudaMalloc - // inside a captured region aborts the capture. An alloc-and-free grows the - // pool only when that class's free list is EMPTY, so with one block free - // and TWO needed live at once it grew nothing at all. - // - // So the driver stops naming a tensor and asks the pool for the demand the - // EAGER step at this shape actually made (`s.demand`, taken at the end of - // the cold step). Working scratch is still freed at DenseForwardLayers return and - // still SAFELY shared between the two graphs -- they replay sequentially on - // one stream. This runs OUTSIDE the capture, which is where a cudaMalloc is - // legal, and it is a no-op once the free list is deep enough. - Pool(b).PreGrowForCapture(b, s.demand); // Option A: build this slot's PERSISTENT device inputs + pinned staging OUTSIDE // the capture (see the 35B driver). The dense fused-preamble arch (27B W4A4) // allocates + fills the persistent cos|sin here (pre-capture); the captured diff --git a/tests/vllm/models/decode_graph_seam_harness.h b/tests/vllm/models/decode_graph_seam_harness.h index f3325a62e..f7673d423 100644 --- a/tests/vllm/models/decode_graph_seam_harness.h +++ b/tests/vllm/models/decode_graph_seam_harness.h @@ -69,6 +69,14 @@ class CaptureCapableCpuBackend final : public vt::Backend { // so it does not go stale when the driver's allocation pattern changes. void* Alloc(size_t bytes) override { ++allocs_; + // THE CAPTURE-WINDOW SPLIT (#2029). A total allocation count cannot see the + // defect this counter exists for: a driver that allocates the SAME blocks, + // in the same classes, differs only in WHEN it asks the driver for them, and + // the whole rule is that a capture region must ask for none. On CUDA an + // `Alloc` here is a `cudaMalloc`, which aborts the capture; on this backend + // it succeeds silently, which is precisely why the count has to be the + // observable rather than the crash. + if (capturing_) ++allocs_in_capture_; return inner_.Alloc(bytes); } void Free(void* p) override { inner_.Free(p); } @@ -99,8 +107,15 @@ class CaptureCapableCpuBackend final : public vt::Backend { bool SupportsAuxStream() const override { return inner_.SupportsAuxStream(); } bool SupportsGraphCapture() const override { return supports_capture_; } - void BeginCapture(vt::Queue&) override { log_.push_back("Begin"); } + void BeginCapture(vt::Queue&) override { + log_.push_back("Begin"); + capturing_ = true; + } void* EndCaptureGraph(vt::Queue&) override { + // Close the window BEFORE the arm-once refusal below, so an abandoned + // capture leaves the flag down rather than charging every later allocation + // in the process to a capture that ended. + capturing_ = false; // Arm-once refusal, the shape of a real `cudaStreamEndCapture` returning // `cudaErrorStreamCaptureInvalidated` / `WrongThread`, or a failing // `cudaGraphInstantiate` — all three of which `Check()` @@ -135,9 +150,17 @@ class CaptureCapableCpuBackend final : public vt::Backend { // the forward rather than the allocator's history (#1352). int64_t copies() const { return copies_; } int64_t allocs() const { return allocs_; } + + // Driver allocations made between `BeginCapture` and `EndCaptureGraph` + // (#2029). This is the number a capture-safety gate asserts, and the + // assertion is ZERO: `DevicePool::Get` reaches `Backend::Alloc` only on a + // free-list miss, and on CUDA that miss is `cudaMalloc: operation not + // permitted when stream is capturing`. + int64_t allocs_during_capture() const { return allocs_in_capture_; } void ResetCounters() { copies_ = 0; allocs_ = 0; + allocs_in_capture_ = 0; } size_t Count(std::string_view what) const { @@ -153,6 +176,8 @@ class CaptureCapableCpuBackend final : public vt::Backend { bool fail_next_end_ = false; int64_t allocs_ = 0; int64_t copies_ = 0; + bool capturing_ = false; + int64_t allocs_in_capture_ = 0; std::vector log_; std::vector tags_; }; diff --git a/tests/vllm/models/test_qwen3_5_decode_graph_seam.cpp b/tests/vllm/models/test_qwen3_5_decode_graph_seam.cpp index ccd90d9f9..e613831f6 100644 --- a/tests/vllm/models/test_qwen3_5_decode_graph_seam.cpp +++ b/tests/vllm/models/test_qwen3_5_decode_graph_seam.cpp @@ -41,6 +41,7 @@ #include #include "decode_graph_seam_harness.h" +#include "vllm/model_executor/models/device_pool.h" #include "vllm/model_executor/models/qwen3_5.h" #include "vllm/model_executor/models/qwen3_5_dense.h" #include "vllm/transformers_utils/hf_config.h" @@ -882,3 +883,173 @@ TEST_CASE("W6: two spec shapes of EQUAL S and different q get two graphs") { CHECK(st2.capture_shapes == 2); // no third ring was opened CHECK(vt::GetGraphBreakStats().segments_captured == 2); } + +// ─── #2029: THE CAPTURE MUST ALLOCATE NOTHING, ON THE LANE THE DEFAULT SERVER TAKES ─── +// +// #2029: with speculation OFF the engine dies at concurrency 8 with +// `vt cuda: cudaMalloc: operation not permitted when stream is capturing`, while +// the SAME binary with `--speculative-config` serves. The asymmetry is not in the +// speculative code. It is in `dbuf`: +// +// const bool dbuf = impl_->dbuf || spec_step; // qwen3_5.cpp +// +// with `impl_->dbuf` false unless `VT_ASYNC_EXECUTOR=1` and `spec_step` false on +// every non-speculative step -- and the #1380 capture pre-grow living INSIDE +// `if (dbuf)`. So the default server captures over a pool nobody prepared, and +// #1380's own failure comes back by the one route its fix does not cover. +// +// WHAT THESE CASES ASSERT, and why it is not the call. `CHECK(pre-grow was +// called)` is a transcription: it stays green when the demand profile is wrong, +// which is the half #1393's own body recorded as ungated ("the fix rests on the +// captured forward demanding no more blocks of any size class than the eager +// forward at that shape did ... no test asserts it"). The observable here is the +// GUARANTEE -- zero `Backend::Alloc` calls between `BeginCapture` and +// `EndCaptureGraph` -- which on CUDA is exactly the `cudaMalloc` that aborts the +// capture, and which `CaptureCapableCpuBackend` counts without a device. +// +// WHY THE POOL IS DRAINED between the cold step and the capture step. In +// production the free list is SHORT rather than empty: every captured `SizeSlot` +// retains its `[S, vocab]` logits and `[S, H]` hidden forever, `DevicePool` is +// keyed by SIZE CLASS, and a ramping server captures more shapes -- which is why +// #2029 fires at c=8 and not at c=2. Reproducing that by arithmetic would make +// the case depend on whether two of this tiny model's tensors happen to collide +// in a class, i.e. on a coincidence rather than on the rule. `Drain` is the same +// condition taken to its limit, it is a production API +// (`DevicePool::Drain`, called at phase changes), and it is what also settles the +// premise above: an empty free list can only be served by the pre-grow, so a +// green here says the cold step's profile COVERS the capture. +// +// The non-vacuity guard is `allocs() > 0`. The pre-grow itself is a driver +// allocation, made OUTSIDE the region, so the fixed driver must allocate in this +// step and must allocate none of it under capture. A case that measured zero +// because nothing ran at all would fail that line. + +namespace { + +// THE LANE THESE TWO CASES DO NOT APPLY TO, and why they say so with a SKIP +// rather than with a pass. +// +// `VT_POOL_BYPASS=1` turns every `DevicePool::Get` into a raw `Backend::Alloc` +// and every `Put` into a real `Free` (`device_pool.h:113-127`, `:246-252`). +// In that lane there is no free list, `Drain` therefore reports 0, +// and `PreGrowForCapture` returns before it grows anything (`:429`). The +// guarantee these cases assert -- that the captured region performs no driver +// allocation -- is FALSE BY DESIGN there, and false identically for the fixed +// and the unfixed driver. It is not a defect the case has found; it is the +// lane deliberately reinstating the per-op alloc/free storm the pool exists to +// remove, so that ASan can tell the pool's retained cache apart from a leak and +// can see a use-after-free of a released block. +// +// `.github/workflows/ci.yml:1598` sets it for the `sanitize-cpu` job, for BOTH +// the `address,undefined` and the `thread` lane. So this is not a TSan +// interaction and not an allocator-footprint effect: the identical +// `REQUIRE( 0 > 0 )` reproduces on an ordinary non-sanitized Release build with +// `VT_POOL_BYPASS=1` in the environment and nothing else changed. +// +// WHAT THE PRECONDITION GUARD ACTUALLY BOUGHT, stated accurately because the +// first reading of the CI failure got it backwards. Without +// `REQUIRE(freed > 0)` these cases do NOT sail through asserting nothing: they +// reach the capture assertion and FAIL it, at 107 and 195 driver allocations +// inside the capture, because under bypass every `Get` is a driver call. The +// guard converts an inevitable failure that names the SYMPTOM into one that +// names the PRECONDITION. That is worth having, and it is a smaller claim than +// "it prevented a vacuous merge". +// +// SO THE CAPTURE-ALLOCATION GUARANTEE IS NOT EXERCISED BY `sanitize-cpu`, on +// either lane, and nothing in this file can change that: `Bypass()` is read +// once into a process-wide function-local static (`device_pool.h:480-486`), so +// no scope, no locally constructed `DevicePool` and no `ActivePoolScope` can +// opt one case back into pooled behaviour. Unsetting the variable for these +// cases would be worse than the gap -- the pool would retain blocks that the +// job's own `ASAN_OPTIONS=detect_leaks=1` then reports as leaks, which is the +// exact confusion the bypass was introduced to prevent. +// +// The predicate MIRRORS `DevicePool::Bypass()` byte for byte rather than +// approximating it. If the two ever disagree, a case would run in a lane whose +// pool is bypassed, or skip in one whose pool is not. +bool PoolBypassLane() { + const char* e = std::getenv("VT_POOL_BYPASS"); + return e != nullptr && e[0] == '1'; +} + +} // namespace + +TEST_CASE("#2029: a NON-speculative Qwen3_5DenseDecodeGraph capture allocates nothing" + " [pooled lane only -- SKIPPED under VT_POOL_BYPASS, where the pool is" + " disabled and the guarantee is false by design]" + * doctest::skip(PoolBypassLane())) { + const HfConfig c = dense::TinyConfig(); + const dense::Qwen3_5DenseWeights w = dense::MakeWeights(c); + REQUIRE_MESSAGE(vt::GraphCaptureEnabled(), + "this gate needs the CAPTURING lane; VLLM_CPP_CUDAGRAPH=0 is set"); + REQUIRE_MESSAGE(std::getenv("VT_ASYNC_EXECUTOR") == nullptr, + "this gate is about the DEFAULT lane; VT_ASYNC_EXECUTOR is set"); + + StaticGraphCpu harness; + vt::Queue q = Q(); + CachePool pool(c, /*num_blocks=*/4, /*block_size=*/16); + vllm::Qwen3_5DenseDecodeGraph graph(w, c, q, /*max_num_reqs=*/4); + + // COLD: the one eager run at this shape, and the step whose per-class peak the + // driver records as `s.demand`. `num_spec_decodes` is 0, so `spec_step` is + // false and this is the lane the default server is on. + graph.Step({11}, {0}, DecodeAttnMeta(0), DecodeGdnMeta(), pool.attn_kv, + pool.gdn_state); + REQUIRE_FALSE(graph.captured()); + + const size_t freed = vllm::Pool(harness.backend()).Drain(harness.backend()); + REQUIRE_MESSAGE(freed > 0, + "the cold step returned no block to the pool, so this case would " + "assert nothing about a short free list"); + harness.backend().ResetCounters(); + + // WARM: the capture. + graph.Step({12}, {1}, DecodeAttnMeta(1), DecodeGdnMeta(), pool.attn_kv, + pool.gdn_state); + REQUIRE(graph.captured()); + CHECK(harness.backend().Count("Begin") == 1); + CHECK(harness.backend().Count("EndCaptureGraph") == 1); + MESSAGE("dense capture step: " << harness.backend().allocs_during_capture() + << " driver allocations INSIDE the capture, " + << harness.backend().allocs() << " in the step"); + CHECK(harness.backend().allocs() > 0); + CHECK(harness.backend().allocs_during_capture() == 0); +} + +TEST_CASE("#2029: a NON-speculative Qwen3_5DecodeGraph capture allocates nothing" + " [pooled lane only -- SKIPPED under VT_POOL_BYPASS, where the pool is" + " disabled and the guarantee is false by design]" + * doctest::skip(PoolBypassLane())) { + const HfConfig c = TinyConfig(); + const Qwen3_5MoeWeights w = MakeWeights(c); + REQUIRE_MESSAGE(vt::GraphCaptureEnabled(), + "this gate needs the CAPTURING lane; VLLM_CPP_CUDAGRAPH=0 is set"); + REQUIRE_MESSAGE(std::getenv("VT_ASYNC_EXECUTOR") == nullptr, + "this gate is about the DEFAULT lane; VT_ASYNC_EXECUTOR is set"); + + StaticGraphCpu harness; + vt::Queue q = Q(); + CachePool pool(c, /*num_blocks=*/4, /*block_size=*/16); + vllm::Qwen3_5DecodeGraph graph(w, c, q, /*max_num_reqs=*/4); + + graph.Step({11}, {0}, DecodeAttnMeta(0), DecodeGdnMeta(), pool.attn_kv, + pool.gdn_state); + REQUIRE_FALSE(graph.captured()); + + const size_t freed = vllm::Pool(harness.backend()).Drain(harness.backend()); + REQUIRE_MESSAGE(freed > 0, + "the cold step returned no block to the pool, so this case would " + "assert nothing about a short free list"); + harness.backend().ResetCounters(); + + graph.Step({12}, {1}, DecodeAttnMeta(1), DecodeGdnMeta(), pool.attn_kv, + pool.gdn_state); + REQUIRE(graph.captured()); + CHECK(harness.backend().Count("Begin") == 1); + CHECK(harness.backend().Count("EndCaptureGraph") == 1); + MESSAGE("MoE capture step: " << harness.backend().allocs_during_capture() + << " driver allocations INSIDE the capture, " + << harness.backend().allocs() << " in the step"); + CHECK(harness.backend().allocs() > 0); + CHECK(harness.backend().allocs_during_capture() == 0); +}