-
Notifications
You must be signed in to change notification settings - Fork 79
Unify RPS and Concurrent Scheduler Paths #233
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
Conversation
a6521a7
to
2e74a1b
Compare
cf81f0a
to
2e10740
Compare
2e10740
to
c91b3ef
Compare
f4a8d66
to
17e1be7
Compare
Signed-off-by: Samuel Monson <[email protected]>
Signed-off-by: Samuel Monson <[email protected]>
Signed-off-by: Samuel Monson <[email protected]>
Signed-off-by: Samuel Monson <[email protected]>
Signed-off-by: Samuel Monson <[email protected]>
17e1be7
to
b7a2eaf
Compare
Signed-off-by: Samuel Monson <[email protected]>
Signed-off-by: Samuel Monson <[email protected]>
Signed-off-by: Samuel Monson <[email protected]>
330ddf5
to
681d505
Compare
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.
Pull Request Overview
Unifies the scheduling methods for both async and synchronous modes by removing the synchronous mode and standardizing on the asynchronous approach. This change is made in preparation for multi-turn conversation support and significantly reduces process IDs at high concurrency.
- Removed the separate synchronous scheduling path and unified on the asynchronous approach
- Moved type definitions from scheduler-specific module to a more general request types module
- Updated the scheduling strategy to use a consistent start time across all processes
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
src/guidellm/scheduler/worker.py | Removed synchronous process loop, unified on async scheduling with updated parameters |
src/guidellm/scheduler/strategy.py | Added start_time field and updated request_times methods to use consistent timing |
src/guidellm/scheduler/scheduler.py | Removed sync/async branching logic, updated queue handling and process management |
src/guidellm/scheduler/result.py | Moved WorkerProcessRequest and WorkerProcessResult dataclasses from worker module |
src/guidellm/scheduler/queues.py | New helper module for queue type imports and MPQueues dataclass |
src/guidellm/request/types.py | Formatting update to all list |
src/guidellm/request/loader.py | Added type hints to abstract methods |
src/guidellm/config.py | Updated max_worker_processes calculation and added scheduler_start_delay setting |
src/guidellm/benchmark/benchmarker.py | Updated import path for RequestT and ResponseT types |
src/guidellm/benchmark/aggregator.py | Updated import path for RequestT and ResponseT types |
src/guidellm/scheduler/init.py | Removed exports that moved to other modules |
src/guidellm/request/init.py | Added RequestT and ResponseT to exports |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
681d505
to
8bdfc83
Compare
Signed-off-by: Samuel Monson <[email protected]>
8bdfc83
to
89b501f
Compare
Signed-off-by: Samuel Monson <[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.
Looks good to me.
Unify the scheduling method used for async and synchronous modes in preparation for multi-turn conversation support. This change with also significantly reduce the number PIDs used by GuideLLM at high concurrency.
Relates to #196