Uh oh!
There was an error while loading. Please reload this page.
feat(graph,arena): SaveForBackward contract with arena pinning (ADR 006, T2.1+T2.2) - #132
Merged
Conversation
…ed frees (ADR 006, T2.2) ArenaPool gains Pin/Unpin per-buffer refcounts (keyed by base-pointer offset, under the existing lock): - Reset raises its effective rewind floor to the end of the highest pinned span instead of skipping holes: nothing at or below the rewound offset can be re-issued by the bump allocator, the simplest provably safe semantics. Watermark consequence (documented): dead unpinned bytes below the raised floor stay retained until the pins release; they are still poisoned at Reset under ZTENSOR_ARENA_POISON=1. - FreeArena on a pinned span is deferred (no poison, no free-list entry) until the last Unpin, which then applies the poison fill and free-list insert exactly as if the free had happened then. Reset drops deferred frees wholesale (the rewind subsumes them; releasing them later could alias the bump path). - Pinned spans are never poisoned; refcount underflow (Unpin without Pin) warns via a swappable sink, never panics. - PinnedBytes/PinnedHighWaterBytes expose the contract's watermark cost. Host-backed test hooks (NewHostBackedArenaForTesting, SetArenaPoisonEnabledForTesting, HostPoisonFillForTesting) are exported from internal/cuda for the graph-level regression tests; internal/ keeps them out of the public API. Refs #128, zerfoo docs/plan-gpu-training-hardening.md S2.2.1.
…arena pinner (T2.2) - gpuapi.BackwardPinner: the pool-side pin capability; CUDAArenaPool implements it by delegating to ArenaPool.Pin/Unpin and exposes PinnedBytes/PinnedHighWaterBytes. - tensor.PinnableStorage: the storage-side interface the graph type-asserts when recording a saved tensor. GPUStorage pins its allocation base pointer with the original allocation size; CPU storages intentionally do not implement it (GC-owned memory cannot be reclaimed behind a live reference), so CPU engines are unaffected. - UnpinForBackward targets the pointer captured at pin time, so the pin stays balanced even when the graph executor's refcount release frees a saved intermediate mid-step (the pool defers the actual free while pinned, but Free zeroes devicePtr). - GPUEngine.ArenaPinnedBytes() surfaces the watermark cost for monitoring. Refs #128.
…ecycle (ADR 006, T2.1) Nodes that need a forward intermediate in Backward now have two sanctioned mechanisms, documented in graph/save_for_backward.go: 1. SaveForBackward (new): the node registers tensors during Forward; the graph records the saved set per node and pins arena-backed storage (tensor.PinnableStorage) so arena Reset / intra-pass reuse cannot recycle the buffers before Backward consumes them. Non-arena storage (CPU engines) is a recorded no-op. 2. Recompute from the live 'inputs ...' Backward already receives -- the documented alternative for cheap intermediates. Plumbing: Builder.Build hands every node implementing SaverAware a per-node Saver bound to its identity. A per-node handle (rather than a context value or a current-node field) keeps saves correctly attributed under ParallelForward, where node Forwards run concurrently; the saved sets carry their own mutex for the same reason. Lifecycle: the graph unpins a node's saved set immediately after that node's Backward returns (success or error), releases all remaining sets at the end of Backward (nodes that never received a gradient), and at the start of the next Forward (forward-only inference loops do not accumulate pins; at most one pass of saved intermediates is retained). Migrated consumer: checkpointNode cached its forward inputs in a struct field and re-read them in Backward for recomputation -- exactly the zerfoo#842/#845 bug shape. It now also registers them via SaveForBackward; the struct field keeps only the Go handles. Key regression test (S2.1.1): the Wolf hazard schedule -- Forward, arena Reset (per-sample ResetPool), Backward -- against a REAL host-backed ArenaPool. A LayerNorm-variance-shaped node using SaveForBackward reads the correct value; the same node using a raw struct-field cache under poison mode reads NaN. Refs #128, zerfoo docs/plan-gpu-training-hardening.md T2.1/S2.1.1.
…nce) Two nodes saving the same tensor each take their own refcounted pin; each Backward releases only its own reference.
Uh oh!
There was an error while loading. Please reload this page.
dndungu added a commit
to zerfoo/zerfoo
that referenced
this pull request
Jun 11, 2026
Pulls graph.Saver/graph.SaverAware and the arena Pin/Unpin lifecycle (zerfoo/ztensor#132, SHA 1cc7793f9acc) needed for the T2.3 migration. Refs #847, zerfoo/ztensor#128.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements ADR 006 decisions 1–3 (decision 4, poison mode, already landed): the save-for-backward contract in the graph plus the arena Pin/Unpin it rests on. Plan tasks T2.1, T2.2, S2.1.1, S2.2.1 of zerfoo
docs/plan-gpu-training-hardening.md.Refs #128.
Why
Nodes cache forward intermediates in struct fields and read them in Backward; the arena reuses/overwrites those buffers first (zerfoo#842 LayerNorm variance, zerfoo#845 gradient buffer, Wolf QK-norm cached inverse). Nothing in ztensor could express "this tensor must survive until backward". Now the graph owns that lifetime, PyTorch-style.
API chosen (T2.1)
graph.Saver[T]—SaveForBackward(ts ...*tensor.TensorNumeric[T]), the handle a node calls during Forward.graph.SaverAware[T]—SetSaver(Saver[T]); Builder.Build wires a per-node Saver into every SaverAware node. A per-node handle (rather than acontext.Contextvalue or a "current node" field on the graph) keeps saves correctly attributed underParallelForward, where several node Forwards run concurrently; the saved-set map carries its own mutex for the same reason. Nodes used outside a Graph get no Saver and must tolerate nil.(*Graph[T]).SaveForBackward(node, ts...)is the underlying recording method for callers that orchestrate nodes manually;SavedForBackward(node)exposes the set for tests/diagnostics.inputs ...remains the documented alternative for cheap intermediates; both mechanisms (and the deprecation of raw struct-field caches) are documented ingraph/save_for_backward.go.Pinning plumbing: graph →
tensor.PinnableStorage(implemented byGPUStorage, which pins the allocation base pointer with the original allocation size) →gpuapi.BackwardPinner(implemented byCUDAArenaPool) →cuda.ArenaPool.Pin/Unpin. CPU storages do not implementPinnableStorage, so CPU engines are completely unaffected.UnpinForBackwardtargets the pointer captured at pin time, so the pin stays balanced even when the executor's refcount release frees a saved intermediate mid-step.Lifecycle: the graph unpins a node's saved set immediately after that node's Backward returns (success or error), releases all remaining sets at the end of Backward (nodes that never received a gradient), and at the start of the next Forward — so forward-only inference loops retain at most one pass of saved intermediates and never accumulate pins.
Reset-vs-pinned semantics chosen (T2.2)
Raise the floor.
Reset()rewinds the bump offset to the end of the highest pinned span instead of the reset floor. This is the simplest provably-safe option: nothing at or below the rewound offset can ever be re-issued by the bump allocator, and the free-list is cleared as before, so no alias of a pinned span can exist.arena_pin.go): all bytes between the reset floor and the highest pinned span — including dead, unpinned buffers below it — stay retained until the pins release; the next Reset after the last Unpin reclaims them. Under poison mode those retained-dead bytes are still NaN-filled at Reset (poisonUnpinnedSpanLockedskips only the pinned spans). Pinned bytes are bounded by what backward genuinely needs;PinnedBytes()/PinnedHighWaterBytes()(also surfaced asGPUEngine.ArenaPinnedBytes()) monitor the cost.FreeArenaon a pinned span is deferred (no poison, no free-list entry) until the last Unpin, which applies the poison fill and free-list insert exactly as if the free had happened then. Reset drops deferred frees wholesale — a deferred block can extend above the raised floor, and releasing it later would alias the bump path.Migrated consumer
graph/checkpoint.gocheckpointNode: it cached the segment's forward inputs inseg.savedInputsand re-read them in Backward for recomputation — exactly the bug shape. It now implementsSaverAwareand registers the saved inputs viaSaveForBackward; the struct field keeps only the Go handles, the contract owns the memory lifetime.Regression-test story
TestSaveForBackward_WolfHazard_*(graph package) drives a real host-backedcuda.ArenaPoolthrough the Wolf hazard schedule — Forward,arena.Reset()(per-sample ResetPool), Backward — with a LayerNorm-variance-shaped node:SaveForBackward: Backward reads the correct value (2.5), pins drop to zero after Backward, and the next Reset reclaims+poisons the span;Arena-level tests (S2.2.1) cover: pinned-survives-Reset (unpinned sibling poisoned), raised-floor retention of dead bytes, deferred FreeArena with poison-on-release, refcounted re-pin, underflow warning, non-arena no-op, PinnedBytes/high-water tracking, Reset-drops-deferred-frees, and zero fills when poison is off. Tensor-level tests cover the GPUStorage→pool delegation incl. unpin-after-free. All host-backed, no GPU needed in CI (poison-test pattern; hooks exported from
internal/cudaonly).Gates
go build ./...✓internal/cudais excluded by CI; new files are vet-clean)graph/graph.go/checkpoint.goleft untouched)go test -race -timeout 300s -count=1 ./...✓Open questions for the E2.3 migration wave
SaverAwareand route every struct-field intermediate through the Saver. Audit list = everyBackwardreading a field written inForward.TrySetwith a different length) between save and backward — acceptable contract, or shouldPinForBackwardbe re-taken on realloc?MarkStepBoundarycurrently has no release hook; next-Forward + end-of-Backward release covers the known loops. If a caller needs pins dropped at an explicit step boundary without a Forward, we can addGraph.ReleaseSaved()trivially.