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

Add prefix cache aware routing #641

Merged
merged 11 commits into from
Feb 10, 2025
Merged

Add prefix cache aware routing #641

merged 11 commits into from
Feb 10, 2025

Conversation

varungup90
Copy link
Collaborator

No description provided.

@varungup90 varungup90 changed the title Add prefix cache aware routing WIP: Add prefix cache aware routing Feb 7, 2025
@varungup90 varungup90 changed the title WIP: Add prefix cache aware routing Add prefix cache aware routing Feb 7, 2025
end = len(unMatchedTokens)
}
chunk := unMatchedTokens[i:end]
prefixHash := xxhash.Sum64(IntArrayToByteArray(chunk))
Copy link
Collaborator

Choose a reason for hiding this comment

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

here. it just consider the current block size? just like to confirm this is not 100% same as vLLM's solution right? their 1st block hash is part of 2nd hash input

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, hash only for current block. No link list kind of behavior that vllm has. For our usecase we do not need that link list behavior.

Copy link
Collaborator

Choose a reason for hiding this comment

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

if that case, we need to do up to O(n) calculations? n=number of blocks

Copy link
Collaborator Author

@varungup90 varungup90 Feb 9, 2025

Choose a reason for hiding this comment

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

Yes. If you are referring to that link list behavior could prevent O(n) calculations then it wont be the case. Total computations stays the same.

Copy link
Collaborator

Choose a reason for hiding this comment

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

it's a little bit different. LinkedList you can do binary search etc for optimization. In this way, we can only do O(n).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Let me look into this, but for our usecase we need to evaluate all blocks to ensure a 50%+ hit rate.

Copy link
Collaborator

@gaocegege gaocegege left a comment

Choose a reason for hiding this comment

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

Some nits.

Copy link
Collaborator

@Jeffwan Jeffwan left a comment

Choose a reason for hiding this comment

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

Let's move a little bit faster and track those TODOs in separate issues

@varungup90 varungup90 merged commit f40c973 into main Feb 10, 2025
9 of 10 checks passed
@varungup90 varungup90 deleted the add-prefix-cache branch February 10, 2025 22:09
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.

3 participants