[AR-Diffusion] Allocate K and V separately so the compiled path stops cloning the pool - #6463
Open
linzhenpl07 wants to merge 1 commit into
Open
Conversation
… cloning the pool
The paged-write custom op declares both key_pool and value_pool as
mutated. They were the two halves of one (2, num_blocks, ...) tensor, so
they aliased a single storage, and inductor's reinplace pass could
re-inplace only the first of the two:
For node auto_functionalized_v2, attempted to reinplace range(0, 2).
We were unable to reinplace [1]
The second falls into the "mutated arg aliases a graph input" case in
_inductor/fx_passes/reinplace.py, so auto_functionalized_v2's clone of
the entire V pool survives into the compiled graph:
buf13 = empty_strided_cuda((545587200,), (1,), torch.bfloat16) # 1.02 GiB
triton_poi_fused_1.run(arg10_1, buf13, 545587200, stream=stream0)
That is one full pool copied per compiled region per denoising step: at
480x768 with 40 layers and 4 DMD steps, roughly 163 GiB of copies per
committed chunk. On sm_120 the copy kernel additionally faults during
inductor's Triton config selection, so the path did not run compiled at
all; elsewhere it presumably ran and was simply slow, which is likely why
this went unnoticed.
Allocate K and V as separate tensors. Indexing is unchanged --
kv_pools[layer][0] and [1] still address K and V -- so no caller moves.
Measured on one RTX PRO 6000 at 480x768 bf16, LingBot-World 2.0, steady
state over chunks 6-9 (the sliding window fills at chunk 6):
eager 4.049 s/chunk
compiled 3.668 s/chunk 1.10x
Before this change the compiled path crashed rather than producing a
number. The plateau still occurs at chunk 6, so compilation introduces
nothing that grows with session length.
Tests: update test_paged_pool_layout_exposes_flat_slot_views for the new
layout, keeping its real invariant (the flat views alias their
block-shaped cache), and add test_key_and_value_caches_do_not_share_storage
to pin what this fix establishes, with a negative control that writing V
leaves K untouched. 83 tests pass.
Signed-off-by: linzhenpl07 <linzhenpl07@gmail.com>
linzhenpl07
requested review from
Gaohan123,
NickCao,
hsliuustc0106,
yenuo26 and
ywang96
as code owners
August 21, 2026 15:50
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
This PR appears to belong to: docs/design/module/cache_management.md. Module owners: @Isotr0py @princepride @SamitHuang @linzhenpl07, please review your own changes and leave a short self-review comment describing what you checked. PRs without author self-review may not be assigned a reviewer. Please take a look when you have a chance. If you would like an automated review, mention @vllm-omni-review-bot in a comment. |
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.
Summary
The AR-Diffusion paged-write custom op declares both
key_poolandvalue_poolas mutated:They were the two halves of one allocation:
so they alias a single storage. Inductor's reinplace pass can then re-inplace only the first of the two mutated arguments:
The second hits the "mutated arg aliases a graph input" case in
_inductor/fx_passes/reinplace.py, soauto_functionalized_v2's clone of the entire V pool survives into the compiled graph:and in the FX graph:
That is one full pool copied per compiled region per denoising step. At 480x768 with 40 layers and 4 DMD steps that is roughly 163 GiB of copies per committed chunk.
Fix
Allocate K and V as separate tensors.
kv_pools[layer][0]and[1]still address K and V, sokey_cache()/value_cache()and every other caller are unchanged.Measured
One RTX PRO 6000 Blackwell (sm_120), LingBot-World 2.0 at 480x768 bf16, single session, steady state over chunks 6-9 — the sliding window (
sink=9+window=9frames, 3 frames per block) fills at chunk 6, and latency is flat within 0.4% after that:1.10x, and the compiled path runs at all.
Before the change, the clone kernel faulted during inductor's Triton config selection:
confirmed under
CUDA_LAUNCH_BLOCKING=1, so it is the clone kernel itself rather than an async report from an earlier one. On other architectures the copy presumably completes and is simply slow, which is likely why this went unnoticed.The plateau still occurs at chunk 6 after the change, so compilation introduces nothing that grows with session length.
Ruled out along the way
Recorded because each of these looks plausible and none of them is the cause — all four act on a different layer than the one that decides the clone:
torch.cuda.empty_cache()released 0.00 GiBdynamic=Trueproducing multiple Triton configsdynamic=Falsestill crashestorch._dynamo.mark_static_address()on the poolsThe clone is a post-grad FX pass decision; none of the above can influence it.
TORCH_LOGS="+torch._inductor.fx_passes.reinplace"names the failing argument directly and is what located it.Test
tests/diffusion/ar_diffusion/— 83 pass, ruff check and format clean.test_paged_pool_layout_exposes_flat_slot_viewsupdated for the new layout, keeping its real invariant: the flat slot views alias their block-shaped cache, so a slot write is visible through the layout the attention kernel reads.test_key_and_value_caches_do_not_share_storageadded, pinning what this fix establishes — K and V have distinct storages per layer, the flat views point at the right one, and a negative control that filling V leaves K untouched.Note
The
(2, num_blocks, ...)shape was described as following FlashAttention's block-table layout. The kernel takeskandvas separate arguments (flash_attn_varlen_func(q=..., k=key_cache, v=value_cache, block_table=...)), so the combined allocation was a convenience rather than a requirement; each cache keeps its own(num_blocks, block_size, num_kv_heads, head_dim)shape. If there is a reason to keep one allocation that I have missed, the alternative would be to declare only one of the two as mutated and write V through a different path — but that seems worse.