Conversation
![lance-gatekeeper[bot]](https://avatars.githubusercontent.com/in/4433141?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
lance-gatekeeper
Bot
added
the
K-changes
Joining a partial prefetch window with the fetched tail copied the whole unread span while the tail was still live, so a header read followed by read() peaked at about twice the unread payload in Rust. On a 12 MiB blob with a 1 MiB window that was 24.1 MB of peak allocation for 12.6 MB of payload, where the plain cursor..size read peaks at 12.6 MB. The window is now reused only when the tail past it is no larger than the bytes reused. The transient extra memory then never exceeds the transfer saved and peak stays under 1.5x the unread span. Larger tails fall back to the single cursor..size read, which re-fetches at most one window. Reads above the scheduler's 16 MiB request split still peak at 2x because the split pieces are joined into one buffer; that is unchanged from before the prefetch reuse and outside this change. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NumG7qmJa1QqiLJe3q2LcS
lance-gatekeeper
Bot
removed
the
K-changes
Contributor
There was a problem hiding this comment.
✅ Gate recommendation: approve.
The prior memory finding is withdrawn because its allocation measurement did not isolate the pre-existing scheduler behavior. This head keeps complete-window reuse, bounds partial-tail concatenation, and falls back to the prior read for large tails; the focused matrix verifies the three paths, transferred bytes, returned data, and final cursor.
lance-gatekeeper
Bot
added
the
K-approved
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
1 participant
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.
While working on the buffering pr for blobfiles in #9251 I noticed that a header read followed by
read()could download the same bytes twice becauseread()ignored the existing prefetch window and always fetchedcursor..size.So now we reuse the prefetched bytes and only fetch the missing remainder. This avoids the duplicate download while preserving the existing cursor behavior.
do_with_cursorwas only used byread(), so the cursor/window bookkeeping is inlined there.