[None][feat] support C++ streaming KV events with multimodal payloads - #19359
Conversation
c095803 to
4fc3763
Compare
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: NVIDIA/TensorRT-LLM/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review. WalkthroughThe PR adds native C++ streaming events for KV-cache blocks. It exposes the sink through bindings, integrates native event draining with Python publication, supports both V2 backends, and adds multimodal event handling and tests. ChangesNative streaming KV-cache events
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant KVCacheManager
participant StreamingEventSink
participant StreamingKVCacheEventManager
participant WirePublisher
KVCacheManager->>StreamingEventSink: submit block lifecycle events
StreamingEventSink->>StreamingKVCacheEventManager: provide drained native DTOs
StreamingKVCacheEventManager->>WirePublisher: publish stored or removed wire events
StreamingKVCacheEventManager->>StreamingEventSink: synchronize native statistics
Suggested reviewers: Merge Risk: ⚪ Minimal · up to This change declares the native KV-event data types and decoder interface consistently with their implementation and consumer. No merge-blocking risk is identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
9850306 to
003d83f
Compare
Signed-off-by: Guan Luo <[email protected]>
Signed-off-by: Guan Luo <[email protected]>
Signed-off-by: Guan Luo <[email protected]>
003d83f to
3e0a0df
Compare
zhaoyangwang-nvidia
left a comment
There was a problem hiding this comment.
Approve with nits.
Signed-off-by: Guan Luo <[email protected]>
|
/bot run --disable-fail-fast |
|
PR_Github #75118 [ run ] triggered by Bot. Commit: |
|
PR_Github #75118 [ run ] completed with state
|
Signed-off-by: Guan Luo <[email protected]>
Signed-off-by: Guan Luo <[email protected]>
|
/bot run |
|
Fixed the x86_64/SBSA |
|
PR_Github #75203 [ run ] triggered by Bot. Commit: |
Address review feedback with a simpler digest encoder and concise streaming documentation that distinguishes multimodal payload support from deferred routing compatibility. Signed-off-by: Guan Luo <[email protected]>
|
Addressed the follow-ups from review #19359 (review) in 1aabf98:
Validation: native library and bindings rebuild passed; 2 targeted cap tests and 228 regression tests passed; Python lint/format and C++ formatting checks passed. No new model-serving or Dynamo routing E2E was run for this follow-up. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
/bot run --disable-fail-fast |
|
PR_Github #76010 [ run ] triggered by Bot. Commit: |
|
PR_Github #76010 [ run ] completed with state
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
/bot run --disable-fail-fast |
|
PR_Github #76136 [ run ] triggered by Bot. Commit: |
|
PR_Github #76136 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #76211 [ run ] triggered by Bot. Commit: |
|
PR_Github #76211 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #76217 [ run ] triggered by Bot. Commit: |
|
PR_Github #76217 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #76233 [ run ] triggered by Bot. Commit: |
|
PR_Github #76233 [ run ] completed with state |
Signed-off-by: GuanLuo <[email protected]>
Signed-off-by: Guan Luo <[email protected]>
|
/bot reuse-pipeline |
|
PR_Github #76308 [ reuse-pipeline ] triggered by Bot. Commit: |
|
PR_Github #76308 [ reuse-pipeline ] completed with state |
Dev Engineer Review
The change adds a native C++ streaming event sink for the C++ KV-cache manager V2 backend. The sink captures stored and removed block events, filters by lifecycle and block state, coalesces events, limits pending entries, and records statistics. Nanobind exposes the sink and event DTOs. Python converts the DTOs into
BlockStoredandBlockRemovedevents for the existing publisher path. The Python backend retains its own event source.The event payload can contain hexadecimal digest strings in
token_idsandmm_keysmetadata for multimodal blocks. Consumers that require integer-only token IDs may not be compatible. Verify wire compatibility, event ordering, buffer-drop behavior, shutdown handling, and API parity across backends.The PR description reports successful native builds, unit tests, formatting, lint, and pre-commit checks. It also reports that the routing E2E test was not rerun after the source-ownership refactor. CI runs PR_Github
#75118and L0_MergeRequest_PR pipeline#61871failed. The supplied evidence does not identify the failed tests. A later run was triggered, but no result is supplied.QA Engineer Review
Changed unit tests cover native event handling, stored and removed events, multimodal payloads, backend validation, attention-layer window filtering, unpublished-parent drops, and negative lifecycle IDs. Test-function names are not available for all added coverage. The changed files are unit tests, not integration tests. The KV-cache manager V2 test directory is already included in the CI lists
l0_cpu.yml,l0_b200.yml,l0_h100.yml, andl0_a10.yml; no test-list files changed. Coverage verdict: needs follow-up, because CI failed and the routing E2E test was not rerun after the refactor.Per-File QA Perspective
cpp/tensorrt_llm/batch_manager/kv_cache_manager_v2/CMakeLists.txt: Adds the event implementation files to the native build. Verify compilation and linking.cpp/tensorrt_llm/batch_manager/kv_cache_manager_v2/streamingEventSink.cpp: Adds native event capture, filtering, coalescing, bounded buffering, and statistics. Verify lifecycle selection, block drops, multimodal handling, and removal behavior.cpp/tensorrt_llm/batch_manager/kv_cache_manager_v2/streamingEventSink.h: Defines native event payloads and the sink API. Verify constructor defaults and consistency with nanobind.cpp/tensorrt_llm/batch_manager/kv_cache_manager_v2/eventData.cpp: Decodes block tokens and multimodal digests. Verify digest encoding andmm_keysalignment.cpp/tensorrt_llm/batch_manager/kv_cache_manager_v2/eventData.h: Adds shared event-token and multimodal-key types. Verify compatibility with Python event structures.cpp/tensorrt_llm/batch_manager/kv_cache_manager_v2/eventManager.cpp: Uses shared decoding for stored blocks. Verify decoded token IDs and cache metadata in emitted events.cpp/tensorrt_llm/batch_manager/kv_cache_manager_v2/eventManager.h: Replaces duplicate event-data declarations with shared types. Verify dependent C++ interfaces.cpp/tensorrt_llm/nanobind/batch_manager/kvCacheManagerV2.cpp: Exposes native sink types and accepts anEventSinkin manager construction. Verify binding signatures and accepted sink types.tensorrt_llm/_torch/pyexecutor/kv_cache/kv_cache_manager_v2.py: Passes the backend choice to the event manager. Verify C++ sink selection and attention-layer filtering.tensorrt_llm/_torch/pyexecutor/kv_cache_events.py: Adds backend-specific event sources and native DTO conversion. Verify draining, publication, shutdown, and statistics synchronization.tensorrt_llm/runtime/kv_cache_manager_v2/__init__.py: Exports native event types for non-Python backends. Verify imports and__all__.tensorrt_llm/runtime/kv_cache_manager_v2/__init__.pyi: Updates public type declarations for the sink and manager inputs. Verify stub and binding parity.tensorrt_llm/runtime/kv_cache_manager_v2/_introspection.py: Adds helpers for forwarding events to the native sink. Verify event forwarding and unavailable-backend errors.docs/source/features/kvcache.md: Documents streaming event scope and multimodal token values. Verify the documented wire contract matches producer behavior.tests/unittest/_torch/executor/kv_cache/test_kv_cache_manager_v2.py: Adds coverage for attention-layer window filtering. This is a unit test; integration test-list registration does not apply.tests/unittest/kv_cache_manager_v2_tests/test_kv_cache_event_manager.py: Adds native event coverage, including unpublished-parent drops and negative lifecycle IDs. This is a unit test; the parent test directory is listed in the CI test lists noted above.tests/unittest/kv_cache_manager_v2_tests/test_streaming_kv_events.py: Covers stored and removed events, multimodal payloads, backend validation, and parallelism constraints. This is a unit test; the parent test directory is listed in the CI test lists noted above.Description
The Python KV cache manager backend is being removed, but the streaming KV-event publisher introduced by #17023 currently depends on Python-side KV manager events.
This change keeps streaming KV events available with the C++ KV cache manager V2 backend:
StreamingEventSinkthat captures stored and removed block events as compact C++ DTOs.BlockStoredandBlockRemovedmsgspec structures and reuse the existing serialization and publisher path.The boundary intentionally remains semantic rather than serialized:
Multimodal wire compatibility
Breaking change for V2 multimodal streaming consumers: previously, streaming
token_idscontained integers and digest-bearing blocks were suppressed. This PR can emit hexadecimal digest strings intoken_idsand addmm_keystoBlockStored. Consumers that assume every token ID is an integer are incompatible. Text-only payloads are unchanged.The final cross-project interface is still being finalized by Dynamo #15095 and TensorRT-LLM #19529. We will revisit multimodal streaming handling separately after both PRs merge, when the interface is finalized, rather than define that final contract in this DTO/backend PR.
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, comment
/bot help.