fix: let an unchanged conversation hit the prompt cache - #734
Open
Javinator9889 wants to merge 1 commit into
Open
Javinator9889 wants to merge 1 commit into
Javinator9889 wants to merge 1 commit into
Conversation
can_use_cache required at least two new messages (messages.size() - 2), but message_checksums_ is updated to the full incoming list after every request. A resend of an unchanged conversation therefore fails the bound and misses, re-prefilling everything -- which is slower than the request that timed out, so the client's retry times out too and the loop sustains itself. Bound by the full incoming length instead. Also guard the case this exposes: with nothing left to prefill, _chunked_insert computes zero chunks and hands an empty logits buffer to sampler->sample(). Refs ROCm#733
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the first defect in #733.
can_use_cacherequires at least two new messages:But
message_checksums_is updated to the full incoming list after everyrequest, so a resend of an unchanged conversation has
message_checksums_.size() == messages.size()and always fails the bound.That is exactly what a client sends after a timeout. The miss re-prefills the
whole conversation — slower than the request that just timed out — so the retry
times out as well and the loop sustains itself. Observed with OpenCode on a
146-message, ~98k-token conversation.
Bounding by the full incoming length lets the resend hit. The second hunk guards
the case that exposes: with nothing left to prefill,
_chunked_insertcomputeszero chunks and returns a default-constructed (empty) logits buffer straight into
sampler->sample(). Without it this change crashes the connection.Verified on
qwen3.5:9b: usage contract and generation output unchanged, and aburied-value probe returns the same string 4/4 on both cold and cached paths.
Note this does not make the fully-cached case cheap — it still re-prefills.
Doing better needs engine-side support; see #733 for the measurements.