[core] invoke save_new_computed_blocks when computed blocks are not empty#1568
Conversation
… empty Signed-off-by: AlpinDale <alpindale@gmail.com>
![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 aims to prevent an unnecessary call to save_new_computed_blocks when there are no new computed blocks. This is a good optimization. The accompanying change in single_type_kv_cache_manager.py correctly prevents a KeyError that would otherwise be introduced. However, the condition added in kv_cache_manager.py relies on an object identity check which is not fully reliable for determining if the block list is empty. I've provided a suggestion for a more robust emptiness check.
| # Append the new computed blocks to the request blocks until now to | ||
| # avoid the case where the new blocks cannot be allocated. | ||
| self.coordinator.save_new_computed_blocks(request.request_id, new_computed_block_list) | ||
| if new_computed_block_list is not self.empty_kv_cache_blocks.blocks: |
There was a problem hiding this comment.
The condition new_computed_block_list is not self.empty_kv_cache_blocks.blocks checks for object identity, but it doesn't robustly check for emptiness. new_computed_block_list could be a different object that still represents an empty list of blocks (e.g., a tuple of empty lists ([], []) vs the empty tuple of tuples ((), ())). This can lead to unnecessary calls to save_new_computed_blocks.
A more reliable way to check if there are any blocks is to iterate through the outer tuple and check if any of the inner sequences are non-empty. Using a generator expression with any() is an efficient way to do this.
| if new_computed_block_list is not self.empty_kv_cache_blocks.blocks: | |
| if any(g for g in new_computed_block_list): |
1 participant
No description provided.