Skip to content
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

Reuse KV cache of prefixes #5572

Draft
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

tohtana
Copy link
Contributor

@tohtana tohtana commented May 27, 2024

This PR implements reusing KV cache prefixes.
See microsoft/DeepSpeed-MII#484 for details.

for i in range(num_already_cached_blocks, n_blocks):
chunk = tokens[:(i + 1) * self.block_size]
hash = token_ids_to_hash(chunk)
if hash not in self.tokens_to_blocks:
Copy link

Choose a reason for hiding this comment

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

Do we still need this if statement if in the for loop you are already starting from num_already_cached_blocks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if it is necessary but there might be a complicated case. Assume we are running two requests.
Request 1 is using cached blocks 0, 1, 2, and just generated the last token of the current block. Then it saves the generated sequence and a hash.
Request 2 is also using the cached block 0, 1, 2, but the generation is a few steps later than Request 1. They are not sharing the last block. But the request may generate exact same tokens for the last block. So, it will try to update the cache with the same hash.
In this case, I didn't want to overwrite the cache.

Copy link

Choose a reason for hiding this comment

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

sounds good, so this is to ensure we are sharing the recently generated blocks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants