diff --git a/SUMMARY.md b/SUMMARY.md new file mode 100644 index 0000000..a4b86e3 --- /dev/null +++ b/SUMMARY.md @@ -0,0 +1,275 @@ +# E5 — MTLResidencySet over the existing malloc'd slabs (experiment branch) + +Branch: `e5/metal-residency-set` (cut from `origin/dev` @ `caa49f7`, per spec — E4 was cut +from `main` @ `72d3d37`; `backend_metal.mm`/`.h` are byte-identical between the two bases, +confirmed via `git diff 72d3d37 origin/dev -- c/backend_metal.mm c/backend_metal.h`). + +## The hypothesis + +E4 (MTLHeap-backed slabs) proved that batching residency declaration kills the GPU stall +(25.9s → 3.9s at cap16, −85%), but changing the *allocation* (heap sub-buffers instead of +malloc'd host memory) brought a +12–13s expert-disk-load tax, suspected first-touch/lock +contention on CPU-writes into GPU-owned heap pages. E5 decouples the two: keep the exact +same malloc'd slabs and per-slab `newBufferWithBytesNoCopy`-wrapped `MTLBuffer`s, and change +**only** residency bookkeeping — declare residency once, ahead of time, on a set attached to +the command queue, instead of once per command buffer via `useResource:`. If the stall +reduction survives without the load-path tax (malloc pages never change ownership), E5 wins. + +## What changed + +Everything is confined to `c/backend_metal.mm`. **No changes to `glm.c` or +`backend_metal.h`** — `coli_metal_register`/`coli_metal_unregister`'s existing signatures and +every call site in `glm.c` (expert_load, uring_load_add, qalloc, kv_alloc, map_of_fd) are +untouched; the residency-set bookkeeping lives entirely inside those two functions' existing +bodies. This is a smaller diff shape than E4, which needed new `backend_metal.h` declarations +and new `glm.c` call sites because it changed the allocation function itself. + +Env-gated `COLI_METAL_RESSET=1`, default OFF, runtime `@available(macOS 15.0, *)` guard with +a one-line stderr fallback when requested on an older OS or when residency-set creation +fails. Gate off ⇒ every new branch is skipped and behavior is byte-for-byte the stock path +(verified by inspection: `g_resset_enabled` starts `false` and nothing sets it except inside +the `COLI_METAL_RESSET` `getenv` branch in `coli_metal_init`, so `resset_add`/`resset_remove`/ +`resset_flush` are no-ops and `moe_submit`'s `useResource:` loop runs unconditionally). + +### Lifecycle (`c/backend_metal.mm`) + +- **Init** (`coli_metal_init`, end of the existing pipeline-setup `@autoreleasepool`): if + `COLI_METAL_RESSET=1` and `@available(macOS 15.0, *)`, create one + `MTLResidencySetDescriptor` (`initialCapacity=4096`, a presize hint only), call + `[g_dev newResidencySetWithDescriptor:desc error:&err]`, and `[g_queue addResidencySet:rs]` + — one set, attached once, for the process lifetime. Failure (old OS or creation error) + prints one stderr line and leaves `g_resset_enabled=false` — stock path. +- **`coli_metal_register`**: after wrapping the buffer exactly as today + (`newBufferWithBytesNoCopy`), calls `resset_add(b)` under the *same* `g_slab_mtx` that + already serializes `g_slabs` mutation from parallel OMP loader threads. `resset_add` calls + `[rs addAllocation:b]` and sets a `g_resset_dirty` flag — **it does not commit**. +- **`coli_metal_unregister`**: calls `resset_remove(b)` (also under `g_slab_mtx`) *before* + clearing the `g_slabs` entry. `resset_remove` calls `[rs removeAllocation:b]` **and commits + immediately** — no batching. See UNCERTAINTIES for why this asymmetry is deliberate. +- **`moe_submit`** (the one function whose `use` list — resolved expert weight/scale slabs — + scales with LRU cache size): calls `resset_flush()` at the top (commits any pending adds + from `resset_add`, under `g_slab_mtx`), then, if `g_resset_enabled`, **skips** the + `for(auto&b:use) [e useResource:b usage:MTLResourceUsageRead];` loop entirely — residency + is already guaranteed by the queue-attached set. Every other `useResource:` call site in the + file (`bind_gemv`'s weight/scale buffers, `coli_metal_attn_decode`/`coli_metal_layer_decode`'s + `Lb`/`Rb`/`kvbW`/`kvbS`/`inB`/`pnB`/`rwB`/`rbB`, `coli_metal_gemm`'s `wb`/`sb`) is + **left completely unchanged**, regardless of the flag — see "Why only `moe_submit`" below. +- **Shutdown** (`coli_metal_shutdown`): `[g_queue removeResidencySet:rs]` then clears the + globals, ahead of the existing `g_queue=nil; g_dev=nil;`. + +### Why only `moe_submit` skips `useResource:` + +Apple's own `MTLResidencySet` docs are explicit: *"Residency sets don't support hazard +tracking, so you need to account for hazards with fences and events."* (confirmed against +the actual SDK header comment and Apple's "Simplifying GPU resource management with residency +sets" guide, both read directly for this design — see UNCERTAINTIES for the exact quotes and +how they were obtained). Dropping `useResource:` therefore risks losing whatever +hazard-tracking value those calls provided. Rather than apply the residency set uniformly and +argue *in general* that hazard tracking isn't load-bearing, this diff draws the line at the +one call site the mechanism history actually implicates: + +`moe_submit`'s `use` vector holds only **read-only** (`MTLResourceUsageRead`), **indirectly +referenced** slab buffers — the kernel (`moe_gemv`) never touches them via `setBuffer:`; it +dereferences raw GPU addresses (`waddr[e]`/`saddr[e]`) baked into a separately-bound address +array (`bag`/`bau`/`bad`/`bsg`/`bsu`/`bsd`), which is exactly the "indirect reference" case +`useResource:` exists for. No GPU-side write ever touches these buffers, so there is no +write-after-write/read-after-write hazard for Metal's tracking to have been serializing in +the first place; the one real hazard — a slab unregistered+freed+reused by the CPU while an +async in-flight `moe_block_begin` command buffer still references it via a baked-in GPU +address — is a **CPU-write race that Metal's hazard tracking never protected against anyway** +(hazard tracking only covers GPU-side command dependencies visible through the Metal API; a +raw host-memory write via `pread`/`memcpy` is invisible to it regardless of `useResource:`). +That race is, and always was, the engine's own responsibility (slot/generation lifecycle: a +slab isn't freed while an outstanding async handle still owns it) — unrelated to E5. + +Every other call site (`bind_gemv`, attention K/V cache writes) either doesn't scale with +cache size (fixed per-layer dense tensors — no perf benefit to touching) or has real +GPU-side write traffic in the same encoder (`Lb`/`Rb` are written by `a_copy` and read by +`a_score`/`a_clat` within one encoder — currently ordered by explicit +`memoryBarrierWithScope:MTLBarrierScopeBuffers` calls already present in `encode_attention`, +not by `useResource:`'s hazard tracking, but touching them wasn't needed for the hypothesis +and was judged not worth the added surface area). Leaving them untouched keeps the diff's +blast radius matched to the one seam the fix-plan's v5 finding actually names. + +### Deferred-commit design (`resset_add` batches; `resset_remove` doesn't) + +`coli_metal_register` is called from parallel OpenMP loader threads in tight bursts +("warmup fan-out" — same phrase E4's audit used for the same threads). Committing on every +single `addAllocation:` would reintroduce a per-slab cost on the load path, which is exactly +what E4's own +12s regression looked like (mutex held across a live Metal call, serializing +loader threads). So `resset_add` only marks `g_resset_dirty`; the commit is deferred to the +next `moe_submit` call, which flushes once via `resset_flush()` before it relies on the set +for residency. + +This is correct — not just fast — because of an existing invariant the codebase already +depends on for `resolve()` to work at all: a slab's `coli_metal_register` call (mutex-guarded) +always completes and releases `g_slab_mtx` before any dispatch that references that slab's +pointer can call `resolve()` for it (the caller in `glm.c` cannot pass a freshly-loaded +expert's pointer to a dispatch before the load — which registers it — returns). Since +`resset_flush()` also takes `g_slab_mtx`, and runs immediately before `moe_submit`'s own +`resolve()` calls in program order, any slab a given `moe_submit` invocation will resolve was +already `addAllocation:`-ed (and marked dirty) strictly before that invocation's +`resset_flush()` runs — so the flush is guaranteed to cover it, regardless of what other +threads are concurrently registering unrelated slabs. + +`resset_remove`, by contrast, commits synchronously and immediately, with no batching, +because the caller (`glm.c`, in every one of the four slab-realloc call sites, and in +`kv_alloc`) frees the underlying host memory *right after* `coli_metal_unregister` returns. +An uncommitted-but-still-set-member allocation pointing at memory the host has already freed +is a potential use-after-free the GPU could act on — deferring that removal is not a +performance-vs-safety tradeoff, it's just unsafe, so it isn't deferred. (The spec's own +lifecycle wording backs this reading: "`coli_metal_register` → add allocation + commit +**(batch commits where call pattern allows)**" carries a batching allowance that +"`coli_metal_unregister` → remove + commit" does not.) + +## Instrumentation parity + +No existing counter's semantics changed. `coli_metal_moe_times`/`coli_metal_moe_counts` +(`g_t_setup`, `g_t_gpu`, `g_t_kernel`, `g_t_scatter`, `g_moe_ok`/`g_moe_fb`/`g_moe_experts`) +are computed exactly as before — `resset_flush()` runs *before* `ts_start = mnow()` in +`moe_submit`, so its cost (whatever it is) is **outside** `g_t_setup` and therefore invisible +to the existing setup/gpu/kernel breakdown. This is a deliberate choice: it keeps the +orchestrator's A/B harness reading the same counters with the same meaning across stock/E4/E5, +but it also means the harness's existing numbers will not show E5's flush cost if it turns +out to be non-negligible — see UNCERTAINTIES. No new counters were added (no `glm.c` +`profile_print` changes), unlike E4's `METAL-HEAP: alloc fallbacks` line, because there was no +`glm.c` touch point to hang a print on without adding one — judged not worth the extra diff +surface for an experiment branch; `[METAL] residency-set: on` / the two fallback stderr lines +from `coli_metal_init` are the only new observability, sufficient to confirm which path a run +took. + +## Per-seam differences vs E4 + +| Seam | E4 (`e4/metal-heap`) | E5 (this branch) | +|---|---|---| +| Allocation | New: `MTLHeap` sub-buffers via `coli_metal_heap_alloc` | Unchanged: same `posix_memalign` + `newBufferWithBytesNoCopy` | +| `glm.c` / `backend_metal.h` | Touched (new alloc/free API, 4 call sites + `expert_host_release`) | **Untouched** | +| Residency scope | Declared once **per command buffer** (`useHeap:`, still inside `moe_submit`) | Declared once **for the process** (queue-attached set), refreshed incrementally at register/unregister | +| Hazard tracking | Heap sub-buffers forced `MTLHazardTrackingModeUntracked` always (allocation-level) | Untouched at the resource level; `moe_submit` alone stops calling `useResource:` (encoder-level), independent of `COLI_METAL_UNTRACKED` | +| Per-buffer vs per-set skip | `[b heap]` (Metal's own `MTLResource.heap` property) checked per buffer — heterogeneous mixes possible if a slab fell back to malloc | Blanket `if (!g_resset_enabled)` — homogeneous by construction, since every registered slab goes through the same `coli_metal_register` path when the gate is on | +| Availability guard | None needed (`MTLHeap` is old API) | `@available(macOS 15.0, *)`, matching this box's macOS 26.5 but required for portability | +| Known regression | +12–13s expert-disk load at cap16 (suspected first-touch/lock contention on heap pages) | None expected — malloc pages never change ownership; **unverified without a run** | + +## What to measure (orchestrator, cap1/cap16, stock vs E4 vs E5) + +1. **GPU stall** (`coli_metal_moe_times` gpu/kernel breakdown) — success: E5 ≈ E4's + −85%-class reduction vs stock at cap16. +2. **Expert-disk load path** (existing load/service-time counters) — success: E5 ≈ stock, + i.e. **no** repeat of E4's +12–13s tax, since allocation is untouched. +3. **tok/s** — should track (1) and (2) together. +4. **md5 within a fixed dispatch composition** — flag on vs off must be byte-identical at a + given cap (the "Output-invariant by construction" hard constraint); flag-on vs flag-on + across cap1/cap16 may legitimately differ (different dispatch composition, per the + fix-plan's "Determinism side-finding"). +5. **`[METAL] residency-set: on` line present in stderr** at flag-on startup, and absent + (or the OS<15/create-failed fallback line) otherwise — cheap sanity check that a run + actually exercised the intended path before trusting its numbers. +6. If the hypothesis holds (E5 stall ≈ E4, E5 load-path ≈ stock, identical output), E5 becomes + the upstream PR candidate and must include the cap-default recalibration flagged in PR + #386's CURRENT-STATE CALIBRATION markers, per the spec's validation plan. + +## Build + +`cd c && make glm METAL=1` and a separate explicit `-Wall -Wextra` compile of +`backend_metal.mm` (the Makefile's `METALXX` line does not itself pass `-Wall -Wextra`, so +"clean under `-Wall -Wextra`" was checked with those flags added explicitly), plus +`cd c && make glm` (plain, non-Metal — unaffected, since this diff never touches `glm.c`), and +`make metal-test` (existing synthetic kernel-correctness unit test — no model, no +`glm52_i4/`, random weights — run once with `COLI_METAL_RESSET` unset and once with +`COLI_METAL_RESSET=1` to numerically exercise `coli_metal_register`/`moe_submit`'s changed +code path, since the task scope excludes running the real model). Exact results in the final +report, not here (build results belong to the report per the task's deliverable split, and +this file is written before the batched build run, per the scheduling constraint). + +## UNCERTAINTIES + +**Everything below is a judgment call, a seam where the residency-set lifecycle interacts +with the existing queue/command-buffer structure, or something unverifiable without a real +model run — flagged per the task's hard requirement.** + +1. **The central design risk: skipping `useResource:` in `moe_submit` gives up Metal's + automatic hazard tracking for that buffer set.** Apple's `MTLResidencySet.h` header + (read directly from this box's SDK at + `/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/.../Headers/MTLResidencySet.h`) + documents the protocol only in terms of residency; Apple's own "Simplifying GPU resource + management with residency sets" guide states plainly: *"You don't need to call + `useResource`/`useHeap`... for allocations in a residency set,"* and separately, the + `MTLResidencySet` class reference states *"Residency sets don't support hazard tracking, + so you need to account for hazards with fences and events."* I reasoned through every + code path that touches `moe_submit`'s `use` buffers (read-only, indirectly referenced, + never concurrently written, freed only after the engine's own slot lifecycle guarantees + no outstanding async reference) and concluded removing `useResource:` there specifically + is safe — but this reasoning is **not the same as having run the model**. If any code + path I didn't trace lets a slab get unregistered while an async `moe_block_begin` handle + is still in flight and reading it, this change removes a mitigation (weak as it may have + been) that existed before. **This is the #1 thing to watch for md5 divergence on**, and + the reason the scope was deliberately narrowed to `moe_submit` alone rather than applied + uniformly. +2. **`resset_add`/`resset_remove`/`resset_flush` all run under `g_slab_mtx`, serializing + concurrent OMP loader threads' residency-set mutations against each other and against + dispatch's flush.** `MTLResidencySet`'s own header says *"all methods are non-threadsafe"* + (confirmed directly from the SDK header), so this serialization is required for + correctness, not optional — but it is structurally the same shape as the bug E4's + audit-round-2 found and fixed (mutex held across a live Metal call, serializing loader + threads during warmup fan-out, suspected root cause of E4's +12s regression). Apple's + guide frames `addAllocation:`/`commit` as lightweight bookkeeping relative to the actual + (deferred, async) page-in work ("Metal makes allocations resident when you call `commit()` + on the first command buffer using the set" — implying `commit()` itself doesn't + synchronously page anything in), which is why I judged holding the mutex across these + calls acceptable unlike E4's `newBufferWithLength:` (a real allocation call). **This is + unverified without profiling a loaded run** — if `commit()` or `addAllocation:` turns out + to be synchronously expensive on this hardware/OS build, this could reproduce E4's + expert-disk-load regression through a different code path, defeating E5's entire premise + (decoupling residency from the load path). Orchestrator: this is the single most important + number to check E5's load-path timing against stock, not just against E4. +3. **`resset_flush()`'s cost sits outside `g_t_setup`/the `moe_times` breakdown** (it runs + before `ts_start = mnow()`), by design, to keep the harness's existing counters meaningful + — but this also means if `commit()` is expensive, it will show up as *general* wall-clock + slowdown / tok/s regression without a corresponding line in the existing instrumentation + pointing at it. No new counter was added for it (see "Instrumentation parity" above) to + avoid a `glm.c` touch; if E5 numbers look off without an obvious cause in the existing + breakdown, `resset_flush`/`commit()` cost is the first place to add a throwaway probe. +4. **`initialCapacity = 4096` on the `MTLResidencySetDescriptor` is an unverified guess.** + It's documented as a presize hint only (no correctness effect either way), chosen to be + "clearly larger than the permanent-weight-tensor + KV-cache + plausible cap16 LRU-slab + count" without actually counting those registrations precisely. Too small just means + internal array growth; not a correctness concern, flagged only because it's a number I + picked without measuring. +5. **Not calling `requestResidency()` proactively.** Apple's guide frames it as an optional + latency-hiding call ("call ahead of time during non-critical moments... to minimize [first + command buffer] latency"), and Blender's Cycles PR (the spec's cited reference + implementation) doesn't appear to use it either per its PR description. Omitted to keep + the lifecycle minimal and match the reference pattern; if profiling shows a + first-command-buffer-after-a-load-burst latency spike, this is the documented lever to try + next, not implemented here. +6. **The deferred-commit correctness argument (item in "Deferred-commit design" above) rests + on a single-writer-before-single-reader program-order guarantee that is true today by + inspection but is not an invariant enforced anywhere in code** (no assertion, no type-level + guarantee) — it's the same kind of implicit ordering `resolve()` itself already depends on + for correctness (a slab must be registered before any dispatch can resolve its pointer), + so this diff doesn't introduce a new category of fragility, but it's worth naming + explicitly rather than leaving implicit. +7. **Async `moe_block_begin`/`moe_block_end` overlap with concurrent `register()` calls** + (background loader threads registering new/different experts while an unrelated MoE block + is still in flight on the GPU) was reasoned through but never exercised in a real + concurrent stress scenario — the synthetic `metal-test` unit test's `run_moe` calls are + single-threaded and synchronous (`coli_metal_moe_block`, not the async `_begin`/`_end` + pair), so it does **not** cover this interleaving. The real engine's `PILOT`/prefetch and + `moe_block_begin`/`_end` overlap path is exactly the concurrency shape most likely to + expose a bug in this design if one exists, and is untested here by construction (out of + scope: no model runs). +8. **`coli_metal_gemm` (prefill path) and `bind_gemv` (attention path) still call + `useResource:` unconditionally, so they get no CPU-overhead benefit from the residency set + even though their buffers are also set members.** This is deliberate (see "Why only + `moe_submit` skips" above) but means E5's win, if any, is scoped to the decode-path MoE + dispatch loop specifically — prefill and attention timing should be unaffected by the flag, + which is itself a testable prediction the orchestrator's harness can check. +9. **API surface verified against this box's actual SDK headers** + (`MTLResidencySet.h`, `MTLDevice.h`, `MTLCommandQueue.h`, `MTLAllocation.h`, + `MTLResource.h` — all read directly, not from memory) and against Apple's own + "Simplifying GPU resource management with residency sets" guide, so the method names/ + signatures (`newResidencySetWithDescriptor:error:`, `addResidencySet:`, + `removeResidencySet:`, `addAllocation:`, `removeAllocation:`, `commit`) are + high-confidence. What is **not** independently verified is runtime behavior beyond what + the docs state and what the synthetic unit test exercises — no substitute for the + orchestrator's real cap-sweep battery. diff --git a/c/backend_metal.mm b/c/backend_metal.mm index b57fa66..9c10533 100644 --- a/c/backend_metal.mm +++ b/c/backend_metal.mm @@ -743,6 +743,7 @@ static id moe_submit(int nb, int D, int Iinter, int fmt, const float *xg, const int *xoff, const int *nr, int R, id xg_buf, id gg_buf, id uu_buf, id hh_buf) { if (!g_dev || (fmt != 1 && fmt != 2)) return nil; + resset_flush(); // E5: commit any pending slab adds before we may skip useResource: below double ts_start = mnow(); std::vector ag(nb),au(nb),ad(nb),sgv(nb),suv(nb),sdv(nb); std::vector> use; use.reserve(nb*2); @@ -764,7 +765,18 @@ static id moe_submit(int nb, int D, int Iinter, int fmt, memcpy([xg_buf contents], xg, (size_t)R*D*4); id cb=[g_queue commandBuffer]; id e=[cb computeCommandEncoder]; - for(auto&b:use) [e useResource:b usage:MTLResourceUsageRead]; + // E5 (COLI_METAL_RESSET=1): the queue-attached MTLResidencySet already guarantees these + // buffers are resident, so skip the per-buffer declaration whose count scales with LRU + // cache size (mechanism history v5). Residency sets don't do hazard tracking (Apple docs), + // but none was load-bearing here: every buffer in `use` is MTLResourceUsageRead-only and + // referenced only indirectly (moe_gemv dereferences waddr[]/saddr[] baked into bag/bsg's + // contents), so there's no GPU-side write to serialize against; the one real hazard -- a + // slab unregistered+freed+reused while an async in-flight CB still reads it -- is a + // CPU-write race outside Metal's hazard tracking either way, held by the engine's own slot + // lifecycle, not by useResource:. See SUMMARY.md UNCERTAINTIES. + if (!g_resset_enabled) { + for(auto&b:use) [e useResource:b usage:MTLResourceUsageRead]; + } auto gemv=[&](id wa,id sa,id xin,id y,int O,int K,int Kin){ int NT=R*O; [e setComputePipelineState:g_moe_gemv];