Skip to content

kv-cache : limit seq_rm to the used cell range - #311

Merged
bri-prism merged 1 commit into
PrismML-Eng:prismfrom
sb32445:pr/kv-seq-rm-bound
Oct 8, 2026
Merged

bri-prism merged 1 commit into
PrismML-Eng:prismfrom
sb32445:pr/kv-seq-rm-bound

Conversation

@sb32445

@sb32445 sb32445 commented Oct 4, 2026

Copy link
Copy Markdown

Overview

llama_kv_cache::seq_rm walked all cells of the cache on every call. Cells outside [used_min(), used_max_p1()) are empty (pos == -1), so the loop now only covers that range. With a large context and speculative decoding (seq_rm after rejected drafts) the full walk showed up in the host profile (0.9 % of the decode step at 114688 cells, 0.02 % at 8192).

RTX 4070, Bonsai 2 27B PTQ1_0 + MTP head (n-max 2), server arguments as in an agent setup (context 114688, reasoning budget, thinking sampling), 8 interleaved A/B pairs: +0.77 % (95 % CI [+0.59, +0.96] %), outputs identical. Depth 0 / 16k / 65k / 100k: outputs identical, +0.4 to +1.8 %, never slower.

Additional information

  • Three-line change in src/llama-kv-cache.cpp, no behavior change: the skipped cells were empty.
  • A functional test suite (80 cases, greedy) gives 80 of 80 identical answers with and without the change. There is no seq_rm test in the repo.
  • Not measured: multiple sequences/slots.

Test results

  • Hardware / software: RTX 4070 12 GB (AD104, cc 8.9, 504 GB/s, 48 MB L2), Linux 6.18, NVIDIA driver 615.71, CUDA 13.4, GCC 16.2; Release build, -DGGML_CUDA=ON -DCMAKE_CUDA_ARCHITECTURES=89.
  • Base: speed numbers were measured on prism at 88c4bc60b. The four commits since (SYCL, WebGPU and cuda: fused FWHT quantizer for 64-wide warps (#303)) do not touch the code paths of this PR; the branch is rebased on 2459f68b5, builds, and the test-backend-ops runs below were repeated on it.
  • Model: Ternary-Bonsai-2-27B (PTQ1_0) with a community MTP draft head (Q8_0), speculative decoding with --spec-type draft-mtp --spec-draft-n-max 2, q4_0 K/V cache, one slot.
  • Method: two builds (with and without this commit) alternating A B B A, paired differences with a 95 % bootstrap interval (there is no environment switch for this change). Run-to-run noise is about 0.04 to 0.12 %, so effects above ~0.3 % are reliable.
  • Agent-like server setup (context 114688, --reasoning-format deepseek --reasoning-budget 16384, thinking sampling 1.0 / 0.95 / 20 / 0.05, fixed seed, 4 prompts x 256 tokens), 8 interleaved pairs: +0.77 % (95 % CI [+0.59, +0.96] %), outputs identical in all runs. This is the favourable case (almost empty cache in a 114688-cell context); the full walk costs 0.02 % of a decode step at 8192 cells and 0.9 % at 114688.
  • Greedy decode at filled context (one run each, no pairs, so only indicative): depth 0 / 16k / 65k / 100k = +1.8 / +0.4 / +0.8 / +0.5 %; output hashes identical at all four depths.
  • A functional test suite (80 cases, greedy, 128k context, MTP n-max 2): 80 of 80 answers identical with and without the change, same pass/fail verdict for every case (64 passed, 6 failed, 10 not scored, both runs).
  • There is no seq_rm test in the repository (the change is a loop bound; skipped cells have pos == -1 and would not have been touched).
  • Not tested: multiple slots / sequences.

Requirements

  • I have read and agree with the contributing guidelines
  • AI usage disclosure: The patches were developed with Claude Code (Anthropic's coding agent): it wrote the code, the measurement scripts and the first drafts of the commit messages and PR texts. I decided what to work on (which kernels and host paths to optimise, based on profiles of my own decode setup). The measurements and checks listed in the PR texts were run in the Claude Code sessions; I did not re-run them independently. I will maintain the changes. Commits where Claude Code was used carry a Co-Authored-By trailer.

seq_rm walked all cells of the cache on every call. Cells outside
[used_min, used_max_p1) are empty, so skip them. With a large context
and speculative decoding (seq_rm after rejected drafts) this showed up
in the host profile.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@sb32445 sb32445 changed the title kv-cache : limit seq_rm to the used cell range kv-cache : limit seq_rm to the used cell range Oct 4, 2026

@bri-prism bri-prism left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking findings in the loop-bound change.

I compared the complete base/head seq_rm methods using the real KV-cell metadata type: 21,000 calls matched under AddressSanitizer and UndefinedBehaviorSanitizer across 1-4 streams, eight sequence IDs, shared cells, holes, empty caches, full clears, negative bounds, reversed intervals, and repeated removals. Cell metadata, sequence-position extrema, used ranges/counts, and allocation heads matched. The changed translation unit also passed a CPU syntax check.

The skipped cells have position -1, and p0 is normalized to 0, so they cannot match the original removal predicate. Capturing the upper bound before erasures preserves the walk while the used set changes.

This was focused metadata validation; I did not independently rerun real-model generation or reproduce the speed measurements.

@bri-prism
bri-prism merged commit 56f21fb into PrismML-Eng:prism Oct 8, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants