Skip to content

kv : change structures in kv-cells for improve operations asymptotic - #27061

Closed
lexasub wants to merge 4 commits into
ggml-org:masterfrom
lexasub:kv-cells
Closed

kv : change structures in kv-cells for improve operations asymptotic#27061
lexasub wants to merge 4 commits into
ggml-org:masterfrom
lexasub:kv-cells

Conversation

@lexasub

@lexasub lexasub commented Aug 14, 2026

Copy link
Copy Markdown

Overview

Change containers for llama-kv-cache for boost lookup and modify
replace std::bitset<LLAMA_MAX_SEQ> -> uint64_t w[N_SEQ_WORDS]
replace std::map<llama_pos, int> seq_pos[LLAMA_MAX_SEQ] -> seq_pos_t seq_pos[LLAMA_MAX_SEQ]

Additional information

May be filled later after discussion

Requirements

@github-actions github-actions Bot added the testing Everything test related label Aug 14, 2026
@lexasub

lexasub commented Aug 14, 2026

Copy link
Copy Markdown
Author

@ngxson, also may be your review may good)

@ggml-gh-bot

ggml-gh-bot Bot commented Aug 14, 2026

Copy link
Copy Markdown

Hi @lexasub, thanks for your contribution!

Per our contribution guidelines, the automated PR checker found the following issue(s) that need your attention:

  • Multiple open PRs from a new contributor: We limit new contributors (those without a previously merged PR) to 1 open PR at a time. You currently have 2 open PRs.

  • Large PR: Large changes require prior discussion (e.g. an issue or RFC) and maintainers may not be able to review this PR as-is. Consider splitting it into smaller, focused PRs.


Please note that maintainers reserve the right to make final decisions on PRs. If you believe there is a mistake, please comment below.

@lexasub

lexasub commented Aug 14, 2026

Copy link
Copy Markdown
Author

Hi @lexasub, thanks for your contribution!

Per our contribution guidelines, the automated PR checker found the following issue(s) that need your attention:

  • Multiple open PRs from a new contributor: We limit new contributors (those without a previously merged PR) to 1 open PR at a time. You currently have 2 open PRs.
  • Large PR: Large changes require prior discussion (e.g. an issue or RFC) and maintainers may not be able to review this PR as-is. Consider splitting it into smaller, focused PRs.

Please note that maintainers reserve the right to make final decisions on PRs. If you believe there is a mistake, please comment below.

Multiple open PRs from a new contributor: Drafts don't count toward the limit. ) now i have only old draft commits.
Large PR: most diff - in test 1151 lines)

@lexasub

lexasub commented Aug 14, 2026

Copy link
Copy Markdown
Author
image image image image rtx 5090 - (pci-e-v3) E5-2620 v2. Muse Glimmer q4

-np 2 -c 262144 -fa on

before changes : (in progress)
PP(input) :
126617 tok,
2319 decode,
TG(output) :
1053 tok
65.14 t/s

after changes :
PP(input) :
126617 tok,
2339 decode,
TG(output) :
1258 tok
65.89 t/s

log (2).log
image
image

@ngxson

ngxson commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

this change is quite excessive tbh, I doubt if this code path is hot enough to be consider

we can proceed if you can proof if this brings significant end-to-end perf improvement (pp or tg measure)

most of the time is spent on ggml's graph sched IIRC, kv cache management is just too small

@lexasub

lexasub commented Aug 14, 2026

Copy link
Copy Markdown
Author

also
image
image
image
image
(my changes best in this case)

@ngxson

ngxson commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

please use proper llama-bench

@lexasub

lexasub commented Aug 14, 2026

Copy link
Copy Markdown
Author

before
image
after (smaller volatility)
image
rtx 5090 - (pci-e-v3) E5-2620 v2. Muse Glimmer q4 . gpu is very good for this cpu. cpu very slow
now retrying with nvidia-sm -lgc 2400, ./bin/llama-bench -m ~/Muse-Glimmer-30B-UD-Q4_K_XL.gguf -ngl 99 -fa on
-pg 1024,256 -pg 8192,256 -pg 32768,256 -pg 65536,256 -pg 126613,256
-r 5 --prio 3 --poll 100 --delay 2 -o csv

@ngxson

ngxson commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

I don't see any significant improvements here, the PR's result is already within master's error margin

@lexasub

lexasub commented Aug 14, 2026

Copy link
Copy Markdown
Author

okay, i go the deeper, llama-bench on profiling didn't show kv-cells usage( but via llama server i see it in profile), i will notify when kv cache give improvement on llama-bench

in my opinion - llama-bench doesn't test prefill, tokenization. this mr improve prefil!

@github-actions github-actions Bot added the ggml changes relating to the ggml tensor library for machine learning label Aug 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ggml changes relating to the ggml tensor library for machine learning testing Everything test related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants