-
Notifications
You must be signed in to change notification settings - Fork 100
feat: concurrent multi stream executor for rec_model. #548
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?
feat: concurrent multi stream executor for rec_model. #548
Conversation
xllm/core/common/global_flags.cpp
Outdated
| "Number of streams for parallel step execution."); | ||
|
|
||
| // --- fixsteps scheduler config --- | ||
| DEFINE_bool(use_fixsteps_scheduler, |
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.
what's the meaning of fixsteps?
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.
fixed_steps
f9e2fc0 to
b3cd0b5
Compare
xllm/core/runtime/worker.cpp
Outdated
| impl_ = new LLMWorkerImpl(parallel_args, device, options); | ||
| if (fLB::FLAGS_enable_concurrent_llm_worker) { | ||
| impl_ = new ConcurrentLLMWorkerImpl( | ||
| parallel_args, device, options, FLAGS_concurrent_execute_stream_num); |
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.
nit: delete fLB:: ?
and maybe we dont need to pass FLAGS_concurrent_execute_stream_num to ConcurrentLLMWorkerImpl function.
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.
Done.
| return; | ||
| } | ||
|
|
||
| // LOG(INFO) << "ContinuousScheduler::step: batch size " << batch.size(); |
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.
nit: delete comment.
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.
Done.
xllm/core/common/global_flags.cpp
Outdated
| "Whether to enable multi stream llm."); | ||
|
|
||
| DEFINE_int32(concurrent_execute_stream_num, | ||
| 2, |
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.
nit: can a single flag be used instead of these two flags? like we use concurrent_execute_stream_num, when concurrent_execute_stream_num=1 we know that use default llm worker. Cause that there to many flags now :( . Just a suggestion.
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.
Done.
27cf6cc to
87fc7cc
Compare
benchmark client cmd:
Normal scheduler and llm_worker, server cmd:
result:
fixedstep_scheduler and concurrent_llm_worker, server cmd:
result: