[Draft] kv aware schedule #134
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces changes to make the KV cache scheduling aware of multi-modal (mm) features by associating mm_hashes with KV cache blocks and request outputs. The changes are propagated through event definitions, block pool management, and the output processor. My review identified a critical bug in vllm/v1/core/block_pool.py related to an incorrect token index calculation, which could lead to incorrect behavior. I also found a minor issue with a missing type hint in vllm/v1/engine/output_processor.py that should be addressed for better code quality.
| for _ in range(num_full_blocks - | ||
| num_cached_blocks)] | ||
| start_token_idx = num_cached_blocks * block_size | ||
| end_token_idx = num_full_blocks + block_size |
There was a problem hiding this comment.
There appears to be a bug in the calculation of end_token_idx. It should be num_full_blocks * block_size to correctly represent the end token index of the full blocks. The current calculation num_full_blocks + block_size mixes block count with block size, which will lead to an incorrect token range and cause multi-modal features to be associated with the wrong blocks.
| end_token_idx = num_full_blocks + block_size | |
| end_token_idx = num_full_blocks * block_size |
| top_p: Optional[float] = None, | ||
| n: Optional[int] = None, | ||
| temperature: Optional[float] = None, | ||
| mm_hashes=None, |
There was a problem hiding this comment.
The type hint for the mm_hashes parameter is missing. For consistency with other parameters and to improve code clarity and maintainability, please add the appropriate type hint.
| mm_hashes=None, | |
| mm_hashes: Optional[list[str]] = None, |
|
do we have a related PR in llm-service repo for this scheduling |
2 participants
Purpose
Test Plan
Test Result
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.