[None][feat] Add KV cache manager V2 support for DSA - #16060
Conversation
8ff6b1c to
69404fe
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #58215 [ run ] triggered by Bot. Commit: |
|
PR_Github #58215 [ run ] completed with state
|
69404fe to
767b3be
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #60273 [ run ] triggered by Bot. Commit: |
|
PR_Github #60273 [ run ] completed with state
|
767b3be to
eeadb01
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #60657 [ run ] triggered by Bot. Commit: |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
WalkthroughDSA now supports V1 and V2 KV cache managers. The change adds indexer cache sizing, pool-index resolution, full-KV mapping recomputation, FP4 byte indexing, and draft replay state restoration. Manager selection, compatibility checks, tests, integration configuration, and kernel documentation were updated. ChangesDSA KV Cache Manager V2
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant draft_kv_cache_context
participant DSAtrtllmAttentionMetadata
participant DSACacheManagerV2
participant Indexer
draft_kv_cache_context->>DSAtrtllmAttentionMetadata: swap draft cache state
DSAtrtllmAttentionMetadata->>DSACacheManagerV2: resolve pool mappings
DSAtrtllmAttentionMetadata->>Indexer: recompute slot and full-KV mappings
Indexer-->>DSAtrtllmAttentionMetadata: return updated mappings
draft_kv_cache_context->>DSAtrtllmAttentionMetadata: restore original state
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
PR_Github #60657 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #60702 [ run ] triggered by Bot. Commit: |
|
PR_Github #60702 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #60752 [ run ] triggered by Bot. Commit: |
|
PR_Github #60752 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #60898 [ run ] triggered by Bot. Commit: |
|
PR_Github #60898 [ run ] completed with state |
zhaoyangwang-nvidia
left a comment
There was a problem hiding this comment.
Approve for trt-llm-runtime-devs with few comments.
| # contents in place can race those copies because CPU writes are not | ||
| # ordered with the CUDA stream. The fresh host buffers stay attached | ||
| # to the metadata until after the draft forward has been enqueued. | ||
| m.host_indexer_k_cache_block_offsets = torch.empty_like( |
There was a problem hiding this comment.
This allocates three fresh pinned host buffers on every draft replay, i.e., every decode step. cudaHostAlloc is expensive and can implicitly synchronize, and PyTorch's caching host allocator doesn't guarantee a hit. Since the shapes are fixed, consider pre-allocating a dedicated set of draft-side replacement buffers once (double-buffering) at metadata init, and just swap references here.
| kv_cache_manager_cls, | ||
| model_config: ModelConfig, | ||
| kv_cache_config: Optional[KvCacheConfig] = None): | ||
| def _fallback_if_unsupported_kv_cache_manager_v2(self, kv_cache_manager_cls, |
There was a problem hiding this comment.
consider renaming to_validate_or_fallback_kv_cache_manager_v2(or similar).
| if self._max_beam_width is not None and self._max_beam_width > 1: | ||
| incompat.append("beam_width > 1") | ||
| sparse_attn_config = model_config.sparse_attention_config | ||
| if (sparse_attn_config is not None |
There was a problem hiding this comment.
This PR introduces two user-visible behavior changes: (a) DSA + STAR context parallelism now raises instead of running; (b) sparse-attention models no longer silently fall back from V2 to V1. Please update doc under docs/source/features/feature-combination-matrix.md or docs/source/features/kvcache.md
Signed-off-by: Yi Zhang <187001205+yizhang-nv@users.noreply.github.com>
Signed-off-by: Yi Zhang <187001205+yizhang-nv@users.noreply.github.com>
Signed-off-by: yizhang-nv <187001205+yizhang-nv@users.noreply.github.com> Signed-off-by: Yi Zhang <187001205+yizhang-nv@users.noreply.github.com>
Signed-off-by: Yi Zhang <187001205+yizhang-nv@users.noreply.github.com>
Signed-off-by: Eric Tsai <ertsai@nvidia.com>
2a417cc to
ffcffbf
Compare
Summary by CodeRabbit
New Features
DSACacheManagerV2.Bug Fixes
Tests
Dev Engineer Review
use_kv_cache_manager_v2.DSACacheManagerV2provides role-based indexer K-cache sizing, pool layout, stable-slot resolution, and byte-per-token estimates.CpType.STAR, and confirm that fallback errors remain actionable.QA Engineer Review
test_dsa_cache_manager_selection_honors_v2_config.test_dsa_metadata_accepts_kv_cache_manager_v2.test_dsa_cache_manager_v2_cache_size_estimate_includes_indexer.test_dsa_cache_manager_v2_indexer_pool_layout.TestPrepareRestoreAttnMetadataForDraftReplay.test_draft_context_recomputes_and_restores_dsa_metadata.TestPrepareRestoreAttnMetadataForDraftReplay.test_recompute_context_kv_gather_mappings_skips_generation.create_dsa_cache_managerto support V2 test setup.TestGLM52.test_nvfp4to enable V2.test-db/orqa/entries. Verdict: needs follow-up.Description
Port the DSA KV cache manager V2 work from liji-nv@492422b onto the latest
main.This change:
DSACacheManagerV2selection throughuse_kv_cache_manager_v2while preserving the V1 path.Role.INDEX_KEYbuffer managed by V2.Role.ALL, runtime quota calculation, and static KV cache warm-up estimation.Test Coverage
git diff --checkpassed.PR Checklist
Please review the following before submitting your PR:
PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.
PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.
Test cases are provided for new code paths (see test instructions)
If PR introduces API changes, an appropriate PR label is added - either
api-compatibleorapi-breaking. Forapi-breaking, includeBREAKINGin the PR title.Any new dependencies have been scanned for license and vulnerabilities
CODEOWNERS updated if ownership changes
Documentation updated as needed
Update tava architecture diagram if there is a significant design change in PR.
The reviewers assigned automatically/manually are appropriate for the PR.
Please check this after reviewing the above items as appropriate for this PR.
GitHub Bot Help
To see a list of available CI bot commands, please comment
/bot help.