Repository navigation
Conversation
Signed-off-by: Shixiaowei02 <39303645+Shixiaowei02@users.noreply.github.com>
…ayout hooks Signed-off-by: Bo Deng <deemod@nvidia.com>
d05717c to
379248c
Compare
|
Thanks for consolidating layout ownership in the manager. To keep this step easy to review, could we preserve the lender’s existing contract and behavior while changing the producer? Consider staging the Tensor/Axis migration, layout-ID version bump, and stricter role admission separately, or explicitly documenting their compatibility effects. Any compatibility projection should derive from one source of truth and cover only layouts it can represent losslessly. Differential checks against the previous implementation would help verify regions, transferred buffers, shards, object keys, and admission decisions for unchanged layouts, alongside existing lifecycle regressions and real-manager GPU checks. |
|
Thanks for this PR. I really like the approach of having each specialized manager declare its own page layout through a set of overridable hooks (
That seems clearly right to me. There are two things I'd like to discuss, both about how cross-rank layout is represented. 1. Should the cross-rank distribution live in the page layout at all? The per-buffer Coupling the two has some costs:
Could the hooks describe only the single-rank page: layers, roles, offsets, sizes, dtype/quant, and whether each buffer is transferred or rebuilt? The relationship between ranks would then be abstracted later, in the transfer stage that actually consumes it. 2. Should the cross-rank relationship be fixed in the current data form? The current vocabulary covers the common cases well. It is still a closed format, though:
Transfers that need conversion or re-layout don't fit, for example requantization, MLA expansion, swizzled layouts, or layouts that depend on runtime state. Each new case would mean extending the format and bumping In #20207 I took a different approach: a per-model These seem complementary. The manager hooks from this PR would publish the single-rank page facts. Planners would consume them, e.g. comparing Happy to discuss. I may be missing a reason the lender or another consumer needs the per-axis ownership at the manager level. |
@coderabbitai summary
Description
Depends on #20534, which is not merged yet. This branch is built on #20534's head (
dbc8015261), so until #20534 merges this PR's diff also contains that commit. Please review only the last commit,379248caaa. I will rebase ontomainonce #20534 lands.This is the first of the RI-10 PRs whose goal is that a cache's layout is declared once, by its manager, and that the lender and every backend read that one description. Supporting a new cache structure then means overriding a few manager hooks.
What changes:
KVCacheManagerV2.get_page_table()builds the page table (ManagerLayout, new modulekv_cache/page_table.py). It reads what the runtime already holds: layer groups, pool groups and their pools, where each buffer lies in its slot, layer configs and the rank's place. It asks hooks only for what the runtime cannot tell, and a subclass overrides only the hooks whose answer differs:_page_buffer_tensors: how a buffer's bytes spread over ranks, as tensors whose axes split over tp/cp. The base declares K/V and their block scales by KV head and index-K replicated. A role that moves and that no hook declares makesget_page_table()fail instead of being guessed._page_untransferred_roles,_page_rebuilds_untransferred,_virtual_layers,_page_layer_ids,_page_kv_heads,_page_dtype,_page_quant_mode.page_layout:BufferDesc.tensorsreplacesmapperandgeometry. The new types areTensor,Axisandheld, with the buildershead_major,token_major,sectioned,replicatedandowned.LAYOUT_ID_VERSIONgoes from 3 to 4, so instances of different builds see different layout ids and share no pages.derive_layoutreturns the manager's table. Its own derivation from theget_disagg_*declarations and from private manager attributes is removed.What does not change: the native disaggregated transfer. It still reads the
get_disagg_*declarations, which stay for now, and so doMapperKindandRoleLayout.Follow-ups, in order:
get_disagg_*declarations.Test Coverage
tests/unittest/_torch/executor/kv_cache/sharing/test_layout_identity.py(CPU): page table construction, tensor and axis validation, layout ids (version 4 encoding), undeclared roles refused, roles that never move, layer naming, quantization.tests/unittest/_torch/executor/kv_cache/test_kvcm2_integration.py: the base manager's default tensors, and refusal of an undeclared role.tests/unittest/_torch/executor/kv_cache/sharing/test_layout_matches_native_page_table.py(GPU): the manager's page table against native's page table across model and parallel layouts.tests/unittest/_torch/executor/kv_cache/test_mamba_cache_manager.py,tests/unittest/_torch/attention/sparse/deepseek_v4/test_deepseek_v4_cache_manager.py,tests/unittest/_torch/attention/sparse/qsa/test_qsa_cache_manager_layout.py(GPU): the Mamba/KDA, DeepSeek-V4 and QSA declarations.Run locally on CPU only:
kv_cacheanddisaggregatedCPU suites match [TRTLLM-16186][feat] Lend KV cache pages and recurrent state in place #20534's results, apart from the layout tests this PR replaces.The GPU tests have not been run yet.
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.