Skip to content

Breaking change: perf: Enable scheduling overlap by default #4174

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
May 15, 2025
Merged

Breaking change: perf: Enable scheduling overlap by default #4174

merged 3 commits into from
May 15, 2025

Conversation

kaiyux
Copy link
Member

@kaiyux kaiyux commented May 9, 2025 •
edited
Loading

This PR:

  1. Renamed the enable_overlap_scheduler argument in PyTorchConfig to disable_overlap_scheduler.
  2. Enabling scheduling overlap by default.

@kaiyux
Copy link
Member Author

kaiyux commented May 9, 2025

/bot run --disable-fail-fast

Copilot
Copilot AI reviewed May 9, 2025
View reviewed changes
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR reverses the overlap scheduler flag by replacing the old “enable_overlap_scheduler” parameter with “disable_overlap_scheduler” (defaulted to False) across the codebase to enable scheduling overlap by default.

  • Inverted the flag logic in all relevant modules (worker, executor, pyexecutor, model_engine, decoder, etc.).
  • Updated CLI examples, documentation, and configuration files to reflect the new “disable_overlap_scheduler” naming and semantics.

Reviewed Changes

Copilot reviewed 47 out of 47 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tensorrt_llm/scaffolding/worker.py Parameter renamed and updated in worker initialization.
tensorrt_llm/executor/worker.py Flag condition inverted for determining scheduling overlap.
tensorrt_llm/commands/* Removed legacy flag usage in serve and eval commands.
tensorrt_llm/_torch/pyexecutor/* Multiple files updated to invert flag logic consistently.
examples/* and docs/* CLI/example scripts and documentation now reference the new flag.

@tensorrt-cicd
Copy link
Collaborator

PR_Github #4651 [ run ] triggered by Bot

@kaiyux kaiyux requested a review from amukkara May 9, 2025 06:13
@tensorrt-cicd
Copy link
Collaborator

PR_Github #4651 [ run ] completed with state SUCCESS
/LLM/main/L0_MergeRequest_PR pipeline #3354 completed with status: 'FAILURE'

@kaiyux kaiyux force-pushed the user/kaiyu/def_schedule_overlap branch from 8e4b83f to dfd073f Compare May 9, 2025 09:03
@kaiyux
Copy link
Member Author

kaiyux commented May 9, 2025

/bot run --disable-fail-fast

@tensorrt-cicd
Copy link
Collaborator

PR_Github #4692 [ run ] triggered by Bot

@tensorrt-cicd
Copy link
Collaborator

PR_Github #4692 [ run ] completed with state SUCCESS
/LLM/main/L0_MergeRequest_PR pipeline #3382 completed with status: 'FAILURE'

@kaiyux kaiyux force-pushed the user/kaiyu/def_schedule_overlap branch from dfd073f to 3c8cf62 Compare May 11, 2025 07:20
@kaiyux
Copy link
Member Author

kaiyux commented May 11, 2025

/bot run --disable-fail-fast

@tensorrt-cicd
Copy link
Collaborator

PR_Github #4767 [ run ] triggered by Bot

@tensorrt-cicd
Copy link
Collaborator

PR_Github #4767 [ run ] completed with state SUCCESS
/LLM/main/L0_MergeRequest_PR pipeline #3443 completed with status: 'FAILURE'

@QiJune
Copy link
Collaborator

QiJune commented May 12, 2025

/bot run --disable-fail-fast

@tensorrt-cicd
Copy link
Collaborator

PR_Github #4790 [ run ] triggered by Bot

@tensorrt-cicd
Copy link
Collaborator

PR_Github #4790 [ run ] completed with state SUCCESS
/LLM/main/L0_MergeRequest_PR pipeline #3464 completed with status: 'FAILURE'

@kaiyux
Copy link
Member Author

kaiyux commented May 14, 2025

/bot run --disable-fail-fast --add-multi-gpu-test

@kaiyux kaiyux enabled auto-merge (squash) May 14, 2025 01:33
@tensorrt-cicd
Copy link
Collaborator

PR_Github #5075 [ run ] triggered by Bot

@tensorrt-cicd
Copy link
Collaborator

PR_Github #5075 [ run ] completed with state SUCCESS
/LLM/main/L0_MergeRequest_PR pipeline #3694 completed with status: 'FAILURE'

@kaiyux
Copy link
Member Author

kaiyux commented May 14, 2025

/bot run --stage-list "DGX_H100-4_GPUs-PyTorch-Others-1"

@tensorrt-cicd
Copy link
Collaborator

PR_Github #5127 [ run ] triggered by Bot

@tensorrt-cicd
Copy link
Collaborator

PR_Github #5127 [ run ] completed with state SUCCESS
/LLM/main/L0_MergeRequest_PR pipeline #3733 (Partly Tested) completed with status: 'FAILURE'

@kaiyux kaiyux force-pushed the user/kaiyu/def_schedule_overlap branch from fdeeb26 to cc02fae Compare May 14, 2025 12:10
@kaiyux
Copy link
Member Author

kaiyux commented May 14, 2025

/bot run --disable-fail-fast --add-multi-gpu-test

@tensorrt-cicd
Copy link
Collaborator

PR_Github #5171 [ run ] triggered by Bot

@lucaslie lucaslie added the AutoDeploy <NV> AutoDeploy Backend label May 14, 2025
@lucaslie lucaslie removed the AutoDeploy <NV> AutoDeploy Backend label May 14, 2025
@tensorrt-cicd
Copy link
Collaborator

PR_Github #5171 [ run ] completed with state FAILURE
/LLM/main/L0_MergeRequest_PR pipeline #3772 completed with status: 'FAILURE'

kaiyux added 3 commits May 15, 2025 00:53
@kaiyux
Breaking change: Enable scheduling overlap by default
752844f 
Signed-off-by: Kaiyu Xie <[email protected]>

Fix

Signed-off-by: Kaiyu Xie <[email protected]>

Fix test_ptp_quickstart_advanced_eagle3

Signed-off-by: Kaiyu Xie <[email protected]>

Fix

Signed-off-by: Kaiyu Xie <[email protected]>

Fix

Signed-off-by: Kaiyu Xie <[email protected]>

Fix

Signed-off-by: Kaiyu Xie <[email protected]>
@kaiyux
Fix
949c323 
Signed-off-by: Kaiyu Xie <[email protected]>
@kaiyux
Fix
a0ac332 
Signed-off-by: Kaiyu Xie <[email protected]>
@kaiyux kaiyux force-pushed the user/kaiyu/def_schedule_overlap branch from cc02fae to a0ac332 Compare May 15, 2025 01:06
@kaiyux
Copy link
Member Author

kaiyux commented May 15, 2025

/bot run --disable-fail-fast --add-multi-gpu-test

@tensorrt-cicd
Copy link
Collaborator

PR_Github #5227 [ run ] triggered by Bot

@kaiyux
Copy link
Member Author

kaiyux commented May 15, 2025

Pre-merge pipeline has passed, merging this in case there are going to be more conflicts.

@kaiyux
Copy link
Member Author

kaiyux commented May 15, 2025

/bot skip --comment "pre-merge pipeline has passed"

@tensorrt-cicd
Copy link
Collaborator

PR_Github #5278 [ skip ] triggered by Bot

@tensorrt-cicd
Copy link
Collaborator

PR_Github #5227 [ run ] completed with state ABORTED

@tensorrt-cicd
Copy link
Collaborator

PR_Github #5278 [ skip ] completed with state SUCCESS
Skipping testing for commit a0ac332

@kaiyux kaiyux merged commit b4e5df0 into NVIDIA:main May 15, 2025
3 checks passed
@rmccorm4 rmccorm4 mentioned this pull request May 30, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@syuoni syuoni syuoni left review comments

Copilot code review Copilot Copilot left review comments

@nv-yilinf nv-yilinf nv-yilinf left review comments

@lucaslie lucaslie lucaslie approved these changes

@hlu1 hlu1 hlu1 approved these changes

@Funatiq Funatiq Funatiq approved these changes

@QiJune QiJune QiJune approved these changes

@amukkara amukkara amukkara approved these changes

@dongxuy04 dongxuy04 Awaiting requested review from dongxuy04

@dcampora dcampora Awaiting requested review from dcampora

@HuiGao-NV HuiGao-NV Awaiting requested review from HuiGao-NV

@suyoggupta suyoggupta Awaiting requested review from suyoggupta

+1 more reviewer

@zhyncs zhyncs zhyncs left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

10 participants

@kaiyux @tensorrt-cicd @QiJune @lucaslie @hlu1 @Funatiq @syuoni @zhyncs @amukkara @nv-yilinf