Repository navigation
serve: a decoder that cannot share its KV gets the state-file route, not the decoder's own prefill - #1203
Conversation
…not the decoder's own prefill
`ONEBIT_KV_SHARE=1` makes the hybrid route hand the prompt over as dma-buf fds, and the handshake has
one failure mode with no recovery in it: when the decoder refuses `?action=share` (a params or model
mismatch, or a hybrid cache on a pin without the recurrent-state arm), `prepare()` returned the refusal
and the provider never ran, so the request fell back to the decoder prefilling the whole prompt -- the
exact cost the hybrid route exists to remove.
Now the handshake failures (share refused, no share_id, no socket) fall back to the state-file route
for that request, which still keeps the prompt off the decoder's prefill path, and say so once:
`... this decoder cannot share its KV, so the state-file route is used`, with `share 0.000 ms` in the
per-request line. A provider that already wrote into the region, or an adopt that failed after it, is
still a plain fallback: falling back there would pay for the prefill twice.
Measured with tests/fake_backend.py in a new mode (`FAKE_KVSHARE_REFUSE=1`, a decoder that answers the
real refusal), one 30-token prompt, same binary before and after:
before hybrid: share answered 400: {...cannot be shared}; the backend prefills this request itself
(no provider launch, no restore, id_slot unset)
after hybrid: share answered 400: {...cannot be shared}; this decoder cannot share its KV, so the
state-file route is used
hybrid: 30 prompt tokens: prefill 0 ms, restore 2 ms, share 0.000 ms, first byte 103 ms
Gates, all on today's main (29ec80c), device-free: `hybrid_kvshare_route` PASS with 6 new checks (the
new case fails 4 of them before the change), `hybrid_route` PASS, `hybrid_e2e` PASS against a real
llama-server and Qwen3-0.6B (identical greedy reply to `--device cpu`, `prompt_n 1, plain 22`),
`ctest -R hybrid` 2/2, `tools/copyright.py --check` clean.
docs/hybrid.md also loses a stale limit: the shared region has covered a hybrid model's recurrent state
since fork 1bit-MONSTER/llama.cpp#181, and the doc still said `?action=share` refuses a hybrid cache.
Same class as engine#941 (a handoff silently thrown away); part of engine#698.
efa2e51 to
f8d39b6
Compare
|
Rebased onto current |
…ke the state-file fallback
The fallback added in the previous commit is deliberately limited to the handshake (share refused, no share_id, no
socket). The boundary is worth a case of its own, because the wrong side of it is expensive rather than wrong-looking:
if an `?action=adopt` refusal also fell back, the engine would re-run the whole prompt through the state-file route
after the provider had already prefilled it -- paying for the prefill twice, which is the failure the route exists to
avoid.
New arm: `FAKE_KVSHARE_ADOPT_REFUSE=1` makes the decoder refuse the adopt the way the real one does when another
sequence holds the cells ("Cannot adopt the shared cells: ..."). The arm asserts four things at once: the reply still
comes through, the provider *did* run and wrote into the region (its marker), the refusal is named (`adopt answered
400`), and `cannot share its KV` -- the fallback line -- does **not** appear.
`tests/hybrid_kvshare_route.sh` PASS, 28 checks (23 before this commit).
|
Added the other side of the boundary as a second commit (d31b14b), because the wrong side of it is expensive rather than visibly wrong: an New arm in |
|
Companion half found while gating this PR against a real decoder, and it is the case this fallback cannot reach on its own: with Root cause is in the fork and two-layered: Measured end to end through this route, one request, real model, flag on: with |
|
Composition check, both PRs together on current against 89 passed / 1 failed of 90 for a |
…fore adopting its cells
The shared route took the region on the provider's exit code alone. `?action=adopt` then hands the decoder
`ids.size()` cells whatever happened, so a provider that exits 0 after writing fewer rows than it was given leaves the
decoder decoding from a region whose last rows were never written -- a wrong answer, not a failed request, and nothing
in the log says so. The file route has carried this cross-check since engine#941 (`provider prefilled N of M tokens`,
then the state file is removed and the request goes out plain); the shared route had no equivalent.
Now it parses the provider's last stdout line the same way and refuses the adopt on a mismatch:
hybrid: provider prefilled 29 of 30 tokens into the shared region; the backend prefills this request itself
A line that does not parse is still not an error (on this route, like on the file route, the region is the result), and
a provider's own `prefill_ms` is taken from the same line, which the file route already did -- so the two routes report
the same number for the same work.
Reproduced first, with a provider that exits 0 and claims one token fewer (`FAKE_KVSHARE_SHORT=1`): before the change
the adopt happened and the run answered from the short region (the per-request line showed `share 0.433 ms, first byte
19 ms`); after it, the three new checks pass -- the claim is named, no cells are adopted, and the decoder prefills the
prompt itself.
Gates: `tests/hybrid_kvshare_route.sh` **PASS 33 checks** (28 before this commit, 23 before the PR), `tests/hybrid_route.sh`
PASS, `ONEBIT_KV_SHARE=1 tests/hybrid_e2e.sh` against a real llama-server + Qwen3-0.6B PASS (greedy reply equal to
`--device cpu`, `prompt_n 1, plain 22`).
|
Third commit (2cf36a9), and it is the one I would have wanted as a reviewer: the shared route adopted a region it had never been told was complete.
Reproduced first with a provider that exits 0 and claims one token fewer ( A provider line that does not parse is still not an error (the region is the result, as the state file is on the file route), and the provider's own Gates after this commit: So the PR now covers three failure paths of the same route, all measured before/after: a decoder that cannot share (state-file fallback), an adopt that is refused (deliberately not a fallback), and a provider whose claim does not match the region (no adopt). Happy to split any of them out if you would rather review them separately. |
bong-water-water-bong
left a comment
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at 2cf36a95.
Both halves are provable on this box with the fake backend, so I ran them rather than reading them. Two binaries built the same way (Release, gcc-16, ninja 1bit in ~/lb/thermal-run.sh): main 29ec80c6 and this head.
this head's binary, this head's tests -> 33 ok, 0 FAIL
main's binary, this head's tests -> 26 ok, 7 FAIL:
FAIL the request was pinned to slot 0
FAIL the fallback to the state file is named
FAIL the provider's state file was restored (share 0.000 ms, the file route)
FAIL the decoder was not left to prefill the prompt itself
FAIL a short claim is named
FAIL no cells were adopted from a short region
FAIL the decoder prefills the prompt itself instead
So the suite is not vacuous on either half: the four fallback checks and the three n_tokens cross-check checks are red on main and green here, and the other 26 stay green across both. The green run's own log lines are the ones the body quotes (share 0.000 ms, first byte on the file route; provider prefilled N of M tokens into the shared region on the short claim).
Read against the code, the boundary is where the body says it is: decoder_cannot_share is set only at the three handshake points (listen failed, share answered non-2xx, reply without share_id), never for a provider failure or a failed adopt — which is what keeps the double-prefill case out. The n_tokens check deliberately does not set it, so a provider that wrote fewer rows returns "not handed over" and the decoder prefills, rather than adopting a region whose last row was never written.
Instrument note, because it cost me a false red and will cost the next reviewer the same: my first run of this suite failed exactly these checks. The cause was mine — I started the build in the same worktree and then checked another PR's branch into it while ninja was running, so the binary was stitched from two trees. A clean build in its own worktree (~/wt/w1203 + ~/wt/b1203) passes. Nothing in the PR was wrong; the numbers above are from the clean pair.
Nit: j["n_tokens"].get<size_t>() on a negative integer would wrap rather than be refused; the field is a count so it does not matter today, and a >= 0 guard is one line.
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
…r exists (Refs #753) (#1317) engine#753 was parked on an NPU-side K/V writer. It exists now, and it is bit-exact out of an imported dma-buf, so the two doc pages that said "not yet" are wrong and the contract the next provider needs was never written down. docs/hybrid.md: the shared route's region is the *decoder's* cache, so its layout is llama.cpp's -- cache_k_l<L> and cache_v_l<L> separate, position-major, each tensor at a 4 KiB boundary (share_layout). A token's K|V row is contiguous in a prefill's output and not in the cache: two destinations per layer, row stride n_embd_k_gqa, and the provider addresses its own mapping of the dma-buf (a driver-made mapping of the import poisons the submission, #812). Nothing is re-checked after the write, because the handoff is llama_kv_cells_adopt. docs/zero-copy.md: target 1's provider status, and the two facts above as the bounds on a device provider. The engine side needs no change for any of it: ?action=share, ?action=adopt and llama_kv_cells_adopt are shipped (#1203), and the provider's gate stays text identity against --device hrx. No kernel internals in either page: the private tree holds the design, the measurement and the traps. Refs #753 Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg>
What
ONEBIT_KV_SHARE=1makes the hybrid route hand the prompt's KV over as dma-buf fds instead of a state file. The handshake had one failure mode with no recovery in it: when the decoder refuses?action=share— a params or model mismatch, or a hybrid cache on a pin without the recurrent-state arm —hybrid::Prefill::prepare()returned the refusal and the provider never ran, so the request fell back to the decoder prefilling the whole prompt. That is the exact cost the hybrid route exists to remove, and it arrives silently: one stderr line, and a request that looks like a plain one.Now the handshake failures (share refused, reply without a
share_id, no socket for the decoder) fall back to the state-file route for that request, which still keeps the prompt off the decoder's prefill path, and say which route ran:Only the handshake falls back. A provider that already wrote into the region, or an
adoptthat failed after it, keeps today's behaviour (the request is forwarded as it came): falling back there would pay for the prompt's prefill twice.Measured, before and after
tests/fake_backend.pylearnsFAKE_KVSHARE_REFUSE=1— the decoder answers the real refusal shape. One 30-token prompt through1bit serve --device hybrid, same build tree, only the code below differs (~/wt/zc-probes/kvshare-fallback-{before,after}/):29ec80c6)hybrid: share answered 400: {"error": {"message": "the KV cache of a hybrid model cannot be shared"}}; the backend prefills this request itself— no provider launch, no restore,id_slotunset...; this decoder cannot share its KV, so the state-file route is usedthenprefill 0 ms, restore 2 ms, share 0.000 ms, first byte 103 ms(the provider ran, the state file was restored into slot 0, the request is pinned to it)Gates (device-free, on today's
main29ec80c6)tests/hybrid_kvshare_route.shtests/hybrid_route.shtests/hybrid_e2e.sh(real llama-server, Qwen3-0.6B)--device cpu,prompt_n 1, plain 22ctest -R hybridtools/copyright.py --checkThe new case asserts the four things that were wrong and two that keep it honest: the provider's state file is restored (
share 0.000 msin the log line is the file route), the request is pinned to slot 0, the fallback is named, and the decoder is not left to prefill the prompt itself.Also in this diff
docs/hybrid.mdcarried a stale limit: "the region isllama_kv_cache, so a hybrid model's recurrent half is not shared and?action=sharerefuses a hybrid cache". The pinned fork has shared the recurrent state since 1bit-MONSTER/llama.cpp#181 (llama_memory_recurrentarm insrc/llama-kv-share.cpp), so the doc said "not supported" about a route this branch now falls back to on a decoder that predates it. The Tests section also gains thehybrid_kvshare_routebullet it never had.Not claimed
ONEBIT_KV_SHAREstays opt-in, and the file route stays the default for everyone else.Same class as engine#941 (a handoff silently thrown away); part of engine#698.