Conversation
BWAAEEEK
requested review from
WoosukKwon,
njhill and
yewentao256
as code owners
September 23, 2026 02:21
Contributor
|
This pull request has merge conflicts that must be resolved before it can be |
BWAAEEEK
force-pushed
the
fix/mrv2-encoder-cudagraph-memory
branch
from
September 25, 2026 07:21
7ac8ba1 to
daf06cf
Compare
Contributor
|
This pull request has merge conflicts that must be resolved before it can be |
Allow the worker and V2 graph profiler to account for encoder CUDA graphs when decoder graphs are disabled. Keep the pre-bootstrap decoder check, MRV1 behavior, eager exclusion, and graph-reservation opt-out semantics. Add regression coverage for profiling eligibility, KV budget subtraction, and cleanup when encoder graph capture fails. Assisted-by: OpenAI Codex Signed-off-by: BWAAEEEK <jooho414@gmail.com>
BWAAEEEK
force-pushed
the
fix/mrv2-encoder-cudagraph-memory
branch
from
September 26, 2026 07:19
daf06cf to
8c75d67
Compare
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
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
While following up on the MRV2 feedback on my earlier PR, I revisited CUDA graph memory accounting in the V2 runner. This led me to a missing case similar to the encoder graph memory accounting I previously worked on in #41714, but in a different runner and configuration.
MRV2 can capture encoder CUDA graphs independently of decoder graphs. With
cudagraph_mm_encoder=Trueandcudagraph_mode=NONE, the encoder still captures graphs, but the Worker skips graph memory profiling. The V2 profiler also returns early based on decoder-only checks. Consequently, automatic KV-cache sizing does not reserve memory for those encoder graphs.This PR:
enforce_eager.NONE, so it should not reserve memory for that case.No kernels, capture sizes, batch-size limits, pool-sharing policy, or memory-estimation formulas are changed. This concerns normal multimodal generation, not the separate
--mm-encoder-onlydeployment mode.Related Work / Duplicate Check
I checked #49224 and its comments, open PR references to that issue, and open/merged PRs for encoder graph memory,
cudagraph_mode=NONE, and MRV2 profiling.profile_cudagraph_memory()returns 0, causing OOM duringcapture_model()#49224. This fixes a remaining eligibility case; it does not reimplement that profiler or claim to fix the original issue in full.12b5ceefaf7f, both its Worker and V2 profiler still skip profiling for decoderNONE.5d28d0504e9b, its Worker still requires decoder graphs to be enabled.The first two open accounting PRs overlap files with this change, but address different failures. This PR deliberately leaves their accounting-policy changes out of scope.
Test Plan
Validation used a fresh Python 3.12 environment and a standard editable installation with the official CUDA 13.0 wheel for the exact main-base commit
a6c47fbbf4187e8ae5396ebdcf93234db617a4ad. Both baseline and fixed sources used the same dependencies and CUDA binaries. All 18 installed extension files matched that wheel by SHA256; loaded-library paths were also checked. There are no C++/CUDA changes in this PR.The focused suites were run both through the environment-verification wrapper and directly with pytest. The direct command, with temporary/cache directories configured outside the repository, was:
The regression subset was also run against an unmodified main worktree using the final tests and
--import-mode=importlib.Model checks use NVIDIA B200, BF16, TP=1, Torch 2.13.0+cu130, and the actual Qwen/Qwen2.5-VL-3B-Instruct weights at revision
66285546d2b821cf421d4f5eb2576359d3770cd3. Configuration:max_model_len=2048,max_num_seqs=2,max_num_batched_tokens=1024,gpu_memory_utilization=0.12, prefix caching disabled, FLASHINFER encoder attention, and one image/no video per prompt.The model harness observes the real profiling/capture returns, KV budget, temporary-pool release, production recapture/replay, and generated token IDs. A separate check runs the repository's
vllm bench throughputwith actual multimodal requests, rather than a synthetic kernel timing loop.Multimodal benchmark configuration
The local invocation uses a read-only wrapper that asserts one image per request before calling the unchanged benchmark CLI. Its standard-install equivalent is:
The nonempty synthetic dataset-path marker avoids the current CLI fallback to text-only
random; the random multimodal dataset does not read that path. The local run uses the pinned downloaded snapshot instead of fetching weights.Test Result
Tested base:
a6c47fbbf4187e8ae5396ebdcf93234db617a4ad.Tested PR head:
7ac8ba13be40e48b81984a73fcfd217f2102d52e.uv pip check: all 228 installed packages are compatible.Tested-base real-model checks:
Both fixed runs released their temporary encoder profiling pools completely before production recapture and had zero replay misses. In each run, the KV budget equals requested memory minus the observed forward-profile non-KV footprint minus the graph reservation. In the matched-environment single-budget comparison, the forward-profile footprint was identical: KV budget changed from 14,439,960,146 to 14,167,330,386 bytes, exactly the 260 MiB reservation.
The full nine-configuration model matrix was rerun on this submission revision and matched wheel:
All generated smoke-test token IDs matched the baseline, including the concurrent pair. Temporary encoder pools were released in every profiled configuration, and every enabled encoder graph run had zero replay misses.
The matched-environment multimodal benchmark completed four warmup and 32 measured image requests per run, with concurrency capped at two. Both runs processed 5,856 prompt tokens and generated 512 output tokens. The wrapper confirmed that every request actually contained one image.
Both CLI runs exited successfully and emitted the same NCCL process-group shutdown warning at exit. These are single short runs checking successful multimodal processing, not evidence of a statistically significant speedup or performance equivalence. This PR fixes a demonstrated memory-reservation omission, not a reproduced OOM or throughput bottleneck.
AI assistance was used for implementation, validation, and drafting.