Revert "[Redo] #26368 (#28771)"

[Jialin]

Purpose

This reverts commit 98b4d38.

Per @gshtras reported offline, the original PR introduced throughput regression. From @gshtras,

main
Avg latency: 222.78142010899805 seconds
main + revert 186352b27
Avg latency: 192.86235034199976 seconds

And we've confirmed the regression locally, and try to fix forward in #29033, but it did not help.

Our learning here is that: Although replacing list[int] to np.ndarray could avoid bumping gc allocation count, but the conversion overhead is way too big and would regress the throughput e2e.

Test Plan & Test Result

vllm bench latency --model meta-llama/Llama-3.1-8B-Instruct --dtype bfloat16 --batch-size 3000 --input-len 128 --output-len 2048 -tp 8 --num-iters-warmup 1 --num-iters 3

---

Essential Elements of an Effective PR Description Checklist

• The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
• The test plan, such as providing test command.
• The test results, such as pasting the results comparison before and after, or e2e results
• (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.
• (Optional) Release notes update. If your change is user facing, please update the release notes draft in the Google Doc.

[gemini-code-assist]

Code Review

This pull request reverts a previous commit that introduced a performance regression by changing token ID representations from list[int] to np.ndarray. The revert seems mostly correct, but I've identified a critical issue in vllm/v1/worker/gpu_model_runner.py where the logic for handling speculative decoding outputs in asynchronous scheduling mode was not fully reverted. This could lead to incorrect outputs. My review includes a suggested fix for this issue.

[chatgpt-codex-connector]

💡 Codex Review

vllm/vllm/v1/worker/gpu_model_runner.py

Lines 2498 to 2500 in 97db3b6

	if self.input_batch.prev_sampled_token_ids is None:
	assert sampled_token_ids.shape[-1] == 1
	self.input_batch.prev_sampled_token_ids = sampled_token_ids

Async scheduler reuses stale prev_sampled_token_ids

In the async scheduling branch, prev_sampled_token_ids is only populated when it is None (lines 2498-2500), and sample_tokens no longer clears it between iterations. After the first batch this condition remains false, so later iterations never refresh the cached sampled tokens. When _prepare_input_ids scatters cached tokens for requests that span iterations, it will reuse stale data from the first iteration, producing incorrect inputs whenever async scheduling processes multiple decode steps.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[wangxiyuan]

TBH, this change breaked oot again and again. lol, any plan to make it strong enough before merge?

[DarkLight1337]

Pretty sure this is final now

[Jialin]

TBH, this change breaked oot again and again. lol, any plan to make it strong enough before merge?

@wangxiyuan n00b question, what's oot? Before landing in the first place, we ensured all CI tests are passed. I'm wondering if there's anything we should do to further improve CI coverage. Thanks for sharing the context.

[DarkLight1337]

OOT stands for Out-Of-Tree. In this case it refers to plugin packages for alternative hardware backends, such as vllm-ascend. Since those backends have their own model runner, any change to the interface of the inputs and outputs may break them.

[Jialin]

OOT stands for Out-Of-Tree. In this case it refers to plugin packages for alternative hardware backends, such as vllm-ascend. Since those backends have their own model runner, any change to the interface of the inputs and outputs may break them.

Thanks for the explanation!

[wangxiyuan]

Never mind. Usually breaking change for oot is acceptable. But this kind of change(do-revert-redo-revert) is really rare