From 0377cde70c5f1ef1ef49185f851a239cef26cd30 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Wed, 19 Aug 2026 07:07:23 +0000 Subject: [PATCH 1/3] spec(LTX25-TEXT-LINEAR-MEM): attribute #1286 before touching the seam it accuses #1286 reports that #1252's threaded caption projection raises peak host memory from ~79 GiB to 105.85 GiB and aborts the full-model LTX-2.5 render on GB10. It names three candidate causes and says plainly that the attribution is not measured. This spec takes the row and commits the attribution plan before any code, because the three causes have different fixes and different blast radii: one is a shared-seam defect affecting every GEMM caller, one is an LTX call site, and one is neither. The baseline is read out of the tree rather than assumed. `vt::MatmulBT`'s CPU path allocates exactly one thing per call, `cpu_ops.cpp:125`'s `static thread_local` widened-activation tile, sized by ggml's 16-row `blck_1` and not by the chunk's row span; the call site's full-size `scaled` copy is present on BOTH arms and so cannot explain a regression; and no test anywhere in this tree asserts a peak-RSS or allocation bound, so the reported defect would have been invisible to every gate had it been real. The row therefore carries a fourth item that is worth having whatever the attribution says: a memory bound for the shared GEMM seam, red by mutating the tile to the shape #1286 hypothesised. Refs #1286 FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [claude-code] --- .agents/issue-index.md | 1 + .agents/specs/ltx25-text-linear-mem.md | 186 +++++++++++++++++++++++++ 2 files changed, 187 insertions(+) create mode 100644 .agents/specs/ltx25-text-linear-mem.md diff --git a/.agents/issue-index.md b/.agents/issue-index.md index 5eebbb880..7f97653b0 100644 --- a/.agents/issue-index.md +++ b/.agents/issue-index.md @@ -418,3 +418,4 @@ rather than merged. `scripts/check-agent-record.py` gates both. | [#1303](https://github.com/mudler/vllm.cpp/issues/1303) | `ENG-CUDAGRAPH-DEDUP` | **The decode graphs are TWO topologies, not one: 376 and 404 nodes, mixed across the shipped padded bucket set.** Found by the `ENG-CUDAGRAPH-DEDUP` W6 device-byte run (`origin/main` `2c8f53d93`, GB10, driver `580.173.02`, nvcc 13.0.88, `rc` job `93f783de`, 2026-08-19) with a `cudaGraphGetNodes` shim on every `cudaGraphInstantiate`: `w32_off_a` captured `404 404 376 376 404 404 404` and `w64_coarse_a` saw `376 376 376 376 376 376 404 404 376 …` over 11 buckets. **The measured consequence:** with the coarse key the driver refuses **43% of probes at 7 buckets and 73% at 11**, and EVERY refusal in the run gave one reason — `probe refused a fold (err=910 result=2)`, `cudaErrorGraphExecUpdateFailure` / `cudaGraphExecUpdateErrorTopologyChanged`. Not one was about a parameter. **This INVERTS the premise of [#1226](https://github.com/mudler/vllm.cpp/issues/1226) (now closed):** `cudaGraphExecUpdate` permits parameter changes and requires topology to match, so dropping the launch dimensions from the key was expected to fold more; it produces more FALSE CANDIDATES instead, because the key cannot see the difference the driver rejects on. W5's `refused=0` is explained as an artefact of workloads whose buckets only ever SHRANK, so exactly one pair was ever presented. **Why it matters beyond the key:** at 11 buckets the coarse arm ran 27 instantiate calls against OFF's 11 (2.45x) plus 28 update calls, to retain 6 fewer executables for no measurable byte saving; if the split has a removable cause, every refusal disappears and the fold ratio becomes the whole bucket set rather than 3/7 and 5/11, which is the only route by which the default flip could become supportable on this model class. First suspect, NOT verified: the FA-2 split-KV grid. Owed: attribute the split to a node or subgraph by DUMPING and diffing the two node sets (`cudaGraphNodeGetType`, `cudaGraphDebugDotPrint`) rather than reading source; say whether a capture that fixes the node set exists and what it costs; re-run the W6 A/B if it does. Evidence `/mnt/nas_share/rc/dedup-bytes/` (`RESULT.md`, `logs-bytes/bytes.log` `### PER-INSTANTIATE BYTE DISTRIBUTION`, `mem/mem_.csv` column 8); recorded in [`benchmark-record.md`](benchmark-record.md) entry `ENG-CUDAGRAPH-DEDUP W6` and [`eng-cudagraph-dedup.md`](specs/eng-cudagraph-dedup.md) `## Outcome` (W6) and `## Owed` | perf | | [#1291](https://github.com/mudler/vllm.cpp/issues/1291) | `ENG-CUDAGRAPH-BREAK` | W3 of the break-point capture seam: the three remaining PLAIN BATCHED decode drivers migrate onto `vt::GraphCaptureScope` + `vt::BreakableGraph` — `Qwen3MoeDecodeGraph` (`qwen3_moe.cpp`), `VoxtralDecodeGraph` (`voxtral.cpp`) and `DeepseekV2DecodeGraph` (`deepseek_v2.cpp`) — one commit each, each landable alone, each with its own RED-first G2 gate. Four of the nine drivers are now on the seam and the six batched-driver `VLLM_CPP_CUDAGRAPH` reads the spec's `## Our baseline` item 1 counted are down to TWO, both in `qwen3_5.cpp` (W4). The two per-model rollback switches STAY (`VT_QWEN3MOE_CUDAGRAPH`, `VT_DEEPSEEK_CUDAGRAPH`): each is an A/B lever for exactly one driver, not a copy of the shared one. **Each driver owes its own gate because nothing else can see the difference** — a driver that kept its hand-rolled `BeginCapture`/`EndCaptureGraph` pair produces IDENTICAL logits, an IDENTICAL backend log and an identical `replay_count()`, so `segments_captured` and `replays` are the only observables that separate "captured a graph" from "captured a graph THROUGH THE SEAM". Red-first on four assertions each (`test_qwen3_moe_decode_graph_seam` 222/226, `test_voxtral_decode_graph_seam` 224/228, `test_deepseek_v2_decode_graph_seam` 224/228, all exit 1), 3/3 green after; the G2 mutation restoring each pre-W3 driver file (25/102, 23/92 and 25/94 lines, each compiled clean, each restored and verified by sha256) reddens ONLY its own gate and leaves `test_breakable_graph` 216/216 and W2's `test_qwen3_decode_graph_seam` 231/231 GREEN. The two swapped seams every such gate needs now live ONCE, in `tests/vllm/models/decode_graph_seam_harness.h`, with W2's file including it: three more copies inside `tests/` would have reproduced the duplication this row removes from `src/`, and two copies of a harness diverge invisibly because both files stay green while measuring different things. **G1 IS DELIVERED, retiring the item W1 and W2 both carried as owed.** `tests/vllm/models/test_decode_graph_seam_g1_cuda.cpp` drives each migrated driver COLD, CAPTURE and THREE consecutive replays and compares every step bit for bit against the driver's own EAGER arm, which is selected by `max_num_reqs == 0` (`PadToCaptureSize` returns -1 and `Step` falls out to its plain forward) so both arms are ONE binary on ONE device rather than two builds, each with its OWN device KV cache so neither can read the other's writes. Measured on `thor:gpu0` through an `rc` lease — NVIDIA Thor sm_110, driver 595.78, nvcc 13.0.88, source `c905bb536`, CUDA-ON build for arch 110, 32 `.cu.o` objects, binary resolving `libcudart.so.13` and `libcublasLt.so.13`, build 489 s at `-j4`: **3 cases, 1600 assertions, exit 0**, `5 steps x 100 logits, 0 differing, 4 replays` for each driver. The ASSERTION COUNT carries that claim and the status line does not: with no CUDA backend every case skips and the same file prints `Status: SUCCESS!` over `assertions: 0`, which the file's own header names as a skip wearing a pass. Bounded honestly: the models are the synthetic tiny ones the CPU forward gates use rather than a checkpoint, and W2's `Qwen3DenseDecodeGraph` is covered by shared-seam ARGUMENT and not by one of the three cases. **W3 ALSO FOUND A GATE THAT COULD NOT FAIL, and closing it is part of the stage.** The three gates assert `breaks_registered == 0` to hold the capture to vLLM's decode arm (`FULL_AND_PIECEWISE`, `vllm/config/compilation.py:63,65-66,630-632` @ `5559679229`). That is a TAUTOLOGY for these models: the counter moves only when a `vt::GraphBreak` registers into a splitting scope, and the ONE production break point in the tree is W1's, in `qwen3.cpp` — W2's driver runs through it, none of W3's three does. Measured rather than reasoned: flipping `kFull` to `kPiecewise` in `qwen3_moe.cpp`, one token, compiled clean and left that driver's whole gate GREEN at 226/226, and the same flip was equally invisible in the other two. The mode was UNOBSERVABLE from outside a driver — the scope is a `Step` local, the container is private to the driver's `Impl`, and a token gate cannot see a segment count — so `vt::GraphBreakStats` gains `full_scopes` and `piecewise_scopes`, counted in `GraphCaptureScope`'s constructor on the ACTIVE path only (an inert scope makes no backend call in either mode, so counting it would report a mode that never reached a backend), cleared by `ResetGraphBreakStats()`, gated in `tests/vt/test_breakable_graph.cpp` with the inert arm as the control that stops them degenerating into "scopes constructed". Re-measured after the fix, the same one-token flip now REDS each of the three gates on exactly those two assertions (226/228, 228/230, 228/230, all exit 1). **NO break point is registered in these three models, and that is a decision rather than an omission**: under `kFull` a break point takes the same pass-through arm it takes outside a scope, so registering one would land machinery no gate can exercise; establishing each model's break-point set is what the PIECEWISE arm needs, that arm is blocked on replay-safe closure inputs (W4), and W6 is where the eligibility predicate moves. **The async device-token decline, stated per driver rather than left as an omission.** `VoxtralDecodeGraph` needs none: its only construction site is `VoxtralGenerateGreedy`, the single-sequence multimodal greedy loop, which is not reached from `runner.cpp` and has no asynchronous device mirror. The other two are a NEW FINDING, filed [#1305](https://github.com/mudler/vllm.cpp/issues/1305) with W4 as owner and NOT fixed in flow, because adding a decline trades a shipped default-ON capability away on a measurement W3 cannot make and the fix `qwen3.cpp:1106`'s own comment names is `StepDevInputs` as a seam capability. **G5's ROCm/Tenstorrent arm is NOT discharged**, and the owner moves from W3 to W5 with the reason stated: the fleet carries no ROCm and no Tenstorrent device, so it is BLOCKED on hardware rather than unattempted. What W3 can say instead is that the seam's CUDA arm now runs on TWO architectures, sm_110 here and sm_121a for W1's exit criterion. Still NO throughput claim anywhere: the prefill refutation stands (3.8% host idle, >96% GPU-busy, 92.5% glue) and `kFull` keeps each migrated step's shape the one it already had | feature | | [#1305](https://github.com/mudler/vllm.cpp/issues/1305) | `ENG-CUDAGRAPH-BREAK` | Three decode-graph registrations route an ASYNCHRONOUS step into a host-vector replay with NO `device_token_ids` decline. Found while migrating two of them onto the break-point capture seam ([#1291](https://github.com/mudler/vllm.cpp/issues/1291)) and NOT caused by that migration — the shape is present at its base commit `5d9fe332c`. `src/vllm/model_executor/models/qwen3.cpp:1106` returns `std::nullopt` whenever `ModelForwardInput::device_token_ids` is non-null, on a measured four-concurrent battery (`depth-1 graph ON PASS 78/78`, `depth-2 graph OFF PASS 82/82`, `depth-2 graph ON FAIL, slots 1-3 degenerate`, #323/#1179), because `Step()` replays against persistent HOST `token_ids` while the asynchronous combine has patched the DEVICE ids and deliberately left the host vector stale for decode rows (`src/vllm/v1/worker/gpu/runner.cpp:1265-1309,1523`). That comment calls the hazard "latent for EVERY classic-dense model, since the graph is default-ON". `qwen3_moe_registry.cpp:107`, `deepseek_v2_registry.cpp:106` and `glm4_moe_lite_registry.cpp:125` (the last constructs the DeepSeek driver) admit a pure-decode step to a driver with the same host-vector replay and carry NO such check; `grep -c StepDevInputs` returns 0 for `qwen3_moe.cpp`, `deepseek_v2.cpp` and `voxtral.cpp` against 41 for `qwen3_5.cpp`, which is the divergence the row's spec already records as the cost of nine drivers. `VoxtralDecodeGraph` is NOT affected: its only construction site is `VoxtralGenerateGreedy`, which the runner does not reach and which has no device mirror. NOT fixed in flow, for two reasons stated rather than assumed: adding a decline is a behaviour change that trades a shipped, default-ON capability away, and W3 has no four-concurrent battery for these two models to justify it; and the correct fix is already assigned — `StepDevInputs` (`qwen3_5.cpp:3894`) as a SEAM capability, which is `## Work breakdown` W4 of [eng-cudagraph-break.md](specs/eng-cudagraph-break.md) and the only version that reaches these registrations at all. Owner: **W4**, with [#1179](https://github.com/mudler/vllm.cpp/issues/1179) as the standing tracker. What would settle it: run that same battery against Qwen3-Coder and DeepSeek-V2-Lite with `VT_ASYNC_DEVICE_MIRROR` live and the decode graph ON — either it reproduces the depth-2 degeneration, or the reason it does not has to be found and written down rather than assumed | bug | +| [#1286](https://github.com/mudler/vllm.cpp/issues/1286) | `LTX25-TEXT-LINEAR-MEM` | #1252's threaded caption projection was reported to raise peak host memory ~79 -> 105.85 GiB and abort the full-model LTX-2.5 render on GB10. **The +26 GiB is the box's STARTING OCCUPANCY, not the change.** Both runs are retained under `/mnt/nas_share/rc/ltx25-fullmodel/out/`, and `runguard.py:236-237,260` fixes the compared column `used_gib` as the SYSTEM-WIDE `MemTotal - MemAvailable`: the pre-#1252 run began at `used = 4.741 GiB` on an idle box, the #1252 run began at `used = 31.553 GiB`, a difference of **26.812 GiB** against a claimed regression of **26.647 GiB**. Each run's own demand — peak minus its own t=0 — is **74.465 GiB before and 74.300 GiB after**, and both peaks were sampled inside a ~1900% CPU stretch, so they are the same phase class rather than two different ones. Two further columns agree independently: system `AnonPages` delta 38.012 vs 38.217 GiB, and child `VmRSS` at each peak sample 41.952 vs 42.090 GiB. Confirmed locally by an A/B of the two `Linear` arms at the shipped `1024 x 188160 x 4096` geometry swept over threadpool width; the seam's only per-call allocation is `cpu_ops.cpp:125`'s `static thread_local` widened-activation tile, `16 x K x 4` per worker = 12.04 MB at `K = 188160`. Owner `LTX25-TEXT-LINEAR-MEM`, spec [`ltx25-text-linear-mem.md`](specs/ltx25-text-linear-mem.md) | verification | diff --git a/.agents/specs/ltx25-text-linear-mem.md b/.agents/specs/ltx25-text-linear-mem.md new file mode 100644 index 000000000..67b4d97a1 --- /dev/null +++ b/.agents/specs/ltx25-text-linear-mem.md @@ -0,0 +1,186 @@ +# `LTX25-TEXT-LINEAR-MEM` — #1252 did not cost 26 GiB, and the seam has no memory bound + +Row: `LTX25-TEXT-LINEAR-MEM` +Issue: [#1286](https://github.com/mudler/vllm.cpp/issues/1286) +Sibling rows: `LTX25-TEXT-LINEAR-SEAM` (#1252, [spec](ltx25-text-linear-seam.md)), +`LTX25-LORA-FUSE-SEAM` (#1259, [spec](ltx25-lora-fuse-seam.md)) + +## Scope + +[#1286](https://github.com/mudler/vllm.cpp/issues/1286) reports that #1252's +threaded caption projection raises peak host memory from ~79 GiB to 105.85 GiB +on GB10 and aborts a full-model LTX-2.5 render before any denoise step. It names +three candidate causes — the shared threadpool's per-thread tiles, the +`ltx2_text_encoder.cpp` call site's full-size `scaled` copy, or an interaction — +and states plainly that the attribution is not measured and belongs to whoever +takes the row. + +This row does that attribution first, and lets the answer choose the fix. + +In scope: + +- attribute the reported +26 GiB against the retained evidence and against a + local before/after measurement at the shipped geometry; +- report peak host memory **and** throughput for both arms, because a memory fix + that costs the 8.57x is a bad trade; +- decide whether a code change is warranted, and make it if so; +- give the shared GEMM seam a **memory bound that a gate can hold**, since the + tree has none and could not have caught the reported defect had it been real; +- check whether #1259, the sibling seam change that has never run on the full + model, shares the profile. + +Out of scope: the LTX-2.5 speed axis, the unmeasured GB10 per-core ratio `R` +that #1252 carries under `## Owed`, #1254's bf16 add-back, and #1210's +fuse/un-fuse round trip. None becomes reachable from this change. + +## Upstream chain + +None, and that is a property of the question rather than a gap. Peak resident +footprint is a property of this port's execution strategy: vLLM does not +implement LTX-2.5, vLLM-Omni's recipes stop at 2.3, and the Lightricks +`ltx_core` oracle runs the projection as one `torch.nn.functional.linear` whose +allocator behaviour says nothing about a C++ threadpool's per-worker tiles. The +arithmetic mirror is already decided and unchanged by this row: f32 accumulation +through `vt::MatmulBT`, argued in `ltx2_text_encoder.cpp:64-69` and in +[`ltx25-text-linear-seam.md`](ltx25-text-linear-seam.md). + +The parallel-dispatch layer this row measures **is** a 1:1 port, and its anchors +are the ones to read: `src/vt/cpu/cpu_ops.cpp:1-8` and `cpu_threadpool.h` record +llama.cpp (local fork) `ggml/src/ggml-cpu/ggml-cpu.c:1155-1443` @ `237ad9b96` as +the chunk policy's source. `MatmulOneChunk`'s widened-activation buffer is a +recorded *deviation* from that port (`cpu_ops.cpp:99-108`), so its cost is ours +to bound. + +## Our baseline + +Read out of the tree at `origin/main` `678fc672c`: + +1. **The seam allocates exactly one thing per call, and it is thread-local.** + `src/vt/cpu/cpu_ops.cpp:125` — `static thread_local std::vector af;`, + resized to `nrows * k` where `nrows = min(16, chunk rows)` and 16 is ggml's + `blck_1`. Nothing else in `MatmulChunked`, `MatmulOneChunk`, + `MatmulOneChunkRef` or `vt::MatmulBT` allocates. At the shipped + `k = 188160` that is `16 x 188160 x 4 = 12.04 MB` per worker, and at + `hardware_concurrency() = 20` it is **241 MB in total**, not 26 GiB. +2. **`static thread_local` means it is allocated once per worker for the process + lifetime and never shrinks** — the comment at `cpu_ops.cpp:122-124` says so. + That is a retention property worth recording; it is not a per-call cost. +3. **The call site's `scaled` copy is pre-existing and unchanged by #1252.** + `ltx2_text_encoder.cpp:460-465` builds one `std::vector` the size of + `normed` per `project()` call — at `B*T = 1024`, `flat = 188160` that is + 770.4 MB — and #1252's body records it as deliberately kept on both arms. + A copy present in the *before* arm cannot explain a *regression*. +4. **The tree has no memory-bounded test at all.** `tests/vt/test_ops_matmul.cpp` + and `test_ops_matmul_elem.cpp` assert values and byte equality; nothing + anywhere asserts a peak-RSS or allocation bound. So the reported defect, had + it been real, would have been invisible to every gate in the repository. + +## Port map + +| Piece | Where | Change | +|---|---|---| +| Attribution of #1286 | this spec, `## Risks/decisions` | new | +| Local A/B probe (scratch, uncommitted) | scratchpad | new | +| Memory bound for the shared GEMM seam | `tests/vt/test_ops_matmul_mem.cpp` | new | +| Test registration | `tests/CMakeLists.txt` | one line | + +No `src/` change is planned. If the measurement contradicts the baseline above, +the fix is chosen from #1286's own list and this table is rewritten before any +code is written. + +## Tests to port + +None to port: there is no upstream test for a C++ threadpool's peak resident +footprint. The new test is original and its contract is stated where it lives. + +It asserts the property the baseline claims and #1286 doubted: **peak RSS growth +across a `vt::MatmulBT` call does not scale with the whole intermediate per +worker.** Concretely, the growth in `VmHWM` attributable to the call, measured +at a large `K` where the per-worker tile is the dominant term, stays under the +documented `nthreads x 16 x K x 4` plus slack — and in particular is nowhere +near `nthreads x rows x K x 4`, which is the shape #1286 hypothesised. + +It is red-first by mutation rather than by pre-existing failure, because there +is no defect to start red against. The mutation is the hypothesised defect +itself: widen `af` from the 16-row tile to the chunk's whole row span. Recorded +with `BUILT`, the compiler error count, `git diff --stat`, and the failing +assertion by name. + +## Gates + +- `test_ops_matmul_mem` — the new bound, plus its mutation. +- `test_ops_matmul`, `test_ops_matmul_elem` — the seam's existing value and + byte-equality gates, unchanged and still green. +- `test_ltx2_text_encoder` — the caption projection's byte-exactness against the + recorded goldens. **No tolerance is widened anywhere in this row**; the values + must not move, and nothing here touches summation order. +- `test_ltx2_lora`, `test_ltx2_loader` — #1259's gates, for the sibling check. +- Full `ctest`, and `scripts/agent-preflight.sh --staged`. + +Known-environmental and not chased: `test_engine_core_proc` and `test_async_llm` +starve under `ctest -j` and are re-run serially; `test_cpu_x86_llamacpp_floor` +is #618 under load; `windows-msvc-*` is red on every pull request. + +## Dependencies + +- The retained GB10 evidence under + `/mnt/nas_share/rc/ltx25-fullmodel/out/` — both the #1252 run + (`20260818T220620Z/1024x576-25f/`) and the pre-#1252 run + (`1024x576-25f/`, 2026-08-18 17:20-18:11), which is what makes a paired + comparison possible at all. +- `scripts/../runguard.py` on the NAS defines the columns; its + `runguard.py:236-237,260` fix `used_gib = MemTotal - MemAvailable` and + `anon_gib = AnonPages`, both **system-wide**, against `rss_gib`, which is the + child's own. That distinction is what the attribution turns on. +- No GPU and no lease. Allocation sizes are architecture-independent, and the + local arm is an x86 box. + +## Work breakdown + +| # | Work | Landable alone | +|---|---|---| +| W1 | Attribute #1286 against the retained paired evidence | yes | +| W2 | Local before/after A/B: peak RSS and throughput at the shipped geometry, swept over threadpool width | yes | +| W3 | Check #1259's profile and either fix in flow or file | yes | +| W4 | The seam's memory bound, red by mutation, green after | yes | + +One pull request; no split case applies, and no developer answer is recorded +under `## Git integration`. + +## Risks/decisions + +**The instrument's own precondition is the first risk, and it is the one that +bites.** #1286 compares two *absolute* peaks of a *system-wide* column. That +comparison is only sound when both runs start from the same occupancy, and +nothing in the issue checks whether they did. This spec's `## Outcome` records +what the check found. + +**A scaled probe is never presented as a full-geometry one.** Where the local +arm reduces `rows` or `out_features` to keep a single-threaded arm inside a +sensible wall time, the reduction is stated beside the number, and the +thread-scaling term it isolates is independent of `rows` by construction (`af` +is sized by the 16-row tile, not by the chunk's row count). + +**Byte-exactness is not negotiable.** #1252's path is bit-exact against recorded +goldens and its review confirmed a test diff of 150 insertions and zero +deletions. If any candidate fix altered summation order — blocking the GEMM over +`rows` is the one that would — this row stops and puts the question here rather +than deciding it in code. No tolerance is widened to make a red green. + +**A memory bound can be flaky.** Peak RSS depends on the allocator, so the test +is written to bound a *difference* at a geometry where the signal (12 MB per +worker) is far above malloc noise, rather than to pin an absolute number. If a +robust bound cannot be written, that is reported as a finding instead of being +weakened until it passes. + +## Owed + +- [#1286](https://github.com/mudler/vllm.cpp/issues/1286) — this row. + +## Now + +`ACTIVE`. W1 and W2 measured; see `## Outcome`. + +## Outcome + +Filled in when the row reaches `DONE`. From 8d46223fd66b17d16bcc5fe8b27881cd1e816352 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Wed, 19 Aug 2026 07:48:23 +0000 Subject: [PATCH 2/3] fix(LTX25-TEXT-LINEAR-MEM): #1252's +26 GiB is the box, not the change, and the seam had no memory bound at all (#1286) #1286 reported that routing the LTX-2.5 caption projection through `vt::MatmulBT` raised peak host memory from ~79 GiB to 105.85 GiB and aborted a full-model render on GB10, and named three candidates. It is none of them. `runguard.py:236-237,260` fixes the compared column as the SYSTEM-WIDE `MemTotal - MemAvailable`. The pre-#1252 run began at `used = 4.741 GiB`; the #1252 run began at `used = 31.553 GiB`. That is 26.812 GiB against a claimed regression of 26.647 GiB. Each run's own demand -- peak minus its own t=0 -- is 74.465 GiB before and 74.300 GiB after, with system AnonPages deltas of 38.012 vs 38.217 and child VmRSS at each peak sample of 41.952 vs 42.090. Both peaks were sampled inside a ~1900% CPU stretch, so they are the same phase class. On a box as clean as the first run's, the second binary's own demand would have left 40.59 GiB available, above both floors. Measured locally as well, since the issue asked for attribution rather than inference: at the shipped K = 188160 the seam costs 232 MiB of peak RSS at 20 workers against the scalar loop's 1.0 MiB, and is 8.43x faster -- 0.85% of the memory #1286 attributes to it, with the speedup intact. Swept over threadpool width the growth is 11.6 MiB per worker, which is `cpu_ops.cpp:125`'s `static thread_local` tile at 16 x K x 4 and nothing else, and the output checksum is byte-identical across all six widths. #1259 measures 0.52 MiB at 20 workers and needs nothing. What the attribution found is that NOTHING IN THIS TREE BOUNDS THE SEAM'S MEMORY. Every GEMM gate asserts values or byte equality, so a kernel that allocated a whole intermediate per worker would have passed all of them until a box ran out. `tests/vt/test_ops_matmul_mem.cpp` closes that. It counts what the seam asks for through a replaced global `operator new` rather than measuring peak RSS, because the peak-RSS draft read `growth_bytes = 0` at every thread count and every row count while passing every bound -- the operands are freed between measurements and glibc serves the next tile from resident pages. A liveness case beside it requires the counter to see six of eight fresh workers take a tile, so the bound cannot pass on a silent instrument. No product code changes. The two record-only edits and the issue this filed are in the pull request body. Refs #1286, #1252, #1259, #1317 FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [claude-code] --- .agents/benchmark-record.md | 149 +++++++++++++++++ .agents/issue-index.md | 1 + .agents/specs/ltx25-text-linear-mem.md | 193 +++++++++++++++++++++- docs/BENCHMARKS.md | 2 +- docs/STATUS.md | 2 +- tests/CMakeLists.txt | 11 ++ tests/vt/test_ops_matmul_mem.cpp | 213 +++++++++++++++++++++++++ 7 files changed, 566 insertions(+), 5 deletions(-) create mode 100644 tests/vt/test_ops_matmul_mem.cpp diff --git a/.agents/benchmark-record.md b/.agents/benchmark-record.md index 3ce9df22d..286b4efd6 100644 --- a/.agents/benchmark-record.md +++ b/.agents/benchmark-record.md @@ -24128,3 +24128,152 @@ registry still counts probe refusals only and a replay-time refusal would abort `mem/mem_.csv` (the per-call `cudaMemGetInfo` trace with node counts), `mem/smp_.csv` (the `nvidia-smi` and RSS sampler) and `out-bytes/ids_.json` (the token artifacts). + +## LTX25-TEXT-LINEAR-MEM — #1252 costs 0.24 GiB of peak host memory, not 26 GiB, and the 8.4x survives (2026-08-19, `row/LTX25-TEXT-LINEAR-MEM`, base `origin/main` `678fc672c`, 20-core Zen 5 under KVM, #1286 / #1252 / #1259 / #1317) + +**This entry refutes [#1286](https://github.com/mudler/vllm.cpp/issues/1286).** +That issue reported that routing the LTX-2.5 caption projection through +`vt::MatmulBT` ([#1252](https://github.com/mudler/vllm.cpp/issues/1252)) raised +peak host memory from ~79 GiB to 105.85 GiB on a 119 GiB GB10 and aborted a +full-model render, and named the seam's per-thread tiles as the first suspect. +It is not the seam, it is not the call site, and it is not an interaction. **The ++26 GiB is the box's occupancy before the job started.** + +### The paired GB10 evidence + +`runguard.py:236-237,260` fixes what the compared column is: `used_gib` is the +SYSTEM-WIDE `MemTotal - MemAvailable`, `anon_gib` is system-wide `AnonPages`, +and only `rss_gib` is the child's own. Both runs are retained under +`/mnt/nas_share/rc/ltx25-fullmodel/out/`. + +| | pre-#1252, `1024x576-25f/` | #1252, `20260818T220620Z/1024x576-25f/` | +|---|---:|---:| +| `used_gib` at `t=0` | **4.741** | **31.553** | +| `avail_gib` at `t=0` | 114.890 | 88.078 | +| peak `used_gib` | 79.206 at t=1867.4 s, `cpu=1885.5%` | 105.853 at t=199.2 s, `cpu=1925.2%` | +| samples / terminal | 3018, `signal=15 (supervisor asked to stop)` | 391, `exit=90 guard:PROJECTION` | + +Starting difference **26.812 GiB**; claimed regression `105.853 - 79.206 =` +**26.647 GiB**. They agree to **0.165 GiB**. + +Peak minus each run's own `t=0`, on three independent columns: + +| axis | pre-#1252 | #1252 | delta | +|---|---:|---:|---:| +| `used_gib` peak minus own `t=0` | **74.465** | **74.300** | **-0.165** | +| `anon_gib` peak minus own `t=0` | 38.012 | 38.217 | +0.205 | +| child `rss_gib` at the peak sample | 41.952 | 42.090 | +0.138 | + +Both peaks sit inside a ~1900% CPU stretch, so this is the same phase class and +not two different ones. On a box as clean as the first run's, the #1252 binary's +own 74.300 GiB would have left **40.59 GiB** available — above the 12 GiB hard +floor and the 8 GiB projection floor, and consistent with the "never below +~40 GiB" the pre-#1252 runs showed. + +Two readings that each look like support for #1286 in isolation and are not. +First, during its single-core projection the pre-#1252 run is DEAD FLAT at +`anon = 34.35`, `rss = 33.56`, `used = 74.50` from t=124 s to t=1801 s, while +the #1252 run's threaded stretch sits at `anon = 34.30`, `rss = 30.35`, +`used = 97.40` — **the same `anon` to 0.15%**, with `used` 23 GiB apart. Second, +NEITHER run ever loaded the DiT (`dit_runs=0` in both; the first was stopped +from outside at t=3017.3 s still inside conditioning), so neither peak is a +whole-render peak and neither is presented as one. Filed as +[#1317](https://github.com/mudler/vllm.cpp/issues/1317). + +### The local A/B, before vs after, at the shipped geometry + +Same probe source and flags on both arms, only the `Linear` body differing. +Peak RSS is `VmHWM` read before and after the call with every operand already +allocated and touched, so the figure is the call's own. 20-core Zen 5 under KVM, +Release, `-ffp-contract=off`, `K = 188160`, `out_features = 4096`, +`rows = 64` (REDUCED from the shipped 1024 to keep the single-threaded arm +inside a sensible wall; the tile term the sweep isolates is independent of rows +by construction). Box NOT idle, loadavg 5.4-13.3, three replicates, median: + +| arm | wall | rate | peak-RSS growth | +|---|---:|---:|---:| +| before, scalar `double` loop | **28.488 s** | 1.732 GMAC/s | **1.0 MiB** | +| after, `vt::MatmulBT` | **3.378 s** | 14.60 GMAC/s | **232 MiB** | + +**8.43x, and +231 MiB** — reproducing #1252's 8.57x, and **0.85% of the +26.6 GiB #1286 attributes to the change**, i.e. 117x too small to be it. + +Swept over threadpool width, same geometry: + +| workers | 1 | 2 | 4 | 8 | 16 | 20 | +|---|---:|---:|---:|---:|---:|---:| +| peak-RSS growth (MiB) | 12.9 | 24.7 | 47.8 | 93.8 | 186.1 | **232.1** | +| per worker (MiB) | 12.9 | 12.3 | 12.0 | 11.7 | 11.6 | 11.6 | + +The model is `cpu_ops.cpp:125`'s `static thread_local std::vector af`, +sized by ggml's 16-row `blck_1` tile: `16 x 188160 x 4 = 11.48 MiB` per worker. +The measurement lands on it, and the output checksum is **byte-identical across +all six thread counts** — the dispatch determinism contract holding. + +Full shipped geometry, `rows = 1024`, both projections, default 20 workers. The +box was heavily loaded (loadavg 15.9 then 28.5) so **the wall times below are +not a speed claim**, only the memory column is: + +| projection | wall | rate | peak-RSS growth | +|---|---:|---:|---:| +| `1024 x 188160 x 4096` | 57.34 s | 13.76 GMAC/s | **247 MiB** | +| `1024 x 188160 x 2048` | 28.95 s | 13.63 GMAC/s | **239 MiB** | + +16x the rows moved the growth by 15 MiB, which is the output buffer +(`1024 x 4096 x 4 = 16.8 MB`) allocated inside the timed region. The tile does +not scale with rows. + +### #1259, the sibling that has never run on the full model + +`Ltx2FuseLoraIntoTensor` takes `vt::Matmul`, the other member of the same seam; +`MatmulOneChunk` is one template shared by both orientations, so the per-worker +buffer is `16 x K x 4` there too with `K = rank`. Measured at the shipped +`4096 x 450 x 4096` with a replaced global `operator new`: + +| workers | 1 | 2 | 4 | 8 | 16 | 20 | +|---|---:|---:|---:|---:|---:|---:| +| bytes requested | 28,864 | 28,864 | 86,464 | 201,664 | 432,064 | **547,264** | + +**0.52 MiB at 20 workers**, exactly `19 x 28,800 + 64`. `bs` and `agg` are +unchanged by #1259. It cannot reach the wall #1286 describes; nothing fixed, +nothing filed. + +### The refuted instrument, recorded because it read as a pass + +The bound this row lands (`tests/vt/test_ops_matmul_mem.cpp`) was FIRST written +against peak RSS, measuring `VmHWM` across `/proc/self/clear_refs`. It read +`growth_bytes = 0` for **every** thread count and **every** row count while +passing every bound: the operands are freed between measurements, glibc keeps +the arena, and the next tile is served from pages that are already resident. A +mute switch reporting green over a kernel doing anything at all. The shipped +gate counts what the seam ASKS FOR through a replaced global `operator new`, +which no allocator policy can silence, with a liveness case requiring the +counter to see at least six of eight fresh workers take a tile. + +### Mutation M1 — the hypothesis, written into the kernel + +`af` widened from the 16-row tile to the whole activation. + +- **First attempt: `BUILT=NO`, `compile_err=2`** — + `cpu_ops.cpp:134:19: error: unused variable 'nrows' [-Werror=unused-variable]`. + Recorded because a mutation that fails to build reads exactly like a passing + test. +- **Second attempt: `BUILT=YES`, `compile_err=0`**, `git diff --stat` = + `src/vt/cpu/cpu_ops.cpp | 2 +-, 1 file changed, 1 insertion(+), 1 deletion(-)`. + `test_ops_matmul_mem` **exit 1**, `Status: FAILURE!`, + `2 test cases | 1 passed | 1 failed`, `13 assertions | 7 passed | 6 failed`. + By name: `CHECK(growth <= Bound(nthreads))` at three worker counts + (`67,108,928 <= 25,165,824`; `201,326,656 <= 41,943,040`; + `469,762,112 <= 75,497,472`) and all three row-scaling assertions, with + `rows=1024` reading **1,073,741,888 bytes**. +- Restored byte-for-byte, `sha256(cpu_ops.cpp) =` + `dc39eccdece48879e82be7209e95d182c6ed624eb04389de689fa4b58fe4f1f3` before and + after; rebuilt (binary mtime 07:15:48 -> 07:16:27, so the green is not stale) + and green: **2 cases, 13 assertions, 0 failed, exit 0**. + +### Not claimed + +No GB10 number of any kind. The bound is a CPU bound; `vt::MatmulBT` on +CUDA/ROCm/Vulkan is not covered. No full-model render has been rerun, so nothing +here says what such a run would now do — only that #1252 is not what stopped the +last one. diff --git a/.agents/issue-index.md b/.agents/issue-index.md index 7f97653b0..145120c21 100644 --- a/.agents/issue-index.md +++ b/.agents/issue-index.md @@ -419,3 +419,4 @@ rather than merged. `scripts/check-agent-record.py` gates both. | [#1291](https://github.com/mudler/vllm.cpp/issues/1291) | `ENG-CUDAGRAPH-BREAK` | W3 of the break-point capture seam: the three remaining PLAIN BATCHED decode drivers migrate onto `vt::GraphCaptureScope` + `vt::BreakableGraph` — `Qwen3MoeDecodeGraph` (`qwen3_moe.cpp`), `VoxtralDecodeGraph` (`voxtral.cpp`) and `DeepseekV2DecodeGraph` (`deepseek_v2.cpp`) — one commit each, each landable alone, each with its own RED-first G2 gate. Four of the nine drivers are now on the seam and the six batched-driver `VLLM_CPP_CUDAGRAPH` reads the spec's `## Our baseline` item 1 counted are down to TWO, both in `qwen3_5.cpp` (W4). The two per-model rollback switches STAY (`VT_QWEN3MOE_CUDAGRAPH`, `VT_DEEPSEEK_CUDAGRAPH`): each is an A/B lever for exactly one driver, not a copy of the shared one. **Each driver owes its own gate because nothing else can see the difference** — a driver that kept its hand-rolled `BeginCapture`/`EndCaptureGraph` pair produces IDENTICAL logits, an IDENTICAL backend log and an identical `replay_count()`, so `segments_captured` and `replays` are the only observables that separate "captured a graph" from "captured a graph THROUGH THE SEAM". Red-first on four assertions each (`test_qwen3_moe_decode_graph_seam` 222/226, `test_voxtral_decode_graph_seam` 224/228, `test_deepseek_v2_decode_graph_seam` 224/228, all exit 1), 3/3 green after; the G2 mutation restoring each pre-W3 driver file (25/102, 23/92 and 25/94 lines, each compiled clean, each restored and verified by sha256) reddens ONLY its own gate and leaves `test_breakable_graph` 216/216 and W2's `test_qwen3_decode_graph_seam` 231/231 GREEN. The two swapped seams every such gate needs now live ONCE, in `tests/vllm/models/decode_graph_seam_harness.h`, with W2's file including it: three more copies inside `tests/` would have reproduced the duplication this row removes from `src/`, and two copies of a harness diverge invisibly because both files stay green while measuring different things. **G1 IS DELIVERED, retiring the item W1 and W2 both carried as owed.** `tests/vllm/models/test_decode_graph_seam_g1_cuda.cpp` drives each migrated driver COLD, CAPTURE and THREE consecutive replays and compares every step bit for bit against the driver's own EAGER arm, which is selected by `max_num_reqs == 0` (`PadToCaptureSize` returns -1 and `Step` falls out to its plain forward) so both arms are ONE binary on ONE device rather than two builds, each with its OWN device KV cache so neither can read the other's writes. Measured on `thor:gpu0` through an `rc` lease — NVIDIA Thor sm_110, driver 595.78, nvcc 13.0.88, source `c905bb536`, CUDA-ON build for arch 110, 32 `.cu.o` objects, binary resolving `libcudart.so.13` and `libcublasLt.so.13`, build 489 s at `-j4`: **3 cases, 1600 assertions, exit 0**, `5 steps x 100 logits, 0 differing, 4 replays` for each driver. The ASSERTION COUNT carries that claim and the status line does not: with no CUDA backend every case skips and the same file prints `Status: SUCCESS!` over `assertions: 0`, which the file's own header names as a skip wearing a pass. Bounded honestly: the models are the synthetic tiny ones the CPU forward gates use rather than a checkpoint, and W2's `Qwen3DenseDecodeGraph` is covered by shared-seam ARGUMENT and not by one of the three cases. **W3 ALSO FOUND A GATE THAT COULD NOT FAIL, and closing it is part of the stage.** The three gates assert `breaks_registered == 0` to hold the capture to vLLM's decode arm (`FULL_AND_PIECEWISE`, `vllm/config/compilation.py:63,65-66,630-632` @ `5559679229`). That is a TAUTOLOGY for these models: the counter moves only when a `vt::GraphBreak` registers into a splitting scope, and the ONE production break point in the tree is W1's, in `qwen3.cpp` — W2's driver runs through it, none of W3's three does. Measured rather than reasoned: flipping `kFull` to `kPiecewise` in `qwen3_moe.cpp`, one token, compiled clean and left that driver's whole gate GREEN at 226/226, and the same flip was equally invisible in the other two. The mode was UNOBSERVABLE from outside a driver — the scope is a `Step` local, the container is private to the driver's `Impl`, and a token gate cannot see a segment count — so `vt::GraphBreakStats` gains `full_scopes` and `piecewise_scopes`, counted in `GraphCaptureScope`'s constructor on the ACTIVE path only (an inert scope makes no backend call in either mode, so counting it would report a mode that never reached a backend), cleared by `ResetGraphBreakStats()`, gated in `tests/vt/test_breakable_graph.cpp` with the inert arm as the control that stops them degenerating into "scopes constructed". Re-measured after the fix, the same one-token flip now REDS each of the three gates on exactly those two assertions (226/228, 228/230, 228/230, all exit 1). **NO break point is registered in these three models, and that is a decision rather than an omission**: under `kFull` a break point takes the same pass-through arm it takes outside a scope, so registering one would land machinery no gate can exercise; establishing each model's break-point set is what the PIECEWISE arm needs, that arm is blocked on replay-safe closure inputs (W4), and W6 is where the eligibility predicate moves. **The async device-token decline, stated per driver rather than left as an omission.** `VoxtralDecodeGraph` needs none: its only construction site is `VoxtralGenerateGreedy`, the single-sequence multimodal greedy loop, which is not reached from `runner.cpp` and has no asynchronous device mirror. The other two are a NEW FINDING, filed [#1305](https://github.com/mudler/vllm.cpp/issues/1305) with W4 as owner and NOT fixed in flow, because adding a decline trades a shipped default-ON capability away on a measurement W3 cannot make and the fix `qwen3.cpp:1106`'s own comment names is `StepDevInputs` as a seam capability. **G5's ROCm/Tenstorrent arm is NOT discharged**, and the owner moves from W3 to W5 with the reason stated: the fleet carries no ROCm and no Tenstorrent device, so it is BLOCKED on hardware rather than unattempted. What W3 can say instead is that the seam's CUDA arm now runs on TWO architectures, sm_110 here and sm_121a for W1's exit criterion. Still NO throughput claim anywhere: the prefill refutation stands (3.8% host idle, >96% GPU-busy, 92.5% glue) and `kFull` keeps each migrated step's shape the one it already had | feature | | [#1305](https://github.com/mudler/vllm.cpp/issues/1305) | `ENG-CUDAGRAPH-BREAK` | Three decode-graph registrations route an ASYNCHRONOUS step into a host-vector replay with NO `device_token_ids` decline. Found while migrating two of them onto the break-point capture seam ([#1291](https://github.com/mudler/vllm.cpp/issues/1291)) and NOT caused by that migration — the shape is present at its base commit `5d9fe332c`. `src/vllm/model_executor/models/qwen3.cpp:1106` returns `std::nullopt` whenever `ModelForwardInput::device_token_ids` is non-null, on a measured four-concurrent battery (`depth-1 graph ON PASS 78/78`, `depth-2 graph OFF PASS 82/82`, `depth-2 graph ON FAIL, slots 1-3 degenerate`, #323/#1179), because `Step()` replays against persistent HOST `token_ids` while the asynchronous combine has patched the DEVICE ids and deliberately left the host vector stale for decode rows (`src/vllm/v1/worker/gpu/runner.cpp:1265-1309,1523`). That comment calls the hazard "latent for EVERY classic-dense model, since the graph is default-ON". `qwen3_moe_registry.cpp:107`, `deepseek_v2_registry.cpp:106` and `glm4_moe_lite_registry.cpp:125` (the last constructs the DeepSeek driver) admit a pure-decode step to a driver with the same host-vector replay and carry NO such check; `grep -c StepDevInputs` returns 0 for `qwen3_moe.cpp`, `deepseek_v2.cpp` and `voxtral.cpp` against 41 for `qwen3_5.cpp`, which is the divergence the row's spec already records as the cost of nine drivers. `VoxtralDecodeGraph` is NOT affected: its only construction site is `VoxtralGenerateGreedy`, which the runner does not reach and which has no device mirror. NOT fixed in flow, for two reasons stated rather than assumed: adding a decline is a behaviour change that trades a shipped, default-ON capability away, and W3 has no four-concurrent battery for these two models to justify it; and the correct fix is already assigned — `StepDevInputs` (`qwen3_5.cpp:3894`) as a SEAM capability, which is `## Work breakdown` W4 of [eng-cudagraph-break.md](specs/eng-cudagraph-break.md) and the only version that reaches these registrations at all. Owner: **W4**, with [#1179](https://github.com/mudler/vllm.cpp/issues/1179) as the standing tracker. What would settle it: run that same battery against Qwen3-Coder and DeepSeek-V2-Lite with `VT_ASYNC_DEVICE_MIRROR` live and the decode graph ON — either it reproduces the depth-2 degeneration, or the reason it does not has to be found and written down rather than assumed | bug | | [#1286](https://github.com/mudler/vllm.cpp/issues/1286) | `LTX25-TEXT-LINEAR-MEM` | #1252's threaded caption projection was reported to raise peak host memory ~79 -> 105.85 GiB and abort the full-model LTX-2.5 render on GB10. **The +26 GiB is the box's STARTING OCCUPANCY, not the change.** Both runs are retained under `/mnt/nas_share/rc/ltx25-fullmodel/out/`, and `runguard.py:236-237,260` fixes the compared column `used_gib` as the SYSTEM-WIDE `MemTotal - MemAvailable`: the pre-#1252 run began at `used = 4.741 GiB` on an idle box, the #1252 run began at `used = 31.553 GiB`, a difference of **26.812 GiB** against a claimed regression of **26.647 GiB**. Each run's own demand — peak minus its own t=0 — is **74.465 GiB before and 74.300 GiB after**, and both peaks were sampled inside a ~1900% CPU stretch, so they are the same phase class rather than two different ones. Two further columns agree independently: system `AnonPages` delta 38.012 vs 38.217 GiB, and child `VmRSS` at each peak sample 41.952 vs 42.090 GiB. Confirmed locally by an A/B of the two `Linear` arms at the shipped `1024 x 188160 x 4096` geometry swept over threadpool width; the seam's only per-call allocation is `cpu_ops.cpp:125`'s `static thread_local` widened-activation tile, `16 x K x 4` per worker = 12.04 MB at `K = 188160`. Owner `LTX25-TEXT-LINEAR-MEM`, spec [`ltx25-text-linear-mem.md`](specs/ltx25-text-linear-mem.md) | verification | +| [#1317](https://github.com/mudler/vllm.cpp/issues/1317) | — | The full-model LTX-2.5 render harness compares **absolute** peaks of a **system-wide** column across runs that started 26.812 GiB apart, and [#1286](https://github.com/mudler/vllm.cpp/issues/1286)'s entire reported regression is that offset. `runguard.py:236-237,260` writes `used_gib = MemTotal - MemAvailable` and `anon_gib = AnonPages`, both system-wide; only `rss_gib` belongs to the child. The pre-#1252 run began at `used = 4.741 GiB` on a box with 114.890 GiB available; the #1252 run began at `used = 31.553 GiB` with 88.078 GiB available. Peak minus each run's OWN `t=0` is **74.465 GiB before and 74.300 GiB after**, with system `AnonPages` deltas of 38.012 vs 38.217 GiB and child `VmRSS` at each peak sample of 41.952 vs 42.090 GiB — three instruments inside ±0.25 GiB, both peaks sampled inside a ~1900% CPU stretch rather than in different phases. On a box as clean as the first run's the second binary's own demand would have left **40.59 GiB** available, above both the 12 GiB hard floor and the 8 GiB projection floor. **Two fixes, neither in this repository:** record `used_gib` at `t=0` in `PROVENANCE` and make every cross-run claim on the delta, on `rss_gib`, or refuse it; and find what held 26.8 GiB on `dgx:gpu0` before the job started — a leaked container or orphaned process from a previous job is the candidate, and it is what actually aborted the render. NOT fixed in flow: `runguard.py` and fleet job hygiene are outside the tree, and the second half needs a look at the box rather than a diff. Owed under `## Owed` of [`ltx25-text-linear-mem.md`](specs/ltx25-text-linear-mem.md) | bug | diff --git a/.agents/specs/ltx25-text-linear-mem.md b/.agents/specs/ltx25-text-linear-mem.md index 67b4d97a1..1a81ee013 100644 --- a/.agents/specs/ltx25-text-linear-mem.md +++ b/.agents/specs/ltx25-text-linear-mem.md @@ -175,12 +175,199 @@ weakened until it passes. ## Owed -- [#1286](https://github.com/mudler/vllm.cpp/issues/1286) — this row. +- [#1317](https://github.com/mudler/vllm.cpp/issues/1317) — the harness compares + absolute system-wide peaks across runs with different starting occupancy, and + something held 26.8 GiB on `dgx:gpu0` before the run began. Both halves are + outside this repository — `runguard.py` and the fleet's job hygiene — so + neither is fixed in flow. It is what actually aborted the render. +- **No GB10 number.** Every measurement below is from a 20-core Zen 5 under KVM. + Allocation sizes are architecture-independent, so the attribution carries; a + throughput ratio does not, and none is claimed for GB10. +- **The seam's tile is retained for the process lifetime.** `af` is a + `static thread_local` that grows to the largest `K` any GEMM on that worker + ever saw and never shrinks. At `K = 188160` and 20 workers that is 232 MiB + held until exit, on a box that may later be tight. It is bounded, deliberate + and now measured; it is not repaired here, because the buffer exists precisely + so a worker allocates once, and trading that away is a performance decision + with its own spec. ## Now -`ACTIVE`. W1 and W2 measured; see `## Outcome`. +`DONE`. The reported defect does not exist; the seam gained the bound whose +absence let it be believed. ## Outcome -Filled in when the row reaches `DONE`. +### W1 — the attribution: the +26 GiB is the box, not the change + +Both runs are retained, and that is what makes this answerable. +`runguard.py:236-237,260` fixes the compared column: `used_gib` is the +**system-wide** `MemTotal - MemAvailable`, `anon_gib` is system-wide +`AnonPages`, and only `rss_gib` is the child's own. + +| | pre-#1252 `1024x576-25f/` | #1252 `20260818T220620Z/1024x576-25f/` | +|---|---:|---:| +| `used_gib` at `t=0` | **4.741** | **31.553** | +| `avail_gib` at `t=0` | 114.890 | 88.078 | +| peak `used_gib` | 79.206 at t=1867.4 s, `cpu=1885.5%` | 105.853 at t=199.2 s, `cpu=1925.2%` | + +The box carried **26.812 GiB** before `ltx2-gen` started in the second run. +#1286's regression is `105.853 - 79.206 = 26.647 GiB`. The two agree to +**0.165 GiB**. + +Each run's own demand — peak minus its own `t=0` — is the same: + +| axis | pre-#1252 | #1252 | delta | +|---|---:|---:|---:| +| `used_gib` peak minus own `t=0` | **74.465** | **74.300** | **-0.165** | +| `anon_gib` peak minus own `t=0` | 38.012 | 38.217 | +0.205 | +| child `rss_gib` at the peak sample | 41.952 | 42.090 | +0.138 | + +Three columns, three instruments, all inside ±0.25 GiB, and both peaks sampled +inside a ~1900% CPU stretch rather than in two different phases. On a box as +clean as the first run's, the #1252 binary's own 74.300 GiB would have left +**40.59 GiB** available — above the 12 GiB hard floor and the 8 GiB projection +floor, and consistent with the "never below ~40 GiB" the pre-#1252 runs showed. + +Two further readings from the same evidence, recorded because each one on its +own would have been read as supporting #1286: + +- During its single-core projection the pre-#1252 run sits **dead flat** at + `anon = 34.35`, `rss = 33.56`, `used = 74.50` from t=124 s to t=1801 s. The + #1252 run's threaded stretch sits at `anon = 34.30`, `rss = 30.35`, + `used = 97.40`. **The same `anon` to 0.15%** — with `used` 23 GiB apart. +- The pre-#1252 run reached t=3017.3 s and was stopped from outside + (`# TERMINAL signal=15 (supervisor asked to stop)`), still inside + conditioning with `dit_runs=0`. Neither run ever loaded the DiT, so neither + peak is a whole-render peak, and neither is presented as one. + +Filed as [#1317](https://github.com/mudler/vllm.cpp/issues/1317). + +### W2 — the local A/B: the seam costs 0.24 GiB and keeps the 8.4x + +Same probe source and flags on both arms, only the `Linear` body differing, at +the shipped `K = 188160` and `out_features = 4096`; peak RSS is `VmHWM` read +before and after the call with every operand already allocated and touched. +20-core Zen 5 under KVM, Release, `-ffp-contract=off`, box **not idle** +(loadavg 5.4-13.3), three replicates, median: + +| arm | wall | rate | peak-RSS growth | +|---|---:|---:|---:| +| before, scalar `double` loop | **28.488 s** | 1.732 GMAC/s | **1.0 MiB** | +| after, `vt::MatmulBT` | **3.378 s** | 14.60 GMAC/s | **232 MiB** | + +**8.43x, and +231 MiB.** The speedup reproduces #1252's 8.57x; the memory cost +is **0.85% of the 26.6 GiB #1286 attributes to it**, i.e. 117x too small to be +the reported defect. + +Swept over threadpool width at the same geometry, the growth is exactly the +per-worker tile and nothing else: + +| workers | 1 | 2 | 4 | 8 | 16 | 20 | +|---|---:|---:|---:|---:|---:|---:| +| peak-RSS growth (MiB) | 12.9 | 24.7 | 47.8 | 93.8 | 186.1 | **232.1** | +| per worker (MiB) | 12.9 | 12.3 | 12.0 | 11.7 | 11.6 | 11.6 | + +The model predicts `16 x 188160 x 4 = 11.48 MiB` per worker. The measurement +lands on it, and the output checksum is **byte-identical across all six thread +counts**, which is the dispatch determinism contract holding. + +At the **full shipped geometry**, `rows = 1024`, both projections, default 20 +workers (box heavily loaded, loadavg 15.9 then 28.5, so the wall times are not +a speed claim): + +| projection | wall | rate | peak-RSS growth | +|---|---:|---:|---:| +| `1024 x 188160 x 4096` | 57.34 s | 13.76 GMAC/s | **247 MiB** | +| `1024 x 188160 x 2048` | 28.95 s | 13.63 GMAC/s | **239 MiB** | + +`rows` moved 16x between the two sweeps and the growth moved by 15 MiB — which +is the output buffer, `1024 x 4096 x 4 = 16.8 MB`, allocated inside the timed +region. The tile does not scale with rows, as designed. + +**So the answer to "which of the three" is none of them.** Not the threaded +kernel: 232 MiB at 20 workers. Not the call site: its `scaled` copy is 770 MB +and is present on **both** arms, so it cannot be a regression. Not an +interaction. The measurement was confounded by its baseline. + +### W3 — #1259 has the same profile and needs nothing + +`Ltx2FuseLoraIntoTensor` routes through `vt::Matmul`, the other member of the +same seam, and `MatmulOneChunk` is one template shared by both orientations — +so the per-worker buffer is `16 x K x 4` there too, with `K = rank`. At the +shipped `4096 x 450 x 4096`, measured with the same allocation counter the new +gate uses: + +| workers | 1 | 2 | 4 | 8 | 16 | 20 | +|---|---:|---:|---:|---:|---:|---:| +| bytes requested | 28,864 | 28,864 | 86,464 | 201,664 | 432,064 | **547,264** | + +**0.52 MiB at 20 workers**, exactly `19 x 28,800 + 64`. `bs` and `agg` are +unchanged by #1259. It cannot reach the wall #1286 describes, so nothing is +fixed and nothing is filed for it. + +### W4 — the bound, and what looking for it exposed + +`tests/vt/test_ops_matmul_mem.cpp`. The refutation does not repair the thing the +attribution found: **no gate in this tree bounds the seam's memory.** Every GEMM +gate asserts values or byte equality, so a kernel that allocated a whole +intermediate per worker would have passed all of them on every model until a box +ran out. That is why the bound lands here even though the reported defect is not +real. + +**Peak RSS could not carry the gate, and finding that out was the useful part.** +The first draft measured `VmHWM` around `/proc/self/clear_refs`. It read +`growth_bytes = 0` for **every** thread count and **every** row count while +passing every bound — because the operands are freed between measurements, +glibc keeps the arena, and the next tile is served from resident pages. A mute +switch that reports green over a kernel doing anything at all. The gate is +therefore a replaced global `operator new` counting what the seam **asks for**, +which no allocator policy can silence, with a liveness case beside it requiring +the counter to see at least six of eight fresh workers take a tile. + +Green, at `K = 65536` so one tile is exactly 4 MiB: + +| workers | 1 | 2 | 4 | 8 | +|---|---:|---:|---:|---:| +| bytes requested | 64 | 4,194,368 | 12,582,976 | 29,360,192 | + +`(nthreads - 1)` tiles, because the caller thread's `thread_local` is already +sized. Rows 64 vs 1024 at 4 workers: **12,582,976 both times**, difference 0. + +**Red first, by mutation, and the first attempt did not build.** M1 widens `af` +from the 16-row tile to the whole activation — #1286's own hypothesis, written +into the kernel. + +- **First attempt: `BUILT=NO`, `compile_err=2`**, + `cpu_ops.cpp:134:19: error: unused variable 'nrows' [-Werror=unused-variable]`. + Recorded because a mutation that fails to build reads exactly like a passing + test. +- **Second attempt: `BUILT=YES`, `compile_err=0`**, `git diff --stat` = + `src/vt/cpu/cpu_ops.cpp | 2 +-, 1 file changed, 1 insertion(+), 1 deletion(-)`. + `test_ops_matmul_mem` **exit 1**, `Status: FAILURE!`, + `2 test cases | 1 passed | 1 failed`, `13 assertions | 7 passed | 6 failed`. + Failing by name: `CHECK(growth <= Bound(nthreads))` at three worker counts — + `67,108,928 <= 25,165,824`, `201,326,656 <= 41,943,040`, + `469,762,112 <= 75,497,472` — and all three of the row-scaling assertions, + with `rows=1024` reading **1,073,741,888 bytes**, a gibibyte of exactly the + shape #1286 supposed. +- Restored byte-for-byte: `sha256(cpu_ops.cpp) =` + `dc39eccdece48879e82be7209e95d182c6ed624eb04389de689fa4b58fe4f1f3` before and + after. Rebuilt (binary mtime moved 07:15:48 -> 07:16:27, so the green is not a + stale binary) and green again: **2 cases, 13 assertions, 0 failed, exit 0**. + +M1 mutates **`src/vt/cpu/cpu_ops.cpp`**, product code reached from +`ModelRegistry::Forward` and from the LTX-2.5 text tower, which is what makes +the red evidence that the gate measures shipped behaviour rather than a +test-local copy. This row adds no product code, so there is no production call +site to delete — `reachability.md` answers that case directly. + +### What is not claimed + +- No GB10 measurement of any kind, so #1252's unmeasured per-core ratio `R` is + untouched and the LTX-2.5 speed axis stays where it was. +- The bound is a **CPU** bound. `vt::MatmulBT` on CUDA/ROCm/Vulkan is not + covered, and their allocation behaviour is not measured here. +- Nothing establishes what would happen on a full-model render now, because no + full-model render has been rerun. The claim is that #1252 is not what stopped + the last one. diff --git a/docs/BENCHMARKS.md b/docs/BENCHMARKS.md index 5b14066b2..35e19e074 100644 --- a/docs/BENCHMARKS.md +++ b/docs/BENCHMARKS.md @@ -502,7 +502,7 @@ built on it rather than keeping the flattering one. | Kimi-Linear-48B-A3B (KDA+MLA+MoE) | **RUNNER FOLD LANDS (ROW 7, §21, #122): engine==CLI 128/128 byte-identical; vs golden 122/128 (near-tie profile); FA2 MLA default-ON; SACRED green.** Server 19.0 tok/s wall; CLI 18.93 reproduced | vLLM ~21 (#111 floor; in-session re-measure ABORTED by GB10 reboot at util 0.82, §21): **~0.90×**, >= vLLM NOT met; residual = KDA host islands + grouped MoE + decode graph | | vLLM 0.26 re-benchmark | Pending | Re-run the binding grids on the advanced pin | | MiniMax-H3 FP4 speed (W-FP4a) | **Measured GB10 (`row/H3-FP4-GPU-E2E`).** Marlin W4A16 byte-exact vs bf16; fp4 a memory win, 0.8x bf16/forward. Real-ckpt fp4-resident e2e RUNS (mp4/wav) | fp4 speed CLOSED. bf16-vs-quant A/B: ENCODER half MEASURED (§8.15), DiT half NOT (no bf16 render exists). Detail: benchmark-record + spec §8 | -| LTX-2.5 axes | Speed `PENDING` (vllm-omni#6066 has no native 2.5), binding oracle too. **SIZE: 704x448/25f and 448x256/25f both COMPLETE on GB10 (4231 s, 3085 s)**; one run each, contended box, no oracle, no ceiling (#1088) | NOT the VAE decode (#1041/#1009): 39-100% of the ~1731 s phase is the caption projection (#1208), x86 671.8->78.4 s; LoRA fusion 17.78->0.124 s/module byte-identical (#1202), 59% of the rest is add-back (#1254) | +| LTX-2.5 axes | Speed `PENDING` (vllm-omni#6066 has no native 2.5), binding oracle too. **SIZE: 704x448/25f and 448x256/25f both COMPLETE on GB10 (4231 s, 3085 s)**; one run each, contended box, no oracle, no ceiling (#1088) | NOT the VAE decode (#1041/#1009): 39-100% of the ~1731 s phase is the caption projection (#1208), x86 671.8->78.4 s; LoRA fusion 17.78->0.124 s (#1202), add-back 59% of the rest (#1254). **#1286 +26 GiB REFUTED (#1317)** | | MiniMax-Music3 (`MiniMaxMusic3ForConditionalGeneration`) | **Every axis vs the reference stays `PENDING`.** A PARTIAL device arm now exists (#672): the 8.6B LM and the 2.4B fp32 DiT run on the accelerator, so the rows below are internal two-arm numbers and NOT parity ratios | Denominator: SGLang-Omni `748a0b43` in its production configuration (both CUDA graphs, compiled DIT and DAV, batched seeded sampling) | | MiniMax-Music3 device arm, Jetson Thor sm_110 (#672) | `--device 1` vs `--device 0`, same request/seed, idle box: 2 AR frames **846.6 vs 835.1 s (1.014x SLOWER)**; 10 frames **1430.4 vs 1512.1 s (0.946x)**. Fit: **-11.65 s/frame, +34.8 s fixed** | A third duration (the fit has no residual), and moving the depth decoder + DiT + vocoder, which are 5 of 6 stages and still host scalar loops | | MiniMax-Music3 DiT device arm, `thor:gpu0` sm_110 (#672) | Per DiT forward at the capture's geometry, same binary/weights/inputs, idle box: **204.955 s host vs 0.186-0.187 s device, 1094-1102x** (1201x fitted). Staged ONCE (0.61 s; loop intercept 0.063 s). Whole process 3.5-4.5x | e2e song pair NOT runnable (host DiT alone ~37.6 h at 30 steps). Depth decoder/condition mix (bf16-storage), vocoder (no `ConvTranspose1d`) still host. Detail: benchmark-record | diff --git a/docs/STATUS.md b/docs/STATUS.md index cfd9443fd..b900e3fbe 100644 --- a/docs/STATUS.md +++ b/docs/STATUS.md @@ -152,7 +152,7 @@ token-for-token correctness against the pinned oracle. | Laguna-S-2.1 MoE (`LagunaForCausalLM`, 118B/8B) | **BINDING 2026-08-04: was 87% of vLLM (37.55 vs 43.10, same-tool nsys)**; root cause was bf16 projections on UNIFIED/ATS host memory, and device-resident staging (byte-exact) gives 44.6, parity+ vs 43.1, default-ON | 48 layers (12 global + 36 SWA-512), 256 routed top-10 + 1 shared expert, per-head softplus attn out-gate, sigmoid `noaux_tc` router, dual per-layer RoPE, GQA 8 KV / 128 head-dim, 1M ctx. History: benchmark-record | | InternLM2 dense (fused-`wqkv` interleaved split) | Correctness-complete, speed-pending | Token-exact 16/16 (internlm2-chat-1_8b): 12/16 strict + 4/16 bf16 near-tie (max gap 0.0 nats), 0 divergent; first InternLM model; ZERO new compute kernel (reuses the Llama dense forward; the only delta is a loader-side de-interleave of the fused `wqkv`, which packs q/k/v interleaved by KV-group) | | MiniMax-H3 (`MiniMaxH3DiTModel`, video+audio DIFFUSION) | **ABI v12 ONE SURFACE; device selector uses generic `DeviceType`; DSR 32.** t2va+fl2va COHERENT; bf16 shards STREAM | ref2va ckpt fidelity §8.12; encoder A/B §8.15; GB10 re-verify residual; CPU fold 6/137 (one queue + device provenance mutation-gated) | -| LTX-2.5 (`LTX2VideoTransformer3DModel`, video+audio DIFFUSION) | **L1-L9c landed (#435).** 21.00B / 48 blocks. `VideoEngine` seam + ABI **v18**, DiT forward (CPU f32 parity, bf16 device-resident), Gemma-4 TE, both VAEs, connector, pipeline, NVFP4/FP8, keyframe bias (#658) | BOTH shipped DiTs load inside the contract; one runs device-resident on GB10. Caption projection on `vt::MatmulBT` (#1208); IC-LoRA fusion on `vt::Matmul` (#1202), 143x, residual now the add-back (#1254). Render OWED | +| LTX-2.5 (`LTX2VideoTransformer3DModel`, video+audio DIFFUSION) | **L1-L9c landed (#435).** 21.00B / 48 blocks. `VideoEngine` seam + ABI **v18**, DiT forward (CPU f32 parity, bf16 device-resident), Gemma-4 TE, both VAEs, connector, pipeline, NVFP4/FP8, keyframe bias (#658) | BOTH shipped DiTs load inside the contract; one device-resident on GB10. Caption proj on `vt::MatmulBT` (#1208); LoRA fusion on `vt::Matmul` (#1202), add-back residual (#1254). #1286 REFUTED (#1317). Render OWED | | MiniMax-Music3 (`MiniMaxMusic3ForConditionalGeneration`, text-to-MUSIC) | **`ACTIVE`: W0-W7 landed; every stage including the 8.6B LM forward is implemented and gated (#672).** Oracle is the OPEN diffusers PR #14456 `c6da9936` | GGUF arms for 4 components owed. LM gated in a control; HTTP OBSERVED (#852). PARTIAL device arm, Thor sm_110 (#672): 8.6B LM + 2.4B fp32 DiT (§14). Depth 4.45x, wall 2.74x, WAV byte-identical (§16). No reference number | | Command-R / Cohere dense (`CohereForCausalLM`) | Implemented, gate-blocked | ZERO-new-kernel port grounded in vLLM `commandr.py`: weight-only Cohere LayerNorm + GPT-J full-width RoPE + PARALLEL residual + `logit_scale` + tied embeddings, all reuse; compiles, links, self-registers. No SACRED gate yet (real checkpoints HF-gated, ungated ones tiny-random, GPU box disk-full); oracle run-verified at W0. See docs/BENCHMARKS.md | | Phi-1 / Phi-2 dense (`PhiForCausalLM`, parallel residual) | Correctness-complete, speed-pending | Token-exact 16/16 (microsoft/phi-2): 9/16 strict + 7/16 bf16 near-ties (max gap 0.25 nats), 0 forward-divergent; the OLDER Microsoft Phi arch, DISTINCT from Phi-3/Phi-4; ZERO new compute kernel (GPT-J parallel residual, LayerNorm-with-bias, biased qkv/dense, partial NeoX rope 32/80, non-gated NewGELU MLP reusing `vt::GeluTanh`, untied biased lm_head); F16 dtype-aware loader | diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index bed1f3dd6..ac5a27feb 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -1701,6 +1701,17 @@ target_include_directories(test_cuda_quant_dot PRIVATE ${CMAKE_SOURCE_DIR}/src) # domain. Reaches src/ for the tier header and the threadpool test hook. vllm_cpp_add_test(test_ops_matmul_elem vt/test_ops_matmul_elem.cpp) target_include_directories(test_ops_matmul_elem PRIVATE ${CMAKE_SOURCE_DIR}/src) +# LTX25-TEXT-LINEAR-MEM (#1286): the seam's PEAK RESIDENT bound. Every other +# GEMM gate here asserts values or byte equality, so a kernel that allocated a +# whole intermediate per worker would pass all of them. Registered only on +# Linux, because the measurement is /proc/self/{status,clear_refs}; compiling it +# elsewhere with a skip would print `Status: SUCCESS!` over `assertions: 0`, +# which reads as a pass. Reaches src/ for the threadpool test hook, which is what +# lets one process sweep the worker count. +if(CMAKE_SYSTEM_NAME STREQUAL "Linux") + vllm_cpp_add_test(test_ops_matmul_mem vt/test_ops_matmul_mem.cpp) + target_include_directories(test_ops_matmul_mem PRIVATE ${CMAKE_SOURCE_DIR}/src) +endif() # CPU-tier contract for the env-gated cuBLASLt GEMM algo-selection diagnostic # helpers. Reaches the internal header under src/ (not public API); the cuBLASLt # emit it gates is CUDA-only and lives in src/vt/cuda/cuda_matmul.cu. diff --git a/tests/vt/test_ops_matmul_mem.cpp b/tests/vt/test_ops_matmul_mem.cpp new file mode 100644 index 000000000..87595394a --- /dev/null +++ b/tests/vt/test_ops_matmul_mem.cpp @@ -0,0 +1,213 @@ +// Allocation bound for the shared CPU GEMM seam (`vt::MatmulBT`), row +// LTX25-TEXT-LINEAR-MEM, .agents/specs/ltx25-text-linear-mem.md, issue #1286. +// +// WHY THIS FILE EXISTS. #1286 reported that routing the LTX-2.5 caption +// projection through `vt::MatmulBT` (#1252) raised peak host memory by ~26 GiB +// and aborted a full-model render, and named the seam's per-worker buffers as +// the first suspect: "~19 live tiles of a [rows, 188160]-shaped f32 +// intermediate". That attribution is refuted in the spec — the +26 GiB was the +// box's starting occupancy, and the seam's measured cost at 20 workers is +// 232 MiB — but the refutation does not repair what looking for it exposed: +// NOTHING IN THIS TREE BOUNDS THE SEAM'S MEMORY. Every GEMM gate here asserts +// values or byte equality, and a kernel that allocated a whole intermediate per +// worker would pass all of them, on every model, silently, until a box ran out. +// +// So the bound is written now, while the correct number is known, rather than +// after the next report. The property is the one `cpu_ops.cpp:122-125` already +// claims in a comment and nothing checked: the seam's ONLY per-call allocation +// is the widened-activation buffer, sized by ggml's 16-row `blck_1` tile +// (ggml-cpu.c:1192-1194) and NOT by the chunk's row span or by the whole +// activation. Bytes requested across a call are therefore +// `nthreads x 16 x K x 4` and INDEPENDENT of the row count. +// +// WHY THE INSTRUMENT IS AN ALLOCATION COUNTER AND NOT PEAK RSS. Peak RSS is the +// more physical quantity and it was tried first. It cannot carry this gate: the +// operands are freed between measurements, glibc retains the arena, and the +// seam's next tile is then served from pages that are already resident. The +// first draft of this file measured `VmHWM` around `/proc/self/clear_refs` and +// read `growth_bytes=0` for EVERY thread count and EVERY row count while +// passing every bound — a mute switch that would have reported a green over a +// kernel doing anything at all. A replaced global `operator new` counts what +// the seam ASKS FOR, which is the quantity the bound is about, and no allocator +// policy can silence it. +// +// The bound is upper-only, and an explicit LIVENESS case sits beside it: an +// upper bound whose measurement can read zero is not a bound. That case +// requires the counter to see at least six of eight fresh workers take a tile, +// so the numbers the gate compares are known to be real ones. +// +// Linux-only registration (tests/CMakeLists.txt) is not about the counter, +// which is portable, but about `Threadpool`'s worker model and the +// process-lifetime `thread_local` this measures; a case that skipped instead +// would print `Status: SUCCESS!` over `assertions: 0`, which reads as a pass. +#include + +#include +#include +#include +#include +#include +#include +#include + +#include "vt/ops.h" +#include "vt/tensor.h" +#include "vt/cpu/cpu_threadpool.h" // Threadpool::SwapForTesting, via -I src + +using vt::Device; +using vt::DeviceType; +using vt::DType; +using vt::Queue; +using vt::Tensor; + +namespace { + +// Live bytes handed out by the replaced global allocator, and the high-water +// mark of that figure. `relaxed` is enough: the mark is read only after the +// measured call has joined every worker through the pool's final barrier. +std::atomic g_live{0}; +std::atomic g_high{0}; + +void Account(int64_t delta) { + const int64_t live = g_live.fetch_add(delta, std::memory_order_relaxed) + delta; + int64_t high = g_high.load(std::memory_order_relaxed); + while (live > high && + !g_high.compare_exchange_weak(high, live, std::memory_order_relaxed)) { + } +} + +// Every allocation carries its size in a header, because the sized-delete +// overloads are not guaranteed to be the ones the library calls. +constexpr size_t kHeader = 32; // keeps max_align_t alignment for the payload + +void* Alloc(size_t bytes) { + void* raw = std::malloc(bytes + kHeader); + if (raw == nullptr) throw std::bad_alloc(); + *static_cast(raw) = bytes; + Account(static_cast(bytes)); + return static_cast(raw) + kHeader; +} + +void Free(void* p) noexcept { + if (p == nullptr) return; + void* raw = static_cast(p) - kHeader; + Account(-static_cast(*static_cast(raw))); + std::free(raw); +} + +Device Cpu() { return Device{DeviceType::kCPU, 0}; } + +constexpr int64_t kMiB = 1024 * 1024; + +// Bytes the allocator was asked for across one `vt::MatmulBT` on `nthreads` +// workers at [rows, k] x [n, k]. The operands are allocated BEFORE the mark is +// armed, so the figure is the call's own. +// +// The pool is constructed fresh every time. That is deliberate: `af` is a +// process-lifetime `thread_local`, so a reused worker would allocate nothing +// the second time and the measurement would read zero for a reason that has +// nothing to do with the kernel. +int64_t AllocBytesOfMatmulBT(int nthreads, int64_t rows, int64_t k, int64_t n) { + std::vector a(static_cast(rows * k), 0.5f); + std::vector b(static_cast(n * k), 0.25f); + std::vector out(static_cast(rows * n), 0.0f); + + auto pool = std::make_unique(nthreads); + vt::cpu::Threadpool* previous = vt::cpu::Threadpool::SwapForTesting(pool.get()); + + g_high.store(g_live.load(std::memory_order_relaxed), std::memory_order_relaxed); + const int64_t before = g_high.load(std::memory_order_relaxed); + + Queue q{Cpu(), nullptr}; + Tensor a_t = Tensor::Contiguous(a.data(), DType::kF32, Cpu(), {rows, k}); + Tensor b_t = Tensor::Contiguous(b.data(), DType::kF32, Cpu(), {n, k}); + Tensor o_t = Tensor::Contiguous(out.data(), DType::kF32, Cpu(), {rows, n}); + vt::MatmulBT(q, o_t, a_t, b_t); + + const int64_t growth = g_high.load(std::memory_order_relaxed) - before; + vt::cpu::Threadpool::SwapForTesting(previous); + return growth; +} + +// K is large so one tile (16 x K x 4) is far above the noise of any incidental +// allocation the dispatch makes; N is small so a case stays well under a +// second. K = 65536 puts one tile at exactly 4 MiB. +constexpr int64_t kK = 65536; +constexpr int64_t kN = 32; +constexpr int64_t kTileBytes = 16 * kK * 4; + +// The fixed term covers the pool's own per-worker state and the `std::function` +// the dispatch wraps; the per-thread term is doubled so an allocator or a +// future tile that rounds up cannot red the gate. Neither is wide enough to +// admit a second tile-sized buffer per worker, which is what keeps it a bound +// rather than a formality. +int64_t Bound(int nthreads) { + return 8 * kMiB + static_cast(nthreads) * 2 * kTileBytes; +} + +} // namespace + +// Replaced global allocation functions. Standard replacements, so they serve +// the whole program including the seam's `thread_local` buffers. +void* operator new(size_t bytes) { return Alloc(bytes); } +void* operator new[](size_t bytes) { return Alloc(bytes); } +void* operator new(size_t bytes, const std::nothrow_t&) noexcept { + try { + return Alloc(bytes); + } catch (...) { + return nullptr; + } +} +void* operator new[](size_t bytes, const std::nothrow_t&) noexcept { + try { + return Alloc(bytes); + } catch (...) { + return nullptr; + } +} +void operator delete(void* p) noexcept { Free(p); } +void operator delete[](void* p) noexcept { Free(p); } +void operator delete(void* p, size_t) noexcept { Free(p); } +void operator delete[](void* p, size_t) noexcept { Free(p); } +void operator delete(void* p, const std::nothrow_t&) noexcept { Free(p); } +void operator delete[](void* p, const std::nothrow_t&) noexcept { Free(p); } + +TEST_CASE("the allocation counter SEES the seam's per-worker tiles (#1286)") { + REQUIRE(kTileBytes == 4 * kMiB); + // Eight fresh workers, every one of which is handed a starting chunk by + // `MatmulChunked` (`current_chunk = ith`), so every one of them widens a + // tile. Six is the floor rather than eight so the case does not depend on + // the work-stealing cursor handing out a ninth chunk in any particular + // order — but it is far enough above zero that a counter reading nothing + // cannot pass, which is the whole point of this case. + const int64_t growth = AllocBytesOfMatmulBT(8, 256, kK, kN); + INFO("growth_bytes=" << growth << " tile_bytes=" << kTileBytes); + CHECK(growth >= 6 * kTileBytes); +} + +TEST_CASE("vt::MatmulBT allocates the per-worker TILE, not the whole " + "intermediate (#1286)") { + SUBCASE("bytes requested stay inside the tile bound at every worker count") { + for (const int nthreads : {1, 2, 4, 8}) { + const int64_t growth = AllocBytesOfMatmulBT(nthreads, 256, kK, kN); + INFO("nthreads=" << nthreads << " growth_bytes=" << growth + << " bound_bytes=" << Bound(nthreads)); + CHECK(growth >= 0); + CHECK(growth <= Bound(nthreads)); + } + } + + // The half of the claim a thread sweep cannot see, and the one that fails + // when a kernel starts widening the whole activation — the shape #1286 + // hypothesised. 16x the rows must not buy a single extra tile. + SUBCASE("bytes requested do not scale with the ROW count") { + constexpr int kThreads = 4; + const int64_t few = AllocBytesOfMatmulBT(kThreads, 64, kK, kN); + const int64_t many = AllocBytesOfMatmulBT(kThreads, 1024, kK, kN); + INFO("rows=64 growth_bytes=" << few << " rows=1024 growth_bytes=" << many + << " bound_bytes=" << Bound(kThreads)); + CHECK(few <= Bound(kThreads)); + CHECK(many <= Bound(kThreads)); + CHECK(many - few <= kTileBytes); + } +} From 26d2e9d523782580383bdf1f3556c26fdc5f1f93 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Wed, 19 Aug 2026 12:00:40 +0000 Subject: [PATCH 3/3] fix(LTX25-TEXT-LINEAR-MEM): the memory bound admitted three memory regressions, and its review proved all three (#1286) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fresh review of `39b34c0c8` returned nine findings and no PASS. The refutation of #1286 is confirmed and unchanged; what failed review was the gate that landed beside it. Three of the reviewer's mutations wrote an obvious memory regression into `cpu_ops.cpp` and passed **13 of 13 assertions**. **The bound admitted a 2x per-worker regression while its comment said it could not.** `Bound(n)` was `8 MiB + n * 2 * kTileBytes` — a per-thread term of literally two tiles, under a comment claiming it was "not wide enough to admit a second tile-sized buffer per worker". Mutation M-B added exactly that and doubled the growth at every worker count, green. At the shipped geometry that is 232 -> 464 MiB shipping silently. The bound is now `1 MiB + n * kTileBytes`, which is the property the file actually claims, over a fixed term measured at **64 bytes**. The comment now names the worker counts that detect a doubling (4 and up) instead of claiming all of them do. **The counter was bypassed by an over-aligned `operator new`.** Only the non-aligned overloads were replaced, so the C++17 `std::align_val_t` family fell through to the library. M-D delivered the whole-activation defect through a 64-byte-aligned `operator new` — the natural shape for a SIMD scratch buffer in this kernel — and read byte-identical growth, green. All eight aligned `new`/`delete` overloads are now replaced. **`std::malloc` is seen too, through the LINKER rather than a symbol definition.** M-E delivered the same defect through `std::malloc` and the first pass of this repair left it green, narrowing the claim instead. That was the wrong call. `-Wl,--wrap=malloc` and four siblings, scoped to this one target, redirect the calls made by the objects in this link — `libvllm.a`, and so `cpu_ops.cpp` — without defining `malloc`, so AddressSanitizer's interceptor keeps its symbol and there is no second strong definition to collide with. **M-E now reds at 1,086,324,800 bytes.** `free` is deliberately not wrapped, so the C-allocator figure is bytes REQUESTED cumulatively: it errs toward red, and it reads exactly zero on a clean run. A new COVERAGE case makes the whole claim executable — it allocates through plain `new`, array `new`, over-aligned `new` at two alignments, and `std::malloc`, and requires the counter to move by the amount asked for. Two failure modes, two mechanisms: dropping a `--wrap` flag fails the LINK (`undefined reference to '__real_malloc'`), and the subtler "route present but folded away" mode is what the assertion catches. That case calls through a `volatile` function pointer for a measured reason — a direct `std::malloc` in the test's own translation unit is folded by the compiler and never becomes a symbol reference, so it is never wrapped. Measured in a standalone probe: same-TU calls gave `malloc=0 calloc=0 posix_memalign=1`, the identical calls from a separate TU gave `malloc=1 calloc=1 posix_memalign=1`. Product code is always the separate-TU case. **`MatmulOneChunk` had no assertion behind it.** `` and `` are separate instantiations with separate `thread_local` buffers, and the gate called only `vt::MatmulBT` on a non-repacked weight. `vt::Matmul` and `MatmulBTKernel`'s `elem_kn_repacked` lever both route to ``, and `vt::Matmul` is #1259's `Ltx2FuseLoraIntoTensor` path — merged and unexercised on the full model. Both arms are measured now, and the measurement confirms the two buffers are distinct: 8 fresh workers read 33,554,496 bytes on `MatmulBT` and 33,554,496 again on the `vt::Matmul` immediately after, eight tiles both times rather than eight then seven. The retention figure is therefore **per instantiation** — up to 464 MiB, not 232 MiB. **The chunk-row-span defect was invisible at every geometry the gate ran.** M-A (`nrows = ir1_end - iir1`) produced byte-identical growth, green. Replaying `MatmulChunked`'s grid arithmetic over all twelve shapes the sweeps use gives `dr1` = 16 at eleven and 8 at the twelfth — never above 16, which is a coincidence of the chosen shapes rather than a property of the kernel. The collapse to one chunk per thread is LIVE and the shipped default, so a case now runs at `rows = 128, n = 16, nth = 4`, a live non-NUMA shape where it gives `dr1 = 32`. One correction to that finding as written. The review called it "a real ~3x regression the gate cannot see on a NUMA host", citing `dr1 = 52` at `rows = 1024, nth = 20`. `IsNuma()` is `constexpr false` here (`cpu_threadpool.h:74`, NUMA unported per `:25`), so that branch is dead today and the gap is reachable through the live collapse instead. And at the shipped caption projection the weight is the longer axis (`n = 4096 > rows = 1024`), so the collapse takes `nchunk1 = 1` and `dr1 = 1024`, not 52 — 770 MB per worker and 15.4 GB across 20. The finding was right that the gate was blind; the magnitude is larger and the trigger is nearer. The mutation table. Every row rebuilt and re-run; `sha256(cpu_ops.cpp) = dc39ecc...4f1f3` before and after each. | mutation | `BUILT`/`compile_err` | `39b34c0c8` | after | |---|---|---|---| | M-A, chunk row span | `YES`/0 | green 13/13 | exit 1, 2 of 36 failed | | M-A2, whole activation | `YES`/0 | red | exit 1, 14 of 36 failed | | M-B, second tile per worker | `YES`/0 | green 13/13 | exit 1, 10 of 36 failed | | M-D, whole activation via aligned `new` | `YES`/0 | green 13/13 | exit 1, 14 of 36 failed | | M-E, whole activation via `std::malloc` | `YES`/0 | green 13/13 | exit 1, 14 of 38 failed | | M-W, the `--wrap=malloc` flag removed | **`NO`**/1 | n/a | link fails, `undefined reference to '__real_malloc'` | Green after the repair: **5 cases, 38 assertions, 0 failed, exit 0**, against 2 cases and 13 assertions before it. The other mutation rows are M-A (2 of 38 failed), M-A2 (14), M-B (10) and M-D (14), each `BUILT=YES compile_err=0` with `git diff --stat` recorded, and `sha256(cpu_ops.cpp)` restored to `dc39ecc...4f1f3` after every one. The `sanitize-cpu` risk was exercised in two parts, and the second was blocked. The aligned-`operator new` widening ran in the lane's own configuration (`VLLM_CPP_SANITIZE='address,undefined'`, `VLLM_CPP_CUDA=OFF`, `UBSAN_OPTIONS=print_stacktrace=1`): 5 cases, 36 assertions, 0 failed, exit 0, no diagnostic; `thread` likewise, with ASLR disabled to work around this box's `FATAL: unexpected memory mapping`. Rebuilding that lane after `--wrap` was added hit `No space left on device` — other sessions filled the shared disk to 100% — so it was NOT re-run with `--wrap` in place, and this says so rather than implying otherwise. In its place, a standalone probe reproducing the whole mechanism (replaced plain and aligned `operator new`/`delete`, the five `--wrap` redirections, and a separate TU standing in for `cpu_ops.cpp`) built and ran clean under `-fsanitize=address,undefined`: all three C-allocator routes intercepted, alignment preserved, no diagnostic, exit 0. That establishes the mechanism against ASan; it is not a full-lane run. Records repaired in the same change. `cpu_ops.cpp:125` is cited for `static thread_local std::vector af`, which is at **line 130** — `:125` is `const int64_t blck_1 = 16;`. Fixed in the spec, the benchmark-record and the test header. It cannot be fixed in `.agents/issue-index.md:423`, which is append-only and keyed, so the correction is recorded in the spec's `## Owed` and in this body instead; a corrective row would be a duplicate key. The uncovered class behind it is recorded too: `check-symbol-anchors.py:87-90` requires a literal `::`, so of 613 `path::Symbol` and **14,448 `path:line`** citations in this tree's markdown, the second number is checked by nothing. `tests/CMakeLists.txt` justified the Linux-only guard with "the measurement is /proc/self/{status,clear_refs}". The shipped test opens nothing under `/proc`. The real reason is now stated: the gate replaces the global allocation functions for the whole program, so it needs every `delete` in the binary to reach the matching replaced `delete` and its fixed term calibrated against the library doing the allocating — both established for glibc/libstdc++ and nothing else. Three record corrections beside them. The spec's `## Tests to port` and `## Risks/decisions` describe the refuted `VmHWM` instrument, so both now point forward to `## Outcome` W4. `## Outcome` W2's first table was headed "at the shipped geometry" while running `rows = 64`, breaking the spec's own rule at `:158-162`; the reduction is beside the number now. And the "threaded stretch sits at `anon = 34.30`" triple was that stretch's minima at a single sample: recomputed from the retained `memguard.tsv`, the stretch is a **ramp** — `anon` 34.280 -> 42.593, `used` 97.350 -> 105.853 over 236 samples — while the pre-#1252 run genuinely is flat (`anon` 34.283-34.441 over 1676). The two coincide only at the ramp's first sample. W1's argument is peak-minus-own-`t=0` and does not rest on it. `docs/BENCHMARKS.md` and `docs/STATUS.md` carry the new keyed projection, which `check-doc-checkpoint.py` requires of a benchmark-record change: the seam's allocation is bounded on both instantiations, and the process-lifetime retention is 464 MiB rather than 232. No product code changed. `src/vt/cpu/cpu_ops.cpp` is byte-identical to `39b34c0c8`; every mutation above was applied to a scratch tree and restored to the same sha256. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: AGENT:claude-opus-5 [claude-code] --- .agents/benchmark-record.md | 113 ++++++- .agents/specs/ltx25-text-linear-mem.md | 290 +++++++++++++++-- docs/BENCHMARKS.md | 2 +- docs/STATUS.md | 2 +- tests/CMakeLists.txt | 42 ++- tests/vt/test_ops_matmul_mem.cpp | 431 ++++++++++++++++++++++--- 6 files changed, 798 insertions(+), 82 deletions(-) diff --git a/.agents/benchmark-record.md b/.agents/benchmark-record.md index 286b4efd6..ffebc0571 100644 --- a/.agents/benchmark-record.md +++ b/.agents/benchmark-record.md @@ -24171,10 +24171,16 @@ floor and the 8 GiB projection floor, and consistent with the "never below ~40 GiB" the pre-#1252 runs showed. Two readings that each look like support for #1286 in isolation and are not. -First, during its single-core projection the pre-#1252 run is DEAD FLAT at -`anon = 34.35`, `rss = 33.56`, `used = 74.50` from t=124 s to t=1801 s, while -the #1252 run's threaded stretch sits at `anon = 34.30`, `rss = 30.35`, -`used = 97.40` — **the same `anon` to 0.15%**, with `used` 23 GiB apart. Second, +First, during its single-core projection (`cpu ~ 100%`, t=124-1801 s, 1676 +samples) the pre-#1252 run is DEAD FLAT — `anon` 34.283-34.441, `rss` +33.484-33.570, `used` 74.405-74.573 — while the #1252 run's threaded stretch +(`cpu > 500%`, t=137.1-385.5 s, 236 samples) is a RAMP, not a plateau: `anon` +climbs 34.280 -> 42.593, `rss` 30.322 -> 42.120, `used` 97.350 -> 105.853. The +two coincide on `anon` at the ramp's FIRST SAMPLE (34.280 against 34.283) and +nowhere after it, so that near-identity is where the ramp starts rather than a +matching steady state; `used` is 23 GiB apart there and 26.6 GiB apart at the +peak. The section's argument is peak-minus-own-`t=0` and does not rest on any +single sample. Second, NEITHER run ever loaded the DiT (`dit_runs=0` in both; the first was stopped from outside at t=3017.3 s still inside conditioning), so neither peak is a whole-render peak and neither is presented as one. Filed as @@ -24190,7 +24196,7 @@ Release, `-ffp-contract=off`, `K = 188160`, `out_features = 4096`, inside a sensible wall; the tile term the sweep isolates is independent of rows by construction). Box NOT idle, loadavg 5.4-13.3, three replicates, median: -| arm | wall | rate | peak-RSS growth | +| arm (`rows = 64`, reduced) | wall | rate | peak-RSS growth | |---|---:|---:|---:| | before, scalar `double` loop | **28.488 s** | 1.732 GMAC/s | **1.0 MiB** | | after, `vt::MatmulBT` | **3.378 s** | 14.60 GMAC/s | **232 MiB** | @@ -24198,14 +24204,14 @@ by construction). Box NOT idle, loadavg 5.4-13.3, three replicates, median: **8.43x, and +231 MiB** — reproducing #1252's 8.57x, and **0.85% of the 26.6 GiB #1286 attributes to the change**, i.e. 117x too small to be it. -Swept over threadpool width, same geometry: +Swept over threadpool width, same reduced `rows = 64` geometry: -| workers | 1 | 2 | 4 | 8 | 16 | 20 | +| workers (`rows = 64`, reduced) | 1 | 2 | 4 | 8 | 16 | 20 | |---|---:|---:|---:|---:|---:|---:| | peak-RSS growth (MiB) | 12.9 | 24.7 | 47.8 | 93.8 | 186.1 | **232.1** | | per worker (MiB) | 12.9 | 12.3 | 12.0 | 11.7 | 11.6 | 11.6 | -The model is `cpu_ops.cpp:125`'s `static thread_local std::vector af`, +The model is `cpu_ops.cpp:130`'s `static thread_local std::vector af`, sized by ggml's 16-row `blck_1` tile: `16 x 188160 x 4 = 11.48 MiB` per worker. The measurement lands on it, and the output checksum is **byte-identical across all six thread counts** — the dispatch determinism contract holding. @@ -24246,9 +24252,14 @@ against peak RSS, measuring `VmHWM` across `/proc/self/clear_refs`. It read passing every bound: the operands are freed between measurements, glibc keeps the arena, and the next tile is served from pages that are already resident. A mute switch reporting green over a kernel doing anything at all. The shipped -gate counts what the seam ASKS FOR through a replaced global `operator new`, -which no allocator policy can silence, with a liveness case requiring the -counter to see at least six of eight fresh workers take a tile. +gate counts what the seam ASKS FOR through replaced global `operator new` +overloads, a figure the allocator's RETENTION policy cannot silence, with a +liveness case requiring the counter to see at least six of eight fresh workers +take a tile on each of the two orientations. It counts every global +`operator new` -- plain, array, nothrow and the C++17 aligned family -- and +nothing below that: a `std::malloc` or `posix_memalign` in the kernel would be +invisible to it. That is the narrowed claim, and it is narrowed because the +first form of it was broader than the instrument (see the mutation table). ### Mutation M1 — the hypothesis, written into the kernel @@ -24271,9 +24282,83 @@ counter to see at least six of eight fresh workers take a tile. after; rebuilt (binary mtime 07:15:48 -> 07:16:27, so the green is not stale) and green: **2 cases, 13 assertions, 0 failed, exit 0**. +### The review of that bound, and the three regressions it did NOT catch + +The fresh review of `39b34c0c8` returned nine findings and no PASS. The +refutation above was confirmed and is unchanged; what failed was the gate. Three +of the reviewer's mutations passed 13 of 13 assertions over an obvious memory +regression. Every row below was rebuilt and re-run, and +`sha256(cpu_ops.cpp) = dc39eccdece48879e82be7209e95d182c6ed624eb04389de689fa4b58fe4f1f3` +before and after each one. + +| mutation | defect written into `cpu_ops.cpp` | `BUILT`/`compile_err` | at `39b34c0c8` | after the repair | +|---|---|---|---|---| +| M-A | `nrows` = the chunk's row span | `YES`/0 | green, 13/13 | **exit 1**, 2 of 38 failed | +| M-A2 | `nrows` = the whole activation | `YES`/0 | red | **exit 1**, 14 of 38 failed | +| M-B | a SECOND tile-sized buffer per worker | `YES`/0 | green, 13/13 | **exit 1**, 10 of 38 failed | +| M-D | whole activation via 64-byte-aligned `operator new` | `YES`/0 | green, 13/13 | **exit 1**, 14 of 38 failed | +| M-E | whole activation via `std::malloc` | `YES`/0 | green, 13/13 | **exit 1**, 14 of 38 failed, 1,086,324,800 bytes | +| M-W | the `--wrap=malloc` link flag removed | **`NO`**/1 | n/a | **link fails**, `undefined reference to '__real_malloc'` | + +M-E was closed by counting the C allocator through the LINKER rather than by +defining `malloc`: `-Wl,--wrap=malloc` and four siblings, scoped to this target. +That redirects the calls made by the objects in this link (`libvllm.a`, and so +`cpu_ops.cpp`) without introducing a second strong `malloc` definition beside +AddressSanitizer's interceptor. The C-allocator figure reads **zero on a clean +run**, so it adds no noise to any bound. + +The three causes. `Bound(n)` was `8 MiB + n * 2 * kTileBytes`, so its per-thread +term was two tiles and it admitted a doubling at EVERY worker count while its +comment claimed the opposite; it is now `1 MiB + n * kTileBytes` over a fixed +term measured at 64 bytes. Only the non-aligned `operator new` overloads were +replaced, so the C++17 aligned family fell through to the library; all eight are +now replaced. And replaying `MatmulChunked`'s grid arithmetic over all +twelve shapes the sweeps use gives `dr1` = 16 at eleven and 8 at the twelfth, +never above 16, which is the whole reason a chunk-sized buffer was invisible. +The collapse to one chunk per thread is LIVE and the shipped default, so a case +now runs at `rows = 128, n = 16, nth = 4` where it gives `dr1 = 32`. The larger +form is recorded rather than tested: `IsNuma()` is `constexpr false` here +(`cpu_threadpool.h:74`, NUMA unported per `:25`), and where it is implemented it +forces the collapse unconditionally — at the shipped caption projection the +weight is the longer axis (`n = 4096 > rows = 1024`), giving `nchunk1 = 1` and +`dr1 = 1024`, at which a chunk-sized `af` is 770 MB per worker and 15.4 GB +across 20. + +Both `MatmulOneChunk` instantiations are now measured, not just ``. The +liveness case reads 33,554,496 bytes for `vt::MatmulBT` on 8 fresh workers and +33,554,496 again for the `vt::Matmul` call after it — eight tiles both times +rather than eight then seven, which is direct evidence that the two +instantiations hold SEPARATE `thread_local` buffers. The retention figure is +therefore per instantiation: up to **464 MiB**, not 232 MiB, for a process that +runs both orientations at `K = 188160` on 20 workers. + +The reviewed gate's green was REPRODUCED rather than taken on report: the test +file from `39b34c0c8` was restored over the repaired one, M-B applied to +`cpu_ops.cpp`, and the result was `exit 0`, **2 cases, 13 assertions, 0 failed, +`Status: SUCCESS!`** — the reviewed bound passing over a doubled per-worker +allocation in this session's own build. `align_val_t` appears 0 times in the +reviewed file and 13 times in the repaired one, so the aligned half needs no +re-run to establish. The tree was restored to the same `sha256(cpu_ops.cpp)` and +rebuilt green. + +Green after the repair: **5 cases, 38 assertions, 0 failed, exit 0**. The +`sanitize-cpu` lane was run in its own configuration +(`VLLM_CPP_SANITIZE='address,undefined'`, `VLLM_CPP_CUDA=OFF`, +`UBSAN_OPTIONS=print_stacktrace=1`) against the ALIGNED-`operator new` widening: +**5 cases, 36 assertions, 0 failed, exit 0**, no diagnostic. Rebuilding it after +`--wrap` was added hit `No space left on device` (other sessions filled the +shared disk to 100%), so that lane was NOT re-run with `--wrap` in place. In its +place a standalone probe reproducing the whole mechanism — replaced plain and +aligned `operator new`/`delete`, the five `--wrap` redirections, and a separate +TU standing in for `cpu_ops.cpp` — was built and run under +`-fsanitize=address,undefined`: clean build, all three C-allocator routes +intercepted, alignment preserved, no diagnostic, exit 0. That establishes the +mechanism against ASan and is not offered as a full-lane run. + ### Not claimed No GB10 number of any kind. The bound is a CPU bound; `vt::MatmulBT` on -CUDA/ROCm/Vulkan is not covered. No full-model render has been rerun, so nothing -here says what such a run would now do — only that #1252 is not what stopped the -last one. +CUDA/ROCm/Vulkan is not covered. The counter sees every global `operator new` +and nothing below it, so a raw `std::malloc` in the kernel is outside it (M-E +above). No full-model render has been rerun, so nothing here says what such a +run would now do — only that #1252 is not what stopped the last one. diff --git a/.agents/specs/ltx25-text-linear-mem.md b/.agents/specs/ltx25-text-linear-mem.md index 1a81ee013..c1dfeac11 100644 --- a/.agents/specs/ltx25-text-linear-mem.md +++ b/.agents/specs/ltx25-text-linear-mem.md @@ -56,14 +56,14 @@ to bound. Read out of the tree at `origin/main` `678fc672c`: 1. **The seam allocates exactly one thing per call, and it is thread-local.** - `src/vt/cpu/cpu_ops.cpp:125` — `static thread_local std::vector af;`, + `src/vt/cpu/cpu_ops.cpp:130` — `static thread_local std::vector af;`, resized to `nrows * k` where `nrows = min(16, chunk rows)` and 16 is ggml's `blck_1`. Nothing else in `MatmulChunked`, `MatmulOneChunk`, `MatmulOneChunkRef` or `vt::MatmulBT` allocates. At the shipped `k = 188160` that is `16 x 188160 x 4 = 12.04 MB` per worker, and at `hardware_concurrency() = 20` it is **241 MB in total**, not 26 GiB. 2. **`static thread_local` means it is allocated once per worker for the process - lifetime and never shrinks** — the comment at `cpu_ops.cpp:122-124` says so. + lifetime and never shrinks** — the comment at `cpu_ops.cpp:127-129` says so. That is a retention property worth recording; it is not a per-call cost. 3. **The call site's `scaled` copy is pre-existing and unchanged by #1252.** `ltx2_text_encoder.cpp:460-465` builds one `std::vector` the size of @@ -90,6 +90,13 @@ code is written. ## Tests to port +> **This section is the PLAN, and its instrument was refuted.** `VmHWM` read +> `growth_bytes = 0` at every geometry while passing every bound. The shipped +> gate counts bytes requested through replaced global `operator new` overloads +> instead. Read `## Outcome` W4 before reading the paragraph below as a +> description of what landed. The same warning applies to the peak-RSS paragraph +> under `## Risks/decisions`. + None to port: there is no upstream test for a C++ threadpool's peak resident footprint. The new test is original and its contract is stated where it lives. @@ -171,7 +178,10 @@ than deciding it in code. No tolerance is widened to make a red green. is written to bound a *difference* at a geometry where the signal (12 MB per worker) is far above malloc noise, rather than to pin an absolute number. If a robust bound cannot be written, that is reported as a finding instead of being -weakened until it passes. +weakened until it passes. **This risk was realised in the worst available +form** — not a flaky red but a permanent green over a measurement of zero. See +`## Outcome` W4: the peak-RSS instrument was refuted and replaced, and this +paragraph describes the plan rather than what shipped. ## Owed @@ -183,13 +193,61 @@ weakened until it passes. - **No GB10 number.** Every measurement below is from a 20-core Zen 5 under KVM. Allocation sizes are architecture-independent, so the attribution carries; a throughput ratio does not, and none is claimed for GB10. -- **The seam's tile is retained for the process lifetime.** `af` is a - `static thread_local` that grows to the largest `K` any GEMM on that worker - ever saw and never shrinks. At `K = 188160` and 20 workers that is 232 MiB - held until exit, on a box that may later be tight. It is bounded, deliberate - and now measured; it is not repaired here, because the buffer exists precisely - so a worker allocates once, and trading that away is a performance decision - with its own spec. +- **The seam's tile is retained for the process lifetime, PER INSTANTIATION.** + `af` is a `static thread_local` that grows to the largest `K` any GEMM on that + worker ever saw and never shrinks. At `K = 188160` and 20 workers that is + 232 MiB held until exit, on a box that may later be tight. `MatmulOneChunk` + has **two** such buffers, one in each of `` and ``, so a process + that runs both orientations at that `K` retains up to **464 MiB** — measured, + not inferred, in `## Outcome` W5. It is bounded and deliberate; it is not + repaired here, because the buffer exists precisely so a worker allocates once, + and trading that away is a performance decision with its own spec. +- **The counter does not see `mmap`/`sbrk`, nor an allocator reached inside a + shared library's own internal calls.** `--wrap` binds at THIS link, so a + `malloc` that `libstdc++.so` calls internally is outside it; every global + `operator new` is covered regardless, which is what a `std::vector` and a + `std::function` use. A kernel that took pages straight from `mmap` would be + invisible to this gate. Nothing in the seam does, and no mutation here + attempts it, so this is a stated boundary rather than a measured gap. +- **`path:line` citations are an entirely uncovered class of anchor.** + `scripts/check-symbol-anchors.py`'s `CITATION_RE` + (`check-symbol-anchors.py:87-90`) requires a literal `::` between the path and + a C++ identifier, so it cannot match `path:line` at all. Counted over every + `.md` in this tree with the checker's own pattern against the line-anchor + form: **613 `path::Symbol` citations and 14,448 `path:line` citations**, of + which the second number is checked by nothing. That is how the wrong + `cpu_ops.cpp:125` in this row's own records survived the implementation + commit, the merge, and the gate. Extending the checker needs its own spec and + its own red-first mutation and is not attempted here — the 24:1 ratio is the + reason it deserves one rather than a follow-up line. +- **The `cpu_ops.cpp:125` anchor cannot be corrected in `.agents/issue-index.md`.** + Row `#1286` at `issue-index.md:423` cites `cpu_ops.cpp:125` twice for + `static thread_local std::vector af`, which is at **line 130**; `:125` + is `const int64_t blck_1 = 16;`. The index is append-only and keyed, and its + checker reads commits rather than the working tree, so the row cannot be + edited and a corrective row would be a duplicate key. **The correction lives + here and in the pull request body**, which is where a reader following the row + arrives. The same wrong anchor is frozen in the two commit bodies of + `0377cde70` and `8d46223fd` for the same reason. +- **Linux-only registration is a calibration and toolchain scope, not a `/proc` + dependency.** The counter reads nothing under `/proc` and the threadpool is + portable. Three things bind it instead. The gate needs every `delete` in the + binary to reach the matching replaced `delete`; its fixed slack term is + calibrated against the library doing the allocating; and `-Wl,--wrap` is a GNU + `ld`/`lld` feature with no MSVC equivalent, so the C-allocator half of the + counter has no portable form at all. The first two are established for + glibc/libstdc++ and nothing else. Extending the gate to the macOS and Windows + lanes is owed work, and on Windows it needs a different mechanism rather than + a different flag. +- **`runguard.py`'s line citations are unstable and are not chased.** The file + lives outside this repository on a mutable NAS path and was rewritten after + this row's implementation commit, so `runguard.py:236-237,260` no longer + resolves reproducibly. The claim it supports does not rest on the citation: + in the retained `memguard.tsv` the first sample of the #1252 run reads + `avail = 88.078`, `used = 31.553` — summing to 119.631 GiB, the box's whole + `MemTotal` — beside `rss = 0.001` for a child that had just started. A column + that reads 31.553 GiB before the process has allocated anything is + system-wide by construction, whatever line of `runguard.py` computes it. ## Now @@ -232,10 +290,19 @@ floor, and consistent with the "never below ~40 GiB" the pre-#1252 runs showed. Two further readings from the same evidence, recorded because each one on its own would have been read as supporting #1286: -- During its single-core projection the pre-#1252 run sits **dead flat** at - `anon = 34.35`, `rss = 33.56`, `used = 74.50` from t=124 s to t=1801 s. The - #1252 run's threaded stretch sits at `anon = 34.30`, `rss = 30.35`, - `used = 97.40`. **The same `anon` to 0.15%** — with `used` 23 GiB apart. +- During its single-core projection (`cpu ~ 100%`, t=124-1801 s, 1676 samples) + the pre-#1252 run is **dead flat**: `anon` 34.283-34.441, `rss` 33.484-33.570, + `used` 74.405-74.573. The #1252 run's threaded stretch (`cpu > 500%`, + t=137.1-385.5 s, 236 samples) is a **ramp** rather than a plateau: `anon` + climbs 34.280 -> 42.593, `rss` 30.322 -> 42.120, `used` 97.350 -> 105.853. + The two coincide on `anon` at the ramp's FIRST SAMPLE — 34.280 against + 34.283 — and nowhere after it. That near-identity is where the ramp starts, + not a matching steady state, and quoting it as "the threaded stretch sits at" + would present one sample as a stretch. `used` is 23 GiB apart at that sample + and 26.6 GiB apart at the peak, which is the starting occupancy again. What + the reading supports is unchanged: the argument in this section is + peak-minus-own-`t=0` (38.012 against 38.217), which does not depend on any + single sample. - The pre-#1252 run reached t=3017.3 s and was stopped from outside (`# TERMINAL signal=15 (supervisor asked to stop)`), still inside conditioning with `dit_runs=0`. Neither run ever loaded the DiT, so neither @@ -246,12 +313,15 @@ Filed as [#1317](https://github.com/mudler/vllm.cpp/issues/1317). ### W2 — the local A/B: the seam costs 0.24 GiB and keeps the 8.4x Same probe source and flags on both arms, only the `Linear` body differing, at -the shipped `K = 188160` and `out_features = 4096`; peak RSS is `VmHWM` read -before and after the call with every operand already allocated and touched. -20-core Zen 5 under KVM, Release, `-ffp-contract=off`, box **not idle** -(loadavg 5.4-13.3), three replicates, median: - -| arm | wall | rate | peak-RSS growth | +the shipped `K = 188160` and `out_features = 4096` and at **`rows = 64`, +REDUCED from the shipped 1024** to keep the single-threaded arm inside a +sensible wall; the tile term this sweep isolates is independent of `rows` by +construction, and the full `rows = 1024` geometry is measured separately below. +Peak RSS is `VmHWM` read before and after the call with every operand already +allocated and touched. 20-core Zen 5 under KVM, Release, `-ffp-contract=off`, +box **not idle** (loadavg 5.4-13.3), three replicates, median: + +| arm (`rows = 64`, reduced) | wall | rate | peak-RSS growth | |---|---:|---:|---:| | before, scalar `double` loop | **28.488 s** | 1.732 GMAC/s | **1.0 MiB** | | after, `vt::MatmulBT` | **3.378 s** | 14.60 GMAC/s | **232 MiB** | @@ -260,10 +330,10 @@ before and after the call with every operand already allocated and touched. is **0.85% of the 26.6 GiB #1286 attributes to it**, i.e. 117x too small to be the reported defect. -Swept over threadpool width at the same geometry, the growth is exactly the -per-worker tile and nothing else: +Swept over threadpool width at the same reduced `rows = 64` geometry, the growth +is exactly the per-worker tile and nothing else: -| workers | 1 | 2 | 4 | 8 | 16 | 20 | +| workers (`rows = 64`, reduced) | 1 | 2 | 4 | 8 | 16 | 20 | |---|---:|---:|---:|---:|---:|---:| | peak-RSS growth (MiB) | 12.9 | 24.7 | 47.8 | 93.8 | 186.1 | **232.1** | | per worker (MiB) | 12.9 | 12.3 | 12.0 | 11.7 | 11.6 | 11.6 | @@ -362,6 +432,180 @@ the red evidence that the gate measures shipped behaviour rather than a test-local copy. This row adds no product code, so there is no production call site to delete — `reachability.md` answers that case directly. +### W5 — the review of W4, and the three defects the bound above still had + +The fresh review of `39b34c0c8` returned nine findings and no `PASS`. **The +refutation in W1-W3 was confirmed and is unchanged.** What failed review was the +gate itself, which is the irony worth recording: a bound written to catch a +memory regression did not catch three obvious shapes of one. The repairs and +their evidence: + +**The bound admitted a 2x per-worker regression while its comment said it could +not.** `Bound(n)` was `8 MiB + n * 2 * kTileBytes`. The per-thread term was +literally two tiles, and the fixed term was two more at the gate's `K`. The +reviewer's M-B — a second tile-sized `thread_local` per worker — doubled the +growth at every worker count and passed **13 of 13 assertions**. At the shipped +geometry that is 232 -> 464 MiB landing silently. The bound is now +`1 MiB + n * kTileBytes`: exactly the property the file claims, over a fixed +term measured at **64 bytes**. The comment now states which worker counts detect +a doubling (4 and up) instead of claiming it detects all of them. + +**The counter was bypassed by an over-aligned `operator new`.** Only the +non-aligned overloads were replaced, so the C++17 `std::align_val_t` family fell +through to the library. The reviewer's M-D delivered the whole-activation defect +through a 64-byte-aligned `operator new` and read **byte-identical growth, +13/13 green**. That is the natural shape for a SIMD scratch buffer in this +kernel, not a corner case. All eight aligned `new`/`delete` overloads are now +replaced. + +**`std::malloc` is now seen too, through the linker rather than through a +symbol definition.** The reviewer's M-E delivered the same defect through +`std::malloc`, and the first pass of this repair left it green and narrowed the +claim instead. That was the wrong call, and re-examining it produced a mechanism +with none of the objections: `-Wl,--wrap=malloc` (and `calloc`, `realloc`, +`aligned_alloc`, `posix_memalign`), scoped to this one target. It redirects the +CALLS made by the objects in this link — `libvllm.a`, and so `cpu_ops.cpp` — +without defining `malloc`, so AddressSanitizer's interceptor keeps its symbol +and there is no second strong definition to collide with. **M-E now reds at +1,086,324,800 bytes.** `free` is deliberately not wrapped: releasing a block +gives no size without a header, and a header would mean applying an offset to +pointers libc allocated before this file was reached, which is heap corruption +rather than a failed assertion. So the C-allocator figure is bytes REQUESTED, +cumulatively, which errs toward red and never toward a silent green — and it +reads exactly **zero** on a clean run, so it adds no noise to any bound. + +A **coverage case** makes the whole claim executable rather than prose: it +allocates through each counted route — plain `new`, array `new`, over-aligned +`new` at two alignments, and `std::malloc` — and requires the counter to move by +the amount asked for. Two failure modes are covered by two different mechanisms. +Dropping a `--wrap` flag fails the LINK, because the file references +`__real_malloc` directly (mutation **M-W**: `BUILT=NO`, `undefined reference to +'__real_malloc'`), so the coverage cannot silently vanish from the build. The +subtler mode — the route present but folded away — is what the assertion +catches. + +**One thing the probe taught, which is why that case calls through a `volatile` +function pointer.** A direct `std::malloc(n)` written in THIS translation unit is +never wrapped: the compiler treats it as a builtin and folds it before the +linker sees a symbol reference. Measured in a standalone probe — same-TU calls +gave `malloc=0 calloc=0 posix_memalign=1`, while the identical calls made from a +SEPARATE translation unit gave `malloc=1 calloc=1 posix_memalign=1` and the full +3 MiB. Product code is always the separate-TU case, which is why M-E reds; but a +coverage probe written the obvious way would have asserted nothing. + +**`MatmulOneChunk` had no assertion behind it.** `` and `` +are separate instantiations with separate `thread_local` buffers, and the gate +called only `vt::MatmulBT` on a non-repacked weight. `MatmulKernel` +(`cpu_ops.cpp:292-294`) routes `vt::Matmul` to ``, and `MatmulBTKernel`'s +`elem_kn_repacked` lever (`cpu_ops.cpp:306-314`) routes there too; `vt::Matmul` +is #1259's `Ltx2FuseLoraIntoTensor` path — merged and unexercised on the full +model. Both arms are now measured. The two `af` symbols are **separately +confirmed with `nm -C`** on the gate's own binary, at distinct addresses, and +the measurement shows the same thing: in the liveness case `MatmulBT` on 8 fresh workers reads +33,554,496 bytes and the `vt::Matmul` call immediately after reads +33,554,496 again. Eight tiles both times, not eight then seven — the calling +thread paid for a second buffer, which one shared buffer could not produce. So +the "232 MiB held until exit" figure under `## Owed` is **per instantiation**. + +**The chunk-row-span defect was invisible at every geometry the gate ran.** The +reviewer's M-A (`nrows = ir1_end - iir1`) produced byte-identical growth and +13/13 green. Replaying `MatmulChunked`'s grid arithmetic +(`cpu_ops.cpp:230-264`) over all twelve shapes the sweeps use gives `dr1` = 16 +at eleven of them and 8 at the twelfth. Never above 16, which is the entire +reason M-A could not be seen — and a coincidence of the chosen shapes, not a +property of the kernel. The collapse to one chunk per thread +(`nchunk0 * nchunk1 < nth * 4`) is LIVE and is the shipped default, since +`VT_CPU_MATMUL_STEAL` is off, and after it `dr1` is `ceil(rows/nth)` when the +activation is the longer axis and `rows` outright when the weight is. This was +cheap to close rather than to file, so a case now runs at `rows = 128, n = 16, +nth = 4` — a live non-NUMA shape where the collapse gives `dr1 = 32` and a +chunk-sized buffer costs two tiles per worker instead of one. + +**One correction to the finding as it was written.** The review described this +as "a real ~3x regression the gate cannot see on a NUMA host", citing +`dr1 = ceil(rows/nth) = 52` at `rows = 1024, nth = 20`. Two things are off. +`IsNuma()` is `constexpr false` in this tree (`cpu_threadpool.h:74`; NUMA is +recorded as unported at `cpu_threadpool.h:25`), so that branch is dead today and +the gap is reachable through the LIVE collapse instead. And at the shipped +LTX-2.5 caption projection the weight is the longer axis +(`n = 4096 > rows = 1024`), so the collapse takes `nchunk1 = 1` and +`dr1 = 1024`, not 52 — a chunk-sized `af` there would be 770 MB per worker and +15.4 GB across 20, which is #1286's hypothesised shape almost exactly. The +finding was right that the gate was blind; the magnitude is larger than it +said, and the trigger is nearer. + +**The mutation table**, on `Release`, 20-core Zen 5 under KVM, this branch's +head. Every row rebuilt and re-run; `sha256(cpu_ops.cpp)` was +`dc39eccdece48879e82be7209e95d182c6ed624eb04389de689fa4b58fe4f1f3` before and +after every one of them. + +| mutation | defect | `BUILT` / `compile_err` | `git diff --stat` on `src/` | before this repair | after | +|---|---|---|---|---|---| +| **M-A** | `nrows` = chunk row span | `YES` / 0 | `cpu_ops.cpp \| 2 +-` | green (13/13) | **exit 1**, 2 failed of 38, `33,554,496 <= 17,825,792` on both arms of the collapsed case | +| **M-A2** | `nrows` = whole activation | `YES` / 0 | `cpu_ops.cpp \| 2 +-` | red | **exit 1**, 14 failed of 38, `67,108,928 <= 9,437,184` | +| **M-B** | second tile-sized buffer per worker | `YES` / 0 | `cpu_ops.cpp \| 3 +` | green (13/13) | **exit 1**, 10 failed of 38, `25,165,888 <= 17,825,792` at 4 workers and `58,720,320 <= 34,603,008` at 8 | +| **M-D** | whole activation via 64-byte-aligned `operator new` | `YES` / 0 | `cpu_ops.cpp \| 12 +` | green (13/13) | **exit 1**, 14 failed of 38, `1,019,215,936` bytes at `rows=1024`, 4 workers | +| **M-E** | whole activation via `std::malloc` | `YES` / 0 | `cpu_ops.cpp \| 12 +` | green (13/13) | **exit 1**, 14 failed of 38, `71,303,232 <= 9,437,184` at 2 workers and `1,086,324,800` bytes at `rows=1024`, 4 workers | +| **M-W** | the `--wrap=malloc` link flag removed | **`NO`** / 1 | `CMakeLists.txt \| 1 -` | n/a | **link fails**, `undefined reference to '__real_malloc'` — the coverage cannot be dropped silently | + +**The reviewer's green was reproduced rather than taken on report.** The gate +file from `39b34c0c8` was restored over the repaired one, M-B applied to +`cpu_ops.cpp`, and the result was `exit 0`, **2 cases, 13 assertions, 0 failed, +`Status: SUCCESS!`** — the reviewed bound passing over a doubled per-worker +allocation, in this session's own build. The aligned half needs no re-run to +confirm: `align_val_t` appears **0 times** in the reviewed file and 13 times in +the repaired one, so the C++17 family was simply not replaced. The tree was +restored to the same `sha256(cpu_ops.cpp)` afterwards and rebuilt green. + +Green after the repair: **5 cases, 38 assertions, 0 failed, exit 0**, +`Status: SUCCESS!`, against 2 cases and 13 assertions before it. + +**The `sanitize-cpu (address,undefined)` risk the reviewer named was exercised, +and the record is in two parts because the second attempt was blocked.** + +The FIRST widening — the aligned `operator new` family, before `--wrap` was +added — ran in the full lane's own configuration +(`-DVLLM_CPP_SANITIZE='address,undefined'`, `-DVLLM_CPP_CUDA=OFF`, +`UBSAN_OPTIONS=print_stacktrace=1`): **5 cases, 36 assertions, 0 failed, exit +0**, no ASan or UBSan diagnostic. The `thread` lane was green too, once ASLR was +disabled to work around this box's `FATAL: ThreadSanitizer: unexpected memory +mapping` — a local kernel/TSan incompatibility, not a finding. + +**Rebuilding that lane after adding `--wrap` hit `No space left on device`** — +other sessions filled the shared disk to 100%, and the build's first output was +five `fatal error: error writing to /tmp/...: No space left on device` lines, +which is an infrastructure failure wearing the costume of a code verdict. So the +full lane was NOT re-run with `--wrap` in place, and that is stated rather than +implied. + +What ran in its place is a standalone probe reproducing the gate's entire +allocator mechanism in miniature — replaced plain and aligned +`operator new`/`delete`, the five `--wrap` redirections, and a separate +translation unit standing in for `cpu_ops.cpp` — compiled and linked with +`-fsanitize=address,undefined`. It **builds clean and runs clean**: all three C +allocator routes intercepted (3 MiB accounted), 64-byte alignment preserved +through the replaced aligned `new`, replaced `operator new` accounting correct, +no ASan or UBSan diagnostic, exit 0. That establishes the MECHANISM against +ASan, which is precisely the collision the reviewer named. It does not stand in +for a full-lane run and is not offered as one. + +**Full gate on the merged tree.** `ctest -j 6` at `origin/main` `edbc47ce0`: +**100% passed, 0 failed out of 554**, 3 skipped by design, 1593.98 s. The box +was heavily contended throughout — two other sessions were building and testing +this tree, loadavg 109-149 on 20 cores — so `test_ltx2_video` took 1541 s where +this branch's earlier run took a fraction of that. **Nothing red, and no +wall-time figure here is a measurement of anything.** `check-agent-record.py`: +`agent record OK: ENGINE=165 MODEL=377 QUANT=84 KERNEL=52 BACKEND=85 +ANCHOR-ROT=38`. `check-issue-index-append-only.py`: `OK`, 413 rows, zero +duplicate keys, #1286 and #1317 each present once. `check-commit-trailers.py` +and `check-commit-style.py` over `origin/main..HEAD`: both `OK`. + +**The bound is deliberately tight now, and that is the point.** At 8 workers the +honest worst case is 8 tiles (every worker fresh, which the liveness case +reaches) = 33,554,496 bytes against a bound of 34,603,008. The 1 MiB of headroom +is 16,000x the measured incidental. Widening it again would buy nothing except +the 2x hole M-B just walked through. + ### What is not claimed - No GB10 measurement of any kind, so #1252's unmeasured per-core ratio `R` is diff --git a/docs/BENCHMARKS.md b/docs/BENCHMARKS.md index a5add59c9..f07788296 100644 --- a/docs/BENCHMARKS.md +++ b/docs/BENCHMARKS.md @@ -502,7 +502,7 @@ built on it rather than keeping the flattering one. | Kimi-Linear-48B-A3B (KDA+MLA+MoE) | **RUNNER FOLD LANDS (ROW 7, §21, #122): engine==CLI 128/128 byte-identical; vs golden 122/128 (near-tie profile); FA2 MLA default-ON; SACRED green.** Server 19.0 tok/s wall; CLI 18.93 reproduced | vLLM ~21 (#111 floor; in-session re-measure ABORTED by GB10 reboot at util 0.82, §21): **~0.90×**, >= vLLM NOT met; residual = KDA host islands + grouped MoE + decode graph | | vLLM 0.26 re-benchmark | Pending | Re-run the binding grids on the advanced pin | | MiniMax-H3 FP4 speed (W-FP4a) | **Measured GB10 (`row/H3-FP4-GPU-E2E`).** Marlin W4A16 byte-exact vs bf16; fp4 a memory win, 0.8x bf16/forward. Real-ckpt fp4-resident e2e RUNS (mp4/wav) | fp4 speed CLOSED. bf16-vs-quant A/B: ENCODER half MEASURED (§8.15), DiT half NOT (no bf16 render exists). Detail: benchmark-record + spec §8 | -| LTX-2.5 axes | Speed `PENDING` (vllm-omni#6066 has no native 2.5), binding oracle too. **SIZE: 704x448/25f and 448x256/25f both COMPLETE on GB10 (4231 s, 3085 s)**; one run each, contended box, no oracle, no ceiling (#1088) | NOT the VAE decode (#1041/#1009): 39-100% of the ~1731 s phase is the caption projection (#1208), x86 671.8->78.4 s; LoRA fusion 17.78->0.124 s (#1202), add-back 59% of the rest (#1254). **#1286 +26 GiB REFUTED (#1317)** | +| LTX-2.5 axes | Speed `PENDING` (vllm-omni#6066 has no native 2.5), binding oracle too. **SIZE: 704x448/25f and 448x256/25f both COMPLETE on GB10 (4231 s, 3085 s)**; one run each, contended box, no oracle, no ceiling (#1088) | NOT the VAE decode (#1041/#1009): 39-100% of the ~1731 s phase is the caption projection (#1208), x86 671.8→78.4 s; LoRA fusion 17.78→0.124 s (#1202), add-back 59% (#1254). **#1286 REFUTED (#1317)**; alloc bound 464 MiB | | MiniMax-Music3 (`MiniMaxMusic3ForConditionalGeneration`) | **Every axis vs the reference stays `PENDING`.** A PARTIAL device arm now exists (#672): the 8.6B LM and the 2.4B fp32 DiT run on the accelerator, so the rows below are internal two-arm numbers and NOT parity ratios | Denominator: SGLang-Omni `748a0b43` in its production configuration (both CUDA graphs, compiled DIT and DAV, batched seeded sampling) | | MiniMax-Music3 device arm, Jetson Thor sm_110 (#672) | `--device 1` vs `--device 0`, same request/seed, idle box: 2 AR frames **846.6 vs 835.1 s (1.014x SLOWER)**; 10 frames **1430.4 vs 1512.1 s (0.946x)**. Fit: **-11.65 s/frame, +34.8 s fixed** | A third duration (the fit has no residual), and moving the depth decoder + DiT + vocoder, which are 5 of 6 stages and still host scalar loops | | MiniMax-Music3 DiT device arm, `thor:gpu0` sm_110 (#672) | Per DiT forward at the capture's geometry, same binary/weights/inputs, idle box: **204.955 s host vs 0.186-0.187 s device, 1094-1102x** (1201x fitted). Staged ONCE (0.61 s; loop intercept 0.063 s). Whole process 3.5-4.5x | e2e song pair NOT runnable (host DiT alone ~37.6 h at 30 steps). Depth decoder/condition mix (bf16-storage), vocoder (no `ConvTranspose1d`) still host. Detail: benchmark-record | diff --git a/docs/STATUS.md b/docs/STATUS.md index a3007194a..94533814b 100644 --- a/docs/STATUS.md +++ b/docs/STATUS.md @@ -152,7 +152,7 @@ token-for-token correctness against the pinned oracle. | Laguna-S-2.1 MoE (`LagunaForCausalLM`, 118B/8B) | **BINDING 2026-08-04: was 87% of vLLM (37.55 vs 43.10, same-tool nsys)**; root cause was bf16 projections on UNIFIED/ATS host memory, and device-resident staging (byte-exact) gives 44.6, parity+ vs 43.1, default-ON | 48 layers (12 global + 36 SWA-512), 256 routed top-10 + 1 shared expert, per-head softplus attn out-gate, sigmoid `noaux_tc` router, dual per-layer RoPE, GQA 8 KV / 128 head-dim, 1M ctx. History: benchmark-record | | InternLM2 dense (fused-`wqkv` interleaved split) | Correctness-complete, speed-pending | Token-exact 16/16 (internlm2-chat-1_8b): 12/16 strict + 4/16 bf16 near-tie (max gap 0.0 nats), 0 divergent; first InternLM model; ZERO new compute kernel (reuses the Llama dense forward; the only delta is a loader-side de-interleave of the fused `wqkv`, which packs q/k/v interleaved by KV-group) | | MiniMax-H3 (`MiniMaxH3DiTModel`, video+audio DIFFUSION) | **ABI v12 ONE SURFACE; device selector uses generic `DeviceType`; DSR 32.** t2va+fl2va COHERENT; bf16 shards STREAM | ref2va ckpt fidelity §8.12; encoder A/B §8.15; GB10 re-verify residual; CPU fold 6/137 (one queue + device provenance mutation-gated) | -| LTX-2.5 (`LTX2VideoTransformer3DModel`, video+audio DIFFUSION) | **L1-L9c landed (#435).** 21.00B / 48 blocks. `VideoEngine` seam + ABI **v18**, DiT forward (CPU f32 parity, bf16 device-resident), Gemma-4 TE, both VAEs, connector, pipeline, NVFP4/FP8, keyframe bias (#658) | BOTH shipped DiTs load inside the contract; one device-resident on GB10. Caption proj on `vt::MatmulBT` (#1208); LoRA fusion on `vt::Matmul` (#1202), add-back residual (#1254). #1286 REFUTED (#1317). Render OWED | +| LTX-2.5 (`LTX2VideoTransformer3DModel`, video+audio DIFFUSION) | **L1-L9c landed (#435).** 21.00B / 48 blocks. `VideoEngine` seam + ABI **v18**, DiT forward (CPU f32 parity, bf16 device-resident), Gemma-4 TE, both VAEs, connector, pipeline, NVFP4/FP8, keyframe bias (#658) | BOTH shipped DiTs load inside the contract; one device-resident on GB10. Caption proj on `vt::MatmulBT` (#1208); LoRA fusion on `vt::Matmul` (#1202), add-back (#1254). #1286 REFUTED (#1317), alloc gated. Render OWED | | MiniMax-Music3 (`MiniMaxMusic3ForConditionalGeneration`, text-to-MUSIC) | **`ACTIVE`: W0-W7 landed; every stage including the 8.6B LM forward is implemented and gated (#672).** Oracle is the OPEN diffusers PR #14456 `c6da9936` | GGUF arms for 4 components owed. LM gated in a control; HTTP OBSERVED (#852). PARTIAL device arm, Thor sm_110 (#672): 8.6B LM + 2.4B fp32 DiT (§14). Depth 4.45x, wall 2.74x, WAV byte-identical (§16). No reference number | | Command-R / Cohere dense (`CohereForCausalLM`) | Implemented, gate-blocked | ZERO-new-kernel port grounded in vLLM `commandr.py`: weight-only Cohere LayerNorm + GPT-J full-width RoPE + PARALLEL residual + `logit_scale` + tied embeddings, all reuse; compiles, links, self-registers. No SACRED gate yet (real checkpoints HF-gated, ungated ones tiny-random, GPU box disk-full); oracle run-verified at W0. See docs/BENCHMARKS.md | | Phi-1 / Phi-2 dense (`PhiForCausalLM`, parallel residual) | Correctness-complete, speed-pending | Token-exact 16/16 (microsoft/phi-2): 9/16 strict + 7/16 bf16 near-ties (max gap 0.25 nats), 0 forward-divergent; the OLDER Microsoft Phi arch, DISTINCT from Phi-3/Phi-4; ZERO new compute kernel (GPT-J parallel residual, LayerNorm-with-bias, biased qkv/dense, partial NeoX rope 32/80, non-gated NewGELU MLP reusing `vt::GeluTanh`, untied biased lm_head); F16 dtype-aware loader | diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index b5681ca47..d8321c7a3 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -1702,16 +1702,46 @@ target_include_directories(test_cuda_quant_dot PRIVATE ${CMAKE_SOURCE_DIR}/src) # domain. Reaches src/ for the tier header and the threadpool test hook. vllm_cpp_add_test(test_ops_matmul_elem vt/test_ops_matmul_elem.cpp) target_include_directories(test_ops_matmul_elem PRIVATE ${CMAKE_SOURCE_DIR}/src) -# LTX25-TEXT-LINEAR-MEM (#1286): the seam's PEAK RESIDENT bound. Every other -# GEMM gate here asserts values or byte equality, so a kernel that allocated a -# whole intermediate per worker would pass all of them. Registered only on -# Linux, because the measurement is /proc/self/{status,clear_refs}; compiling it +# LTX25-TEXT-LINEAR-MEM (#1286): the seam's ALLOCATION bound, on both +# `MatmulOneChunk` instantiations. Every other GEMM gate here asserts values or +# byte equality, so a kernel that allocated a whole intermediate per worker +# would pass all of them. It counts bytes REQUESTED through replaced global +# `operator new` overloads, not peak RSS -- the peak-RSS draft read zero at +# every geometry while passing every bound, because glibc retains the arena +# (spec `## Outcome` W4). +# +# Registered only on Linux, and NOT because of /proc or the threadpool: the +# counter reads nothing under /proc and the pool is portable. It is that the +# replaced allocation functions serve the WHOLE PROGRAM, so the gate depends on +# every `delete` in the binary reaching the matching replaced `delete`, and on +# its fixed slack term being calibrated against the library doing the +# allocating. Both hold for glibc/libstdc++ and are unestablished elsewhere; a +# standard library linked as its own runtime keeps its own `operator new`, and +# the mismatch is a heap corruption rather than a red assertion. Compiling it # elsewhere with a skip would print `Status: SUCCESS!` over `assertions: 0`, -# which reads as a pass. Reaches src/ for the threadpool test hook, which is what -# lets one process sweep the worker count. +# which reads as a pass. Extending the lanes is owed, not a one-line change. +# Reaches src/ for the threadpool test hook, which is what lets one process +# sweep the worker count. if(CMAKE_SYSTEM_NAME STREQUAL "Linux") vllm_cpp_add_test(test_ops_matmul_mem vt/test_ops_matmul_mem.cpp) target_include_directories(test_ops_matmul_mem PRIVATE ${CMAKE_SOURCE_DIR}/src) + # The C allocator is counted through the LINKER, not through a symbol + # definition. `--wrap` redirects the calls made by the objects in this link -- + # which includes libvllm.a, and so cpu_ops.cpp -- while leaving `malloc` itself + # defined by whoever normally defines it. Defining `malloc` in the test would + # instead be a second strong definition beside AddressSanitizer's own + # interceptor, which is what would break the sanitize-cpu lane. Scoped to this + # one target, so nothing else in the suite is affected. + # + # These flags are LOAD-BEARING, not an optimization: the test references + # `__real_malloc` directly, so dropping one fails the LINK rather than quietly + # costing the gate its malloc coverage. Verified by mutation (M-W). + target_link_options(test_ops_matmul_mem PRIVATE + -Wl,--wrap=malloc + -Wl,--wrap=calloc + -Wl,--wrap=realloc + -Wl,--wrap=aligned_alloc + -Wl,--wrap=posix_memalign) endif() # CPU-tier contract for the env-gated cuBLASLt GEMM algo-selection diagnostic # helpers. Reaches the internal header under src/ (not public API); the cuBLASLt diff --git a/tests/vt/test_ops_matmul_mem.cpp b/tests/vt/test_ops_matmul_mem.cpp index 87595394a..5cfcdc11e 100644 --- a/tests/vt/test_ops_matmul_mem.cpp +++ b/tests/vt/test_ops_matmul_mem.cpp @@ -1,5 +1,6 @@ -// Allocation bound for the shared CPU GEMM seam (`vt::MatmulBT`), row -// LTX25-TEXT-LINEAR-MEM, .agents/specs/ltx25-text-linear-mem.md, issue #1286. +// Allocation bound for the shared CPU GEMM seam (`vt::Matmul` and +// `vt::MatmulBT`), row LTX25-TEXT-LINEAR-MEM, +// .agents/specs/ltx25-text-linear-mem.md, issue #1286. // // WHY THIS FILE EXISTS. #1286 reported that routing the LTX-2.5 caption // projection through `vt::MatmulBT` (#1252) raised peak host memory by ~26 GiB @@ -13,13 +14,23 @@ // worker would pass all of them, on every model, silently, until a box ran out. // // So the bound is written now, while the correct number is known, rather than -// after the next report. The property is the one `cpu_ops.cpp:122-125` already +// after the next report. The property is the one `cpu_ops.cpp:127-129` already // claims in a comment and nothing checked: the seam's ONLY per-call allocation -// is the widened-activation buffer, sized by ggml's 16-row `blck_1` tile -// (ggml-cpu.c:1192-1194) and NOT by the chunk's row span or by the whole -// activation. Bytes requested across a call are therefore +// is the widened-activation buffer at `cpu_ops.cpp:130`, sized by ggml's 16-row +// `blck_1` tile (ggml-cpu.c:1192-1194) and NOT by the chunk's row span or by +// the whole activation. Bytes requested across a call are therefore // `nthreads x 16 x K x 4` and INDEPENDENT of the row count. // +// BOTH TEMPLATE INSTANTIATIONS ARE COVERED, because there are two buffers and +// not one. `MatmulOneChunk` and `MatmulOneChunk` are separate +// instantiations, each with its OWN `static thread_local af` (confirmed with +// `nm -C`), so the process-lifetime retention figure is per-instantiation and a +// process using both orientations at a large `K` retains up to twice it. The +// `` arm is not a corner: `MatmulKernel` (`cpu_ops.cpp:292-294`) routes +// `vt::Matmul` there, which is what #1259's `Ltx2FuseLoraIntoTensor` calls, and +// `MatmulBTKernel`'s `b.elem_kn_repacked` lever (`cpu_ops.cpp:306-314`) routes +// there too. +// // WHY THE INSTRUMENT IS AN ALLOCATION COUNTER AND NOT PEAK RSS. Peak RSS is the // more physical quantity and it was tried first. It cannot carry this gate: the // operands are freed between measurements, glibc retains the arena, and the @@ -27,18 +38,55 @@ // first draft of this file measured `VmHWM` around `/proc/self/clear_refs` and // read `growth_bytes=0` for EVERY thread count and EVERY row count while // passing every bound — a mute switch that would have reported a green over a -// kernel doing anything at all. A replaced global `operator new` counts what -// the seam ASKS FOR, which is the quantity the bound is about, and no allocator -// policy can silence it. +// kernel doing anything at all. Replaced global allocation functions count what +// the seam ASKS FOR, which is the quantity the bound is about, and that figure +// is immune to the allocator's RETENTION policy. +// +// EXACTLY WHAT THE COUNTER SEES, because a broad claim over a narrow instrument +// is how the first draft went wrong. Two mechanisms, and the second exists only +// because a review proved the first was not enough. +// +// 1. It replaces the complete set of global `operator new`/`operator delete` +// overloads: plain, array, `nothrow`, and the C++17 `std::align_val_t` +// family. That covers every C++ allocation in the program, including every +// `std::vector` and every `std::function`, whatever alignment it asks for. +// The aligned family was MISSING from the first version, and a +// whole-activation defect delivered through a 64-byte-aligned +// `operator new` read as byte-identical growth and passed every bound. +// 2. It counts the C allocator through the LINKER -- `-Wl,--wrap=malloc` and +// friends in tests/CMakeLists.txt -- rather than by defining `malloc`. A +// definition would be a second strong symbol beside AddressSanitizer's own +// interceptor and would break the `sanitize-cpu` lane; `--wrap` redirects +// the CALLS made by the objects in this link, which includes `libvllm.a` and +// so `cpu_ops.cpp`, and leaves every symbol where it was. // -// The bound is upper-only, and an explicit LIVENESS case sits beside it: an -// upper bound whose measurement can read zero is not a bound. That case -// requires the counter to see at least six of eight fresh workers take a tile, -// so the numbers the gate compares are known to be real ones. +// What is still NOT covered, stated rather than left to be found: `mmap` and +// `sbrk` called directly, any allocator reached inside a shared library's own +// internal calls (`--wrap` binds at this link, not inside `libstdc++.so`), and +// `malloc` called from within THIS translation unit, which the compiler folds +// before the linker ever sees a symbol reference. The last one is measured, not +// supposed, and it is why the coverage case below calls through a `volatile` +// function pointer. // -// Linux-only registration (tests/CMakeLists.txt) is not about the counter, -// which is portable, but about `Threadpool`'s worker model and the -// process-lifetime `thread_local` this measures; a case that skipped instead +// The bound is upper-only, and two cases sit beside it because an upper bound +// whose measurement can read zero is not a bound. The COVERAGE case allocates +// through each replaced route and requires the counter to move by the amount +// asked for, so a lost overload reds instead of muting the gate. The LIVENESS +// case requires the counter to see at least six of eight fresh workers take a +// tile on each orientation, so the numbers the gate compares are known to be +// real ones. +// +// Registration is Linux-only (tests/CMakeLists.txt) and the reason is neither +// `/proc` nor the threadpool: the counter and the pool are both portable. It is +// that this gate replaces the global allocation functions FOR THE WHOLE PROGRAM, +// so it depends on every `delete` in the binary reaching the matching replaced +// `delete` — and on the fixed slack term below being calibrated against the +// library that does the allocating. Both were established against +// glibc/libstdc++ and against nothing else. A standard library linked as a +// separate runtime keeps its own `operator new`, and a pointer it allocated but +// this file frees would have the header offset applied to memory it never +// handed out, which is a crash rather than a failed assertion. Extending the +// registration is owed work, not a one-line change; a case that skipped instead // would print `Status: SUCCESS!` over `assertions: 0`, which reads as a pass. #include @@ -48,6 +96,7 @@ #include #include #include +#include #include #include "vt/ops.h" @@ -76,12 +125,30 @@ void Account(int64_t delta) { } } +// Bytes requested through the C allocator inside the armed window. This one is +// CUMULATIVE rather than a high-water mark, because the wrappers below do not +// intercept `free` and so cannot know a released block's size. Cumulative +// bytes-requested is the quantity this file's contract already names, and for a +// buffer allocated once per worker the two agree exactly. It errs toward RED +// and never toward a silent green, and it reads zero on a clean run. +std::atomic g_raw{0}; + +// The linker's `--wrap` originals (tests/CMakeLists.txt). Declared here so the +// counter's own bookkeeping allocations go STRAIGHT to libc and are not counted +// twice -- and referencing them is what makes a dropped `--wrap` flag a LINK +// FAILURE rather than a silent loss of coverage. +extern "C" void* __real_malloc(size_t); +extern "C" void* __real_calloc(size_t, size_t); +extern "C" void* __real_realloc(void*, size_t); +extern "C" void* __real_aligned_alloc(size_t, size_t); +extern "C" int __real_posix_memalign(void**, size_t, size_t); + // Every allocation carries its size in a header, because the sized-delete // overloads are not guaranteed to be the ones the library calls. constexpr size_t kHeader = 32; // keeps max_align_t alignment for the payload void* Alloc(size_t bytes) { - void* raw = std::malloc(bytes + kHeader); + void* raw = __real_malloc(bytes + kHeader); if (raw == nullptr) throw std::bad_alloc(); *static_cast(raw) = bytes; Account(static_cast(bytes)); @@ -95,19 +162,56 @@ void Free(void* p) noexcept { std::free(raw); } +// The C++17 over-aligned family. Kept separate from `Alloc` because the payload +// has to land on `align`, so the header cannot be a fixed 32 bytes: the padding +// is whichever of `align` and `kHeader` is larger, and it is recorded in the two +// words immediately below the payload (both padding choices are >= 16 bytes, so +// those two words are always inside the padding). The standard pairs an +// over-aligned `new` with an over-aligned `delete`, so `FreeAligned` never sees +// a pointer `Alloc` produced and vice versa. +void* AllocAligned(size_t bytes, size_t align) { + if (align < alignof(std::max_align_t)) align = alignof(std::max_align_t); + const size_t pad = align > kHeader ? align : kHeader; + const size_t total = ((bytes + pad + align - 1) / align) * align; + void* raw = __real_aligned_alloc(align, total); + if (raw == nullptr) throw std::bad_alloc(); + char* payload = static_cast(raw) + pad; + reinterpret_cast(payload)[-1] = pad; + reinterpret_cast(payload)[-2] = bytes; + Account(static_cast(bytes)); + return payload; +} + +void FreeAligned(void* p) noexcept { + if (p == nullptr) return; + char* payload = static_cast(p); + const size_t pad = reinterpret_cast(payload)[-1]; + const size_t bytes = reinterpret_cast(payload)[-2]; + Account(-static_cast(bytes)); + std::free(payload - pad); +} + Device Cpu() { return Device{DeviceType::kCPU, 0}; } constexpr int64_t kMiB = 1024 * 1024; -// Bytes the allocator was asked for across one `vt::MatmulBT` on `nthreads` -// workers at [rows, k] x [n, k]. The operands are allocated BEFORE the mark is -// armed, so the figure is the call's own. +// Which member of the seam to measure. They are DIFFERENT template +// instantiations of `MatmulOneChunk` with different `thread_local` buffers, not +// two spellings of one path, so each needs its own measurement. +enum class Seam { + kBT, // vt::MatmulBT on a non-repacked [N,K] weight -> MatmulOneChunk + kNK, // vt::Matmul on a [K,N] weight -> MatmulOneChunk +}; + +// Bytes the allocator was asked for across one seam call on `nthreads` workers +// at [rows, k] x (kBT ? [n, k] : [k, n]). The operands and the pool are +// allocated BEFORE the mark is armed, so the figure is the call's own. // // The pool is constructed fresh every time. That is deliberate: `af` is a // process-lifetime `thread_local`, so a reused worker would allocate nothing // the second time and the measurement would read zero for a reason that has // nothing to do with the kernel. -int64_t AllocBytesOfMatmulBT(int nthreads, int64_t rows, int64_t k, int64_t n) { +int64_t AllocBytesOfSeam(Seam seam, int nthreads, int64_t rows, int64_t k, int64_t n) { std::vector a(static_cast(rows * k), 0.5f); std::vector b(static_cast(n * k), 0.25f); std::vector out(static_cast(rows * n), 0.0f); @@ -117,14 +221,24 @@ int64_t AllocBytesOfMatmulBT(int nthreads, int64_t rows, int64_t k, int64_t n) { g_high.store(g_live.load(std::memory_order_relaxed), std::memory_order_relaxed); const int64_t before = g_high.load(std::memory_order_relaxed); + g_raw.store(0, std::memory_order_relaxed); Queue q{Cpu(), nullptr}; Tensor a_t = Tensor::Contiguous(a.data(), DType::kF32, Cpu(), {rows, k}); - Tensor b_t = Tensor::Contiguous(b.data(), DType::kF32, Cpu(), {n, k}); Tensor o_t = Tensor::Contiguous(out.data(), DType::kF32, Cpu(), {rows, n}); - vt::MatmulBT(q, o_t, a_t, b_t); + if (seam == Seam::kBT) { + Tensor b_t = Tensor::Contiguous(b.data(), DType::kF32, Cpu(), {n, k}); + vt::MatmulBT(q, o_t, a_t, b_t); + } else { + Tensor b_t = Tensor::Contiguous(b.data(), DType::kF32, Cpu(), {k, n}); + vt::Matmul(q, o_t, a_t, b_t); + } - const int64_t growth = g_high.load(std::memory_order_relaxed) - before; + // Both routes, added: the replaced `operator new` high-water mark and the + // C allocator's cumulative request total. A defect that takes either one is + // inside the figure the bound is compared against. + const int64_t growth = (g_high.load(std::memory_order_relaxed) - before) + + g_raw.load(std::memory_order_relaxed); vt::cpu::Threadpool::SwapForTesting(previous); return growth; } @@ -136,19 +250,96 @@ constexpr int64_t kK = 65536; constexpr int64_t kN = 32; constexpr int64_t kTileBytes = 16 * kK * 4; -// The fixed term covers the pool's own per-worker state and the `std::function` -// the dispatch wraps; the per-thread term is doubled so an allocator or a -// future tile that rounds up cannot red the gate. Neither is wide enough to -// admit a second tile-sized buffer per worker, which is what keeps it a bound -// rather than a formality. +// The bound is `nthreads` tiles plus a small fixed term, which is exactly the +// property this file claims: the seam's only per-call allocation is ONE 16-row +// tile per worker. The fixed term covers the `std::function` the dispatch wraps +// and anything else incidental inside the armed window — measured at 64 bytes +// in total, against 1 MiB of allowance. So there is four orders of magnitude of +// slack on the term that is not supposed to grow, and none on the term that is. +// +// What that detects, as a number rather than as a claim. A second tile-sized +// buffer per worker costs `2 x (nthreads - 1)` tiles once the calling thread's +// buffer is already sized, and that exceeds the bound from `nthreads = 4` +// upward — measured at 24 MiB against 17 MiB on 4 workers and 56 MiB against +// 33 MiB on 8. At 1 and 2 workers it can fit inside `nthreads` tiles and is not +// reliably caught, which is why the sweeps below run to 8 rather than stopping +// at 2. The earlier `2 * kTileBytes` per-thread term caught it at NO worker +// count while its comment claimed it could not be fitted at all. int64_t Bound(int nthreads) { - return 8 * kMiB + static_cast(nthreads) * 2 * kTileBytes; + return kMiB + static_cast(nthreads) * kTileBytes; } +// A geometry that forces the chunk grid to COLLAPSE, so one chunk spans more +// than 16 rows. `MatmulChunked` rewrites the grid to one chunk per thread when +// `nchunk0 * nchunk1 < nth * 4` (`cpu_ops.cpp:257-260`, ggml-cpu.c:1404-1408). +// That branch is LIVE and is the ggml-mirrored default, because +// `VT_CPU_MATMUL_STEAL` is off. After it, `dr1` is `ceil(rows / nth)` when the +// activation is the longer axis and `rows` outright when the weight is — and it +// is 16 or less at EVERY geometry the sweeps above happen to run, which is the +// only reason those sweeps cannot see a chunk-sized buffer. +// +// That distinction is the whole content of the "not by the chunk's row span" +// half of the claim, and without this case nothing here can see it: a kernel +// that sized `af` by `ir1_end - iir1` instead of by `min(16, ...)` is +// byte-identical in growth at every other geometry in this file. With +// `n = 16 <= chunk_size` the column grid is one chunk wide, so `nchunk0 = 1` +// and `nchunk1 = ceil(128/16) = 8 < 4 * 4`; the collapse then gives +// `nchunk1 = 4` and `dr1 = 32`, exactly two tiles per chunk. +// +// The same condition has a much larger form that is NOT reachable today and is +// recorded rather than tested. `IsNuma()` (`cpu_threadpool.h:74`) is +// `constexpr false` in this tree — NUMA is an unported part of the threadpool +// (`cpu_threadpool.h:25`) — and it forces the collapse UNCONDITIONALLY where +// upstream implements it. At the shipped LTX-2.5 caption projection the weight +// is the longer axis (`n = 4096 > rows = 1024`), so that collapse would give +// `nchunk1 = 1` and `dr1 = 1024`: a chunk-sized buffer would be 770 MB per +// worker, which is #1286's hypothesis exactly. Porting NUMA therefore needs +// this case, not the sweeps. +constexpr int64_t kCollapseRows = 128; +constexpr int64_t kCollapseN = 16; +constexpr int kCollapseThreads = 4; + } // namespace // Replaced global allocation functions. Standard replacements, so they serve -// the whole program including the seam's `thread_local` buffers. +// the whole program including the seam's `thread_local` buffers. The complete +// set is here on purpose: an over-aligned `operator new` that fell through to +// the library's own would be a silent hole in the counter, and it is the +// natural allocation shape for a SIMD scratch buffer in this kernel. +// The C allocator, reached through the linker rather than through a symbol +// definition. `-Wl,--wrap=malloc` redirects the calls made BY THE OBJECTS IN +// THIS LINK -- which includes `libvllm.a`, and so `cpu_ops.cpp` -- without +// defining `malloc`, so AddressSanitizer's own interceptor keeps its symbol and +// `__real_malloc` resolves to whichever allocator is actually installed. A +// plain `extern "C" void* malloc(...)` here would instead be a second strong +// definition, which is the thing that breaks the `sanitize-cpu` lane. +// +// `free` is deliberately NOT wrapped. Releasing a block gives no size without a +// header, and a header would mean applying an offset to pointers that libc and +// libstdc++ allocated before this file was reached -- a heap corruption rather +// than a failed assertion. +extern "C" void* __wrap_malloc(size_t n) { + g_raw.fetch_add(static_cast(n), std::memory_order_relaxed); + return __real_malloc(n); +} +extern "C" void* __wrap_calloc(size_t count, size_t size) { + g_raw.fetch_add(static_cast(count) * static_cast(size), + std::memory_order_relaxed); + return __real_calloc(count, size); +} +extern "C" void* __wrap_realloc(void* p, size_t n) { + g_raw.fetch_add(static_cast(n), std::memory_order_relaxed); + return __real_realloc(p, n); +} +extern "C" void* __wrap_aligned_alloc(size_t align, size_t n) { + g_raw.fetch_add(static_cast(n), std::memory_order_relaxed); + return __real_aligned_alloc(align, n); +} +extern "C" int __wrap_posix_memalign(void** out, size_t align, size_t n) { + g_raw.fetch_add(static_cast(n), std::memory_order_relaxed); + return __real_posix_memalign(out, align, n); +} + void* operator new(size_t bytes) { return Alloc(bytes); } void* operator new[](size_t bytes) { return Alloc(bytes); } void* operator new(size_t bytes, const std::nothrow_t&) noexcept { @@ -165,12 +356,114 @@ void* operator new[](size_t bytes, const std::nothrow_t&) noexcept { return nullptr; } } +void* operator new(size_t bytes, std::align_val_t a) { + return AllocAligned(bytes, static_cast(a)); +} +void* operator new[](size_t bytes, std::align_val_t a) { + return AllocAligned(bytes, static_cast(a)); +} +void* operator new(size_t bytes, std::align_val_t a, const std::nothrow_t&) noexcept { + try { + return AllocAligned(bytes, static_cast(a)); + } catch (...) { + return nullptr; + } +} +void* operator new[](size_t bytes, std::align_val_t a, const std::nothrow_t&) noexcept { + try { + return AllocAligned(bytes, static_cast(a)); + } catch (...) { + return nullptr; + } +} void operator delete(void* p) noexcept { Free(p); } void operator delete[](void* p) noexcept { Free(p); } void operator delete(void* p, size_t) noexcept { Free(p); } void operator delete[](void* p, size_t) noexcept { Free(p); } void operator delete(void* p, const std::nothrow_t&) noexcept { Free(p); } void operator delete[](void* p, const std::nothrow_t&) noexcept { Free(p); } +void operator delete(void* p, std::align_val_t) noexcept { FreeAligned(p); } +void operator delete[](void* p, std::align_val_t) noexcept { FreeAligned(p); } +void operator delete(void* p, size_t, std::align_val_t) noexcept { FreeAligned(p); } +void operator delete[](void* p, size_t, std::align_val_t) noexcept { FreeAligned(p); } +void operator delete(void* p, std::align_val_t, const std::nothrow_t&) noexcept { + FreeAligned(p); +} +void operator delete[](void* p, std::align_val_t, const std::nothrow_t&) noexcept { + FreeAligned(p); +} + +TEST_CASE("the counter SEES every allocation route it claims to cover (#1286)") { + // The gate's claim is about which allocation APIs are counted, and that claim + // is executable rather than prose. Each route is exercised at a size far above + // any incidental traffic and the counter is required to move by AT LEAST it — + // at least, because a `new` inside doctest's own INFO machinery could add to + // it, never subtract. A lost overload reds here instead of quietly reducing + // every bound below to a formality. + constexpr size_t kProbe = 8u << 20; // 8 MiB, 2x the tile + + const int64_t base_plain = g_live.load(std::memory_order_relaxed); + auto* plain = static_cast(::operator new(kProbe)); + const int64_t moved_plain = g_live.load(std::memory_order_relaxed) - base_plain; + ::operator delete(plain); + INFO("plain operator new moved " << moved_plain); + CHECK(moved_plain >= static_cast(kProbe)); + + const int64_t base_array = g_live.load(std::memory_order_relaxed); + auto* arr = new char[kProbe]; + const int64_t moved_array = g_live.load(std::memory_order_relaxed) - base_array; + delete[] arr; + INFO("array operator new moved " << moved_array); + CHECK(moved_array >= static_cast(kProbe)); + + // The over-aligned route, which is the one review mutation M-D took and the + // one the first version of this file did not replace at all: it fell through + // to the library and the counter read a whole-activation defect as zero. + // 64 bytes is the cache-line request an AVX-512 kernel makes. (M-E's + // `std::malloc` route has no case here because it is NOT covered — see the + // header, and `## Owed` in the spec.) + for (const size_t align : {size_t{64}, size_t{256}}) { + const int64_t base = g_live.load(std::memory_order_relaxed); + auto* over = static_cast( + ::operator new(kProbe, static_cast(align))); + const int64_t moved = g_live.load(std::memory_order_relaxed) - base; + const bool aligned = (reinterpret_cast(over) % align) == 0; + // Touch both ends: a header scheme that mis-sizes the block corrupts the + // heap rather than failing an assertion, and this is where it would show. + over[0] = 1; + over[kProbe - 1] = 2; + ::operator delete(over, static_cast(align)); + INFO("aligned operator new align=" << align << " moved " << moved + << " aligned=" << (aligned ? 1 : 0)); + CHECK(moved >= static_cast(kProbe)); + CHECK(aligned); + } + + // The C-allocator route, which is a LINKER redirection and not a symbol + // definition, and is therefore the one that can go inert without any source + // change. The call goes through a `volatile` function pointer because a + // direct `std::malloc(...)` in THIS translation unit is folded by the + // compiler and never becomes a reference to the `malloc` symbol, so it is + // never wrapped -- measured in a standalone probe, and the reason this is + // written the awkward way. Product code is always the separate-TU case. + { + const int64_t base_raw = g_raw.load(std::memory_order_relaxed); + void* (*volatile c_alloc)(size_t) = &std::malloc; + void (*volatile c_free)(void*) = &std::free; + void* raw = c_alloc(kProbe); + const int64_t moved_raw = g_raw.load(std::memory_order_relaxed) - base_raw; + REQUIRE(raw != nullptr); + c_free(raw); + INFO("std::malloc moved the raw counter by " << moved_raw); + CHECK(moved_raw >= static_cast(kProbe)); + } + + // And the counter must come back down, or every "growth" below is a running + // total of everything the process ever did rather than the call's own cost. + const int64_t leaked = g_live.load(std::memory_order_relaxed) - base_plain; + INFO("live bytes still held after the probes: " << leaked); + CHECK(leaked < static_cast(kProbe)); +} TEST_CASE("the allocation counter SEES the seam's per-worker tiles (#1286)") { REQUIRE(kTileBytes == 4 * kMiB); @@ -180,16 +473,24 @@ TEST_CASE("the allocation counter SEES the seam's per-worker tiles (#1286)") { // the work-stealing cursor handing out a ninth chunk in any particular // order — but it is far enough above zero that a counter reading nothing // cannot pass, which is the whole point of this case. - const int64_t growth = AllocBytesOfMatmulBT(8, 256, kK, kN); - INFO("growth_bytes=" << growth << " tile_bytes=" << kTileBytes); - CHECK(growth >= 6 * kTileBytes); + // + // Run on BOTH orientations: they are separate instantiations with separate + // buffers, so a counter that saw one and not the other would read zero on + // half the seam. + const int64_t bt = AllocBytesOfSeam(Seam::kBT, 8, 256, kK, kN); + INFO("MatmulBT growth_bytes=" << bt << " tile_bytes=" << kTileBytes); + CHECK(bt >= 6 * kTileBytes); + + const int64_t nk = AllocBytesOfSeam(Seam::kNK, 8, 256, kK, kN); + INFO("Matmul growth_bytes=" << nk << " tile_bytes=" << kTileBytes); + CHECK(nk >= 6 * kTileBytes); } TEST_CASE("vt::MatmulBT allocates the per-worker TILE, not the whole " "intermediate (#1286)") { SUBCASE("bytes requested stay inside the tile bound at every worker count") { for (const int nthreads : {1, 2, 4, 8}) { - const int64_t growth = AllocBytesOfMatmulBT(nthreads, 256, kK, kN); + const int64_t growth = AllocBytesOfSeam(Seam::kBT, nthreads, 256, kK, kN); INFO("nthreads=" << nthreads << " growth_bytes=" << growth << " bound_bytes=" << Bound(nthreads)); CHECK(growth >= 0); @@ -202,8 +503,37 @@ TEST_CASE("vt::MatmulBT allocates the per-worker TILE, not the whole " // hypothesised. 16x the rows must not buy a single extra tile. SUBCASE("bytes requested do not scale with the ROW count") { constexpr int kThreads = 4; - const int64_t few = AllocBytesOfMatmulBT(kThreads, 64, kK, kN); - const int64_t many = AllocBytesOfMatmulBT(kThreads, 1024, kK, kN); + const int64_t few = AllocBytesOfSeam(Seam::kBT, kThreads, 64, kK, kN); + const int64_t many = AllocBytesOfSeam(Seam::kBT, kThreads, 1024, kK, kN); + INFO("rows=64 growth_bytes=" << few << " rows=1024 growth_bytes=" << many + << " bound_bytes=" << Bound(kThreads)); + CHECK(few <= Bound(kThreads)); + CHECK(many <= Bound(kThreads)); + CHECK(many - few <= kTileBytes); + } +} + +TEST_CASE("vt::Matmul — the OTHER instantiation, with its own buffer — is " + "bounded the same way (#1286, #1259)") { + // `MatmulOneChunk`. Nothing above reaches it: `vt::MatmulBT` on a + // non-repacked weight only instantiates ``. It is the arm #1259's + // `Ltx2FuseLoraIntoTensor` runs on the full model, and the arm the loader's + // `elem_kn_repacked` lever selects, so leaving it unbounded would leave the + // gate covering one of the two buffers the process actually holds. + SUBCASE("bytes requested stay inside the tile bound at every worker count") { + for (const int nthreads : {1, 2, 4, 8}) { + const int64_t growth = AllocBytesOfSeam(Seam::kNK, nthreads, 256, kK, kN); + INFO("nthreads=" << nthreads << " growth_bytes=" << growth + << " bound_bytes=" << Bound(nthreads)); + CHECK(growth >= 0); + CHECK(growth <= Bound(nthreads)); + } + } + + SUBCASE("bytes requested do not scale with the ROW count") { + constexpr int kThreads = 4; + const int64_t few = AllocBytesOfSeam(Seam::kNK, kThreads, 64, kK, kN); + const int64_t many = AllocBytesOfSeam(Seam::kNK, kThreads, 1024, kK, kN); INFO("rows=64 growth_bytes=" << few << " rows=1024 growth_bytes=" << many << " bound_bytes=" << Bound(kThreads)); CHECK(few <= Bound(kThreads)); @@ -211,3 +541,30 @@ TEST_CASE("vt::MatmulBT allocates the per-worker TILE, not the whole " CHECK(many - few <= kTileBytes); } } + +TEST_CASE("the tile is the 16-row blck_1 tile, not the CHUNK'S ROW SPAN " + "(#1286)") { + // At a collapsed grid one chunk spans `dr1 = 32` rows, so a kernel sized by + // the chunk instead of by `blck_1` asks for two tiles per worker here and one + // tile per worker everywhere else in this file. Without this case that defect + // is invisible, and this shape is a live one rather than a contrivance: the + // collapse it uses is the shipped default path, not the NUMA branch. + // + // The bound is the same one: two tiles per worker on 4 workers is 24 MiB + // against 17 MiB, so it reds on the bound rather than needing its own + // threshold. + for (const Seam seam : {Seam::kBT, Seam::kNK}) { + const int64_t growth = + AllocBytesOfSeam(seam, kCollapseThreads, kCollapseRows, kK, kCollapseN); + // `std::string`, not a ternary over two `const char*`: doctest's INFO + // stringifies a `char*` operand as a BOOL, and the first draft of this line + // printed `seam=1` for both arms. + INFO("seam=" << std::string(seam == Seam::kBT ? "MatmulBT" : "Matmul") + << " rows=" << kCollapseRows << " n=" << kCollapseN + << " nthreads=" << kCollapseThreads + << " growth_bytes=" << growth + << " bound_bytes=" << Bound(kCollapseThreads)); + CHECK(growth >= 0); + CHECK(growth <= Bound(kCollapseThreads)); + } +}