Chris Tolliday

Wait for cyclic values before publication Summary: Before D120884622 returns a deferred value to its caller, the constructors it depends on must finish, even if cycle detection temporarily allowed unfinished pointers during restoration. Separate local payload construction from transitive readiness. A cycle-breaking reference may be stored during construction, but an owning result must not escape while a reachable constructor is unfinished or has failed. Track unfinished construction dependencies in storage-scoped readiness groups. Complete a group only after its writers and outgoing dependencies succeed, and propagate failures to its dependents. Keep ordinary ready-value lookups outside the readiness graph. Serialize enrollment and publication with the existing heap waiter mutex, using checked release stores and one readiness notification per affected heap. Preserve construction context through pagable arc bodies, with explicit publication boundaries for `OwnedFrozen` and `Globals`. Readable arena walks hide pending and failed graphs; physical indexing and destruction still visit initialized payloads. Keep removed heap owners alive until after the readiness lock is released. Add deterministic success, failure, unwind, same-thread, cross-heap, ancestor-chain, and nested owning-cache regressions. These checks cover publication safety separately from the existing deadlock-detection tests. An owning page-in that re-enters its own unfinished constructor is now rejected, rather than returning an in-progress pointer that can escape. This includes a frozen value -> `Arc` body -> `OwnedFrozen` back-edge. Production Arc page-out already blocks on the corresponding native ownership cycle; the regression uses synthetic owning wire to exercise `recover_root_from_pagable_in` without claiming that native cycle was previously supported. Return the existing typed heap-identity conflict error from `ActiveClaim::depend_on` instead of asserting during construction. Check that the conflicting binding is preserved without enrolling a readiness dependency. Reviewed By: Nero5023 Differential Revision: D120884617 fbshipit-source-id: ec1fc0a99451ce8380f5846a2108a81df825ce0d

Walk partially deserialized arenas safely Summary: The delayed and concurrent field reads enabled by D120884622 require profiling, serialization indexing, and destruction to tolerate heap slots that have been allocated but not initialized. Represent claimed-but-unwritten slots explicitly as `ArenaEntry::Uninitialized`. Use retained slot metadata to size them, include their addresses in serialization indexes, count their allocation bytes in profiles, and skip their payloads during destruction and value collection. Keep allocated chunk ranges as raw pointers, not shared byte slices. Decode the header with an acquire load before constructing any value reference, paired with release publication of completed values. Snapshot chunk boundaries under the claim lock and release it before profiling callbacks; ordinary native-heap summaries retain allocation-free iteration. Check index freshness again under the claim lock, then build and publish without releasing it. This coalesces concurrent refreshes and prevents stale publication. Pass only decoded entries through ordered visitors, and bind `FrozenHeapArc::iter_values` to its owning heap. Regression coverage includes failed and parked slots, publication and a new claim during a profiling callback, concurrent index refreshes, ordered visitors, and both allocator directions. The existing cycle-detection protocol is unchanged. Reviewed By: Nero5023 Differential Revision: D120093511 fbshipit-source-id: b38a2fc51d9e4cb3c1e1277de83b37c15853dfc7

Bind a heap's dependencies without reading them Summary: Although individual values are restored on demand, reading one heap still fetches its transitive dependency rows. Register unread dependency heaps instead, so field deferral in D120884622 also avoids those reads. Each dependency slot fetched its row and then its dependencies' rows, even when no value in those heaps was materialized. In a deep, widely shared graph such as a transitive set, this reads and retains data that the command may never use. A dependency slot now binds a skeleton instead: a `FrozenFrozenHeap` under the identity written beside the slot - name and seal nonce - with an empty arena and no ref list, holding the row's key. At first pointer resolution, its row is read, its identity checked against the slot's, and its dependencies bound as skeletons. The body header is read and the value claim proceeds as before. The heap that lists a skeleton retains it exactly as it retained a fully read dependency, so the page-in's retention and the brand argument for its values are unchanged; only when the row is read moves. Pointers can now name a heap whose row nothing has read yet, in two ways: - The target's row was written or read on this storage. Then the identity index answers with its key; it is bound, read on claim, and retained by the heap whose value pointed at it, since no ref list of that heap's accounts for it. This is the common case and the one packs rely on. - The target sits below a skeleton nobody has read, and the index has no entry - a daemon restart, say. A pointer from a value only ever names a heap in its own heap's closure, so reading rows breadth-first from that heap reaches it, and each row read on the way binds and retains the next. The deserialization context therefore carries the heap it is reading values out of, as does `DeferredFrozenValues`. Re-serializing a restored heap reuses its stored representation, so its skeleton dependencies are never read for that; a heap serialized from its arena writes each dependency's identity from the slot that bound it, so an unread skeleton needs no row for that either. `HeapDeserializationState` holds either the row's key (a skeleton) or the row once read (recipe and where its body starts); reading the row is idempotent under a race, and `heaps_loaded` and `heap_retained_blob_bytes` now count rows read rather than heaps bound. Where the OSS build's published `pagable` predates naming an arc slot by key, every dependency is read in place as before; nothing there reaches the skeleton paths. The owner retention established in D119898509 also covers targets discovered through lazy binding. The origin heap additionally bounds fallback traversal when the identity index has no entry. Historical measurement on an earlier revision/order of this stack: `pagable_bench --mode=recompute` against `fbcode//buck2:buck2` in a warm second checkout, with a private `allocator-stats.cpp` edit triggering a relink. | metric | before | after | | ------------------------ | ------------: | ------------: | | heaps_loaded | 46,488 | 25,829 | | heap_retained_blob_bytes | 1.70 GB | 1.26 GB | | paging memory restored | 4.13 GB | 3.64 GB | | claimed_values | 3,631,512 | 3,631,512 | | page_in_count | 47,820 | 47,820 | | critical-path page-in | 16.7s | 17.0s | | recompute wall | 66.2s | 66.6s | The counters show 44% fewer heap rows read, 440 MB less retained row data, and 490 MB less reported memory restored, with identical claim and page-in counts. These samples do not establish a wall-time improvement. The reported critical-path page-in counter is not a measure of total paging wall-time overhead; fetching an unclaimed heap's row can still delay required work. These results describe the measured stack, not an isolated attribution to this commit or a remeasurement after the review fixes. Reviewed By: Nero5023 Differential Revision: D119898510 fbshipit-source-id: 9afbe20b1ee5c2da53ca32f2695fbb1725f017d6

