llama: re-create the KV cache when flash attention resolves to disabled (performance and layout bug) - #26460
Open
wanghqc wants to merge 1 commit into
Open
Conversation
With -fa auto, the KV cache is created before flash attention support is probed: cparams.flash_attn still holds the requested value, so V is laid out for flash attention. The probe in resolve_fused_ops runs later, during the first sched_reserve, and may turn flash attention off - nothing updates the cache layout when it does. Unfused attention needs a transposed V, so every graph then inserts a cont(transpose(v)) per layer - a full copy of the V cache on each decode step, in the branch llama-graph.cpp marks "note: avoid this branch". Re-create the memory module once flash attention has resolved. This can only trigger on the first reserve (resolve_fused_ops clears cparams.auto_fa), when the cache is still empty, so no cached data is moved or lost. The re-reserve rebuilds the worst-case graphs against the corrected layout. params_mem is hoisted to the enclosing scope so the re-creation can reuse it. Qwen3-4B-Q4_0, tg32 at depth 4096, -fa auto resolved to disabled vs an explicit -fa 0: Adreno 840 3.72 -> 10.20 t/s (-69.0% -> parity) Adreno X2-90 8.01 -> 24.54 t/s (-67.5% -> parity) Output is byte-identical across -fa 0, -fa 1, auto-resolved-on and auto-resolved-off, before and after the change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
This PR is to fix a performance bug with FA set to auto.
Context
attn_v_trans = !flash_attn:Problem
Under
LLAMA_FLASH_ATTN_TYPE_AUTO— the default in bothllama_context_default_paramsand the CLI — these two steps happen in the wrong order.cparams.flash_attnstill holds the requested value (true), so V gets the flash-attention layout.resolve_fused_opsruns later, during the firstsched_reserve, and may then turn flash attention off.cont(transpose(v))per layer, per graph — a materialized copy of the full V cache on every decode step, growing with context depth.Fix
sched_reserve()also runs during decode, where discarding the cache would lose live KV data. That cannot happen here: the condition is gated oncparams.auto_fa, which is set once in the constructor and cleared inside resolve_fused_ops, so it can only be true on the first reserve — inside the constructor, before any token has been processed.Performance
Qwen3-4B-Q4_0, tg32 at depth 4096. Each row compares -fa auto (resolved to disabled) against an explicit -fa 0, before and after this change:
The fix restores parity with
-fa 0in every case.How the CUDA row was measured
CUDA supports flash attention for every configuration tested (12 of 12 resolve to enabled), so the auto-resolved-off path cannot be reached there naturally. To measure it, a small test harness (not part of this PR) was applied on top of the fix, adding two environment toggles:
GGML_CUDA_FA_FORCE_DECLINE=1— the CUDA backend declinesFLASH_ATTN_EXTat the decode probe shape (ne[1] == 1), so-fa autoresolves to disabled through the normalresolve_fused_opspath, the same way a real backend decline occurs.LLAMA_FA_NO_RECREATE=1— skips the re-creation added by this PR, so the before/after arms come from a single binary.The CUDA row therefore isn't a claim that CUDA users hit this today — they don't. It shows the cost is incurred entirely in the fallback branch in
llama-graph.cpp, independent of which backend declined: any backend that ever resolves flash attention off pays it.Correctness
Model output is byte-identical across
-fa 0,-fa 1, auto-resolved-on and auto-resolved-off, before and after the change. This is a layout and performance fix.Additional information
Requirements