Repository navigation
fix: fix vlm cuda graph shape stability - #30868
Merged
mickqian merged 8 commits intoJul 19, 2026
Merged
Conversation
mickqian
requested review from
BBuf,
Edwardf0t1,
Fridge003,
HaiShaw,
Oasis-Git,
Ying1123,
ch-wan,
hebiao064,
hnyls2002,
ispobock,
merrymercy,
yhyang201 and
yuan-luo
as code owners
July 11, 2026 11:53
github-actions
Bot
added
Multi-modal
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces optimizations and fixes for compiling multimodal models (such as Qwen3-VL), including dynamic dimension marking for mRoPE token positions, a dummy forward pass to warm up deepstack embeddings, and an eager fallback for rotary embeddings during CUDA graph capture. Feedback on these changes highlights a critical bug in resolve_max_seqlen where passing a raw torch.Tensor will cause an AttributeError, a performance concern regarding the global use of dynamic=True in torch.compile which prevents kernel specialization, and unreachable dead code left behind by the generalized stream fallback in the CUDA piecewise backend.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Collaborator
Author
|
CI collection failure comes from the MoE pytest file introduced by sibling #30866, not from this PR’s diff. I added the required standalone |
Collaborator
Author
|
/tag-and-rerun-ci |
mickqian
force-pushed
the
codex/vlm-graph-shape-stability
branch
from
July 13, 2026 00:42
5338e54 to
1bba625
Compare
mickqian
changed the title
mickqian
force-pushed
the
codex/vlm-graph-shape-stability
branch
from
July 17, 2026 03:13
c2422dc to
7ebaf72
Compare
merrymercy
reviewed
Jul 19, 2026
| attn_backend.init_forward_metadata(fb) | ||
| self._run_forward(fb, num_tokens) | ||
|
|
||
| def run_dummy_multimodal_deepstack_forward( |
Collaborator
There was a problem hiding this comment.
this looks hacky.
- do we really need it?
- this is only for tc_piecewise backend, can we move it under python/sglang/srt/model_executor/runner_backend/tc_piecewise_cuda_graph_backend.py
- btw, we are going to deprecate tc_piecewise backend soon in favor of breakable backend. why do you still need to fix it for tc_piecewise?
Collaborator
Author
There was a problem hiding this comment.
- yes deepstack is only for qwen-vl but it also brings solid performance gain
- sure
- I previous have no context of that, will shift to bcg from now on
2 of 5 tasks
Zhylkaaa
pushed a commit
to Zhylkaaa/sglang
that referenced
this pull request
Jul 29, 2026
bowenwan6
added a commit
to bowenwan6/sglang
that referenced
this pull request
Aug 5, 2026
Qwen3-VL routes an ``input_deepstack_embeds`` kwarg to the language
model's forward on image requests. Under breakable-CUDA-graph replay
the tensor was silently dropped: the captured graph never traced the
DeepStack ``add_`` branch, and ``replay_layer_forward`` did not copy
the live tensor anywhere, so image outputs on any DeepStack-carrying
model on the BCG allowlist matched a zero-DeepStack run instead of
the eager reference. No shipped configuration triggers this today
(``Qwen/Qwen3.5-*`` releases ship with ``deepstack_visual_indexes=[]``
and Qwen3-VL is not on ``multimodal_breakable_cuda_graph_supported_model_archs``),
so this is a latent regression that would activate on either config
change; closing it defensively removes the trap.
Fix mirrors the existing ``input_embeds`` slot-and-copy pattern:
* ``Qwen3VLForConditionalGeneration`` declares an explicit class
attribute ``supports_bcg_deepstack_replay = True``. Its MoE
subclass ``Qwen3VLMoeForConditionalGeneration`` inherits the flag.
Non-Qwen3-VL archs (Qwen2.5-VL, Qwen3, Cohere2Vision, KimiK25,
MiniMaxM3) do not set the attribute and default to ``False`` via
``getattr``.
* ``PrefillCudaGraphRunner.__init__`` computes
``deepstack_replay_width = hidden_size × num_deepstack_embeddings``
only when the capability flag is ``True`` AND the instance reports
``num_deepstack_embeddings > 0`` — otherwise ``0``. The width is
passed to both ``PrefillInputBuffers.create`` (which allocates a
single ``[max_num_tokens, width]`` bf16 buffer) and
``build_prefill_registry`` (which registers the matching optional
``GraphSlot``).
* ``_run_forward``'s BCG capture branch passes the slot buffer as
the ``input_deepstack_embeds`` kwarg when ``has_slot`` reports the
slot is registered, so Dynamo traces the ``add_`` branch into the
captured graph. Otherwise it falls through to today's four-
positional call unchanged.
* ``replay_layer_forward`` mirrors the ``input_embeds`` copy: when
the slot is registered, it validates the live kwarg (present,
``numel() > 0``, matching per-row shape, dtype, device) and copies
into the slot with a zeroed padded tail; missing / empty /
mismatched inputs land in an ``else`` branch that zeros the whole
slice so a stale image contribution cannot leak across bucket-
reuse into a text-only request.
Zero cost for models that do not declare the capability: no
``PrefillInputBuffers`` field allocated, no registry slot, no
captured-graph kwarg, no replay-side copy — the entire block is
guarded by ``has_slot()``.
Refs sgl-project#30868 (PCG DeepStack Dynamo warmup) and sgl-project#30872 (input_embeds
BCG slot). Preserves ``run_dummy_multimodal_deepstack_forward`` for
the TC piecewise path unchanged.
Chronostasys
pushed a commit
to MindLab-Research/sglang
that referenced
this pull request
Aug 24, 2026
4 of 5 tasks
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
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.
Summary
Before / after behavior
Qwen3-VL-32B-Instruct on H100 TP=4, with 128/256/512-pixel image requests: before, the first request at each unseen visual-token length created a PCG/Dynamo replacement compilation and its visible first-request pause. after, the startup-warmed graph is reused across that sweep (
__recompiles=0). This is a first-request latency-stability result; no steady-state latency uplift is claimed.Validation
PYTHONPATH=python pytest -q test/registered/unit/compilation/test_torch_compile_decoration.py(3 passed)cu_seqlens, piecewise fallback, and CUDA graph behavior.CI States
Latest PR Test (Base): ❌ Run #29639402817
Latest PR Test (Extra): ❌ Run #29639402686