Skip to content

generate invoking doesn't require detokenization for beam search - #33563

Closed
gameofdimension wants to merge 102 commits into
vllm-project:mainfrom
gameofdimension:patch-1
Closed

gameofdimension wants to merge 102 commits into
vllm-project:mainfrom
gameofdimension:patch-1

Conversation

@gameofdimension

@gameofdimension gameofdimension commented Feb 2, 2026

Copy link
Copy Markdown

Purpose

beam.text = tokenizer.decode(tokens)

The code here demonstrates detokenization isn't required for generate(), and skipping it offers minor latency benefits.

Test Plan

Test Result

model Qwen3-1.7B
GPU NVIDIA A30
dataset An internal dataset containing 106 samples,
with sequence lengths ranging from 958 to 1,230 tokens
max_tokens 5
beam_width 20

latency improvement

mean p99
baseline 0.178 0.208
this pr 0.201 0.232

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 gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a performance optimization to the beam_search method. By setting detokenize=False for the generate calls within the beam search loop, it avoids unnecessary work since the intermediate text is not used. The final sequences are correctly decoded at the end of the function. This is a good and safe optimization.

@gameofdimension

Copy link
Copy Markdown
Author

@aarnphm @chaunceyjiang
Would you mind reviewing this?

@njhill njhill left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @gameofdimension.

This is actually also included in https://github.com/vllm-project/vllm/pull/29133/files which stalled. Maybe you could also include an explanatory comment similar to that one.

@@ -347,6 +347,7 @@ async def beam_search(
logprobs=logprobs_num,
max_tokens=1,
temperature=temperature,
detokenize=False,

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

As the review bot pointed out, we never use generate()'s returned text - the real detokenization happens later in this function.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sorry, I meant adding a comment in the code next to this line, as is in the other PR I referenced.

@gameofdimension gameofdimension changed the title generate API doesn't require detokenization for beam search generate invoking doesn't require detokenization for beam search Feb 3, 2026
gameofdimension and others added 18 commits February 5, 2026 09:54
Signed-off-by: G.O.D <32255912+gameofdimension@users.noreply.github.com>
Signed-off-by: felix01.yu <felix01.yu@vipshop.com>
…3551)

Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
Signed-off-by: felix01.yu <felix01.yu@vipshop.com>
Signed-off-by: felix01.yu <felix01.yu@vipshop.com>
Signed-off-by: rabi <ramishra@redhat.com>
Signed-off-by: felix01.yu <felix01.yu@vipshop.com>
…project#32790)

Signed-off-by: dafrimi <dafrimi@nvidia.com>
Signed-off-by: felix01.yu <felix01.yu@vipshop.com>
Signed-off-by: Isotr0py <mozf@mail2.sysu.edu.cn>
Signed-off-by: felix01.yu <felix01.yu@vipshop.com>
Signed-off-by: shanjiaz <zsjwpianpian@gmail.com>
Signed-off-by: felix01.yu <felix01.yu@vipshop.com>
…for the GB series (vllm-project#33540)

Signed-off-by: Kebe <mail@kebe7jun.com>
Signed-off-by: youkaichao <youkaichao@gmail.com>
Co-authored-by: Thomas Vegas <tvegas@nvidia.com>
Co-authored-by: youkaichao <youkaichao@gmail.com>
Signed-off-by: felix01.yu <felix01.yu@vipshop.com>
Signed-off-by: DarkLight1337 <tlleungac@connect.ust.hk>
Signed-off-by: felix01.yu <felix01.yu@vipshop.com>
Signed-off-by: Matthew Bonanni <mbonanni@redhat.com>
Signed-off-by: felix01.yu <felix01.yu@vipshop.com>
…-project#33039)

Signed-off-by: Yang <lymailforjob@gmail.com>
Signed-off-by: felix01.yu <felix01.yu@vipshop.com>
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Signed-off-by: felix01.yu <felix01.yu@vipshop.com>
Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Signed-off-by: felix01.yu <felix01.yu@vipshop.com>
…max loras. Multiple cuda graphs are captured for each num of active-loras. (vllm-project#32005)

Signed-off-by: Yu Gong <yu3.gong@gmail.com>
Signed-off-by: felix01.yu <felix01.yu@vipshop.com>
Signed-off-by: Matthew Bonanni <mbonanni@redhat.com>
Signed-off-by: felix01.yu <felix01.yu@vipshop.com>
…lm-project#31914)

Signed-off-by: vasiliy <vasiliy@fb.com>
Signed-off-by: felix01.yu <felix01.yu@vipshop.com>
Signed-off-by: Siyuan Fu <siyuanf@nvidia.com>
Co-authored-by: Pavani Majety <pmajety@nvidia.com>
Signed-off-by: felix01.yu <felix01.yu@vipshop.com>
Signed-off-by: Patrick von Platen <patrick.v.platen@gmail.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Signed-off-by: felix01.yu <felix01.yu@vipshop.com>
@mergify

mergify Bot commented Feb 5, 2026

Copy link
Copy Markdown
Contributor

Documentation preview: https://vllm--33563.org.readthedocs.build/en/33563/

@mergify mergify Bot added documentation Improvements or additions to documentation ci/build deepseek Related to DeepSeek models llama Related to Llama models multi-modality Related to multi-modality (#4194) new-model Requests to new models performance Performance-related issues nvidia rocm Related to AMD ROCm labels Feb 5, 2026
@mergify mergify Bot added the cpu Related to CPU backends label Feb 5, 2026
@github-project-automation github-project-automation Bot moved this to Ready in NVIDIA Feb 5, 2026
@github-project-automation github-project-automation Bot moved this to Todo in AMD Feb 5, 2026
@github-project-automation github-project-automation Bot moved this from Todo to Done in AMD Feb 5, 2026
@github-project-automation github-project-automation Bot moved this from Ready to Done in NVIDIA Feb 5, 2026
@gameofdimension

gameofdimension commented Feb 5, 2026

Copy link
Copy Markdown
Author

The commit history is messed up. closed

wait PR 29133 to be merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci/build cpu Related to CPU backends deepseek Related to DeepSeek models documentation Improvements or additions to documentation frontend kv-connector llama Related to Llama models multi-modality Related to multi-modality (#4194) new-model Requests to new models nvidia performance Performance-related issues qwen Related to Qwen models rocm Related to AMD ROCm speculative-decoding structured-output v1

Projects

Status: Done
Status: Done
Status: Done

Development

Successfully merging this pull request may close these issues.