[None][doc] Clarify KV cache host offload limits - #17246
Conversation
Signed-off-by: zq <zhouquan1511@163.com>
|
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 (1)
WalkthroughThe KV cache documentation adds licensing metadata and explains that host-memory offloading supports reuse and preemption but does not increase the active GPU-resident context length. ChangesKV cache documentation
Estimated code review effort: 1 (Trivial) | ~3 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/source/features/kvcache.md (1)
165-171: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse established terms for reuse.
Line [166] uses a long construction, and “later onboarding” in Line [169] is not defined in this section. Use “active request” and “later reuse” consistently.
Proposed wording
-Host offloading is a reuse and preemption mechanism, not a way to extend the -active context length beyond what the GPU KV cache can hold for a request that -is currently running. +Host offloading supports reuse and preemption. It does not extend the active +context length beyond the GPU KV cache capacity of an active request. ... -evicted or paused blocks available for later onboarding, which avoids +evicted or paused blocks available for later reuse, which avoids🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/source/features/kvcache.md` around lines 165 - 171, Update the host-offloading explanation near the active-request discussion to use the established terms “active request” and “later reuse” consistently. Replace the undefined “later onboarding” wording and simplify the long reuse description without changing the distinction between active GPU-resident blocks and evicted CPU-resident blocks.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/source/features/kvcache.md`:
- Around line 165-175: Add the required NVIDIA copyright header to
docs/source/features/kvcache.md, preserving the existing documentation content
and formatting.
---
Nitpick comments:
In `@docs/source/features/kvcache.md`:
- Around line 165-171: Update the host-offloading explanation near the
active-request discussion to use the established terms “active request” and
“later reuse” consistently. Replace the undefined “later onboarding” wording and
simplify the long reuse description without changing the distinction between
active GPU-resident blocks and evicted CPU-resident blocks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: aff68332-342b-4c9d-b235-fc0fa990bd7f
📒 Files selected for processing (1)
docs/source/features/kvcache.md
| request attend directly to KV blocks that remain only in CPU memory. To support | ||
| a longer active context, increase the GPU KV cache budget, reduce other memory | ||
| pressure, use a smaller model or KV cache dtype, use an attention-window feature | ||
| when applicable, or run on GPUs with more available memory. |
There was a problem hiding this comment.
This is a correct statement of the capabilities of the current system and the intended use of the offloading feature. For the record, we tried attending to blocks in host memory as a way to expand the capacity of the system beyond what GPU memory alone could provide, but throughput as deemed too low. This was an undocumented feature for some time, but was removed some time ago. Current system does not have the ability to attend directly to blocks in host memory.
Signed-off-by: zq <zhouquan1511@163.com>
|
Thanks for the review — I pushed Changes made:
Validation:
Note: Full Sphinx docs build was not run locally because |
Summary
Test Plan
git diff --checkpython -m sphinx --versionfails locally withNo module named sphinx)Refs #7281
Dev Engineer Review
docs/source/features/kvcache.md.git diff --checkpassed. A full Sphinx build was not run because Sphinx was unavailable.QA Engineer Review
No test changes.