[TRTLLM-13333][feat] Add prefetch_reuse_blocks and configurable prefetch count - #15149
Conversation
3c0c41e to
b8557ba
Compare
📝 WalkthroughWalkthroughThe pull request adds disk-to-host KV cache block prefetch for context requests. A new configuration field controls prefetch scope, core storage implements block matching and migration, the V2 manager wraps and orchestrates, and the executor integrates prefetch into batch preparation. ChangesKV Cache disk-to-host prefetch for context requests
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
tests/unittest/kv_cache_manager_v2_tests/test_kv_cache_manager_v2.py (1)
1387-1388: 💤 Low valueConsider adding a docstring to explain the test objective.
This test validates a new prefetch feature across cache tiers. A brief docstring would clarify the verification goal: that
prefetch_reuse_blocks()correctly migrates matching reusable pages from disk tier to host tier when called with tokens that match committed blocks in the radix tree.📝 Suggested docstring
class TestPrefetchReuseBlocks(TestNoBatching): def test_prefetch_reuse_blocks(self) -> None: + """Verify prefetch_reuse_blocks migrates matching pages from disk to host. + + Commits known tokens to populate reuse tree, forces eviction to disk via + quota shrink, then confirms prefetch moves matching pages from disk to host. + """ self.prepare(64 << 20, 128 << 20, 128 << 20, 36, 128, 1, kv_buf_size=32768)🤖 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 `@tests/unittest/kv_cache_manager_v2_tests/test_kv_cache_manager_v2.py` around lines 1387 - 1388, Add a concise docstring to the TestPrefetchReuseBlocks class or its test_prefetch_reuse_blocks method that states the test objective: it verifies that prefetch_reuse_blocks() migrates matching reusable pages from the disk tier to the host tier when supplied tokens correspond to committed blocks in the radix tree, and documents expected outcome (which pages are moved and any assertions made); reference the prefetch_reuse_blocks() call, the radix tree/committed block matching, and the cache tiers (disk tier, host tier) so future readers can quickly understand the verification goal.
🤖 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 `@tensorrt_llm/_torch/pyexecutor/py_executor.py`:
- Around line 2416-2417: The attribute _scheduler_manages_kv_suspend is
referenced in the scheduling loop and in _prefetch_for_context_requests but
never initialized on PyExecutor, which causes AttributeError at runtime; fix by
adding an explicit initialization (e.g., self._scheduler_manages_kv_suspend =
False) in PyExecutor.__init__ so the flag always exists, or alternatively guard
its usage with hasattr(self, "_scheduler_manages_kv_suspend") before
dereferencing it in the scheduling code that checks
isinstance(self.kv_cache_manager, KVCacheManagerV2) and in
_prefetch_for_context_requests; ensure the initializer approach is preferred so
other methods (like _prefetch_for_context_requests) can rely on a consistent
boolean.
- Around line 2414-2419: The _commit_kv_cache_stats hook is never invoked so
KVCacheManagerV2 never receives commit_scheduled_kv_cache_stats calls and stats
drift; locate where batches are scheduled/committed in the scheduling flow
(where ScheduledRequests instances are finalized/returned) and add a call to
_commit_kv_cache_stats(scheduled_batch) after a batch is scheduled/committed
(guarded by self._scheduler_manages_kv_suspend like the method does). Ensure
this invocation uses the same ScheduledRequests object passed through scheduling
so KVCacheManagerV2.commit_scheduled_kv_cache_stats(scheduled_batch) runs; also
add the same call at the other scheduling exit point corresponding to the second
occurrence noted (the analogous spot referenced around where the second dead
hook would be invoked).
In `@tensorrt_llm/_torch/pyexecutor/resource_manager.py`:
- Around line 3077-3089: The function prefetch_for_context_tokens uses
CacheLevel but the symbol isn't imported; add an import for CacheLevel at the
top of the module (where other cache-related types are imported) so the call
HOST_LEVEL = CacheLevel(1) resolves; update the import block to include
CacheLevel (alongside existing imports such as ReuseScope or related cache
types) so prefetch_for_context_tokens and its call to
self.impl.prefetch_reuse_blocks(reuse_scope, tokens, HOST_LEVEL) no longer raise
NameError or fail linting.
In `@tests/unittest/kv_cache_manager_v2_tests/test_kv_cache_manager_v2.py`:
- Line 1402: The call to kv_cache.resume(stream) should assert its return value
for consistency with other resume() checks; capture the result of
kv_cache.resume(stream) into a variable and add an assertion that it indicates
success (e.g., assert result is True or assert result), updating the test
invocation of kv_cache.resume(stream) so the test explicitly verifies the resume
succeeded using the same success expectation used elsewhere in this file.
---
Nitpick comments:
In `@tests/unittest/kv_cache_manager_v2_tests/test_kv_cache_manager_v2.py`:
- Around line 1387-1388: Add a concise docstring to the TestPrefetchReuseBlocks
class or its test_prefetch_reuse_blocks method that states the test objective:
it verifies that prefetch_reuse_blocks() migrates matching reusable pages from
the disk tier to the host tier when supplied tokens correspond to committed
blocks in the radix tree, and documents expected outcome (which pages are moved
and any assertions made); reference the prefetch_reuse_blocks() call, the radix
tree/committed block matching, and the cache tiers (disk tier, host tier) so
future readers can quickly understand the verification goal.
🪄 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: 9cd1fe66-aac2-4137-a824-6bf638338ef5
📒 Files selected for processing (5)
tensorrt_llm/_torch/pyexecutor/py_executor.pytensorrt_llm/_torch/pyexecutor/resource_manager.pytensorrt_llm/llmapi/llm_args.pytensorrt_llm/runtime/kv_cache_manager_v2/_core/_kv_cache_manager.pytests/unittest/kv_cache_manager_v2_tests/test_kv_cache_manager_v2.py
|
/bot run --disable-fail-fast |
|
PR_Github #53203 [ run ] triggered by Bot. Commit: |
1b696e8 to
e3463b8
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #53261 [ run ] triggered by Bot. Commit: |
|
PR_Github #53261 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #53456 [ run ] triggered by Bot. Commit: |
|
PR_Github #53456 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #53757 [ run ] triggered by Bot. Commit: |
|
PR_Github #53757 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
7334f22 to
cf8f6fd
Compare
|
/bot run --disable-fail-fast |
67420e4 to
4fc32cd
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #54262 [ run ] triggered by Bot. Commit: |
4fc32cd to
7a6c1a7
Compare
|
/bot run --disable-fail-fast |
2 similar comments
|
/bot run --disable-fail-fast |
|
/bot run --disable-fail-fast |
|
PR_Github #54301 [ run ] triggered by Bot. Commit: |
|
PR_Github #54262 [ run ] completed with state |
|
PR_Github #54301 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #54406 [ run ] triggered by Bot. Commit: |
59e4208 to
225b380
Compare
|
PR_Github #54406 [ run ] completed with state
|
- Add KVCacheManagerV2.prefetch_for_context_tokens(): pre-stage disk-tier radix-tree blocks to host via a transient KV cache (create/prefetch/close), matching the ReuseScope salt derivation used by _create_kv_cache so the transient cache hits the same blocks. - Add disk_prefetch_num_reqs to KvCacheConfig (default 4, 0 disables). - Schedule prefetch for queued context requests in PyExecutor, tracking already-prefetched request IDs and clearing them on termination. - Add HOST_LEVEL/DISK_LEVEL constants to kv_cache_manager_v2._common. Signed-off-by: Lizhi Zhou <1432185+reasonsolo@users.noreply.github.com>
Signed-off-by: Lizhi Zhou <1432185+reasonsolo@users.noreply.github.com>
225b380 to
2cb014f
Compare
|
/bot run --disable-fail-fast |
|
PR_Github #54541 [ run ] triggered by Bot. Commit: |
|
PR_Github #54541 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #54674 [ run ] triggered by Bot. Commit: |
|
PR_Github #54674 [ run ] completed with state |
…tch count (NVIDIA#15149) Signed-off-by: Lizhi Zhou <1432185+reasonsolo@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: GitLab CI Bot <gitlab-ci@nvidia.com>
…tch count (NVIDIA#15149) Signed-off-by: Lizhi Zhou <1432185+reasonsolo@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: GitLab CI Bot <gitlab-ci@nvidia.com>
Performance Summary
KV Cache Metrics (RANK 0, cumulative across all 30s intervals)
New Features
Tests
Description
Test Coverage
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.