Repository navigation
[Other] serve: an all-cached image turn never wakes the elastic encoder - #2455
jerry78424 wants to merge 3 commits into
Conversation
|
Two small points from reading the branch (tests
|
Both points from Niko1221#2454/Niko1221#2455 review (qiuzan2001): cached() returned the hit without the move-to-end encode() does on its hit path, so the re-sent pictures this path exists to answer were the oldest entries in the dict - encode()'s oldest-first eviction (past 64 entries) took exactly them, and the turn after paid the encode and the elastic start this PR removes. The lookup now refreshes the order inside the same lock. The description said the branch is only entered when vision.elastic; the code did not check. The lookup is now computed only for an elastic encoder: a non-elastic turn takes today's path byte-for-byte (FIFO, encode, cache hits inside encode()) and does not pay the key_of() hashing either. Two tests: a cache lookup marks the picture most recently used (the eviction order of three stored pictures flips to its benefit); an elastic=False encoder sees encode_all on every turn, hits included, with no make-room request. Suite: test_vision_cache_wake 8, plus test_vision_elastic / test_vision_busy / test_images - 36 OK.
|
Both points are right - fixed in bf73c58.
Docs (VRAM_ELASTIC/DETAILS) carried the wake rule only one way; they now say both halves (bdbb0e6). Suite on the branch: serve 791 OK (15 skipped), the vision four 36 OK. |
Both points from Niko1221#2454/Niko1221#2455 review (qiuzan2001): cached() returned the hit without the move-to-end encode() does on its hit path, so the re-sent pictures this path exists to answer were the oldest entries in the dict - encode()'s oldest-first eviction (past 64 entries) took exactly them, and the turn after paid the encode and the elastic start this PR removes. The lookup now refreshes the order inside the same lock. The description said the branch is only entered when vision.elastic; the code did not check. The lookup is now computed only for an elastic encoder: a non-elastic turn takes today's path byte-for-byte (FIFO, encode, cache hits inside encode()) and does not pay the key_of() hashing either. Two tests: a cache lookup marks the picture most recently used (the eviction order of three stored pictures flips to its benefit); an elastic=False encoder sees encode_all on every turn, hits included, with no make-room request. Suite: test_vision_cache_wake 6, plus test_vision_elastic / test_vision_busy / test_images - 36 OK.
88032e4 to
bdbb0e6
Compare
The hash cache and its .sve files survive an elastic unload, but every image turn still took the encode FIFO and _vision_up() - so a client that re-sends its pictures (which is what most do) paid the elastic start again on every turn after an idle stop, text-only turns included. prepare() now maps the turn's images through the cache first: when every one is a hit the turn skips the FIFO, the start and the encode; a miss keeps today's path unchanged. serve/test_vision_cache_wake.py (CPU-only) covers it; the serve suite passes (348 tests, 3 skipped). Resolves Niko1221#2454.
Both points from Niko1221#2454/Niko1221#2455 review (qiuzan2001): cached() returned the hit without the move-to-end encode() does on its hit path, so the re-sent pictures this path exists to answer were the oldest entries in the dict - encode()'s oldest-first eviction (past 64 entries) took exactly them, and the turn after paid the encode and the elastic start this PR removes. The lookup now refreshes the order inside the same lock. The description said the branch is only entered when vision.elastic; the code did not check. The lookup is now computed only for an elastic encoder: a non-elastic turn takes today's path byte-for-byte (FIFO, encode, cache hits inside encode()) and does not pay the key_of() hashing either. Two tests: a cache lookup marks the picture most recently used (the eviction order of three stored pictures flips to its benefit); an elastic=False encoder sees encode_all on every turn, hits included, with no make-room request. Suite: test_vision_cache_wake 6, plus test_vision_elastic / test_vision_busy / test_images - 36 OK.
The elastic paragraphs in VRAM_ELASTIC.md and DETAILS.md told only when the encoder starts (a picture's turn, if it is not running). Since this branch a turn whose pictures are all in the hash cache takes neither the FIFO turn nor the start - the cache and its .sve files survive the stop, and most clients re-send every picture of the conversation on every turn - so both paragraphs now carry the other half of the rule, and the bullet points at the new unit tests. Docs only; the code and its suite are unchanged (791 OK, 15 skipped, serve discover on this branch).
bdbb0e6 to
ed4145d
Compare
|
Thanks for the pull request. We're resetting the tracker for 0.2.0-beta. There are more than 1,300 open issues and pull requests, too many for us to review properly, so we're closing everything that isn't being worked on right now. Closed here does not mean rejected. To reopen it takes about two minutes:
The rules are in CONTRIBUTING.md. |
Both points from Niko1221#2454/Niko1221#2455 review (qiuzan2001): cached() returned the hit without the move-to-end encode() does on its hit path, so the re-sent pictures this path exists to answer were the oldest entries in the dict - encode()'s oldest-first eviction (past 64 entries) took exactly them, and the turn after paid the encode and the elastic start this PR removes. The lookup now refreshes the order inside the same lock. The description said the branch is only entered when vision.elastic; the code did not check. The lookup is now computed only for an elastic encoder: a non-elastic turn takes today's path byte-for-byte (FIFO, encode, cache hits inside encode()) and does not pay the key_of() hashing either. Two tests: a cache lookup marks the picture most recently used (the eviction order of three stored pictures flips to its benefit); an elastic=False encoder sees encode_all on every turn, hits included, with no make-room request. Suite: test_vision_cache_wake 6, plus test_vision_elastic / test_vision_busy / test_images - 36 OK.
|
/reopen |
|
The check passed, but GitHub would not reopen it (Validation Failed: ). If the branch was deleted, restore it or open a new pull request and link this one. |
Problem and result
An elastic image encoder is woken (seconds of start, VRAM the expert cache gives up) on every turn that re-sends pictures it has already encoded - which is what most clients do on every turn. A turn whose images are all in the hash cache needs no encoder process at all.
Measured on RTX 5090 32 GB, Windows 11 (26200), engine 0.1.44.1, nvfp4,
elastic_idle_s: 10(table and verbatim log lines in #2454): before - a text-only turn with a re-sent picture cost a 2.3 s start + 2085 MiB taken from the expert cache; after - 722 ms with no start line. A re-sent-picture turn: 1277 ms, no start. New pictures unchanged (3979-4213 ms, still start).Related discussion
Resolves #2454. Independent of #2641 (the tool-picture wake lands in this same method - textual overlap only; the two PRs conflict in neither order, verified by cherry-pick).
Approach
serve/server.py:Vision.key_of()/Vision.cached()- a hash-cache lookup that answers with the process down (the cache and its .sve files survive an unload, as before) and marks a hit most recently used, asencode()does on its hit path.prepare()maps the turn's images through the cache first, for an elastic encoder only: when every one is a hit the turn skips the encode FIFO,_vision_up()and the encode. A miss - alone or alongside hits - keeps today's path byte-for-byte: FIFO, make-room, start, encode.serve/test_vision_cache_wake.py(new, CPU-only, no engine): a re-sent picture does not wake a stopped encoder; a new picture alongside cached ones still wakes it; the lookup answers with the process down and keeps the re-sent picture from aging out of the 64-entry cache; a non-elastic turn keeps today's path even when every picture is cached.docs/VRAM_ELASTIC.md,docs/DETAILS.md: both halves of the elastic rule - a picture's turn starts the encoder, a turn of pictures the cache knows takes neither the turn nor a start.Validation
Passed:
python -m unittest serve.test_vision_elastic serve.test_vision_busy serve.test_vision_cache_wake serve.test_images: the vision four 36 OK - ate88ee39d(v0.2.0-beta-12), rebased 2026-10-20, Windows 11 26200, Python 3.12 venv, CPU-only tests.Failed: none observed.
Not run: AMD/HIP and layer-split machines (a layer split refuses elastic anyway; its start failure is #2435). No native files are touched, so no native builds are required.
Limits