Repository navigation
Conversation
|
This pull request has merge conflicts with main. Please rebase it so it can be reviewed and merged. |
c43cbca to
805bc12
Compare
9b0a5c0 to
4eb0a44
Compare
FeathBow
left a comment
There was a problem hiding this comment.
Requesting changes for the two points below; the mechanism itself holds up on this card.
Passed on an sm_89 card with Qwen3-4B: the gate file with the b16 drafter, the two new tests with the b16 and the DSpark block7 drafters (the DSpark numbers match your L20 run exactly), and a zero budget launch that keeps the prefix cache off. Two mutations of the restore, copying nothing and copying block i from slot i+1, both fail the warm gate on its accept rate floor, so the gate has the power it claims.
Publish on finish: the last step's sealed blocks never reach the cache, so the next turn of a conversation plain decodes whenever that step crossed a block boundary. Measured 2 of 12 two turn conversations, the same two with both drafters; details inline.
Draft state bound: with sharing, the per pool token billing no longer covers the lane. One seed request, then N warm requests on the same 4000 token prefix with 16 output tokens, b16 drafter, card total 49.1 GB:
| N | main (prefix cache off with the drafter) | this PR |
|---|---|---|
| 64 | finished in 27 s, peak 41.7 GB | finished, peak 47.9 GB |
| 96 | finished in 42 s, peak 41.7 GB | no request finished or failed in 15 minutes, pinned at 48.5 GB, no error reported, killed by timeout |
On main the 96 requests cannot all be resident (each owns 250 pool blocks of 9051) and run in waves; here they share 250 blocks, are admitted together, and each allocates about 220 MB of draft state, about 21 GB against a reservation near 9 GB. Details and two possible bounds inline.
Two smaller notes inline on the long prefix publish and the per miss warn.
| Ok(()) | ||
| } | ||
|
|
||
| /// Publish captures for the blocks the previous step committed. Called on |
There was a problem hiding this comment.
Please also publish when a request finishes. The blocks sealed by its last step never get here, so the next turn's all or nothing lookup misses whenever that step crossed a block boundary. Twelve two turn conversations on this card (260 token prompt, 20 to 67 response tokens, then prompt + response + a short suffix): 10 draft on the second turn, 2 plain decode with a KV hit, the same two with both drafters, and both are the boundary cases (block 18 sealed at token 288 of 291, block 19 at token 304 of 304). The seal already holds those rows, and drop_request still has the request's KV handle before it drops it, so the drop command can carry the final hashes and kv_position and the lane can publish from the seal before discarding the state.
| /// DSpark with a non-zero `PEGAINFER_SPEC_CAPTURE_CACHE_MB`); EAGLE-3 and a | ||
| /// zero-budget drafter still force it off. The capture cache is a fixed | ||
| /// term in the draft reservation; the draft's own KV is still billed per | ||
| /// pool block, which is not a per-request bound once blocks are shared |
There was a problem hiding this comment.
This TODO carries the default now. With the prefix cache forced off, every active request owned its context in the pool, so the per pool token billing covered what the lane allocates per request. With sharing it does not: 96 warm requests on a shared 4000 token prefix sat at 48.5 of 49.1 GB without finishing or failing until a 15 minute timeout killed the process, and 64 already reached 47.9 GB; the same 96 on main finish in 42 s at 41.7 GB. Before this ships as the default the lane needs a bound it can act on: count the bytes its request states hold against the reservation and treat a request that would exceed it like a capture miss, or keep the capture cache opt in until the draft state is billed by reference.
| .take(target_blocks) | ||
| .skip(state.published_blocks) | ||
| { | ||
| let _ = cache.insert(ctx, *hash, &state.pending_context.buffer, i * block)?; |
There was a problem hiding this comment.
At 128 MiB the cache has 327 slots, so a request longer than that publishes over its own head: 64 warm requests on an 8000 token prefix all hit the KV cache and none restored, and every other conversation was evicted on the way. Skipping the publish when the request's block count exceeds the slot count, and saying in the doc that the budget caps the longest restorable prefix (about 5k tokens at the default), would keep this from surprising people.
| req.request_id | ||
| ); | ||
| let Some(slots) = cache.lookup(&req.capture_prefix_hashes) else { | ||
| log::warn!( |
There was a problem hiding this comment.
Once the LRU churns this fires for most requests; debug level or a counter.
Prefix caching was force-disabled whenever a drafter was loaded: a hit skips the prefill that produces the drafter's target hidden rows, and those rows are not in the KV cache. Store them per sealed 16-token KV block, keyed by the block's SequenceHash lineage, in a fixed-budget (LRU) capture cache; a hit restores them into the draft's pending context and the first draft round rebuilds the draft KV from them. - PEGAINFER_SPEC_CAPTURE_CACHE_MB (default 128 MiB, 0 = off): a fixed term in DFlashMemoryReservation, so the KV pool shrinks to fit. - A capture-cache miss keeps the target KV hit but leaves the request without drafter state; the all-or-nothing gate falls back to plain decode (correctness unchanged). - EAGLE-3 has no capture cache yet and keeps the old spec-on => APC-off behavior; a zero budget does the same. - Widen the verify plan's page-list bound to the by-reference worst case (max_batch x pages-per-context), since shared prefix pages are counted once per request. - Add dflash_warm_prefix_keeps_drafting: cold fill, plain baseline, then a warm request that must hit, stay lossless up to bf16 ties, and increase the draft/accept counters. - Docs updated. The draft KV reservation is still billed per pool block; that is not a per-request upper bound once blocks are shared (tracked in pegainfer-project#1238). Signed-off-by: bohaotu <bohaotu@pony.ai>
- Do not reserve or allocate the capture cache when the prefix cache is off (--no-prefix-cache): the lane gets an explicit enable flag and the reservation gets the capture budget and prefix block size. - Reserve the per-request partial-block seal and the lane shift buffer; update the reservation test. - Gate the restore on prefill eligibility and require the matched hash count to equal cached_tokens; add source/destination range guards and a seal-ledger debug assert. - Strengthen the warm gate: compare the warm accept rate against the cold rate, and add a 1 MiB capture-cache child gate proving a miss falls back to plain decode instead of seeding a shifted context. - Docs: state the side-cache lifetime honestly (own LRU; a block-owned payload with one lifetime is the pegainfer-project#1238 follow-up) and the seal/accounting. Signed-off-by: bohaotu <bohaotu@pony.ai>
An explicit PEGAINFER_SPEC_CAPTURE_CACHE_MB=0 disables the cache just like --no-prefix-cache, but the reservation and the lane still sized the per-request seal from the page size. Gate both on the actual budget so the seal and its reservation disappear together, and refresh the stale "spec forces APC off" comment in the launch adapter. Signed-off-by: bohaotu <bohaotu@pony.ai>
The metrics snapshot exposes no spec counters until a step has been served; the pre-run baseline treats that as zero instead of failing the gate before the cold request runs. Signed-off-by: bohaotu <bohaotu@pony.ai>
The prefill path still passed the KV page size to new_request_state, so a no-capture engine allocated the 2-block seal but had no publisher to drain it; a long gated run then overflowed the seal. Pass the lane's capture block size (0 when the cache is off) and tighten the warm gate's accept-rate floor to match the measured cold/warm equality. Signed-off-by: bohaotu <bohaotu@pony.ai>
Signed-off-by: bohaotu <bohaotu@pony.ai>
Signed-off-by: bohaotu <bohaotu@pony.ai>
…hing - Publish the blocks sealed by a retired request's final served step from its seal before the draft state is dropped; the next draft/verify that would normally publish never runs. - Charge each request's draft KV/context/pending bytes against the per-token term the KV pool reserved for it, so N requests sharing one prefix cannot allocate N prompts of state the pool charged once. Over-budget requests take the capture-miss path and plain-decode. - Stop publishing a prefix longer than the capture cache's slot count while still draining the seal; it could never restore whole and would evict every other conversation. - Make capture misses debug-level; they are normal once the LRU churns. - Add a finish-boundary gate and document the one uncapturable case (a turn ending exactly on a boundary ends on the verify bonus token). Signed-off-by: bohaotu <bohaotu@pony.ai>
cbe8ae7 to
51a6c21
Compare
|
Both blocking points and the two smaller notes are fixed in Publish on finish. The drop path now carries the request's final block hashes + One caveat for your 304/304 case: when the final token is the boundary token, it is the verify bonus, whose target hidden was never forwarded, so that last block cannot be completed on finish even with the drop publish. The recoverable case — the final step crosses a boundary and leaves a partial tail after it — is what the new gate pins. Recovering the exact-boundary case needs the joint cap (re-prefill one block); I added it to the #1238 follow-ups and the PR body. Draft-state bound. The reservation now exposes the per-request share of its per-token term (56,320 B/token: draft KV + projected-context scratch + pending capture; the lane-level tail scratch is excluded). The executor passes the reserved draft term ( Long-prefix publish. A request with more blocks than the cache has slots no longer inserts (it could never restore whole and would evict every other conversation, including its own head); the seal still drains so it cannot overflow. The docs now state that the budget caps the longest restorable prefix (~5.2k tokens at the 128 MiB default). Per-miss warn. Moved to debug, with a debug ledger line per publish ( CI on the current head is green. |
|
Stage 2 (paged draft KV in the target's block space) is a larger follow-up, so it stays out of this PR. Full design, risks, and gates are in the issue: #1238 (comment) Summary of the plan:
Stage 1 in this PR remains the capture-cache implementation under review. |
FeathBow
left a comment
There was a problem hiding this comment.
Requesting changes once more, for one thing the round one fixes uncovered. The four points from round one are closed and measured on the same sm_89 card with both drafters.
Closed: the finish publish recovers the case I measured (the conversation whose last step sealed block 18 at token 288 of 291 now drafts on turn 2; 11 of the 12 conversations draft, and the new finish gate passes with both drafters); the draft state bound holds (the 96 shared prefix requests that wedged the card last round finish in 2.4 s at 40.8 GB, below main's 41.7 GB); the slot cap and the debug level are in; a zero budget still keeps the prefix cache off.
The remaining conversation is the boundary case you documented, and it is a KV side defect rather than a capture limitation. apply_speculative in the kvbm fork appends the whole committed span, bonus included, and then registers every block the appends completed, so a block completed by the bonus becomes matchable before the bonus has any KV. apply_decode registers before appending the new dangling token and apply_prefill leaves the sampled token dangling for exactly this reason. With the prefix cache forced off under the drafter nothing ever matched those blocks; with this PR the next turn does, and its prefill attends to a row that holds a rejected draft's KV or nothing. Ten turns per drafter that end exactly on a block boundary:
| last token of turn 1 | cases | turn 2 prefix match | turn 2 drafts |
|---|---|---|---|
| prefill's sampled token, never forwarded | 2 | one block short, as it should be | yes |
| verify bonus token, never forwarded | 8 | the full turn, including the block whose last row was never written | no |
Turn 2 warm against the same prompt on a fresh engine: in 12 of the 16 bonus endings the first token's logprob moves by 0.05 to 0.94, while the four controls that sit one block short of a full match move it by at most 0.04. The capture miss is the safe half of this event; the plain decode that follows still runs on the corrupted prefix. The fix belongs in the fork's apply_speculative: register what the forward computed, then append the last token of the span without registering, the way apply_decode already does. After that every registered block has all its rows forwarded, the finish publish covers everything the KV side can match, and the caveat in the body, the doc and the gate comment can go.
One doc point inline: past the budget the whole decode batch stops speculating, because the step decision is all or nothing and a plain decode drops the restored states. 64 requests on a shared 4000 token prefix: 41 restored, 23 refused, 0 draft rounds for anyone; 4 requests: 44 rounds; main runs 704 rounds for the 64 in waves.
| - **Lifetime:** the capture cache is a lane-owned side cache with its own LRU and budget, not part of the KV block's refcount. A capture entry can be evicted while its KV block survives; the hit then falls back (below). Moving the payload into the KV block state so both share one lifetime is the follow-up tracked in #1238. | ||
| - **Budget:** `PEGAINFER_SPEC_CAPTURE_CACHE_MB` (default 128 MiB; explicit `0` disables it, as does `--no-prefix-cache`). The budget is a fixed term in `DFlashMemoryReservation`, so the KV pool shrinks to fit. 128 MiB holds ~327 blocks (~5k tokens), and that is exactly the longest prefix that can be restored: a request with more full blocks than the cache has slots is not published at all. Publishing it could never produce a whole-prefix hit, and it would evict every other conversation (and its own head) on the way. | ||
| - **Restore:** a prefix match hands the matched block hashes to the worker, which copies the rows into the draft's `pending_context`: the suffix prefill appends the rest and the first draft round rebuilds the draft KV from the restored rows. `draft_ready` still comes from the capture set + completed flag. | ||
| - **Publish on finish:** the last served step has no next draft/verify to publish from, so a retired request drains its final seal before its state is dropped. Without it, the next turn plain-decodes whenever that step crossed a block boundary (measured 2 of 12 two-turn conversations). One case is still uncapturable: a turn whose final token lands exactly on the boundary ends on the verify's bonus token, whose target hidden was never forwarded, so that last block cannot be completed on finish. Re-prefilling one block on a capture miss (the joint-cap follow-up in #1238) is what would recover it. |
There was a problem hiding this comment.
This caveat describes a KV side defect, and fixing it removes the caveat. apply_speculative in the kvbm fork appends the committed span, bonus included, and then registers every block the appends completed, so a block completed by the bonus is matchable before the bonus has any KV. apply_decode registers before appending the new dangling token and apply_prefill leaves the sampled token dangling for exactly this reason, and the box shows the asymmetry: a turn whose only generated token came from prefill matches one block short on the next turn and drafts, every turn that ends on a verify bonus at a boundary matches the full turn and plain decodes, and the warm first token's logprob moves by 0.05 to 0.94 against at most 0.04 for the controls. Registering before appending the span's last token, as the decode path does, makes the next turn match only rows that were forwarded; then this sentence, the one in the body and the gate comment stop being true and can go.
| .min(self.model.max_position_embeddings()); | ||
| if !self.can_create_state(max_cache_len) { | ||
| log::debug!( | ||
| "Qwen3 DFlash draft state budget is full; {:?} keeps its KV hit but will not \ |
There was a problem hiding this comment.
Per request this is right; per batch the step decision is all or nothing and the plain decode drops every restored state in the batch, so the first refused request ends speculation for everyone sharing the step. Measured with 64 requests on a shared 4000 token prefix: 41 restored, 23 refused here, 0 draft rounds for all 64 (the publish ledger shows only the seed); with 4 requests, 44 rounds. Worth one sentence in the doc next to the bound, with the ceiling it implies: the budget divided by max_cache_len times 56,320, which is 41 requests at 4k tokens on this card.
Summary
Implements #1238: keep prefix caching on with the DFlash/DSpark drafter by persisting the drafter's captured target hidden rows per sealed KV block.
DFlashCaptureCache(new): fixed-budget, LRU, keyed by the KV block'sSequenceHash, one 16-token block per slot (5 × 2560bf16 = 25.6 KB/token on Qwen3-4B).PEGAINFER_SPEC_CAPTURE_CACHE_MB(default 128 MiB; explicit0= off, as is--no-prefix-cache), charged as a fixed term inDFlashMemoryReservationtogether with the per-request partial-block seal and the lane shift buffer.pending_context; the suffix prefill appends the rest and the first draft round rebuilds the draft KV from the restored rows.spec-on => APC offbehavior.max_batch × pages-per-context), because shared prefix pages are listed once per request.Review fixes (head
51a6c21a):kv_position, and the lane drains the seal into the capture cache before dropping the state. Previously the next turn plain-decoded whenever the last served step crossed a block boundary.max_cache_lenagainst the memory the pool set aside for draft state; a request that would exceed it takes the capture-miss path and plain-decodes. This keeps N requests sharing one prefix from each allocating a prompt of state the pool charged once.Verification
cargo check -p pegainfer-qwen3cargo clippy -p pegainfer-qwen3 --all-targets -- -D warningscargo test -p pegainfer-qwen3 --lib→ 133 passeddeepseek-ai/dflash_qwen3_4b_block7, release), full gate file 10 passed / 1 ignored (168 s):dflash_warm_prefix_keeps_drafting→ 1 passed; cold and warm both 4 draft rounds / 27 accepted.dflash_capture_cache_miss_falls_back_to_plain_decode→ 1 passed; 400 cached tokens, drafts 4 → 4.dflash_finish_publishes_last_sealed_block(new) → 1 passed; the drop publish reportstarget_blocks=50frompublished=49, and the next turn drafts.dflash_spec_levers_keep_losslessness→ 1 passed; the no-prefix, gated, and trimmed children as before.Caveats
Refs #1238.