-
-
Notifications
You must be signed in to change notification settings - Fork 8.3k
Implement Async Scheduling #19970
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Implement Async Scheduling #19970
Conversation
Signed-off-by: Woosuk Kwon <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @WoosukKwon, 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 new asynchronous scheduling mechanism to the system. It involves creating a dedicated AsyncScheduler
class, making it configurable via new engine arguments and CLI options, and refactoring existing scheduler and model runner components to support its unique operational flow. The changes aim to enable more flexible and potentially optimized request processing by altering how tokens are managed and KV cache blocks are allocated in an asynchronous context.
Highlights
- New Async Scheduler Implementation: Introduced a new
AsyncScheduler
class, inheriting from the baseScheduler
, specifically designed to handle asynchronous scheduling logic. This includes distinct behaviors for token management and KV cache block allocation. - Configurability and CLI Integration: Added an
async_scheduling
boolean option toSchedulerConfig
andEngineArgs
, which is also exposed as a command-line argument (--async-scheduling
), allowing users to easily enable or disable the new asynchronous scheduling mode. - Refactored Scheduler Logic: The base
Scheduler
class has been refactored to extract common logic into new, overridable methods such as_free_encoder_inputs
,update_request
, andupdate_before_output
. This modularization enables theAsyncScheduler
to implement its specific token and KV cache handling. - Modified Token and KV Cache Management: Adjusted how token IDs are appended and KV cache blocks are managed within both the
AsyncScheduler
and thegpu_model_runner
. In async mode, token updates and block caching are handled differently, often in a more deferred or single-token-at-a-time manner. - Batching Strategy Adaptation: The
multiproc_executor
now setsmax_concurrent_batches
to2
when asynchronous scheduling is enabled, indicating a tailored batching strategy for this new mode. - Temporary Distributed Executor Backend Fix: A temporary
# FIXME
change was added tovllm/config.py
to default thedistributed_executor_backend
tomp
(multiprocessing) whenworld_size
is1
, overriding the previousuni
(unified) setting.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces asynchronous scheduling, which is a significant feature. The changes are well-structured, with a new AsyncScheduler
and modifications to support it throughout the codebase. The use of a feature flag async_scheduling
is a good approach for introducing this functionality.
I've identified a couple of points for improvement:
- A
FIXME
comment invllm/config.py
could use more context. - There appears to be a redundant method call in
vllm/v1/core/sched/async_scheduler.py
that could lead to inefficiency.
Overall, this is a solid implementation. Addressing these points will improve the code's clarity and correctness.
self._free_encoder_inputs(request) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The _free_encoder_inputs
method is called here in update_before_output
. However, the base Scheduler
class already calls this method in update_from_output
. Since AsyncScheduler
inherits from Scheduler
and doesn't override update_from_output
, this will result in _free_encoder_inputs
being called twice for each request.
While the method might be idempotent, this is inefficient and can be confusing. It seems this call should be removed from update_before_output
to avoid the redundant call.
👋 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 🚀 |
# OPTIMIZATION: Avoid list(set) if the set is empty. | ||
if cached_encoder_input_ids: | ||
for input_id in list(cached_encoder_input_ids): | ||
mm_positions = request.mm_positions[input_id] | ||
start_pos = mm_positions.offset | ||
num_tokens = mm_positions.length | ||
if start_pos + num_tokens <= request.num_computed_tokens: | ||
# The encoder output is already processed and stored | ||
# in the decoder's KV cache. | ||
self.encoder_cache_manager.free_encoder_input( | ||
request, input_id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really related to this PR, but I think we can completely remove the list(set)
call so which makes it a bit more readable and remove the need for the check
# OPTIMIZATION: Avoid list(set) if the set is empty. | |
if cached_encoder_input_ids: | |
for input_id in list(cached_encoder_input_ids): | |
mm_positions = request.mm_positions[input_id] | |
start_pos = mm_positions.offset | |
num_tokens = mm_positions.length | |
if start_pos + num_tokens <= request.num_computed_tokens: | |
# The encoder output is already processed and stored | |
# in the decoder's KV cache. | |
self.encoder_cache_manager.free_encoder_input( | |
request, input_id) | |
for input_id in cached_encoder_input_ids: | |
mm_positions = request.mm_positions[input_id] | |
start_pos = mm_positions.offset | |
num_tokens = mm_positions.length | |
if start_pos + num_tokens <= request.num_computed_tokens: | |
# The encoder output is already processed and stored | |
# in the decoder's KV cache. | |
self.encoder_cache_manager.free_encoder_input( | |
request, input_id) |
@@ -253,7 +261,9 @@ def schedule(self) -> SchedulerOutput: | |||
request, | |||
num_new_tokens, | |||
num_draft_tokens=num_draft_tokens, | |||
num_lookahead_tokens=self.num_lookahead_tokens) | |||
num_lookahead_tokens=self.num_lookahead_tokens, | |||
delay_cache_blocks=self.is_async, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we want to preserve this behavior:
vllm/docs/design/v1/prefix_caching.md
Line 143 in e6327c9
4. If an allocated block is already full of tokens, we immediately add it to the Cache Block, so that the block can be reused by other requests in the same batch. |
Signed-off-by: Woosuk Kwon <[email protected]>
Essential Elements of an Effective PR Description Checklist
supported_models.md
andexamples
for a new model.Purpose
This PR implements async scheduler with minimal code modifications.
Test Plan
Test Result
(Optional) Documentation Update