[GG] perf(dcp): preallocate budgeted CKV layer prefetch#177
[GG] perf(dcp): preallocate budgeted CKV layer prefetch#177voipmonitor wants to merge 2 commits into
Conversation
📝 WalkthroughWalkthroughAdds configurable B12X MLA CKV prefetch depth and workspace budgets, persistent ring-backed registry state, asynchronous lookahead gathers in ChangesB12X CKV Prefetch
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant forward_mqa
participant CKVPrefetchStateRegistry
participant CKVPrefetchWorkspacePool
participant GatherStream
forward_mqa->>CKVPrefetchStateRegistry: resolve layer prefetch state
CKVPrefetchStateRegistry->>CKVPrefetchWorkspacePool: acquire ring workspace
forward_mqa->>GatherStream: schedule lookahead gather
GatherStream-->>forward_mqa: record completion event
forward_mqa->>CKVPrefetchStateRegistry: wait or consume pending gather
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@vllm/v1/attention/backends/mla/b12x_mla_sparse.py`:
- Around line 1605-1608: Update reset_kv_cache_binding_state() to reset the
builder-owned _CKVPrefetchStateRegistry, clearing its layer_caches and
pending_layers entries when KV caches are replaced. Ensure subsequent
for_workspace calls cannot reuse state referencing profiling or freed cache
tensors, while preserving the existing legacy attribute cleanup.
- Around line 364-369: The close method releases ckv_workspace_slot before
guaranteeing completion of side-stream writes. Update close to synchronize the
relevant completion event, or record the slot view on the owning stream, before
calling workspace_pool.release; keep begin_step on the normal path and preserve
clearing ckv_workspace_slot and ckv_workspace after safe release.
- Around line 2050-2059: The staging layout in the ring-buffer setup must avoid
sharing local_buffer across buf_idx values. Update the records/local_buffer
addressing around _dcp_gather_ckv so each ring slot stages into its own
_ckv_local_capacity region, while preserving the per-slot gathered_buffer
layout; alternatively, ensure the synchronous fallback gather is explicitly
ordered after pending side-stream work.
- Line 316: The CKV prefetch state currently tracks liveness through per-call
storage wrappers and private storage identity. Update the workspace tracking
around `_CKVPrefetchState` and its callers to retain `weakref.ref(workspace)`
directly, and derive identity from documented tensor/storage metadata instead of
`storage._cdata`; apply the change at both the workspace registration logic and
the `workspace_storage_ref` assignment so state remains live while the workspace
allocation is tracked.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: cfe8c514-e86b-4e72-ac5d-b7a1fc0d5e3c
📒 Files selected for processing (4)
tests/v1/attention/test_b12x_ckv_prefetch_policy.pytests/v1/attention/test_b12x_mla_dcp_workspace.pyvllm/envs.pyvllm/v1/attention/backends/mla/b12x_mla_sparse.py
|
Review follow-up is in
Validation:
|
Summary
Preallocate a bounded ring of full-CKV DCP gather workspaces before KV-cache memory planning, then use it to overlap the next eligible MLA layer gather with current-layer compute.
The feature is controlled by:
VLLM_B12X_MLA_CKV_PREFETCH_DEPTH(default1;0keeps the synchronous path)VLLM_B12X_MLA_CKV_PREFETCH_WORKSPACE_MIB(default1024MiB per lane)The requested depth is reduced automatically when the ring would exceed the configured budget. Pool exhaustion is a hard error rather than an implicit allocation.
Why this version
The earlier stacked implementation in #160 allocated lookahead buffers lazily, after vLLM had already measured non-KV memory. That made the reported KV budget optimistic and could fail later when the ring was first materialized.
This PR is a direct, standalone change against
dev/gilded-gnosis. It creates the pool during backend initialization, before KV planning, and keeps target/draft plus DBO lanes isolated. It contains no replicated-indexer or sparse selected-record CKV changes.Measured effect
GLM-5.2 NVFP4, RTX PRO 6000 Blackwell, TP8/DCP4/MTP0, A16, full-CKV gather:
At the measured 400k profile, the bounded ring increased non-KV allocation from 410.2 to 738.4 MiB/GPU and reduced KV capacity by 25,344 tokens (1.14%). Depths 2 and 3 did not improve throughput, so depth 1 remains the default while larger depths stay explicit experiments.
After review lifecycle hardening, the final clean-branch TP8/DCP4 64k smoke reached 5,355 tok/s versus 5,367 tok/s before the review fix (-0.22%, noise), with 2,185,216 KV tokens and the two-slot depth-1 pool active.
Validation
63 passed:tests/v1/attention/test_b12x_ckv_prefetch_policy.pytests/v1/attention/test_b12x_mla_dcp_workspace.pyruff checkandruff format --checkpass for all modified files.This supersedes only the prefetch portion of #160. It does not supersede #159 (replicated indexer) or #161 (selected-record decode CKV).
Summary by CodeRabbit
Performance
Configuration
Compatibility