Skip to content

feat(kimi): reuse the attention state a previous turn already built; stop leaking Lc/Rc - #787

Merged
JustVugg merged 2 commits into
devfrom
feat/kimi-kv-prefix
Aug 2, 2026
Merged

feat(kimi): reuse the attention state a previous turn already built; stop leaking Lc/Rc#787
JustVugg merged 2 commits into
devfrom
feat/kimi-kv-prefix

Conversation

@JustVugg

@JustVugg JustVugg commented Aug 2, 2026

Copy link
Copy Markdown
Owner

Stacked on #786, which adds kv_prefix.h. Retarget to dev once that merges.

Same defect: a chat client resends the whole transcript each turn, and kimi_k3.c re-processed turns 1..N-1 from scratch. The cost of a message grew with the conversation, and every replayed position pulled its experts off disk again.

K3 needs the shared record more than the other engines, not less

There is no single KV to inspect. 69 of its layers are KDA — Kimi Delta Attention, a recurrent linear-attention state — and only the 24 MLA layers keep Lc/Rc. So "how much of this prompt does the current state already cover" cannot be read off any buffer; it has to be recorded where the tokens are fed. That is exactly what kv_prefix.h does, and it is why I did not want this logic inlined per engine.

The prefill loop already carried absolute positions:

for(int i=0;i<np;i+=chunk){ ... lo=step_chunk(m,ids+i,i,C); }

so reuse is starting it at reuse instead of 0.

Separately: kv_alloc leaked on every request

Serve calls it per turn, and it callocs over the previous pointers without freeing them:

