Repository navigation
docs(memory-design): write the motivation for an ORT reviewer; fix a missing edge in the diagram - #917
Conversation
…missing edge in the architecture diagram Two problems with taking this to the ORT team as written. The motivation named no ORT mechanism, so an ORT reader could reasonably conclude this is a GenAI-side concern. It also never answered the first objection anyone who knows ORT's memory model will raise -- that these are configuration knobs: gpu_mem_limit, arena extend strategy, OrtArenaCfg, shared allocators on OrtEnv. Rewritten around that objection: the limitation is that ORT gives each pool a bound but no way for one holder to lend to another or be reclaimed mid-generation. Names the concrete mechanisms -- BFC high-water behaviour and where end-of-Run shrinkage does and does not help, custom device allocators not being BFC-wrapped so the governed and arena paths are two implementations, RegisteredAllocator leaking because ORT does not expose the last session user. Adds three measurements up front, because an architecture proposal with no evidence of harm will be read as speculation: a load-time budget split that stranded 1.611 GB and cost 1.68x in streamed bytes (#857/#866); weight bytes measured from the external-data file rather than referenced extents, wrong by exactly 2.00x (#853/#856); and managing single-touch weights measuring 30x slower than the OS (#864/#874). The first two are accounting failures a single authority prevents by construction; the third is why the design also has to say when not to manage. The diagram had a real omission. The appendix assigns transient-peak reporting to InferenceSession/EP, and I8 requires admission to use exact bytes including transient peaks -- but there was no Session-to-manager edge at all. That is the principal ORT-side integration point, so its absence made the ORT work look smaller than it is. Added it, added the matching required change, and stated that the session is a reporter rather than a holder so the edge is not mistaken for an ownership change. Also split control-plane from data-plane edges, since only the control plane is new work and reviewers should be able to see that at a glance. Corrected one overclaim of my own while writing this: ORT does support arena shrinkage at a Run boundary, so 'does not return memory' was wrong. The accurate criticism is narrower -- shrinkage is caller-scheduled at a Run boundary, not demand-driven, so it cannot answer pressure that arrives mid-generation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d60eb808-7cc6-4abc-b48d-2a6dd3841624
|
Rendered the diagram with @mermaid-js/mermaid-cli\ before merging rather than assuming the syntax was fine — this file is going to another team, and a broken diagram would discredit the rest of it. Clean 43 KB SVG, the new \ ransient peak + granularity report\ edge present, no syntax error (the only \�rror\ strings in the output are mermaid's own \�rror-icon/\�rror-text\ CSS classes, which every mermaid SVG carries). |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #917 +/- ##
==========================================
- Coverage 77.78% 77.74% -0.05%
==========================================
Files 351 352 +1
Lines 140874 145872 +4998
Branches 140874 145872 +4998
==========================================
+ Hits 109582 113403 +3821
- Misses 26885 27988 +1103
- Partials 4407 4481 +74
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
🔴 Benchmark Regression DetectedComparison of criterion micro-benchmarks: PR head vs merge-base, measured on the same runner in the same job (base first → PR second).
Visual flags: Host infoWhat this cannot catch
|
Two problems with taking #907 to the ORT team as written.
The motivation was written for us, not for them
It named no ORT mechanism, so an ORT reader could reasonably conclude this is a GenAI-side concern that does not touch their code. And it never answered the first objection anyone who knows ORT's memory model will raise: these are configuration knobs — set
gpu_mem_limit, choose an arena extend strategy, sizeOrtArenaCfg, share allocators throughOrtEnv.Rewritten around that objection, because it is a fair one and the answer is the whole point of the design: ORT gives each pool a bound; it gives no holder a way to lend to another, and no way to reclaim from a holder that is over-provisioned right now. It now names the concrete mechanisms — BFC high-water behaviour and precisely where end-of-
Runshrinkage does and does not help, custom device allocators not being BFC-wrapped (so the governed path and the arena path are two implementations today, not one policy), andRegisteredAllocatorleaking because ORT does not expose the last session user.Also puts three measurements up front. An architecture proposal with no evidence of harm reads as speculation, and we now have the harm:
device_budget − kv_bytes_per_token × max_contextsplit that stranded 1.611 GB from the first token; making it elastic cut streamed bytes 1.68× (Weight budget is reduced by the full-context KV reservation and never reclaims it: 1.611 GB idle while weights stream 3.94 GB/step (2.75x floor reduction available) #857/Make CUDA weight-residency budget elastic against KV occupancy (#857) #866)The first two are accounting failures that a single authority prevents by construction, which is the argument for the design. The third is why it also has to say when not to manage.
The diagram had a real omission
The appendix assigns transient-peak reporting to
InferenceSession/EP, and I8 requires admission to derive exact bytes including transient peaks — but there was no edge fromSessiontoProcessMemoryManagerat all. That is the principal ORT-side integration point, so leaving it out made the ORT-side work look smaller than it is and hid the one interface ORT actually has to add.Added the edge, added the matching entry to Required architecture changes, and stated explicitly that the session is a reporter, not a holder, so the new edge is not misread as an ownership change. Also split control-plane (solid) from data-plane (dotted) edges, since only the control plane is new work and a reviewer should be able to see that without reading the prose.
One overclaim of my own, corrected
I first wrote that the BFC arena "does not return memory". That is wrong — ORT supports arena shrinkage at a
Runboundary, and an ORT reviewer would have caught it immediately and discounted everything around it. The accurate criticism is narrower and still holds: shrinkage is caller-scheduled at aRunboundary, not demand-driven, so it cannot answer pressure that arrives mid-generation.Docs only.