Repository navigation
[Core] Optimize Qwen2.5-Omni Token2Wav buffer loading - #7061
Conversation
Signed-off-by: akshatvishu <akshatnayak197@gmail.com>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
This PR appears to be related to model: qwen-omni. Model owners: @amy-why-3459 @psv666 @akshatvishu, please review your own changes and leave a short self-review comment describing what you checked. PRs without author self-review may not be assigned a reviewer. Please take a look when you have a chance. If you would like an automated review, mention @vllm-omni-review-bot in a comment. |
|
Self-review: The change is limited to persistent buffer discovery in Token2Wav and replaces 219 repeated |
Omni ReviewBot: three questions on the performance claim@akshatvishu this PR reads as a performance or value claim:
Before the full evidence checklist, three short questions:
When you answer, the evidence that settles it is: base and head SHA, hardware, model, workload, warm-up and repeat count, mean or percentiles with their spread, and a correctness/quality-equivalence signal; an end-to-end claim also needs stage attribution. |
|
@vllm-omni-review-bot All the claims, reason for bottleneck as well as A/B testing is already provided in the pr body itself |
linyueqian
left a comment
There was a problem hiding this comment.
Correct and cheap. state_dict() excludes exactly the buffers whose names sit in their module's _non_persistent_buffers_set, so walking named_modules() once to collect those qualified names and filtering named_buffers() against the set reproduces the old membership test without rebuilding the state dict per buffer; it is the same pattern vLLM's own loaders and this repo's diffusion weight loaders already use. The test pins persistent root and nested buffers included, a non-persistent one excluded, and zero state_dict() calls. The A/B numbers in the description are from ROCm on an older vLLM, which is fine for a startup-time change of this shape.
Static read at 40ccac04; fork head, no PR code executed. pre-commit and DCO are green; the PR has no ready label so no general lane has run, and it needs one before merge.
Resolves #7060.
Summary
find_all_registers()calledstate_dict()once for every named buffer. Qwen2.5-Omni made 219 calls during each startup, even though every call traversed the same module tree.The loader now collects nonpersistent buffer names from
_non_persistent_buffers_setand walks the named buffers once. The test checks that persistent child buffers remain included, nonpersistent buffers remain excluded, andstate_dict()is not called.The code follows the existing vLLM buffer loading pattern. vLLM-Omni also uses the same check in its diffusion weight loaders.
Benchmark
Qwen/Qwen2.5-Omni-3B2.12.0+git6bbd2607.2.532110.28.0+rocm7230.28.1.dev47The server was started with:
All three stages used device 0 with GPU memory utilization values of 0.35, 0.25, and 0.15. The benchmark used one warmup per version and five measured runs per version. The measured runs alternated between the baseline and optimized commits, and the model files were already cached.
state_dict()/healthThe startup ranges overlapped, so the PR claims the direct reduction in Token2Wav loading work rather than a stable full startup improvement.
The logged GPU memory figures for stages 0 and 1 were unchanged. Stage 0 used 9.94 GiB for weights and non-PyTorch memory with 2.49 GiB of peak activation memory. Stage 1 used 1.95 GiB with 1.18 GiB of peak activation memory. No GPU memory improvement is claimed.
Test
Command:
pre-commit run --filespassed for both changed files.