Repository navigation
Conversation
ch-wan
left a comment
There was a problem hiding this comment.
Summary
This PR enables unified-memory PD for hybrid MHA/SWA by registering whole-buffer page envelopes, translating virtual ids to physical ids before Mooncake transfer, gating compaction while destinations can still be referenced, and forcing cpu-tensor retraction with startup checks for unsupported combos. The envelope addressing matches the existing MLA contract (raw_ptr + physical_page * page_bytes), SWA state still goes through translate_loc_from_full_to_swa, and spec+Mooncake is correctly refused until a backend implements separate draft indices. The remaining risk is decode-side unified SWA admission/prealloc: joint FULL/SWA budgeting is skipped unless SWA-tail mode is on, and alloc_extend_swa_tail asserts a seq/prefix/delta identity that decode HiCache does not satisfy.
Issue counts by severity
- bugs: 2
- suggestions: 3
- nits: 0
Co-authored-by: Yonghao Zhuang <yhzhuang@meta.com>
Co-authored-by: Yonghao Zhuang <yhzhuang@meta.com>
Co-authored-by: yhzhuang <yhzhuang@fb.com>
Co-authored-by: Yonghao Zhuang <yhzhuang@meta.com>
Minimize both component reclaim quotas under the joint capacity predicate so a blocked compaction path cannot turn one allocation shortfall into an all-SWA eviction. Original prod_inference commit: 56c7082a92bc0c9024585387460e64b065652918 Co-authored-by: Yonghao Zhuang <yhzhuang@meta.com>
ZYHowell
left a comment
There was a problem hiding this comment.
Reviewed the incremental change from #36729 567f4f5d0f13 to this head 606649367b4f, following the allocator if/else feedback. One remaining duplicated rejection workflow is identified below; inherited lower-PR changes are not counted again.
Validation: an isolated CPU comparison of the current method and a common rejection tail matched return values, reservation-call arguments, log/abort/output effects, and message precedence in 1,536 cases. No serving or PD transfer test was run for this review.
Retain shared virtual-ID lifecycle in a common base, gate eager-pool post-capture planning, and preserve capacity realization results. Keep allocator-owned admission policy and existing tri-pool behavior.
Consolidate the historical PR iterations through the last upstream merge (f83adaf), preserving its exact tree. Later review and refactor commits remain separate.
Retain shared virtual-ID lifecycle in a common base, gate eager-pool post-capture planning, and preserve capacity realization results. Keep allocator-owned admission policy and existing tri-pool behavior.
ZYHowell
left a comment
There was a problem hiding this comment.
Reviewed the PD-specific delta from inherited capacity commit 089673a0e6 to 3c9040a7362e. The previous duplicated over-capacity rejection tail is addressed, and unified admission dispatch is consistently first. One small remaining conditional simplification is inline; it preserves the unified reservation call and optional static SWA constraint.
The isolated check matched return values and unified-call arguments in 960 boundary combinations. Validation was limited to source inspection and isolated CPU checks of the suggested simplifications; no GPU, serving, PD, or storage end-to-end validation was run.
Cherry-picked from a6eb82d.
Motivation
Unified memory pools expose virtual token IDs while disaggregated transfer backends operate on physical buffers. PD transfers therefore need an explicit page-envelope contract and physical index translation, including distinct target and independent-draft index vectors when their layouts differ.
Stacked after #36729; prerequisite #36403 is merged.
Modifications
Accuracy Tests
Not applicable; this changes KV transport addressing rather than model math.
Speed Tests and Profiling
Not run; no transfer benchmark environment was used locally.
Test Plan
e09999ea68. Merged OSS maina63efd9056into the bottom PR and propagated normal merges; no rebase or force-push. Final merge previews also pass against maindc5f59c3a2.withsyntax after the first CPU CI exposedunittest.enterContextbeing unavailable on Python 3.10. Its 8 tests pass locally. No new tests were added for this conflict refresh.run-ci-extrais absent). The initial separate MLX run failed in the upstream, unchangedtest_scheduler_mixin.pymock-ingestion contract.Local validation commands
Python commands below were executed through an existing
uv runenvironment (Python 3.12, Torch 2.11); environment-specific paths are omitted.Original commits
3f245db06c33bca46e62b60c0ed0a3ed9edd3353d036c1443db3c7b3ab86d4194c7e1dab5b5dbd3dChecklist
CI States
Latest PR Test (Base): Not run yet⚠️ Not enabled -- add
Latest PR Test (Extra):
run-ci-extralabel to opt in.Latest PR Test (AMD ROCm 10): ➖ No AMD PR run found for this commit.