Skip to content

[Perf] Skip detokenization in offline beam search - #50333

Merged
vllm-bot merged 1 commit into
vllm-project:mainfrom
samuelkim7:offline-beam-search-skip-detok
Aug 10, 2026
Merged

vllm-bot merged 1 commit into
vllm-project:mainfrom
samuelkim7:offline-beam-search-skip-detok

Conversation

@samuelkim7

@samuelkim7 samuelkim7 commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

What & why

Follow-up to #46422, which skipped detokenization in online beam search and noted the offline path has the identical pattern.

Offline beam search asks for logprobs = 2 * beam_width on every internal per-step request. The engine detokenizes all of those token ids into strings on every step, but beam search never reads them. It ranks beams by cum_logprob and decodes the final text itself. This PR sets detokenize=False on the two internal SamplingParams, same as #46422 did for the online path.

As with #46422, the per-step Logprob entries in the returned sequences no longer carry decoded_token strings. Final output text is unchanged.

Benchmark

A100, Qwen/Qwen3-1.7B (same model as #46422), offline LLM.beam_search, 8 prompts of 256 tokens, 32 output tokens, temperature 0, enforce_eager. One arm per process. Output token ids are sha256-compared across arms as a hard gate.

beam width wall clock (before → after) speedup outputs identical
4 2.67s → 2.13s 1.25x yes
8 3.58s → 2.71s 1.32x yes
20 9.41s → 6.61s 1.42x yes

The win grows with beam width, matching the O(beam_width) per-step cost #46422 measured on the online path (1.80x at width 20 there; eager mode inflates the per-step GPU time here, so these numbers are conservative).

Correctness

Output token ids are bit-identical between arms at all three beam widths on the A100 run (sha256 gate). I also verified output equality on CPU (TinyLlama-1.1B, beam widths 4 and 8, 3 interleaved rounds each). The existing tests/samplers/test_beam_search.py suite covers the offline path against HF reference outputs in CI.

Duplicate check

No open PR applies this to the offline path. #47630 touches the same file for allowed_token_ids handling and does not conflict with this change.

Done with Claude Code assistance, mostly on the benchmark harness and runs. I reviewed the change and validated the results.

Signed-off-by: samuelkim7 <samuelmwkim@gmail.com>

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@github-actions

Copy link
Copy Markdown

👋 Hi! Thank you for contributing to the vLLM project.

💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels.

PRs do not trigger a full CI run by default. Reviewers with write access and configured trusted contributors can comment /ci run whenever CI signals are needed.

Once the PR is approved or has the ready label, the PR author can also use /ci run or /ci retry. New commits do not start CI automatically.

If you have any questions, please reach out to us on Slack at https://slack.vllm.ai.

Agent Guidelines

IMPORTANT: 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.

🚀

@mergify mergify Bot added the frontend label Jul 29, 2026
@DarkLight1337

Copy link
Copy Markdown
Member

Thanks for optimizing

@DarkLight1337
DarkLight1337 enabled auto-merge (squash) August 10, 2026 16:11
@DarkLight1337

Copy link
Copy Markdown
Member

/ci run

@github-actions github-actions Bot added the ready ONLY add when PR is ready to merge/full CI is needed label Aug 10, 2026
@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #83168 for commit 7547034eb8ac.

@samuelkim7

Copy link
Copy Markdown
Contributor Author

/ci retry

@github-actions

Copy link
Copy Markdown

✅ Queued 4 failed job(s) for retry in Buildkite CI #83168.

@vllm-bot
vllm-bot merged commit 8977ea8 into vllm-project:main Aug 10, 2026
66 of 73 checks passed
pjdurden added a commit to pjdurden/vllm that referenced this pull request Aug 13, 2026
vllm-project#50333 disabled detokenization on both offline beam-search paths but
added no test, so a regression would be silent: the search loop only
reads token IDs and logprob values, and nothing fails loudly if the
per-step params start detokenizing again.

Add GPU-free tests asserting detokenize is False on the plain per-step
sampling params and on the per-beam params built for the
structured-output path.

Signed-off-by: pjdurden <prajjwalchittori1@gmail.com>
zyp2014 pushed a commit to zyp2014/vllm that referenced this pull request Aug 21, 2026
Signed-off-by: samuelkim7 <samuelmwkim@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

frontend ready ONLY add when PR is ready to merge/full CI is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants