Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .agents/parity-ledger.md

Large diffs are not rendered by default.

174 changes: 174 additions & 0 deletions .agents/specs/async-discrete-device-combine.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,174 @@
# ENG-ASYNC-SCHED W4 — discrete-CUDA device-resident sampled tokens

Row: `ENG-ASYNC-SCHED` W4 (engine matrix). Prerequisite W3 is landed and
default-ON. This spike scopes the remaining leaf: making the async-scheduling
overlap real on a **discrete** CUDA GPU, where W3 currently degrades to a
synchronizing host path.

## The defect this closes

W3 landed the device combine/scatter kernels
(`src/vt/cuda/cuda_combine_tokens.cu`) and wired them into the runner, but both
call sites are gated on `is_integrated_gpu()`:

- `src/vllm/v1/worker/gpu/runner.cpp:821` — device combine, else host combine.
- `src/vllm/v1/worker/gpu/runner.cpp:1763` — device scatter, else host scatter
preceded by `vt::GetBackend(dev.type).Synchronize(queue_)`.

The gate is correct as written: the kernels take the runner's **host**
`std::vector` buffers directly, which is only legal where the platform reports
pageable device access (GB10's UMA). A discrete GPU answers `false` and takes
the host branch, so:

1. the host must know step N's sampled token before it can build step N+1's
`input_ids`, which forces a full-device `Synchronize` inside
`sample_tokens_async`; and
2. the depth-2 `step_with_batch_queue` loop (`src/vllm/v1/engine/core.cpp:115`)
therefore cannot overlap anything — the scheduler is async, the runner is not.

Measured on the local RTX 5070 Ti (`is_integrated_gpu()` == false), 2026-07-25,
attribution-complete node-mode nsys: 497 `cudaStreamSynchronize` calls following
the 256-byte sampled-ID D2H, **20.975 s total, 42.20 ms/call**, against a binding
mean TPOT of 43.72 ms. The matched vLLM trace instead waits on sampled output via
`cudaEventSynchronize` (1,342 calls) AFTER launching the next batch. Evidence:
`docs/bench-evidence/qwen35-4b-main-repair-20260725.md`.

## What upstream does (the mirror obligation)

Read from `${VLLM_SOURCE}` (installed release tree, vLLM 0.24.0 — cited as
installed-release source, NOT as the parity pin):

- `vllm/v1/worker/gpu/states.py:64` —
`self.last_sampled_tokens = torch.zeros(max_num_reqs, 1, dtype=torch.int64,
device=device)`. It is a **GPU tensor**, unconditionally, on every platform.
There is no integrated/discrete branch upstream at all.
- `vllm/v1/worker/gpu/input_batch.py:296-360` — the combine kernel loads
`last_sampled_tokens_ptr + req_state_idx` on device and stores into the device
`input_ids`.
- `vllm/v1/worker/gpu/input_batch.py:449-473` — the post_update kernel stores the
freshly sampled id back into `last_sampled_tokens_ptr + req_state_idx` on
device.
- `vllm/v1/worker/gpu/states.py:132` `remove_request` — **upstream never
condenses**. A finished request's slot goes onto `free_indices` and is reused;
the req_state index is stable for the request's lifetime, which is exactly why
the GPU tensor never has to be permuted, and why `idx_mapping` (batch row ->
req_state) exists.

So the discrete path is not a new design; it is the upstream design, which our
W3 leaf implemented only for the UMA case.

## The one real complication: our batch condenses

`InputBatch::condense()` (`src/vllm/v1/worker/gpu/input_batch.cpp:554`) moves the
last live row into a freed slot (`last_sampled_tokens[empty] =
last_sampled_tokens[last]`, line 641), and `swap_states` (line 737) swaps two
rows. Upstream's free-index pool has neither. With a device-resident buffer the
host no longer holds the values it would need to perform those moves, and reading
them back would reintroduce the synchronize this row exists to delete.

Resolution: keep the moves on the HOST as bookkeeping, but record them and replay
them ON DEVICE in stream order. `InputBatch` gains an ordered pending-op log
(seed on `add_request`, move on `condense`, swap on `swap_states`); the runner
drains it each step and applies it with one small kernel BEFORE the combine. The
ops are host-known (indices, and a host-known value for the seed), so no device
read is needed, and stream ordering makes the replay exact rather than racy.
Rejected alternative: put `last_sampled_tokens` in pinned mapped host memory so
the existing kernels work unchanged — the host `condense` read of a value the
device wrote is then an unsynchronized read, which is precisely the
removal/condensation hazard, and it would be latent rather than loud.

## Work breakdown

- **W4a** Runner-owned persistent device buffers sized to the batch bound:
`last_sampled` [max_num_reqs], `prefill_len` [max_num_reqs], `query_start_loc`
[max_num_reqs+1], `seq_lens` [max_num_reqs], `input_ids`
[max_num_batched_tokens], plus a pinned host staging buffer so the per-step H2D
is a real async copy rather than a pageable staging copy.
- **W4b** `InputBatch` pending-op log + the device replay kernel
(`LaunchApplyLastSampledOps`), with RED-first unit coverage of seed/move/swap
ordering.
- **W4c** `ModelForwardInput::device_token_ids` (default `nullptr`, so every
other model and the whole non-async path is byte-identical) honored by the
Qwen3.5 dense/MoE `EmbedInto`, so the forward embeds the DEVICE ids the combine
patched instead of a host vector.
- **W4d** Flip both `is_integrated_gpu()` gates to select
UMA-in-place / discrete-device-buffer rather than device / host, and delete the
`Synchronize` on the discrete branch.
- **W4e** Remove the other per-step barrier: `EmbeddingKernelCuda`
(`src/vt/cuda/cuda_ops.cu:638-669`) does a `cudaMalloc` + `cudaMemcpyAsync` +
**`cudaStreamSynchronize`** + `cudaFree` for its out-of-range flag on EVERY
call (531 calls, 12.6 ms total in the same trace). That is negligible while the
engine is serialized and becomes a hard barrier the moment it is not, so W4
is not measurable until it is gone. Fix: a persistent per-device flag plus a
deferred check (read the previous call's flag through a completed event), which
keeps the loud failure at the cost of at most one step of latency. **W4e must
land before the W4 A/B, or the A/B measures the wrong thing.**

## Gates

- Correctness: `test_qwen35_plain_weights --no-skip` 3/3 unchanged; the CPU-tier
combine/scatter/input-batch suites unchanged; a new RED-first unit test for the
pending-op replay.
- Identity: token-for-token identity across `{default, VT_ASYNC_RUNNER=1,
VT_ASYNC_RUNNER=0}` on the 128-request benchmark corpus, including a run whose
requests finish at staggered lengths so `condense` actually fires.
- Speed: same-binary A/B under one `flock /tmp/gpu`, >=3 repetitions, against the
matched vLLM arm on the identical corpus. The target is the TPOT axis; TTFT
must be watched (upstream pays a TTFT premium for async scheduling).
- No 4B result implies anything about the 27B/35B release gates.

## What was actually built (2026-07-27)

Implemented as scoped, with three deviations worth recording because each was
forced by something the spike did not anticipate:

1. **The forward reads the device ids through a scoped override, not a
parameter.** `ModelForwardInput::device_token_ids` reaches the two Qwen3.5
registry forwards, which publish it as an RAII, thread-local
`detail::DeviceTokenIdsScope` that the embed consults. The alternative was a
defaulted pointer parameter on five entry points plus the decode-graph class.
The override is consumed on first use so a second, unrelated embed inside the
same forward (the multimodal helper embeds a prompt and then single tokens)
cannot pick up ids that were never meant for it.
2. **The embed PATCHES a prefix rather than embedding the runner's buffer
directly.** The decode-graph path does not embed `token_ids` as given — it
embeds a version padded up to the captured batch size, real rows first. So
the correct operation is "overwrite the first `count` rows", which is right
for the padded case and degenerates to "overwrite everything" on the eager
path. Embedding the runner's buffer directly would have silently mis-shaped
the graph path.
3. **The per-step uploads copy from PAGEABLE host memory on purpose.** The
spike said "pinned staging so the copies are real async copies". That is a
correctness trap here: pinned copies are truly asynchronous, so reusing one
staging buffer for the next upload can overwrite bytes an in-flight DMA has
not read, and with a depth-2 scheduler that window spans steps. For a
pageable source the driver stages the bytes before `cudaMemcpyAsync` returns,
so the source is immediately reusable. These arrays are a few kilobytes at
the front of a step; the staged copy is the better trade against per-upload
regions plus a per-step event.

A fourth correction came from the shared-layer device-leakage ratchet: the
mirror's enable predicate first asked "is this device CUDA and not integrated",
which put a `kCUDA` token in the device-agnostic layer and failed the ratchet.
The right question was already available and is more precise — `vt::Backend::
UnifiedMemory()`, i.e. "is device memory addressable from the host". A unified
device (GB10, and the CPU backend trivially) keeps the in-place path; a device
with separate memory needs the mirror. Same behaviour, no device-type test.

Also landed alongside: `VT_ASYNC_DEVICE_MIRROR=0`, the same-binary rollback that
returns a discrete GPU to the pre-W4 host path WITHOUT disabling async scheduling
(which `VT_ASYNC_RUNNER=0` would also do). That separation is what makes an
honest A/B of W4 alone possible.

W4e (the embedding barrier) landed as a persistent ring of flag slots with a
deferred, event-queried check. The reporting contract changed deliberately and
its test changed with it: an out-of-range id is now raised no later than the NEXT
embedding on the queue, carrying the same message and the same id, instead of on
the offending call. The gather itself is unchanged, so the offending call never
read out of bounds either way.

## Status

`ACTIVE` — implemented; gates and the A/B are the closing step. Nothing here may
be called DONE until the token-identity gate and the same-binary A/B are on the
record.
Loading