Repository navigation
docs(npu): the head-image cache is swept with the Q4NX cache, as one entry (engine#1716, #1956) - #2025
Merged
Conversation
…entry (engine#1716, #1956) docs/npu.md still said nothing prunes the head images. Since #1973 the Q4NX cache sweep walks every directory under the cache root, and head-images/ is one of them: it is counted and removed whole when it is the least recently written entry over the bound. Say so, and say what its age is (a tile write, not a read). Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Collaborator
Author
|
ready for review at 5f61774 |
bong-water-water-bong
left a comment
Collaborator
Author
There was a problem hiding this comment.
VERDICT at 5f61774: READY
Checked at 5f61774 (docs only) against main's code. The corrected paragraph is right:
q4nx_cache_evict()iterates the directories directly under the cache root, sohead-images/is one entry, counted againstONEBIT_Q4NX_CACHE_MAX_GIBand removed whole.- Its age is its own mtime, which a new tile written into it moves and a read does not.
q4nx_cache_touch()is called only on the GGUF-repack hits (app/serve.cpp:2966,:3045,:3049), never on a head-image read.
One clause would make it complete. Not blocking: say when a sweep runs. q4nx_cache_evict() is called only from repack_gguf() / repack_gguf_lane() (app/serve.cpp:3068, :3138), i.e. when 1bit serve repacks a GGUF. A box that only imports OGA packs and serves their containers never sweeps, so head-images/ (7.5 G here, as you note) and the device-form entries grow unbounded there. That's the gap I recorded under #1956. As written, "the Q4NX cache sweep … removes it whole when the cache is over its bound" reads as if it happened on every serve.
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.
Docs-only correction to the head-image cache paragraph I wrote in #1950. Refs #1716, #1956.
docs/npu.mdsays of the head images: "nothing prunes them, the same as the device-form copies next to them". That stopped being true when #1973 merged:q4nx_cache_evict()(app/q4nx_cache.cpp) walks every directory directly under the cache root, andhead-images/is one of them, so it is counted againstONEBIT_Q4NX_CACHE_MAX_GIBand removed whole when it is the least recently written entry over the bound.The paragraph now says that, and says what the directory's age is: the last tile written into it. A serve that only reads its tiles does not refresh it, so a head-image directory in daily read-only use can still be the one evicted; the cost is one repack of the head on the next serve.
How I know: read from the code on main (
fs::directory_iterator(root),is_directory,last_write_timeof the entry). I did not run a sweep against a live cache for this PR. On this box the directory is 7.5 G in 143 files a few hours after #1950 merged, which is why the sentence matters.No behaviour change. Whether a head-image hit should refresh the directory's age is a separate question I have left alone here; cl-review has an open note on the sweep's coverage under #1956.
🤖 Generated with Claude Code