[Bugfix] set default set cuda_graph_sizes to min(self.max_num_seqs * 2, 512) #20628
Conversation
Signed-off-by: izhuhaoran <[email protected]>
izhuhaoran
requested review from
WoosukKwon,
hmellor,
houseroad,
mgoin,
robertgshaw2-redhat,
simon-mo,
tlrmchlsmth and
youkaichao
as code owners
|
👋 Hi! Thank you for contributing to the vLLM project. 💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels. Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging. To run CI, PR reviewers can either: Add 🚀 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @izhuhaoran, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request introduces a significant improvement to how CUDA graph capture sizes are defaulted within the SchedulerConfig. By moving from a fixed default to a dynamically calculated one based on max_num_seqs, the system can more intelligently manage memory usage and optimize startup performance, particularly in resource-constrained environments. This change addresses potential OOM issues and reduces unnecessary graph captures.
Highlights
- Dynamic Default for CUDA Graph Sizes: The default value for
cuda_graph_sizesinSchedulerConfighas been updated. Instead of a fixed[512], it now dynamically defaults to[min(max_num_seqs * 2, 512)]when no explicit value is provided. - Memory and Performance Optimization: This change aims to prevent Out-of-Memory (OOM) errors in scenarios with tight memory and small
max_num_seqs, while also reducing startup time by avoiding the capture of excessively large CUDA graphs. - Configuration Logic Update: New logic has been added to the
__post_init__method ofSchedulerConfigto implement this dynamic default, ensuring it's applied only whencuda_graph_sizesis not set by the user. - Documentation Clarity: The docstring for
cuda_graph_sizeshas been revised to clearly explain the new default behavior and its conditions.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces a sensible change to the default value of cuda_graph_sizes, making it dynamic based on max_num_seqs. This should help prevent OOM errors and reduce startup times in certain scenarios. The implementation looks correct. I've only pointed out a minor inconsistency in the updated docstring that should be fixed for clarity.
| 2. if one value is provided, then the capture list would follow the | ||
| pattern: [1, 2, 4] + [i for i in range(8, cuda_graph_sizes + 1, 8)] |
There was a problem hiding this comment.
The docstring for cuda_graph_sizes appears to have a small error. When one value is provided, the example pattern uses cuda_graph_sizes directly in the range function, but since cuda_graph_sizes is a list, this would raise a TypeError. The implementation correctly uses cuda_graph_sizes[0], so the docstring should be updated to match for clarity.
| 2. if one value is provided, then the capture list would follow the | |
| pattern: [1, 2, 4] + [i for i in range(8, cuda_graph_sizes + 1, 8)] | |
| 2. if one value is provided, then the capture list would follow the | |
| pattern: [1, 2, 4] + [i for i in range(8, cuda_graph_sizes[0] + 1, 8)] |
|
@mgoin @yeqcharlotte would you mind taking a look and reviewing this PR when you have a moment? Thanks! |
@ProExpertProg mentioned some concerns wrt chunked prefill. Empirically this does have some slight perf impact. I don’t think we fully understand best way to tune it would be nice to leave a TODO on it to dig deeper @izhuhaoran cc: @zou3519 to also take a look |
…2, 512) (vllm-project#20628) Signed-off-by: izhuhaoran <[email protected]>
…2, 512) (vllm-project#20628) Signed-off-by: izhuhaoran <[email protected]>
…2, 512) (vllm-project#20628) Signed-off-by: izhuhaoran <[email protected]> Signed-off-by: Jinzhen Lin <[email protected]>
…2, 512) (vllm-project#20628) Signed-off-by: izhuhaoran <[email protected]>
4 participants
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.Purpose
This is a follow-up PR related to #20062 . Based on the discussion in #20062, this PR default set cuda_graph_sizes to [min(max_num_seqs * 2, 512)]. This avoids OOM in tight memory scenarios with small max_num_seqs, and prevents capture of many large graphs (>512) that would greatly increase startup time with limited performance benefit.