Repository navigation
[2/N][Perf][Core] Cut the per-step cost of the omni prefix cache - #9193
Conversation
Signed-off-by: ZhengWG <zwg0606@gmail.com>
Signed-off-by: ZhengWG <zwg0606@gmail.com>
Signed-off-by: ZhengWG <zwg0606@gmail.com>
Signed-off-by: ZhengWG <zwg0606@gmail.com>
Signed-off-by: ZhengWG <zwg0606@gmail.com>
Signed-off-by: ZhengWG <zwg0606@gmail.com>
Signed-off-by: ZhengWG <zwg0606@gmail.com>
Signed-off-by: ZhengWG <zwg0606@gmail.com>
Signed-off-by: ZhengWG <zwg0606@gmail.com>
Signed-off-by: ZhengWG <zwg0606@gmail.com>
Signed-off-by: ZhengWG <zwg0606@gmail.com>
Signed-off-by: ZhengWG <zwg0606@gmail.com>
Signed-off-by: ZhengWG <zwg0606@gmail.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
This PR appears to belong to: docs/design/module/ar_runtime.md. Module owners: @Gaohan123 @fake0fan @Sy0307 Routing: @Gaohan123 via module of the changed files, CODEOWNERS; @fake0fan via module of the changed files; @Sy0307 via module of the changed files @ZhengWG, please review your own changes and leave a short self-review comment describing what you checked. PRs without author self-review may not be assigned a reviewer. Please take a look when you have a chance. If you would like an automated review, mention @vllm-omni-review-bot in a comment. |
Omni ReviewBot triage noteAutomated triage of commit
These are automated triage suggestions only — the final decision belongs to the maintainers. |
hsliuustc0106
left a comment
There was a problem hiding this comment.
@ZhengWG For c05f9be7af8c2c33abcfc7b22a3b592feab9e8b9, the model, hardware and workload details are useful, but #<PR1> + these commits and “main” do not identify the measured revisions.
- Supply the exact baseline, PR1 and combined measured-head SHAs, with per-round result files mapped to each revision. Confirm the vLLM version and matching model/checkpoint, GPU configuration, workload, cache settings and measurement conditions for each comparison; keep the v0.31 startup result separate from the v0.32 throughput results.
Acceptance: the reported cache-overhead and startup improvements can be traced to a reproducible matched baseline/head comparison, with any PR1-dependent effect distinguished from this PR. This is an evidence request, not a demonstrated code defect; I have not independently run these benchmarks.
amy-why-3459
left a comment
There was a problem hiding this comment.
Went through the manager/controller/runner-mixin changes and the decode-buffer tests. The ordering looks right to me: the flush runs before new_step_starts on any non-pure step, so finishes, preemptions, and new hits always see the buffered rows. The _hit_spans check in save_outputs is also safe, since publish clears those spans. Two smaller points, both about device memory on the buffered path (inline).
Also, the branch currently conflicts with main, so the _register_step_writes rebase mentioned in the description is still pending.
| cfg, steps = self._prefix_cache_decode_cfg, self._prefix_cache_decode_steps | ||
| assert cfg is not None and steps is not None | ||
| n, nbytes = len(writes), sum(t.numel() * t.element_size() for t in rows.values()) | ||
| if n > cfg.staging_capacity_tokens or nbytes > cfg.gpu_staging_bytes: |
There was a problem hiding this comment.
This buffer is capped by cfg.gpu_staging_bytes, but its clones aren't charged to the controller's GPU-byte budget, which uses the same 512 MiB limit for its own device clones. So the worst case is roughly 2× gpu_staging_bytes of device memory held for the cache, plus the torch.stack copy at flush. All of that is allocated after KV-cache profiling, and Qwen3-Omni stage 0 runs at gpu_memory_utilization: 0.9. The realistic decode footprint is small, but could the buffer use its own, much smaller cap, or take a budget ticket?
| hidden = rows.pop(hidden_key) | ||
| cache = self.omni_prefix_cache | ||
| assert cache is not None | ||
| step_id = cache.save_outputs( |
There was a problem hiding this comment.
Each buffered row is now copied on device three times: val.clone() when it is parked, torch.stack here, and then hidden_states[:n].clone() / val[:n].clone() again in _split_step_outputs. The stacked tensors are already fresh, unaliased storage, so the last clone is redundant for this caller. It's cheap at decode sizes, but it's easy to skip, e.g. with a flag telling save_outputs that its inputs are already owned.
Signed-off-by: ZhengWG <zwg0606@gmail.com>
Signed-off-by: ZhengWG <zwg0606@gmail.com>
…p-overhead Signed-off-by: ZhengWG <zwg0606@gmail.com>
@amy-why-3459 Already fixed. |
amy-why-3459
left a comment
There was a problem hiding this comment.
Re-checked the two new commits (e5de4d3, 2a36aa2); the merge from main is not covered.
Both earlier points look resolved:
- Each parked decode step is now charged through
reserve_device_bytesbefore the clone, and the flush releases the whole hold beforesave_outputscharges the stacked rows through its own ticket, so the buffer and the step clones share onegpu_staging_bytesbudget. The reserve runs outside the state lock, and the new test checks that. owned=Truedrops the third device copy. The unpadded guard in_split_step_outputsmakes sure a live, padded buffer can't take that path by mistake.
Nothing further from me on this delta.
Contributor intent (required human answers)
1. What outcome do you want to achieve?
Reduce prefix-cache overhead during decode and first-request startup.
2. What is the highest-leverage variable for that outcome?
The main variable is CPU time spent on engine-thread cache bookkeeping and host-mirror initialization.
3. How will this work move that variable?
Buffer pure-decode rows, batch writes, vectorize slot mapping, and eliminate unnecessary host-mirror pinning and zero-filling.
4. Whose customer or user needs does this address, and what are those needs?
Users serving multimodal models with prefix caching need higher throughput and lower startup latency, even when cache hit rates are low.
Purpose
Part [2/N] of the series that turns the omni prefix cache on for Qwen3-Omni and MiniCPM-o (#7902): [1/N] #9162 fixes the collapse under concurrent hits, this PR removes the per-step cost at 0% hit. It applies to main on its own; whichever of #9162 and this PR lands second needs a small rebase in
_register_step_writes.With the cache on, every step is saved whether or not anything hits. At 0% hit, main loses up to 6.5% on Qwen3-Omni, 19–20% on MiniCPM-o decode, and 21.6% on Qwen3-Omni decode with varying output lengths (C64). Under that last load py-spy puts 27–32% of the engine thread in
save_outputs: each save opened one write per request, so every request paid a slot-table update, a commit and a pool scatter. The host mirror was also pinned and zero-filled on the first request (+6.8 s for Step-Audio 2 mini).This PR:
JOIN_NEXT_STEP) rows share oneWriteTask; each request still tracks it, so finish/abort and hit reads are unchanged. Deferred writes stay per request.block_sizesteps at once (bounded bystaging_capacity_tokensandgpu_staging_bytes). Any other step first writes the buffer, partial blocks included.step_slots_cpu: 302 → 21 µs for a 16-request decode step.torch.emptyhost mirror: only CPU copies touch it; reads zero the slots that were absent when planned.Test Plan
vLLM Version: 0.32.0
vLLM-Omni Commit: #9162 + these commits
Test bases GPU runs on main
da572cddb38383deae9caef33b8d490e292ef071.pytest tests/core tests/worker/test_gpu_ar_model_runner.py tests/worker/test_gpu_ar_model_runner_cuda.py tests/config/test_config_factory.py.qwen3_omni_moe.yaml, 2× H20-3e, 4 rounds) and MiniCPM-o 4.5 V1 (minicpmo_4_5_turn.yaml, 1× H20, 3 rounds), cache off vs on, fresh server per run, rounds alternating off / on on the same GPUs. Main cache on was measured the same way. py-spy (20 s) on the stage-0 engine duringDECV_c64.enable_prefix_caching(true / false); for MiniCPM-o the overlay also sets stage 0max_model_len: 17408and the server runs with--trust-remote-code. Run a warmup bench, then the benches below. Hit rates are the delta of stage 0'svllm:prefix_cache_{hits,queries}_totalon/metricsbefore and after each bench.server, overlay, bench and accuracy commands
Test Result
P1024_h0_c8P4096_h0_c1P1024_h50_c64DEC100_c32DEC100_c64DECV_c32DECV_c64P256_h0_c32P1024_h0_c32P1024_h0_c64P4096_h0_c32P4096_h0_c64P8192_h0_c32P16384_h0_c32P16384_h0_c64DEC100_c32DEC100_c64DECV_c32DECV_c64BEFORE SUBMITTING: read CONTRIBUTING.md and run the precheck-pr skill with the code agent for a self-check against project conventions.
Note
To avoid agent spam, vLLM-Omni limits contributors without write access to 5 open PRs. If this blocks well-intentioned critical work, please contact a committer to request bypass list access.
(anything written below this line will be removed by GitHub Actions)