Skip to content

[Perf] Avoid a per-prefill-step GPU->CPU sync in the KDA chunk kernels - #51540

Open
njhill wants to merge 3 commits into
vllm-project:mainfrom
njhill:kda-chunk-indices-nosync
Open

njhill wants to merge 3 commits into
vllm-project:mainfrom
njhill:kda-chunk-indices-nosync

Conversation

@njhill

@njhill njhill commented Aug 9, 2026 •

Copy link
Copy Markdown
Member

prepare_chunk_indices derives per-sequence chunk counts with .tolist(), so passing it a device cu_seqlens blocks the caller. The count also fixes the Triton grid, so it has to reach the host either way; compute it from the host-side cu_seqlens we already have and copy the result across, as gdn_attn already does for its own prefill slice.

Models from vllm/models/kimi_k3 using FLA hit this on prefill batches, on NVIDIA and ROCm. The existing chunk_indices describes the decode-peeled prefill_query_start_loc, so add non_spec_chunk_indices alongside it and thread an optional chunk_indices through chunk_kda_with_fused_gate. It defaults to None and falls back to deriving on device, so other callers are unaffected.

Verified host- and device-derived indices match in value, dtype and length. Not run on Kimi hardware.

Note this applies to Kimi Linear but not Kimi K3 with cuda since the latter uses FlashKDA rather than FLA.

Claude was used for this.

`prepare_chunk_indices` derives per-sequence chunk counts with `.tolist()`,
so passing it a device `cu_seqlens` blocks the caller. The count also fixes
the Triton grid, so it has to reach the host either way; compute it from the
host-side cu_seqlens we already have and copy the result across, as
`gdn_attn` already does for its own prefill slice.

Both models served from `vllm/models/kimi_k3` hit this on prefill batches,
on NVIDIA and ROCm. The existing `chunk_indices` describes the decode-peeled
`prefill_query_start_loc`, so add `non_spec_chunk_indices` alongside it and
thread an optional `chunk_indices` through `chunk_kda_with_fused_gate`. It
defaults to None and falls back to deriving on device, so other callers are
unaffected.

Building it costs 200-650us per prefill step, so it is opt-in via a builder
flag: the Kimi paths opt in, Qwen3-Next pays nothing.

Verified host- and device-derived indices match in value, dtype and length.
Not run on Kimi hardware.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Signed-off-by: Nick Hill <nickhill123@gmail.com>
@njhill
njhill force-pushed the kda-chunk-indices-nosync branch from 5406779 to 0eedbb7 Compare August 10, 2026 20:03
@njhill
njhill marked this pull request as ready for review August 10, 2026 20:09

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@njhill njhill added the ready ONLY add when PR is ready to merge/full CI is needed label Aug 10, 2026
@njhill

njhill commented Aug 10, 2026

Copy link
Copy Markdown
Member Author

/ci run

@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #83206 for commit 0eedbb770324.

njhill and others added 2 commits August 10, 2026 15:45
Co-authored-by: Benjamin Chislett <bchislett@nvidia.com>
Signed-off-by: Nick Hill <nickhill123@gmail.com>
@njhill

njhill commented Aug 10, 2026

Copy link
Copy Markdown
Member Author

/ci run

@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #83229 for commit 14b8fe82ade5.

@Fangzhou-Ai

Copy link
Copy Markdown
Collaborator

hi @njhill thanks for your PR! I saw you've also changed the path for ROCm side; I wonder if we have tested this PR on any AMD hardware?

@AndreasKaratzas

Copy link
Copy Markdown
Member

Test-wise, full CI eval on ROCm is in parity with nightly:

But as @Fangzhou-Ai mentioned, let's ensure perf-wise that we are not introducing any regressions.

@mergify

mergify Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @njhill.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify Bot added the needs-rebase label Aug 13, 2026
justtestingthingsx pushed a commit to meandmyboiclaude/vllm that referenced this pull request Aug 16, 2026
…sted GDN metadata

vllm-project#52297 (picked as the two commits below this one) hoists the per-batch GDN
spec metadata out of per-group build() into compute_common_gdn_attn_metadata.
Two carried club mechanisms lived in the deleted region and are relocated
into the hoisted shape:

1. ReplaySSM spec-sequence mask (use_cache_spec_kernel): the mask must come
   from ~is_prefilling & (qlen > 0), not from num_decode_draft_tokens_cpu
   (stale on draft-less steps; -1 for dropped-draft rows). Now an opt-in
   branch of compute_common_gdn_attn_metadata, driven from prepare_attn via
   cache_config.use_replayssm_spec + the is_prefilling tensor, and from
   build_for_cudagraph_capture (q=1 capture batches must still route spec).

2. Stale zero-accept guard (vllm-project#51508-subset + vllm-project#40756 clamp, which upstream's
   hoist dropped): the batch-level half (stale mask + two-sided clamp) lives
   in the hoisted function; the per-group half (NULL_BLOCK_ID masked_fill on
   spec_state_indices_tensor) stays in GDN build(), fed by a new
   stale_spec_reqs element threaded through MambaHybridAttnMetadata
   (GDN-only kwarg; spec-row-shaped, unsliced).

The context_lens_tensor unconditional compute (072646c) auto-merged into
the hoisted build() and needed no relocation.

Verified: py_compile all three; test_gdn_rollback_identity 49/49; gather
repro unchanged (documents upstream vllm-project#51540, fixed by chain patch at boot).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@khluu

khluu commented Sep 14, 2026

Copy link
Copy Markdown
Member

Fresh exact H200 evidence for this fix: the Kimi-Linear-48B-A3B Disaggregated DP EP lane has now failed three consecutive runs with the same prepare_chunk_indices implicit-sync error:

All three die on the first request in vllm/third_party/flash_linear_attention/ops/index.py:30: chunk_indices.to(..., non_blocking=True) is copying an unpinned CPU tensor while the GPU sync checker is active. This PR's metadata-builder + async_tensor_h2d path is the right structural fix, but the branch is currently conflicting. Could you rebase it and run this exact H200 lane before merge?

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

k3 kimi needs-rebase ready ONLY add when PR is ready to merge/full CI is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants