Conversation
…apture_size When max_cudagraph_capture_size is set explicitly and cudagraph_capture_sizes is left unset, _set_cudagraph_sizes() only built the stepped 8/16 token grid. The uniform decode sizes (request count times decode query length) were computed inside the `max_cudagraph_capture_size is None` branch, so with speculation on they were never appended, the inferred list ended at the last grid entry, and the explicit maximum was truncated down to it. With max_num_seqs=6 and six speculative tokens the widest uniform decode batch is 42 tokens. Setting max_cudagraph_capture_size=42 produced [1, 2, 4, 8, 16, 24, 32, 40], the warning "Truncating max_cudagraph_capture_size to 40", and the 42 token verification step ran eager (vllm-project#54933). Compute the uniform decode sizes whenever the capture list is inferred here, bounded by the possibly explicit maximum, rather than only when the maximum is also inferred. An explicit cudagraph_capture_sizes list is still left as configured and the platform default ceiling still applies when no maximum is given. The large hunk is the uniform size block moving out one nesting level; with whitespace ignored it is a 14 line change. Reproducer from the issue on current main, before and after: before: max=40 sizes=[1, 2, 4, 8, 16, 24, 32, 40] after: max=42 sizes=[1, 2, 4, 7, 8, 14, 16, 24, 28, 32, 40, 42] Adds a parametrized regression test for a maximum equal to, above and below the widest uniform batch, and one guarding that an explicit list is never extended. Existing cudagraph sizing tests in the file still pass. Fixes vllm-project#54933 Signed-off-by: bojiang3 <bli314159@gmail.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
bojiang3
requested review from
ProExpertProg,
WoosukKwon,
houseroad,
mgoin,
robertgshaw2-redhat,
tlrmchlsmth,
yewentao256 and
youkaichao
as code owners
September 2, 2026 18:43
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughChangesUniform decode CUDA graph sizing
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The fix adds missing bounded CUDA-graph sizes for an existing configuration path while preserving explicitly configured lists. No actionable merge-blocking risk remains beyond normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Linked Issues checkExplanation The implementation and regression tests satisfy issue ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
This pull request has merge conflicts that must be resolved before it can be |
MirkoDeVita98
added a commit
to MirkoDeVita98/vllm
that referenced
this pull request
Sep 24, 2026
When `max_cudagraph_capture_size` is set explicitly and `cudagraph_capture_sizes` is left unset, `_set_cudagraph_sizes()` builds the stepped 8/16 grid and then truncates the scalar maximum down to the last grid entry. Decode batches between that entry and the configured maximum have no captured graph and fall back to eager execution. asked 12 -> resolved 8 sizes [1, 2, 4, 8] asked 50 -> resolved 48 sizes [1, 2, 4, ..., 40, 48] asked 100 -> resolved 96 sizes [1, 2, 4, ..., 88, 96] This is the non-speculative counterpart of vllm-project#54933. That issue, and vllm-project#55004 which fixes it, concern `uniform_decode_sizes`, which are only computed when `decode_query_len > 1`; with plain decode the list still ends on the grid. Append the maximum to the inferred list when the grid did not reach it, next to the existing `max_num_batched_tokens` append that preserves the same property. The value appended is the one already clipped to `max_num_batched_tokens`, so it cannot exceed the token budget, and an explicit `cudagraph_capture_sizes` list is still left exactly as configured. Costs one extra captured graph. Closes: vllm-project#58523 Signed-off-by: MirkoDeVita98 <mirko.devita@icloud.com>
1 task
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
Fixes #54933.
When
max_cudagraph_capture_sizeis set explicitly andcudagraph_capture_sizesisleft unset,
_set_cudagraph_sizes()only builds the stepped 8/16 token grid. Theuniform decode sizes (request count times decode query length) were computed inside
the
max_cudagraph_capture_size is Nonebranch, so with speculation on they were neverappended, the list ended at the last grid entry, and the explicit maximum was
truncated down to it.
Concretely,
max_num_seqs=6with six speculative tokens has a widest uniform decodebatch of 42 tokens. Setting
max_cudagraph_capture_size=42produced[1, 2, 4, 8, 16, 24, 32, 40], a warning "Truncating max_cudagraph_capture_size to40", and the 42 token verification step dispatched eager.
Changes
Compute the uniform decode sizes whenever the capture list is being inferred here,
bounded by the (possibly explicit) maximum, instead of only when the maximum is also
inferred. An explicit
cudagraph_capture_sizeslist is still left exactly asconfigured, and the platform default ceiling still applies when no maximum is given.
The hunk in
vllm/config/vllm.pylooks large because the uniform size block movedout one nesting level; with whitespace ignored it is a 14 line change.
Reproducer from the issue, before and after, on current main:
The extra 7, 14 and 28 entries are the same request count grid the inferred maximum
path already produces, now also present under an explicit maximum.
Test Plan
tests/compile/test_config.py:test_explicit_max_cudagraph_capture_size_covers_uniform_decode, parametrized overa maximum equal to, above and below the widest uniform batch.
test_explicit_cudagraph_capture_sizes_are_left_as_configured, guarding that anexplicit list is never extended.
Test Result
New tests pass together with the existing cudagraph sizing tests in that file
(
respects_platform_ceiling,keep_all_sizes_bounded,respect_sequence_parallelism,caps_widest_ngram_decode_batch). Run on a DGX Spark (GB10) against current main.🤖 Generated with Claude Code
Summary by CodeRabbit