static void kv_alloc(Model *m, int max_t){
    Cfg *c=&m->c; m->max_t=max_t;
    m->Lc=calloc(c->n_layers,sizeof(float*));   // old Lc leaked
    m->Rc=calloc(c->n_layers,sizeof(float*));   // old Rc leaked

n_layers × max_t × (kv_lora + qk_rope) floats per turn — hundreds of MB over a conversation on the 24 MLA layers at 4k context.

The fix belongs in this PR rather than its own because prefix reuse requires keeping those buffers across turns: growing them discards the positions the record describes. The lifetime question had to be answered either way, and answering it correctly happens to close the leak.

What is NOT verified — please read this part

Kimi K3 is ~1.6 TB and I do not have the checkpoint. So this PR has:

  • ❌ no speed number for this engine
  • ❌ no end-to-end identity run for this engine

and it has:

  • ✅ a clean build
  • tests/test_kv_prefix.c covering the decision logic exhaustively — divergence at the first and at the last recorded token, prompts shorter than and equal to the record, buffer overrun, taint, NULL
  • ✅ the same mechanism measured on DeepSeek V4: 5.23× on a second turn reusing 82% of its prompt, output byte-identical to a cold engine

I would rather ship a PR that says what it has not checked than one that lets you assume otherwise.

If you have the weights@brad-evony, anyone from #748 — the numbers would be genuinely useful. K3_PREFIX_LOG=1 prints what was reused:

[PREFIX] reusing 45 of 55 prompt tokens (82%)

and the thing worth checking is that a multi-turn conversation gives the same replies as one-shot prompts containing the same transcript.

🤖 Generated with Claude Code

@JustVugg
JustVugg force-pushed the feat/kimi-kv-prefix branch from 2d52469 to 192701e Compare August 2, 2026 21:44
@JustVugg JustVugg added bug Difetto verificato nel codice model-support Supporto a nuovi modelli performance Velocità / tok-s / ottimizzazioni labels Aug 2, 2026
@JustVugg
JustVugg changed the base branch from feat/inkling-kv-prefix to dev August 2, 2026 22:09
JustVugg and others added 2 commits August 3, 2026 00:12
…stop leaking Lc/Rc

Same defect as the Inkling PR this stacks on: a chat client resends the whole
transcript each turn and kimi_k3.c re-processed turns 1..N-1 from scratch, so
the cost of a message grew with the conversation and every replayed position
pulled its experts off disk again.

K3 needs the shared record more than the other engines do, not less. There is
no single KV to inspect: 69 of its layers are KDA (Kimi Delta Attention, a
RECURRENT linear-attention state) and only the 24 MLA layers keep Lc/Rc. So
"how much of this prompt does the current state already cover" is not something
that can be read off any buffer — it has to be recorded where the tokens are
fed, which is what kv_prefix.h does. The prefill loop already carried absolute
positions (step_chunk(m, ids+i, i, C)), so reuse is starting it at `reuse`
instead of 0.

SEPARATELY, kv_alloc leaked on every request. Serve calls it per turn, and it
callocs Lc/Rc over the previous pointers without freeing them:

    m->Lc=calloc(c->n_layers,sizeof(float*));
    m->Rc=calloc(c->n_layers,sizeof(float*));

That is n_layers x max_t x (kv_lora + qk_rope) floats per turn — hundreds of MB
over a conversation on the 24 MLA layers at 4k context. The fix belongs here
rather than in its own PR because prefix reuse REQUIRES keeping those buffers
across turns: growing them discards the positions the record describes, so the
lifetime question had to be answered either way.

NOT MEASURED HERE. Kimi K3 is ~1.6 TB and I do not have the checkpoint, so
there is no speed number and no end-to-end identity run for this engine. What
is verified: it compiles, tests/test_kv_prefix.c covers the decision logic
exhaustively, and the same mechanism measured 5.23x on DeepSeek V4 with
byte-identical output. Anyone with the weights: the numbers would be welcome,
and K3_PREFIX_LOG=1 prints what was reused.

Stacked on feat/inkling-kv-prefix, which adds kv_prefix.h.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Same defect CI caught on the Inkling side, in the same place. Freeing and
re-allocating the MLA buffers on a longer prompt discards every position
already computed -- and a conversation's prompt is longer every turn, so the
state was thrown away immediately before the point of using it. Reuse could
never fire in the one case it exists for.

Lc/Rc are laid out [position][kv_lora] and [position][qk_rope], so unlike
inkling's head-major K/V this grow is a straight prefix copy with no re-layout.
The 69 KDA layers need nothing here: their recurrent state does not scale with
max_t and survives on its own.

K3_PREFIX_LOG now also reports a refusal and the state behind it (held, cap,
prompt, diverged). On the Inkling side that diagnostic is what distinguished
'the engine decided not to reuse' from 'the harness lost the output', and it
turned an unreadable CI failure into a one-line diagnosis.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@JustVugg
JustVugg force-pushed the feat/kimi-kv-prefix branch from 192701e to 863139e Compare August 2, 2026 22:12
@JustVugg
JustVugg merged commit d3bc866 into dev Aug 2, 2026
13 checks passed
ZacharyZcR pushed a commit to ZacharyZcR/colibri that referenced this pull request Aug 5, 2026
DeepSeek V4 was the only engine in the tree that re-prefilled the whole
context on every request. colibri.c has done same-slot prefix reuse since
it had a serve path; inkling.c (JustVugg#786) and kimi_k3.c (JustVugg#787) got it through
the shared kv_prefix.h. This adds the third user of that header.

The visible effect is the one people report as "the second message is
slower than the first": turn ten was paying for turns one through nine
again, with the dense tensors and expert cache already warm.

## Why the reusable case is narrow, and why that is enough

The window attention state cannot be truncated to an arbitrary position.
The sliding window is a ring, and the compressor carries recurrent
kv_state/score_state rather than per-position rows -- unlike GLM's MLA
rows, which are position-addressed and self-contained, so colibri.c can
truncate to any shared prefix and even copy rows between slots.

What this state can do is keep going. So the case handled here is the
exact one a conversation produces: turn N+1's prompt begins with every id
turn N fed, prompt and reply alike, and only the tail is new.
kv_prefix_reuse returns 0 unless the record is a strict prefix of the new
prompt, so an identical prompt, a shorter one, or a divergent one all fall
back to a full reset and prefill.

## Correctness

Positions stay absolute: the fresh tail is prefilled with start=reuse, so
every token sees the same position it would have in a cold run. Only the
batch indexing shifts, since the batch now holds the tail alone.

The record tracks what was actually fed, not what was asked for. The last
generated token is emitted but never fed back, so it is recorded inside
the decode loop as each token enters the state rather than in bulk
afterwards -- recording after the loop would claim one token too many and
corrupt the next turn. Every failure path taints the record, because a
half-updated attention state matches neither the old ids nor the new.

kv_prefix_alloc failing is not an error. Per the header's contract it
leaves the record empty, reuse returns 0, and every request prefills in
full -- exactly the behaviour before this change.

## Tests

tests/test_deepseek_v4_prefix.py drives the SUBMIT/DATA/DONE protocol
directly and runs each second turn twice: once continuing a warm session,
once against a freshly started engine. It asserts they agree token for
token, so the test fails if reuse changes the output, and separately that
reuse actually fired -- an optimisation that silently never engages would
otherwise pass every correctness check.

  PASS prefix reuse: 11 tokens reused, output identical to a cold prefill
  PASS prefix repeat: identical prompt re-prefills, answer unchanged
  PASS prefix reset: divergent prompt re-prefills and matches cold

Wired into `make deepseek-v4-tiny-check`; the existing 11 checks still
pass. The DONE frame gained a trailing reuse count so the test can measure
it; openai_server.py parses `len(fields) >= 7`, so older readers ignore it.

V4_PREFIX_LOG=1 prints the reuse length per request.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@JustVugg
JustVugg deleted the feat/kimi-kv-prefix branch August 5, 2026 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Difetto verificato nel codice model-support Supporto a nuovi modelli performance Velocità / tok-s / ottimizzazioni

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant