[Frontend] improve vllm bench <bench_type> --help display #20430
Conversation
There was a problem hiding this comment.
Summary of Changes
Hello @reidliu41, 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 significantly improves the user experience for vLLM's benchmark command-line interface by enhancing its help system. It enables a more granular and searchable help display for bench <bench_type> commands, making it much easier for users to discover and understand the numerous configuration options available. The changes involve both integrating the new help mechanism and refining the underlying parsing logic to support multi-part commands.
Highlights
- Enhanced CLI Help: Integrated the filtered help functionality (
--help=<keyword>) into thevllm bench <bench_type>subcommands, allowing users to search for specific arguments or argument groups. - Updated Help Epilog: Modified the general CLI help epilog to explicitly mention
vllm bench <bench_type>as a command that supports the advanced--help=<keyword>search feature. - Improved Subcommand Parsing Logic: Refined the internal logic for
show_filtered_argument_or_group_from_helpto correctly identify and apply the filtered help to multi-part subcommands (likebench latency), ensuring accurate help display for complex command structures.
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
The code changes introduce the --help=<keyword> feature to the vllm bench subcommands. A potential issue exists in the subcommand detection logic, where argument values might incorrectly trigger the help function. A more robust approach would involve verifying the subcommand's position in sys.argv.
vllm/entrypoints/utils.py
Outdated
| if " " in subcommand_name: | ||
| # For "bench serve" - check if both parts are in sys.argv | ||
| parts = subcommand_name.split() | ||
| if not all(part in sys.argv for part in parts): | ||
| return | ||
| else: | ||
| # For simple commands like "serve" | ||
| if subcommand_name not in sys.argv: | ||
| return | ||
| # If this is "serve" but "bench" is also present, | ||
| # let "bench serve" handle it | ||
| if subcommand_name == "serve" and "bench" in sys.argv: | ||
| return |
There was a problem hiding this comment.
The current logic to check if the subcommand is active is not robust. It checks for the presence of subcommand parts anywhere in sys.argv, which can lead to incorrect behavior. For example, if a user runs vllm bench throughput --some-option latency --help=foo, this function would be incorrectly triggered for the bench latency subcommand because both "bench" and "latency" are present in sys.argv.
A more robust approach is to check if the subcommand parts appear in the correct order at the beginning of the command-line arguments (right after the script name). This can be done by slicing sys.argv. This change also simplifies the code by removing the if/else logic for single vs. multi-word subcommands and makes it more maintainable.
| if " " in subcommand_name: | |
| # For "bench serve" - check if both parts are in sys.argv | |
| parts = subcommand_name.split() | |
| if not all(part in sys.argv for part in parts): | |
| return | |
| else: | |
| # For simple commands like "serve" | |
| if subcommand_name not in sys.argv: | |
| return | |
| # If this is "serve" but "bench" is also present, | |
| # let "bench serve" handle it | |
| if subcommand_name == "serve" and "bench" in sys.argv: | |
| return | |
| parts = subcommand_name.split() | |
| # sys.argv[0] is the program name. The subcommand follows. | |
| # e.g., for `vllm bench latency`, sys.argv is `['vllm', 'bench', 'latency', ...]` | |
| # and subcommand_name is "bench latency". | |
| if len(sys.argv) <= len(parts) or sys.argv[1:1 + len(parts)] != parts: | |
| return |
|
👋 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 🚀 |
Signed-off-by: reidliu41 <reid201711@gmail.com>
cb1cb11 to
75d4bab
Compare
Signed-off-by: reidliu41 <reid201711@gmail.com>
|
updated |
…ct#20430) Signed-off-by: reidliu41 <reid201711@gmail.com>
…ct#20430) Signed-off-by: reidliu41 <reid201711@gmail.com>
…ct#20430) Signed-off-by: reidliu41 <reid201711@gmail.com>
…ct#20430) Signed-off-by: reidliu41 <reid201711@gmail.com>
…ct#20430) Signed-off-by: reidliu41 <reid201711@gmail.com> Signed-off-by: avigny <47987522+avigny@users.noreply.github.com>
…ct#20430) Signed-off-by: reidliu41 <reid201711@gmail.com>
…ct#20430) Signed-off-by: reidliu41 <reid201711@gmail.com>
…ct#20430) Signed-off-by: reidliu41 <reid201711@gmail.com> Signed-off-by: Jinzhen Lin <linjinzhen@hotmail.com>
…ct#20430) Signed-off-by: reidliu41 <reid201711@gmail.com>
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.Purpose
bench <bench_type>, so add --help= to search:Test Plan
Test Result
(Optional) Documentation Update