Skip to content

[core][scheduler] simplify and improve scheduler #6867

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

Merged
merged 9 commits into from
Aug 1, 2024
Merged

[core][scheduler] simplify and improve scheduler #6867

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
58504ea
remove copy
youkaichao Jul 27, 2024
18f8c03
remove policy
youkaichao Jul 27, 2024
86ab85b
fix tests
youkaichao Jul 27, 2024
e32d7d7
update _schedule_prefills
youkaichao Jul 27, 2024
1d82484
update _schedule_running
youkaichao Jul 27, 2024
6775398
update _schedule_swapped
youkaichao Jul 27, 2024
4b451bd
fix tests
youkaichao Jul 27, 2024
554110d
remove policy
youkaichao Jul 27, 2024
917221e
update tests
youkaichao Jul 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion tests/core/block/e2e/test_correctness.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ def test_v1_v2_greedy_equality_with_cow(baseline_llm_generator,

# Allow only 2 sequences of ~128 tokens in worst case.
# Note 16 = 128/block_size
"num_gpu_blocks_override": 2 * (16 + 1),
"num_gpu_blocks_override": 2 * (16 + 2),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just curious: What is this change for?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is some corner case, that swapping order change in this PR makes the number of blocks not enough.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@youkaichao Oh why the swapping order changes? I thought the change in this PR doesn't actually change the FCFS logic at all.

Copy link
Member Author

Choose a reason for hiding this comment

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

The running queue is FCFS. The swapping queue is somewhat complicated, when you swap in / swap out again and again (which happens in this test case).

That being said, I don't think we have strict FCFS even before this PR, when we consider swap in / swap out again and again. I think we can only guarantee strict FCFS after we use priority queue in the future.

}
])
@pytest.mark.parametrize("baseline_llm_kwargs", [{
Expand Down
Loading