QVAC-24112 fit: budget against real memory availability - #214
QVAC-24112 fit: budget against real memory availability#214simon-iribarren wants to merge 6 commits into
Conversation
Three related fixes to the memory premises behind common_fit_params, from hardware evidence gathered on a 24 GiB Apple M4 Pro (QVAC-24112): - ggml-cpu: report available memory instead of free = total. On macOS this is physical minus wired and compressor pages; on Linux it is MemAvailable. Reporting total made every host-memory fit trivially pass: an 18 GiB model on a 24 GiB machine "fit" regardless of what was running. - ggml-metal: clamp unified-memory free by system-wide availability. currentAllocatedSize is per-process, so a fresh process (e.g. a fit preflight subprocess) saw an idle device no matter how much other processes had wired. Measured: an 11 GiB resident model in another process did not move the fit verdict at all. - common/fit: consult the host row. The nd == 0 arm budgeted against total host memory; it now uses the available figure. For nd >= 1, devices that share physical memory with the host (Apple silicon Metal, integrated GPUs) are folded together with the host row into one combined budget - the per-device rows cannot see that sum. A host-side deficit also forces context reduction and, for pinned parameters, a FAILURE instead of a silent pass. Availability is deliberately physical minus non-evictable (wired + compressor), not free + inactive: the kernel compresses and evicts anonymous and file-backed pages under pressure, and a free+inactive budget rejects loads that demonstrably run. Known limits, unchanged by this patch: the fitter's own projection runs a few hundred MiB high near the boundary, and macOS memory pressure is bistable - a partial offload can pass every budget and still die at decode once the compressor saturates. Projection cannot be denial-grade on this platform; consumers must verify at load (the llm-llamacpp probe decode does).
|
| } | ||
| // A host-side deficit must force a reduction even when the device | ||
| // rows individually have surplus. | ||
| global_surplus -= host_shared_deficit; |
There was a problem hiding this comment.
host_shared_deficit forces entry into step 2 but is absent from sum_used_target, and nothing downstream clamps — so the interpolation can return a context larger than the model was trained for.
Point 3 of the summary comment establishes this and traces it 32768 → 47104. What follows is what happens after that, which is not covered there.
sum_used_target - sum_projected_used is identically global_surplus + host_shared_deficit. Before this PR, global_surplus < 0 implied sum_used_target < sum_projected_used, so :392-393 always produced a genuine reduction. Now global_surplus can be negative purely because of the deficit while sum_used_target still exceeds sum_projected_used — ratio > 1.
Single device, default 1 GiB margin, figures in GiB: device free 18 / projected 14, so projected_free_per_device[0] = 4 and the pre-PR code returned here. Host free 8 / projected 11 → host_free_after = −3 → deficit 4 → global_surplus = −1, step 2 runs. sum_used_target = 17, sum_projected_used = 14, min-ctx 13 → ratio 4 → with hp_nct 131072, n_ctx = 512000.
Nothing downstream catches it. :394 rounds to a 256 multiple but does not clamp to hp_nct. :397 computes (hp_nct - cparams->n_ctx) in uint32_t, which wraps, so the log reports a nonsensical reduction. :400 returns success, and common/common.cpp:1245 discards the status and loads with the mutated cparams. For nd > 1 there is no return at :400, so the inflated context reaches get_memory_for_layers (:561) and poisons every step-3 and step-4 probe.
Suggested fix: subtract the deficit from the reduction target as well as from the entry gate, so both sides of the comparison are measured against the same budget; and clamp separately, since no correct input should ever produce a context above hp_nct.
// :362-369, alongside the per-device margins
sum_used_target -= host_shared_deficit;
// :394, before the rounding
cparams->n_ctx = std::min(cparams->n_ctx, hp_nct);Also compute the :397 subtraction in int64_t.
There was a problem hiding this comment.
Confirmed, including the traced 32768→47104 and the uint32 wrap in the log. Fixed in eb4cf48 exactly along your suggestion: the deficit is subtracted from sum_used_target alongside the margins so both sides of the comparison measure the same budget, the result is clamped to hp_nct before the 256-rounding, and the reduction log is computed in int64. In c998c91 the interpolation moved into common_fit_reduced_n_ctx() (pure, declared in fit.h), and your traced case is a fixture in tests/test-fit-params.cpp asserting the clamp — it fails on the pre-fix arithmetic.
| // Apple silicon: every Metal device is unified with system memory. | ||
| ggml_backend_reg_t reg = ggml_backend_dev_backend_reg(dev); | ||
| const char * reg_name = reg == nullptr ? nullptr : ggml_backend_reg_name(reg); | ||
| if (reg_name != nullptr && strcmp(reg_name, "Metal") == 0) { |
There was a problem hiding this comment.
ggml_dev_shares_host_memory() compares the registry name against "Metal", but the backend registers as "MTL" — so the branch is dead, and the PR's validation cannot have exercised it.
Point 1 of the summary comment reports the mismatch itself. The consequence it does not draw is evidential.
Known limits reports -ngl 48 passing the improved fit by ~630 MiB and still dying at decode, attributed to macOS memory-pressure bistability. A dead comparison is a simpler and fully sufficient explanation — and it means no row of the validation table exercised the combined host+device budget on Metal at all. The one row that does show a real improvement ("11 GiB resident in another process → reduces the budget") comes from the Metal .free clamp, a separate mechanism that does work.
For the record: GGML_METAL_NAME is "MTL" (ggml-metal.cpp:13), returned verbatim at :863; ggml_backend_metal_device_get_type() returns _GPU, never _IGPU (:685), so the first arm at :184 does not rescue it. The device name is "MTL%d" and the description is the MTLDevice's own name, so neither is "Metal" either.
Suggested fix: matching GGML_METAL_NAME works, but the better repair deletes the string match. The backend already computes the answer — ggml-metal-device.m:828 sets has_unified_memory from mtl_device.hasUnifiedMemory — it is simply unreachable from common/, living on the Metal-internal ggml_metal_device_props rather than the public ggml_backend_dev_props. Exposing it there retires the __APPLE__ && __aarch64__ guard, the strcmp and this whole class of bug at once. Either way, add a fixture asserting the helper returns true for a Metal device: the current matrix cannot distinguish "fold works" from "fold is dead".
There was a problem hiding this comment.
Confirmed and fixed in eb4cf48, taking your better repair: the string match is gone entirely. ggml_backend_dev_props gains a public memory_unified flag (memset in ggml_backend_dev_get_props keeps it false for backends that do not set it), Metal fills it from MTLDevice.hasUnifiedMemory via the internal props you pointed at, and the helper reads the flag plus the existing IGPU type arm. The evidential point was right too — the fold was dead during my validation, and the one row that improved came from the Metal .free clamp alone. The fixture you asked for exists twice over now: tests/test-unified-memory-props.cpp asserts the flag is alive on Apple silicon (and that CPU/BLAS do not claim it), and after c998c91 the fold arithmetic itself is a pure function with table-driven tests, so a dead fold fails on any machine. Re-validated with the fold engaged: Gemma ngl=48 @1k — the measured loads-but-cannot-decode false positive — now reports does-not-fit.
| // Budget against what the host can actually still hand out. Using | ||
| // total physical memory here meant a CPU-only fit always succeeded, | ||
| // no matter the model size or what else was running. | ||
| sum_free = dmds_full.back().free; |
There was a problem hiding this comment.
The probe forces LLAMA_LOAD_MODE_NONE, so the host budget charges mmap'd weights as resident against an availability metric that deliberately counts file-backed pages as free.
Point 4 of the summary comment states the contradiction. What it does not give is why it is unconditional, or what the user actually sees.
Unconditional: fit.cpp:57-58 sets mparams_copy.no_alloc = true and mparams_copy.load_mode = LLAMA_LOAD_MODE_NONE on the probe, so mb.model is always the full weight size counted as a buffer — whatever the caller's real load_mode. The caller's default is LLAMA_LOAD_MODE_MMAP (src/llama-model.cpp:2575), under which those bytes are file-backed and evictable. Meanwhile the new availability metric subtracts only wired + compressor pages and deliberately leaves file-backed pages in the pool — which the description argues for at length, correctly. Both sides cannot be right: supply says those pages are available, demand says they must be resident.
What the user sees: not an error. cparams->n_ctx is set to n_ctx_min at :381 before the probe at :382 and left there when the interpolation branch at :390 is not taken; the throw at :424 then fires; and common/common.cpp:1245 calls common_fit_params(...) without assigning its return value at all, so FAILURE is discarded and :1253 loads with the mutated cparams. The result is a context silently clamped to 4096. tools/fit-params/fit-params.cpp:37-40 instead exit(1)s, so the two callers disagree about the same verdict.
This runs unprompted: fit_params = true and n_ctx = 0 are both defaults (common/common.h:474, :449), and CPU-only is the most common deployment.
To be clear about what is not the finding: reducing context when a machine genuinely cannot fit the model is correct and intended. The defect is that demand is inflated by the size of pages supply has already declared available, so the reduction fires far more often, and much harder, than the evidence justifies. The description's own data points the same way — a 10.8 GiB model at 32k ran at 69 tok/s while a stricter budget said no.
Suggested fix: exclude the mmap-backed portion of mb.model from the host budget when the caller's real load_mode == LLAMA_LOAD_MODE_MMAP; the probe still has the caller's mparams before it overwrites them at :57-58, so the information is in hand. Separately, only mutate cparams on a path that succeeds, or restore it before throwing — that decouples this from whatever the caller does with the status.
There was a problem hiding this comment.
Confirmed — this was the strongest finding, since it contradicted the PR's own availability argument. Fixed in eb4cf48: host demand is now measured in the same currency as availability. When the caller's real load_mode == LLAMA_LOAD_MODE_MMAP (checked on the caller's mparams before the probe overwrites them, as you suggested), the host row's mb.model is excluded from the budget — in the nd==0 arm, the min-ctx sample, and the fold. MLOCK modes deliberately stay charged as resident. The silent-clamp half is handled at the wrapper: common_params_fit snapshots mparams/cparams and restores both on FAILURE/ERROR, so common_init_from_params discarding the status now loads with the caller's originals instead of a context mutated mid-reduction — this also un-splits the two callers' behavior you noted. Re-validated: CPU-only Gemma @32k under mmap reports fits (matching the observed 0.1 tok/s decode — feasibility, not performance), and auto-ctx reduces to a sane value instead of clamping to 4096.
| // CPU can pass both individual rows, load, and then fail every decode | ||
| // because the two allocations plus the rest of the system exceed physical | ||
| // memory. Uses the first margin as the host margin. | ||
| int64_t host_shared_deficit = 0; |
There was a problem hiding this comment.
host_shared_deficit reaches only the step-1 gates and global_surplus; steps 2, 3 and 4 still budget per device, and on Android's Vulkan iGPU that double-counts one physical pool today.
Point 4 of the summary comment covers the dead-end path where pinned configs fall through to the pre-existing throws. This is the larger half, and it is not gated behind the "MTL" fix.
The symbol's entire reach is :300 declared, :311 assigned, :313 logged, :318, :324, :349 read. Nothing else. Step 3's ceiling is still device-local:
targets.push_back(dmds_full[id].free - margins[id]); // :612Why this is live now, not after the Metal repair. The first arm of ggml_dev_shares_host_memory() — GGML_BACKEND_DEVICE_TYPE_IGPU at :184 — needs no string match. Vulkan returns _IGPU for any eIntegratedGpu (ggml-vulkan.cpp:20641, assigned :21668), which covers Mali, Adreno, Intel iGPUs and AMD APUs; CUDA does the same for prop.integrated (ggml-cuda.cu:4713-4715). And for an integrated device ggml_backend_vk_get_device_memory sums every heap — if (is_integrated_gpu || (heap.flags & eDeviceLocal)) at :20541 — so dmds_full[id].free is the whole system budget, not a device-local slice.
So on Android the two rows treated as independent, dmds_full[0] and dmds_full.back(), are measurements of the same physical pool. shared_projected is non-zero, the deficit fires, step 1 forces the descent — and steps 2–4 then re-count that pool per device. targets[0] becomes whole-pool − 1 GiB, leaving nothing for the host row's own model, context and compute. Step 1 counts the shared pool once; steps 2–4 go back to counting it twice.
Same applies to the projected-margin reporting at :687, :824, :832, and to the MoE strategy at :583-595, where global_surplus_cpu_moe sums device rows only — so moving experts to "system memory" on a unified device registers as a surplus while freeing no physical memory at all.
There is also a reachable dead end: host_shared_deficit > 0 with global_surplus >= 0 (device free 18 / projected 14, host free 19 / projected 6 → deficit 2, surplus +1). Step 1 refuses to early-return, step 2 does nothing, and control falls to :427/:449, which throw if the user set -ngl or -ot — the exact configuration that produced the deficit. That is the half point 4 already covers.
Suggested fix: thread the shared pool into the step-3/4 targets so the ceiling reflects the pool rather than the device — for shared-memory devices cap targets[id] at host.free - margin_host - host_projected and re-derive as layers move, instead of at dmds_full[id].free. Then close the dead-end branch so a deficit with a nominal surplus does not fall through to the pinned-parameter throws.
There was a problem hiding this comment.
Confirmed, and thank you for flagging that the IGPU arm makes this live on Android today — I had wrongly treated the fold as Metal-gated. Partially fixed in eb4cf48: step-3 targets for shared-memory devices are capped by the pool (host.free − host_margin − resident host demand), which with the mmap exclusion is a reasonable static bound since host-side weights no longer count as resident; and the dead-end branch now throws a message naming the host shortfall when every device row met its own margin (the deficit-with-nominal-surplus shape). What this commit does NOT do, deliberately: re-derive the cap as layers move, extend the pool accounting into the step-2 sums for nd≥1 (a host-only deficit with an auto context fails conservatively instead of reducing host-side KV), or touch the MoE surplus/reporting paths you listed. Those need the per-assignment threading you sketched, and I would rather land that as its own change with Android Vulkan validation than fold it in here untested — both limits are named in the commit message and I can open the follow-up issue if you agree with the split.
| } | ||
|
|
||
| if (nd == 1) { | ||
| if (projected_free_per_device[0] >= margins[0] && host_shared_deficit == 0) { |
There was a problem hiding this comment.
Adding host_shared_deficit to the early-return conditions lets host pressure alone push execution into step 2, where two unguarded integer divisions live at :393 and :376.
Point 2 of the summary comment reports the :393 site, and correctly splits the outcome by architecture. Two things it does not cover: the second denominator, and why the enabling change is here rather than there.
Both denominators are unguarded:
:393—/ (sum_projected_used - sum_projected_used_min_ctx), zero whenever device-side memory does not vary with context size.:376—sum_projected_model / std::min(uint32_t(mparams->n_gpu_layers), hp_ngl), zero whenn_gpu_layers == 0andnd > 1.
The enabling change is this line. Pre-PR, step 2 was reached only after a per-device early return failed, which needed the device to be nearly full — and with -ngl 0 device usage is ~0, so projected_free_per_device[id] was large and the early return fired. With && host_shared_deficit == 0 here (and changes_needed = host_shared_deficit > 0 at :324), host pressure alone now forces the descent.
And on that configuration the zero denominator is not a corner case, it is the only case: nd >= 1 with n_gpu_layers == 0 is ordinary — model->devices is populated independently of n_gpu_layers — and in that state the KV cache is entirely host-side, so the device rows are context-independent and sum_projected_used == sum_projected_used_min_ctx exactly.
Confirming the architecture split, since it decides the impact: AArch64 sdiv/udiv return 0 for a zero divisor and do not trap, so on Apple silicon, Android and arm64 servers there is no crash — cparams->n_ctx stays at n_ctx_min and :400 returns "entire model can be fit by reducing context", a context clamped to 4096 on precisely the configuration where reducing context frees nothing. On x86-64, idiv raises #DE → SIGFPE; the handler at :852-861 catches common_params_fit_exception and std::runtime_error, and a division fault bypasses both, so a preflight subprocess dies uncatchably. Both sites are reached before the n_gpu_layers already set by user throw at :427, so that check shields neither.
Suggested fix: guard both denominators and clamp the std::min(...) to at least 1, so a context-independent device row yields "no reduction possible" instead of a trap or a zero. Independently, add a catch (const std::exception &) fallback in common_fit_params — a preflight gate should never be able to take the process down.
There was a problem hiding this comment.
Confirmed on both counts — I had only seen the :393 site. Both denominators are guarded in eb4cf48: the interpolation computes used_delta once and treats <= 0 as "no reduction possible" (your -ngl 0 analysis is right that it is the only case there, not a corner), and the per-layer estimate clamps the divisor to at least 1. common_params_fit also gains a catch (const std::exception &) → ERROR fallback. The zero-delta case is a table fixture in tests/test-fit-params.cpp after c998c91. To be explicit about the residual: a SIGFPE is not an exception, so the guards are what remove the fault — the catch-all covers other escapes, not division.
| if (nd > 0) { | ||
| int64_t shared_projected = 0; | ||
| for (size_t id = 0; id < nd; id++) { | ||
| if (ggml_dev_shares_host_memory(devs[id])) { |
There was a problem hiding this comment.
common_params_fit and common_get_device_memory_data have no tests anywhere in the tree, and the new registry call adds a second live dependency to the part that holds the bugs.
There is no fit target in tests/CMakeLists.txt, no test file referencing common_fit_params or fit.h, and tools/fit-params/ is a manual CLI. The only consumers are tools/fit-params/fit-params.cpp:34, tools/llama-bench/llama-bench.cpp:2302 and common/common.cpp:1245.
That matters here specifically because the defects in this PR are pure arithmetic in common_params_fit_impl and provable from the algebra with no hardware at all — a table-driven test over (dmds, margins, hp_nct, n_ctx_min) would have caught the context-inflation bug on its first run. The description is candid that coverage is manual and single-machine, but locates the gap in the inputs; the untested part that actually contains the defects is the decision arithmetic.
The PR also moves the wrong way on testability. common_params_fit_impl already obtained its inputs by loading a real model; this line adds a backend-registry query on the decision path, so the step-1 gate now cannot be exercised without a live Metal, CUDA or Vulkan device.
Suggested fix: extract the decision logic into a pure function over (dmds_t, std::vector<bool> shares_host, margins, hp_nct, n_ctx_min) — taking a shares_host vector instead of calling the registry — and add a test-fit-params ctest. Passing the vector is what makes the gate testable on any machine, and it makes the "MTL" bug directly assertable rather than hardware-gated. Fixtures worth having: nd == 0 deficit; nd == 1 device-surplus with host-deficit, asserting n_ctx <= hp_nct; nd == 1 discrete GPU; nd == 2 mixed shared/discrete.
There was a problem hiding this comment.
Agreed, and done in c998c91 along your suggested lines: the two decision sites — the fold and the interpolation — are extracted as pure functions (common_fit_shared_pool_deficit, common_fit_reduced_n_ctx, declared in fit.h for tests), the registry query is hoisted to a single touchpoint filling a shares_host vector so the arithmetic takes flags rather than live devices, and tests/test-fit-params.cpp is a table-driven ctest covering your four fixtures: nd==0 deficit, device-surplus/host-deficit asserting n_ctx <= hp_nct (the traced inflation shape), nd==1 discrete non-folding, and nd==2 mixed shared/discrete — plus the zero-delta division case. Runs on any machine, no devices. Where I stopped short of the full extraction: steps 3–4 still probe through live loads, so the pure surface is the step-1/2 arithmetic where all five defects lived; going further means restructuring upstream-derived code that this fork rebases across fabric bumps, and I wanted the delta to stay rebaseable. The "MTL" class of bug is covered from the other side by tests/test-unified-memory-props.cpp, which asserts the flag is actually set by the backend.
Addresses all five review findings on the previous commit (qvac-fabric-llm.cpp#214): - The fold was dead code: ggml_dev_shares_host_memory matched registry name "Metal" but the backend registers as "MTL". Replace the string match with a public ggml_backend_dev_props.memory_unified flag set by the backend itself (Metal from MTLDevice.hasUnifiedMemory; memset in ggml_backend_dev_get_props keeps it false elsewhere; Vulkan iGPUs are covered by the existing IGPU type arm). tests/test-unified-memory-props.cpp asserts the flag is alive on Apple silicon, so a dead fold can no longer validate. - Guard both step-2 divisions the host-forced descent made reachable: the context interpolation delta is exactly 0 when device rows are context-independent (n_gpu_layers == 0), and the per-layer estimate divided by n_gpu_layers directly. Faults on x86-64, silent zeros on AArch64. - Measure both sides of the context interpolation against the same budget: the deficit now also reduces sum_used_target, the result is clamped to the training context, and the reduction log is computed in int64 (it could interpolate 32768 -> 47104 and report a wrapped "reduction"). - Charge host demand in the same currency as host availability: the probe loads with LLAMA_LOAD_MODE_NONE, but under the caller's real LLAMA_LOAD_MODE_MMAP the weight pages are file-backed and evictable - exactly the pages the availability metric leaves in the pool. The mmap'd model portion is now excluded from the host budget (nd == 0 and the fold). - Contain the blast radius: step-3 targets for shared-memory devices are capped by the pool (host free - host margin - resident host demand) instead of the device row alone; a host-only deficit with pinned n_gpu_layers now throws a message naming the host shortfall instead of blaming the pin; and common_fit_params snapshots and restores mparams/cparams on FAILURE/ERROR, since common_init_from_params ignores the status and would otherwise load with a context silently clamped mid-reduction. A catch-all maps unexpected exceptions to ERROR. Known residual limits, documented for follow-up: for nd >= 1 the step-2 reduction sums remain device-side (a host-only deficit with an auto context fails conservatively instead of reducing the host-side KV), and the step-3 pool cap is static across the descent rather than re-derived per assignment. Re-validated on a 24 GiB M4 Pro with the fold provably engaged: the Gemma 4 31B ngl=48 false positive (loads, cannot decode) now reports does-not-fit; Gemma ngl=99 stays does-not-fit; gpt-oss-20B @32k and Qwen3.5 9B @131k stay fits; CPU-only mmap fits match observed behavior with no silent context clamping; the previous SIGFPE path returns a structured FAILURE with the host-shortfall message.
The step-1 shared-pool fold and the step-2 context interpolation are pure arithmetic, and every defect found in review lived in them. Extract them as common_fit_shared_pool_deficit and common_fit_reduced_n_ctx (declared in fit.h for tests), hoist the backend-registry query to a single touchpoint that fills a shares_host vector, and add a table-driven ctest (tests/test-fit-params.cpp) covering: discrete-GPU non-folding, the measured device-surplus/combined-deficit shape, mixed shared/discrete devices, the traced context-inflation case (asserting the clamp to the training context), the zero-delta division case, and the interior interpolation. The fixtures run on any machine with no live devices, which is what makes the previous dead-fold class of bug directly assertable.
|
@amangupta-tether @iancris — all findings confirmed and addressed in two commits; every inline thread has a detailed reply. Summary against your four points:
Plus the testability finding: the decision arithmetic is extracted into two pure functions with a table-driven ctest ( Declared residual limits (in the commit message and thread replies): for Full fixture matrix re-run on the 24 GiB M4 Pro after the fixes: Gemma ngl=99 stays does-not-fit, ngl=48 flips to does-not-fit (matches the measured decode failure), gpt-oss @32k and Qwen9B @131k stay fits, CPU-only mmap verdicts match observed behavior, and the old SIGFPE path returns a structured FAILURE. |
…llm.cpp into fix/fit-host-memory-budget # Conflicts: # tests/CMakeLists.txt
test-unified-memory-props asserted that Apple silicon must enumerate a Metal device, but ci/run.sh configures -DGGML_METAL=OFF unless GG_BUILD_METAL is set. The gpu-vulkan-apple and gpu-webgpu-apple jobs run on Apple silicon with Metal off, so both failed on "no Metal device enumerated" while the build never had the backend. Define LLAMA_TEST_EXPECT_METAL from tests/CMakeLists.txt only when GGML_METAL is ON and require it in the #if. The assertion still runs where it carries its regression value (gpu-metal, macos-latest-arm64), so a dead fold still cannot validate. Verified on M-series: GGML_METAL=OFF enumerates BLAS+CPU and passes; GGML_METAL=ON enumerates MTL0 with memory_unified=1 and passes. Assisted-by: Claude Opus 5
| // The device's "free" is the same physical pool the host row | ||
| // draws from; leave room for the host's own demand and margin or | ||
| // step 3 fills the pool and starves the host. | ||
| const int64_t pool_target = dmds_full.back().free - margins[0] - host_resident_full; |
There was a problem hiding this comment.
Minor: shared-pool budget isn't partitioned across multiple shares_host devices
common/fit.cpp:710
if (shares_host[id]) {
const int64_t pool_target = dmds_full.back().free - margins[0] - host_resident_full;
target = std::min(target, pool_target);
}pool_target is computed the same way for every device where shares_host[id] is true, and each one is capped against the full pool_target independently rather than a running/shared budget. That's correct for the single-shared-device case this PR validates (Metal-only on Apple Silicon), but if a run ever has two or more shares_host devices at once — e.g. a build with both GGML_METAL and GGML_VULKAN enabled on Apple Silicon, where Vulkan-via-MoltenVK enumerates the same GPU as an IGPU device — each device could independently fill up to pool_target, so the combined assignment could exceed the actual pool.
Not something this project's CI currently builds or tests (the gpu-vulkan-apple job runs with GGML_METAL=OFF), so this isn't blocking. But common_fit_shared_pool_deficit already sums correctly across all shares_host devices for the step-1/2 gate — might be worth reusing that here too, or at least leaving a comment/TODO noting the assumption, since tests/test-fit-params.cpp doesn't cover the multi-shared-device case.
The per-device target for a shares_host device capped against the full pool budget, computed identically for every such device. With one shared device (Metal on Apple silicon, the case this PR validates) that is correct, but two or more could each fill the whole budget and together overrun the pool. Extract common_fit_shared_pool_target and divide the budget by the number of shares_host devices. A negative budget is returned whole, since dividing it would understate the shortfall the descent has to reduce against. At one shared device the arithmetic is unchanged, so the validated Apple path keeps its measured behaviour. Cover it in test-fit-params, which had no multi-shared-device case. Assisted-by: Claude Opus 5
Scaffolding, not a change to land. REVERT THIS COMMIT BEFORE MERGING. Adds a vcpkg overlay port pinning qvac-fabric to the head of tetherto/qvac-fabric-llm.cpp#214 ("QVAC-24112 fit: budget against real memory availability", base temp-10297) at 1d14f65, so this PR's split-mode 'tensor' work is compiled and unit-tested against that fit change before either lands. Relevant because tensor mode disables auto-fit, and #214 rewrites how fit budgets device memory. Follows the /rollout-phase-a overlay pattern with one deliberate narrowing: the overlay-ports key is added to packages/llm-llamacpp only, not to all 7 fabric consumers, because only this package needs validating here. The other six keep resolving qvac-fabric from the registry. - vcpkg-overlays/ports/qvac-fabric/{portfile.cmake,vcpkg.json} copied verbatim from the registry port, with REF v${VERSION} replaced by the literal commit (no tag exists for a PR head) and the matching SHA512. - Overlay version is 10297.214.0 rather than the registry's 10297.0.0 so the vcpkg install log distinguishes the two. A silently-wrong overlay path does not error — it falls back to the registry and validates the OLD fabric — so the distinct version is what makes the check honest. default-registry.baseline is deliberately NOT bumped, and the consumer's `version>=` is left at 10297.0.0; the overlay bypasses version resolution and 10297.214.0 satisfies the existing pin either way. Verified locally on macOS arm64: vcpkg reports qvac-fabric[core,gpu-backends,llama]:arm64-osx@10297.214.0 built from source, and addon-test passes 213/213 against it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
4 participants
Problem
QVAC-24112. Hardware evidence from the QVAC-22629 advisory-fit work (24 GiB Apple M4 Pro, full write-up in the tether workspace under
tasks/task-qvac-9515-sdk-device-resource-monitoring/qvac-22629-local-hardware-evidence.md) showedcommon_fit_paramsverdicts wrong in both directions, traced to its memory premises:ggml_backend_cpu_device_get_memoryreports*free = *totalon non-Windows, and thend == 0arm budgets against.totalanyway. An 18 GiB model on a 24 GiB machine "fits" on CPU no matter what is running.nd ≥ 1.common_get_device_memory_datacomputes it, the decision loop ignores it. A partial offload (48/61 layers on GPU) passed while its host+device sum exceeded what the machine could serve — it loads, then every decode fails.recommendedMaxWorkingSetSize − currentAllocatedSizesees nothing other processes have wired. Measured: an 11 GiB model resident in another process did not move the verdict at all — a fit/preflight subprocess always sees an idle device.Fix
MemAvailable; Windows already usedullAvailPhys.hasUnifiedMemorydevices, clamp free by the same system-wide availability, so cross-process wiring finally registers.nd == 0budgets against.free; fornd ≥ 1, devices sharing physical memory with the host (Apple silicon Metal,GGML_BACKEND_DEVICE_TYPE_IGPU) are folded with the host row into one combined budget. A host-side deficit forces context reduction, and for pinned parameters surfaces as FAILURE through the existing throws.Availability is deliberately physical − non-evictable (wired + compressor), not free+inactive: the kernel compresses/evicts anonymous and file-backed pages under pressure, and a free+inactive budget rejects loads that demonstrably run (verified: a 10.8 GiB model at 32k ran at 69 tok/s while free+inactive said no).
Validation (M4 Pro 24 GiB,
llama-fit-params, margin 1024)Re-run after the review fixes, with the fold provably engaged
(
tests/test-unified-memory-props.cppasserts Metal reportsmemory_unified;the first version of this table ran against a dead fold — see review).
-ngl 99 -c 1024-ngl 48 -c 1024-ngl 99 -c 32768-c 131072-c 32768(mmap)-ngl 0with a device presentPlus two device-free ctests:
test-fit-params(table-driven decisionarithmetic — the traced context-inflation case fails against the pre-fix
algebra) and
test-unified-memory-props.Known limits (declared, follow-up material)
nd ≥ 1the step-2 reduction sums remain device-side: a host-onlydeficit with an auto context fails conservatively instead of reducing the
host-side KV.
rather than re-derived per assignment; the MoE surplus and margin reporting
paths still sum device rows only.
(gpt-oss @128k f32 KV: real 17.7 GiB, projected past the margin) — accuracy,
not budget premise.
never be denial-grade on this platform; the companion llm-llamacpp probe
decode (QVAC-24114, QVAC-24114 fix: fail the load when the model cannot decode qvac#4039) verifies at load.
Testing
cmake --build build --target llama-fit-paramsclean on macOS arm64 (Metal).MemAvailablepath built but not exercised on real Linux; Windows path untouched.