Move storage tests into a separate module Summary: Move the existing `storage::traits::tests` module into `storage/traits/tests.rs`, retaining its `#[cfg(test)]` gate, module path, helpers, and test behavior. This keeps the storage implementation readable and separates the later paging changes from this mechanical test move. Reviewed By: Nero5023 Differential Revision: D121691599 fbshipit-source-id: 2337d78c2d3104836edb40f77003ad45b83fcd46

Look up a heap's row by its identity Summary: D119898510 skips intermediate dependency heaps, while D120884648 introduces shared packs outside the original dependency lists. Resolve these targets directly by heap identity and retain them through the referencing heap so returned values remain valid after storage is dropped. Normally, reading owners' dependency lists binds the heaps their value pointers name. A pointer into a heap no list read so far names instead needs a direct lookup from `HeapRefId` to `DataKey`. `StarlarkHeapKeyIndex` provides that storage-scoped, in-memory map. A new `StorageContext::observe_arc_data_keys` hook populates it whenever pagable associates a heap arc with a key. When a pointer names a heap the scope has not bound, resolution consults the index before reporting an unbound heap: it fetches the row by key, binds it into the scope, and continues with that heap. Contexts restoring values into a heap carry that owner. The owner retains resolved target heaps, including scope-cache hits, and ownership and serialization walks include these discovered dependencies. Lookup alone would not keep returned pointers valid after the storage cache is dropped. The additional owning references last until the referencing heap is dropped. Ordinary eagerly read dependency graphs already have their targets bound. The new fallback supports unlisted targets now and the skipped intermediates in D119898510. D120884648 registers shared-pack keys here when writing them. The index does not survive a daemon restart: targets unreachable through dependency lists require their writer to rebuild the index before resolution. D119981963 makes identity include the heap name and seal nonce, distinguishing separate evaluations under one name. Each sealed heap maps to its stored row. The pagable test module is fbcode-only until OSS pagable supports these APIs. Reviewed By: Nero5023 Differential Revision: D119898509 fbshipit-source-id: bb0b810cb49d5f0d86edd36d42f40085e0bc0461

Fail instead of hang when a paged-out value can't be hydrated Summary: Currently paging deserialization failures cause DICE to hang, which seems to be because `WorkerCancelled` finishes the task *without ever producing a result*, so every awaiting computation blocked forever. Surface the failure as a cancelled computation instead, so awaiters resolve promptly. The `MatchPagedOut` / `CheckDepsPagedOut` lookup arms now finish the task with a result via `finish_hydration_failure` on hydration failure. A paged-out value is a cache entry and is always recomputable, so a hydration failure can and should degrade to a recompute rather than a hard error, which is done in the next diff. This also reports a soft_error propagated from DICE via the DiceEvent tracker. As is, this doesn't propagate the error message for deserialization failures except via the soft_error because the next diff will need another channel for propagating deserialization errors if they don't fail builds (and making this diff propagate the error correctly is not straightforward). Reviewed By: Nero5023 Differential Revision: D113503770 fbshipit-source-id: 815eb7ac62213da8ef8508b1bf3610d39df3e66f

