Repository navigation
[KK] Release dense MLA priming buffers after preparation - #953
voipmonitor wants to merge 2 commits into
Conversation
Keep compiled launchers and caller-owned serving workspaces alive without retaining temporary Q, output and metadata buffers for every layer and capacity. Add BF16 and FP8 lifetime regression checks. Co-authored-by: OpenAI Codex <noreply@openai.com> Signed-off-by: voipmonitor <voipmonitor@users.noreply.github.com>
Co-authored-by: OpenAI Codex <noreply@openai.com>
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughDense MLA preparation now synchronizes the priming call before discarding temporary operands. ChangesDense MLA preparation
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The change removes redundant retention of temporary preparation buffers while serving continues to use its own buffers. No actionable merge-blocking defect was identified; normal dependency and GPU test checks remain necessary. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change shortens temporary GPU-buffer lifetimes without demonstrating new access or privileges. Remaining risk concerns whether outstanding GPU work always finishes before those buffers are released. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
|
Closing: this change is already in |
Behavior
Release the temporary query, output, lengths, page-table and scratch tensors used to prime B12X dense MLA after synchronized preparation. Published plans retain compiled launchers; serving binds its own buffers.
Keeping the priming operands in
PreparedCall.ownersalso kept their allocations alive for the plan's lifetime. This consumes memory needed by KV storage and speculative CUDA graphs. The preparation session synchronizes the priming callback before discarding it, so these operands do not need publication ownership.No arithmetic, precision, cache layout or public configuration changes. This applies to the B12X dense-MLA backend, not only Kimi-K3.
Validation
275b2f96239f0e4330ec4a69d5b9e20eea13422b.Evidence:
kimi-x4t-serving-20260930/evidence/mla-preparation-ownership-{before,after}.log,mla-graph-reuse-tests.json, and the CC8 receipts linked from #906. AI assistance: OpenAI Codex; no human-review claim is implied.Summary by CodeRabbit