context : do not re-reserve the scheduler when toggling causal_attn - #28751
Conversation
|
This seems correct but it is not necessarily true for all models, we would need to check |
Thanks for pointing this out, you're right. I grepped the usage of Two reasons why I think this is still safe:
I also tested Qwen3.8-Flash-Next with the flag flipped via the API after context creation, between prompt and generation, and compared my branch's result vs master with identical results. If you'd rather not rely on the reallocation at all, I'd be happy to handle |
|
It's safe but it will likely fail CI when we add tests for that model as we run CI with |
|
I've tested this change locally, using DeepSeek-V4-Flash-Vision-Exp. Previously, I was seeing the graph re-reserved before processing any image, between every image, and once to switch back to text generation. Each re-reserve took about 5 second on my machine. Now, processing of multiple images is significantly faster end-to-end, even though the encode/decode time per-image has not changed. I've tested with a few different multi-turn, multi-image tasks. As far as I can tell, the quality and content of the model's output has not changed. I really appreciate this change, and hope it can be worked into mainline at some point. Thanks! 4.09.576.687 I slot create_check: id 0 | task 649 | created context checkpoint 1 of 64 (pos_min = 1006, pos_max = 1006, n_tokens = 1007, size = 17.021 MiB)
4.17.603.589 I slot print_timing: id 0 | task 649 | prompt processing, n_tokens = 418, progress = 0.39, t = 8.03 s / 52.05 tokens per second
4.17.603.591 I slot operator(): id 0 | task 649 | cached n_tokens = 1425, memory_seq_rm [1425, end)
4.17.603.716 I slot process_mtmd: id 0 | task 649 | encoding mtmd batch from idx = 1425, n_chunks = 1
4.17.809.659 I decoding image batch 1/1, n_tokens_batch = 380
4.31.952.234 I image decoded (batch 1/1) in 14143 ms
4.31.952.519 I slot process_mtmd: id 0 | task 649 | encoding mtmd batch from idx = 1805, n_chunks = 1
4.32.087.821 I decoding image batch 1/1, n_tokens_batch = 380
4.41.292.756 I image decoded (batch 1/1) in 9205 ms
4.41.293.001 I slot process_mtmd: id 0 | task 649 | encoding mtmd batch from idx = 2185, n_chunks = 1
4.41.427.390 I decoding image batch 1/1, n_tokens_batch = 380
4.50.433.660 I image decoded (batch 1/1) in 9006 ms
4.50.433.931 I slot process_mtmd: id 0 | task 649 | encoding mtmd batch from idx = 2565, n_chunks = 1
4.50.567.375 I decoding image batch 1/1, n_tokens_batch = 380
4.58.957.757 I image decoded (batch 1/1) in 8390 ms
4.58.958.001 I slot process_mtmd: id 0 | task 649 | encoding mtmd batch from idx = 2945, n_chunks = 1
4.59.075.577 I decoding image batch 1/1, n_tokens_batch = 328
5.07.485.631 I image decoded (batch 1/1) in 8410 ms
5.07.485.878 I slot process_mtmd: id 0 | task 649 | encoding mtmd batch from idx = 3273, n_chunks = 1
5.07.636.161 I decoding image batch 1/1, n_tokens_batch = 372
5.16.466.875 I image decoded (batch 1/1) in 8830 ms
5.16.940.675 I slot print_timing: id 0 | task 649 | prompt processing, n_tokens = 2648, progress = 1.00, t = 67.37 s / 39.31 tokens per second
5.16.940.681 I slot operator(): id 0 | task 649 | cached n_tokens = 3655, memory_seq_rm [3655, end)
5.16.941.142 I slot init_sampler: id 0 | task 649 | init sampler, took 0.12 ms, tokens: text = 1439, total = 3659
5.16.943.886 I slot create_check: id 0 | task 649 | created context checkpoint 2 of 64 (pos_min = 3654, pos_max = 3654, n_tokens = 3655, size = 17.021 MiB)
5.28.926.566 I slot print_timing: id 0 | task 649 | n_gen = 100, tg = 8.45 t/s, tg_3s = 8.54 t/s |
You're right. This has now been fixed in my second commit. |
Thank you for testing! |
5012372 to
421fa4b
Compare
|
Rebased on the current master without conflicts, same three commits still. @am17an does the second commit address the GGML_SCHED_NO_REALLOC concern? With it, qwen4exp builds the same graph for both causal_attn values, and toggling the flag under that setting no longer reallocates. |
|
I'm still no sure. @ngxson can you check if this makes sense? |
`llama_context::set_causal_attn()` marks the scheduler to do a full re-reserve on every change of the flag. For vision inputs, this flag is flipped twice around each non-causal image chunk for Gemma models, resulting in two expensive `sched_reserve()` passes per image. This is especially slow for multi-image or video inputs. The cost of a re-reserve scales with context and ubatch configurations, so larger settings pay more per image (see table below). The re-reserve is unnecessary in this case because `causal_attn` only changes the values written to KQ mask, not tensor shapes or any other buffer sizes. Note: `causal_attn` is a graph reuse key (`llm_graph_params` via `cparams`), so a new graph is built regardless of `sched_need_reserve`, so this doesn't change the graph rebuilding behaviour. llama-server with gemma-4-26B-A4B Q4_0 + BF16 mmproj, 130-token images, cache_prompt=false, prompt_ms median of 3 (before -> after): | images | config | H200 before -> after | RTX 4090 before -> after | |-|-|-|-| | 1 | `-c 8192 -ub 512` | 134 -> 105 ms (1.27×) | 201 -> 119 ms (1.69×) | | 24 | `-c 8192 -ub 512` | 2278 -> 1562 ms (1.46×) | 3559 -> 1748 ms (2.04×) | | 24 | `-c 32768 -ub 2048` | 5379 -> 1584 ms (3.40×) | 13377 -> 1759 ms (7.61×) | Generated output remains identical before and after.
The block/cell bias path was selected on cparams.causal_attn, so the causal and non-causal graphs differed in tensor shapes and ops. With the re-reserve removed (previous commit), a runtime flip resulted in reallocating the compute buffers, which would fail under GGML_SCHED_NO_REALLOC. This commit selects the block path from the mask shape only, independent of causal_attn. causal_attn is instead passed to set_input_qsa. causal_attn is fixed per graph as it's part of the reuse key. Causal values are unchanged. Non-causal values now follow the reference rule, where every visible block competes on score and only unpooled cells are always selected.
421fa4b to
0cbdaaf
Compare
|
@am17an here is a check for the GGML_SCHED_NO_REALLOC concern that anyone can run. I added a toggle step to test-llama-archs (branch: sihanyu03/llama.cpp@context-causal-attn-no-rereserve...context-causal-attn-no-rereserve-test). After the normal decode it sets causal_attn off, decodes n_ubatch/2 then n_ubatch tokens, sets it back on and decodes once more. Build with
I also did a real model check on the rebased head for Qwen3.8-Flash-Next: perplexity on wikitext-2 with I'd be happy to add the test to this PR or send it as a follow-up, whichever you prefer (tested on CUDA and CPU). Could someone also approve the CI run? The same head is green on my fork's CI. |
|
@am17an thanks for approving the CI runs, everything is green now. The PR is ready for review, would you or one of the other reviewers have time to take a look please? |
|
@ggerganov @ngxson PTAL. I think it's a good change |
|
@am17an would you be willing to give a formal review yourself, or should it wait for a core maintainer? |
|
I think disabling the causal_attn reserve per #28927 is OK. Regarding the Qwen4 changes - can't tell if it makes sense. My preference is instead of making more changes to rewrite the Qwen4 inference graph and memory because the current implementation seems very inefficient. |
|
We can remove the Qwen4 changes, those are only because it will fail |
|
@ggerganov thanks for taking a look. This PR predates #28927 and its first commit is the identical change. Given your preference for a rewrite over more qwen4exp changes, which do you prefer:
Happy to do whichever you choose. |
ggerganov
left a comment
There was a problem hiding this comment.
Let's keep them to not break the CI. We have to be careful to not use the llama-memory-hybrid-idx memory for other models before it is rewritten.
It's being used for GLM-5.3 next. I think post that model we should not use it before a refactor. |
|
Actually, I am not sure if the inefficiency of Qwen4 is just the graph, or both the graph + memory. Do you have an estimate? |
|
The memory/attention is the main problem there. After the GLM 5.3 next PR we can re-use the same ideas from there to make it better. |
Conflict: src/models/qwen4exp.cpp - upstream ed7ac35 (ggml-org#28751) drops cparams.causal_attn from the blk_bias predicate and carries it into llm_graph_input_qsa instead (it is part of the graph reuse key); the fork's !gather gate on the same predicate stays, since the gather path needs the per-cell bias as its attention mask. Upstream's [TAG_QWEN4_REIMPLEMENT] note and the fork's shared-MTP helper both kept.
|
@sihanyu03 @am17an Could you take a look at this failing test: https://github.com/ggml-org/llama.cpp/actions/runs/36400625405/job/108857353856#step:11:3174 - seems to happen when pipeline parallelism is enabled (i.e. env |
|
It looks like a pre-existing bug that has surfaced because we removed the re-reserve which only surfaces during PP or maybe the reserve was actually specifically added for this case. I don't have time to investigate this so we can revert this PR or add this diff. |
|
Yes I think it's the same issue as #26873. I will look into it |
|
Root cause confirmed, and it is indeed the mechanism described on #26873. The image batch with embedding input has a different graph shape, so the allocator re-plans on it and the worst-case plan is lost. Every later graph that is larger in any tensor re-plans again, and with pipeline parallelism each re-plan costs a full barrier across the devices, and under GGML_SCHED_NO_REALLOC the re-plan on the text batch after the image is flagged as unexpected, which is the CI failure. The removed per-flip reserve was masking this for models that toggle causal_attn. Glimmer in #26873 never toggles, which is why this issue appeared before this PR. @am17an's diff would fix the CI failure but ties the reserve to the flag again and pays two reserves per image under pipeline parallelism, so multi-GPU users wouldn't benefit from this PR. What I am preparing instead (which should be done before tomorrow) is re-reserve once when a decode switches from embedding inputs back to token inputs, which restores the worst-case plan, costs one reserve per image run on any setup, and covers the #26873 models too. This would fix the failing CIs. However, this doesn't tackle the underlying limitation which would be a much bigger change. |
Hm, that shouldn't be the case. If it were the case, then the previous CI with a single GPU would have also failed. It's something related to the pipeline parallelism. |
|
When a graph shape changes re-allocation is allowed but if the graph shape doesn't change and number of outputs change then it will crash. This is what is happening at least in the PP case. |
…ggml-org#28118 and ggml-org#28751 do not apply (MTP uses bounded rollback, no causal toggles) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018co7fcJoEQXMxLezg7HpMU
|
Yes sorry, I meant the scheduler graph changes, not the llama graph. The worst-case plan is created for a text graph, which does not read My previous suggestion had some issues. A better solution could instead be a dummy read of |
|
@sihanyu03 #29634 should resolve the problem. Please confirm it works as expected for your use case. |
…gml-org#28751) * context : do not re-reserve the scheduler when toggling causal_attn `llama_context::set_causal_attn()` marks the scheduler to do a full re-reserve on every change of the flag. For vision inputs, this flag is flipped twice around each non-causal image chunk for Gemma models, resulting in two expensive `sched_reserve()` passes per image. This is especially slow for multi-image or video inputs. The cost of a re-reserve scales with context and ubatch configurations, so larger settings pay more per image (see table below). The re-reserve is unnecessary in this case because `causal_attn` only changes the values written to KQ mask, not tensor shapes or any other buffer sizes. Note: `causal_attn` is a graph reuse key (`llm_graph_params` via `cparams`), so a new graph is built regardless of `sched_need_reserve`, so this doesn't change the graph rebuilding behaviour. llama-server with gemma-4-26B-A4B Q4_0 + BF16 mmproj, 130-token images, cache_prompt=false, prompt_ms median of 3 (before -> after): | images | config | H200 before -> after | RTX 4090 before -> after | |-|-|-|-| | 1 | `-c 8192 -ub 512` | 134 -> 105 ms (1.27×) | 201 -> 119 ms (1.69×) | | 24 | `-c 8192 -ub 512` | 2278 -> 1562 ms (1.46×) | 3559 -> 1748 ms (2.04×) | | 24 | `-c 32768 -ub 2048` | 5379 -> 1584 ms (3.40×) | 13377 -> 1759 ms (7.61×) | Generated output remains identical before and after. * qwen4exp : make the indexer bias shape independent of causal_attn The block/cell bias path was selected on cparams.causal_attn, so the causal and non-causal graphs differed in tensor shapes and ops. With the re-reserve removed (previous commit), a runtime flip resulted in reallocating the compute buffers, which would fail under GGML_SCHED_NO_REALLOC. This commit selects the block path from the mask shape only, independent of causal_attn. causal_attn is instead passed to set_input_qsa. causal_attn is fixed per graph as it's part of the reuse key. Causal values are unchanged. Non-causal values now follow the reference rule, where every visible block competes on score and only unpooled cells are always selected. * context : state the causal_attn shape rule in the comment * cont : add TODOs --------- Co-authored-by: Georgi Gerganov <ggerganov@gmail.com>
…gml-org#28751) * context : do not re-reserve the scheduler when toggling causal_attn `llama_context::set_causal_attn()` marks the scheduler to do a full re-reserve on every change of the flag. For vision inputs, this flag is flipped twice around each non-causal image chunk for Gemma models, resulting in two expensive `sched_reserve()` passes per image. This is especially slow for multi-image or video inputs. The cost of a re-reserve scales with context and ubatch configurations, so larger settings pay more per image (see table below). The re-reserve is unnecessary in this case because `causal_attn` only changes the values written to KQ mask, not tensor shapes or any other buffer sizes. Note: `causal_attn` is a graph reuse key (`llm_graph_params` via `cparams`), so a new graph is built regardless of `sched_need_reserve`, so this doesn't change the graph rebuilding behaviour. llama-server with gemma-4-26B-A4B Q4_0 + BF16 mmproj, 130-token images, cache_prompt=false, prompt_ms median of 3 (before -> after): | images | config | H200 before -> after | RTX 4090 before -> after | |-|-|-|-| | 1 | `-c 8192 -ub 512` | 134 -> 105 ms (1.27×) | 201 -> 119 ms (1.69×) | | 24 | `-c 8192 -ub 512` | 2278 -> 1562 ms (1.46×) | 3559 -> 1748 ms (2.04×) | | 24 | `-c 32768 -ub 2048` | 5379 -> 1584 ms (3.40×) | 13377 -> 1759 ms (7.61×) | Generated output remains identical before and after. * qwen4exp : make the indexer bias shape independent of causal_attn The block/cell bias path was selected on cparams.causal_attn, so the causal and non-causal graphs differed in tensor shapes and ops. With the re-reserve removed (previous commit), a runtime flip resulted in reallocating the compute buffers, which would fail under GGML_SCHED_NO_REALLOC. This commit selects the block path from the mask shape only, independent of causal_attn. causal_attn is instead passed to set_input_qsa. causal_attn is fixed per graph as it's part of the reuse key. Causal values are unchanged. Non-causal values now follow the reference rule, where every visible block competes on score and only unpooled cells are always selected. * context : state the causal_attn shape rule in the comment * cont : add TODOs --------- Co-authored-by: Georgi Gerganov <ggerganov@gmail.com>
…gml-org#28751) * context : do not re-reserve the scheduler when toggling causal_attn `llama_context::set_causal_attn()` marks the scheduler to do a full re-reserve on every change of the flag. For vision inputs, this flag is flipped twice around each non-causal image chunk for Gemma models, resulting in two expensive `sched_reserve()` passes per image. This is especially slow for multi-image or video inputs. The cost of a re-reserve scales with context and ubatch configurations, so larger settings pay more per image (see table below). The re-reserve is unnecessary in this case because `causal_attn` only changes the values written to KQ mask, not tensor shapes or any other buffer sizes. Note: `causal_attn` is a graph reuse key (`llm_graph_params` via `cparams`), so a new graph is built regardless of `sched_need_reserve`, so this doesn't change the graph rebuilding behaviour. llama-server with gemma-4-26B-A4B Q4_0 + BF16 mmproj, 130-token images, cache_prompt=false, prompt_ms median of 3 (before -> after): | images | config | H200 before -> after | RTX 4090 before -> after | |-|-|-|-| | 1 | `-c 8192 -ub 512` | 134 -> 105 ms (1.27×) | 201 -> 119 ms (1.69×) | | 24 | `-c 8192 -ub 512` | 2278 -> 1562 ms (1.46×) | 3559 -> 1748 ms (2.04×) | | 24 | `-c 32768 -ub 2048` | 5379 -> 1584 ms (3.40×) | 13377 -> 1759 ms (7.61×) | Generated output remains identical before and after. * qwen4exp : make the indexer bias shape independent of causal_attn The block/cell bias path was selected on cparams.causal_attn, so the causal and non-causal graphs differed in tensor shapes and ops. With the re-reserve removed (previous commit), a runtime flip resulted in reallocating the compute buffers, which would fail under GGML_SCHED_NO_REALLOC. This commit selects the block path from the mask shape only, independent of causal_attn. causal_attn is instead passed to set_input_qsa. causal_attn is fixed per graph as it's part of the reuse key. Causal values are unchanged. Non-causal values now follow the reference rule, where every visible block competes on score and only unpooled cells are always selected. * context : state the causal_attn shape rule in the comment * cont : add TODOs --------- Co-authored-by: Georgi Gerganov <ggerganov@gmail.com>
Overview
This PR removes the
sched_need_reserve = trueline fromllama_context::set_causal_attn(), which previously marked the scheduler to do a full re-reserve on every change of the flag, and fixes the one architecture (qwen4exp) whose graph shape depended on the flag.The re-reserve is unnecessary in this case because
causal_attnonly changes the values written to the KQ mask, not tensor shapes or any other buffer sizes.qwen4expwas the one exception that selected the per block/cell bias path based on the flag, so the causal and non-causal graphs differed in tensor shapes and ops. The second commit selects the block path independent ofcausal_attn, from the mask shape only and makes the non-causal case follow the reference rule: every visible block competes on score and only unpooled cells are always selected. The causal case works as before.Additional information
For vision inputs, this flag is flipped twice around each non-causal image chunk for Gemma3, Gemma4, and DeepSeek-V4, resulting in two expensive
sched_reserve()passes per image. This is especially slow for multi-image or video inputs.The cost of a re-reserve scales with context and ubatch configurations, so larger settings pay more per image (see table below).
Note:
causal_attnis a graph reuse key (llm_graph_paramsviacparams), so a new graph is built regardless ofsched_need_reserve, so this doesn't change the graph rebuilding behaviour.Speedup table: llama-server with gemma-4-26B-A4B Q4_0 + BF16 mmproj, 130-token images, cache_prompt=false, prompt_ms median of 3 (before -> after):
-c 8192 -ub 512-c 8192 -ub 512-c 32768 -ub 2048Generated output remains identical before and after.
Requirements