Measure the memory paging actually moves, from the allocator Summary: The DataKey byte counters say how much paging wrote and read, not how much memory it freed. Those differ in ways that never wash out: an arc shared with a still-resident value is serialized but not dropped, an arc already on disk is dropped but not written, and `evict_keys` skips nodes that are missing, vacant or injected after their bytes were already counted. Differencing two growing counters accumulates all of that. Measure the memory instead, at the two points it actually moves: - page-out, on the core state thread in `evict_keys`. The graph holds the last reference — `page_out_value` consumed and dropped the worker's copy before the eviction was queued — so that is where the free happens, and jemalloc charges a free to the thread performing it whichever thread allocated it. - page-in, around the synchronous deserialize in `hydrate`. No await inside the window, so tokio cannot migrate us mid-measurement. Both take the net of allocated and deallocated so transient buffers cancel. An arc that is not actually freed contributes nothing, which is the case the byte counters get wrong. The counters come from jemalloc's `thread.{allocated,deallocated}p`, reached through buck2's own weak-symbol jemalloc bridge in `facebook/mem_frag` — the folly Rust binding only exposes `bool`. The pointers are resolved once per thread and `volatile`, since the compiler otherwise assumes an allocation cannot change global state. dice takes the reader as an injected `fn` so it stays allocator-agnostic and OSS-clean; the daemon installs it under `fbcode_build`. `offloaded - restored` is an upper bound on what paging is currently saving: it does not subtract values paged out and later invalidated, whose memory would have been reclaimed anyway. Bounding that is left for a follow-up. Reviewed By: Nero5023 Differential Revision: D117141211 fbshipit-source-id: 31139583123beb9857dc2814aa5cd36e730b9807

Create single choke point for VersionedGraph node mutation Summary: Structural refactor that funnels all `VersionedGraph` node mutation through 2 choke points - a `NodeMut` drop and `retain_injected`. This enables the next diff to maintain a list of page-out candidates that is kept in sync with the state of the actual values in `nodes`. The raw node map becomes private to the store module and is reached only through: `node_mut` (an in-place mutation guard over a present node), `node_entry` (mutate-or-insert resolved in one `entry()` lookup, yielding a `NodeMut` guard when present or a `VacantSlot` when absent), and `nodes()` (read-only access for iteration, lookup, and count). Reviewed By: JakobDegen Differential Revision: D112270660 fbshipit-source-id: b47e741da13cd40eeedef5b0c6a4e9a6765465f4

Measure heap- vs value-granularity cost of Starlark page-in Summary: Paging in a Starlark value retains far more than the value. A `FrozenValue` is serialized as `(HeapRefId, value_index)` and the heap is a separate content-addressed `DataKey`, so resolving one pointer requires fetching that heap's whole blob — and `deserialize_refs_and_skip_body` walks the entire transitive ref closure eagerly. The blob and a slot table covering every value in the heap are then retained for the heap's lifetime so any value can still be claimed later. Value bodies are already lazy; this is the part that is not. Adds counters that distinguish the size of values loaded vs actually read, only if `BUCK2_STARLARK_PARTIAL_DESER_STATS=1` is set, which is set by `pagable_bench`. Later diffs optimize paging so that the values loaded are closer to values read. On `fbcode//aps_models/ads/launchers/fm:icvr_launcher_multiapp` (97.9GB peak RSS), of 23.17GB restored by an incremental build: 13.35GB is retained heap blobs, 1.99GB is slot tables, and 0.45GB is the values actually built — a 33.9x ratio. 302,925 of 399,387 heaps loaded (76%) never had a single value read. `buck2 debug hydration status` independently corroborates the total via allocative. Note that this adds the `fbcode_build` to the starlark crate, since the starlark crate now depends on a pinned git release of buck2 and this requires buck2 changes. This can be dropped later once the buck2 git dependency from starlark is bumped. Reviewed By: Nero5023 Differential Revision: D119193087 fbshipit-source-id: 2c0794c009ab186fdaa41d57f61ed0a1d76d75bd

Prepare arena entries and claim locking Summary: The safe heap traversal added by D120093511 needs coordinated allocation and inspection, so this prepares explicit entry classification and a shared lock protecting slot claims. Prepare partially deserialized arenas for safe walking without changing their visitor behavior. Classify each entry once through the existing `AValueHeapEntry::state()` into `ArenaEntry::{Value, Forward, Reservation}`, and use that classification for sizing and dispatch. Keep the arena pointer immutable in `HeapDeserializationState`, with a separate `claims` mutex and atomic `serialization_index_dirty` flag. Claim allocation and sentinel installation remain under the same lock; index refresh holds it through freshness checking, construction, registration, and clearing the dirty flag. The index-publication regression verifies that registration excludes new claims and does not clear the dirty flag early. Atomic header decoding, raw chunk ranges, uninitialized-slot handling, and decoded ordered visitors remain in the following arena-walk fix. Reviewed By: Nero5023 Differential Revision: D120096124 fbshipit-source-id: ab646175ca6e47151f0bbce3c19a154d0f2b860b