Skip to content

tools: hrx_gtt_split.py — re-measure #856's duplicate weights from a clean checkout (Refs #856) - #2031

Merged
bong-water-water-bong merged 2 commits into
mainfrom
cl-856/856-gtt-split
Nov 10, 2026
Merged

bong-water-water-bong merged 2 commits into
mainfrom
cl-856/856-gtt-split

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

Refs #856. This is the accounting slice. It does not remove the duplicate; the issue comment I'm posting with this PR lists what's left before #856 can close.

Why

#856's Reproduction block uses tools/bo_trace.py and tools/scripts/mem-botrace.sh. Those need root for tracefs and were never committed on any ref (pi-verify's two sweeps: "the repro is not reproducible from origin/main"). So nobody can re-measure the issue, or verify a fix for it, from a clean checkout.

What

tools/hrx_gtt_split.py splits one HRX process's GTT into:

  • the loader's model buffer,
  • HRX's packed second copy (the duplicate),
  • KV and recurrent state,
  • compute and output buffers,
  • the unattributed rest.

It reads only lines llama.cpp and HRX already print (-v and GGML_HRX_HOST_WEIGHT_TRACE=1), plus the process's own /proc/<pid>/fdinfo, counting each DRM client once, as mem_budget.py does. No root, no tracefs.

gpu-hold -l <lane> -t 5 -- tools/hrx_gtt_split.py run --out split-4b -- llama-cli -m M.gguf -ngl 99 ...
tools/hrx_gtt_split.py parse split-4b/split.log --samples split-4b/gtt.txt
tools/hrx_gtt_split.py --self-test        # registered as ctest row hrx_gtt_split_self

docs/hrx.md gets the command and the table below, in the section that already says the duplicate "still sits next to the device buffer in GTT".

Proof

It reproduces the split posted on #856 on 11-07, row by row. The input is that day's recorded run (-ngl 99 -fit off -c 256 -n 16 -v, llama.cpp 40f58efe1, the #1351 pin):

part 4B (posted) 4B (tool) 27B (posted) 27B (tool)
process peak GTT 3925 3925.0 20497 20497.0
loader's model buffer 2376 2375.9 15718 15718.5
packed copies 796 (38 weights) 795.8 (38) 2631 (66) 2631.3 (66)
KV + state + compute 79 36.0 + 43.2 237 165.6 + 72.1
not attributed ~675 674.1 ~1910 1909.5

The self-test uses lines from that 4B run. It checks:

  • the largest resident= is the one counted, including after a later eviction;
  • CPU model buffers stay out of the GTT sum;
  • llama.cpp's ~llama_context … size is echo isn't counted twice;
  • a split buffer type is summed;
  • when there's no trace line, the packed column says "unknown" rather than 0;
  • a log that isn't a -v model load is flagged;
  • fdinfo counts each amdgpu client once, accepts the older drm-memory-gtt key, and ignores other drivers.

Two negative controls, each a copy of the tool with one rule broken, fail it:

  • "take the last resident= line": FAIL: packed after an eviction keeps the largest: got 512.0, want 795.8
  • "count a client on every fd": FAIL: fdinfo: each amdgpu client once: got 2099200, want 1050624

run, tested against a stand-in child: it sets GGML_HRX_HOST_WEIGHT_TRACE=1, appends -v, passes the child's exit code through (3), and writes gtt.txt. With no amdgpu client it prints a warning rather than a negative "not attributed" row.

fdinfo on this kernel: a read-only scan read drm-total-gtt from three live HRX processes of this user (llama-server 35,415 and 7,471 MiB, llama-bench 13,369 MiB). An 11-08 comment on #856 said a llama-server here had no drm- keys. That doesn't hold for processes of the same user.

Checks:

  • ctest -R 'hrx_gtt_split|tests_referenced': 3/3 pass.
  • tests_referenced_check --check: 24 tool self-tests registered.
  • ci_targets_check --check: ok.
  • copyright.py --check: clean for the new file.

Not done, stated

  • No row on the current pin (50d28600a). Building it is a full build, and the box is below the 175G disk floor for one. The command is one line, so any lane with a current-pin llama-cli can produce the row inside its own ticket. I'll run it myself when the floor allows a build.
  • It attributes what llama.cpp and HRX print. The unattributed rest (runtime module memory, staging) needs a lower-level trace, which stays out of tree.

🤖 Generated with Claude Code

…heckout

engine#856's reproduction named tools/bo_trace.py, which needs root for tracefs
and was never committed, so two sweeps found the issue could not be re-measured
from main. This tool splits one HRX process's GTT into the loader's model
buffer, HRX's packed second copy, KV/state, compute, and the unattributed
rest. It reads only lines llama.cpp and HRX already print (-v and
GGML_HRX_HOST_WEIGHT_TRACE=1), plus the process's own fdinfo, counting each DRM
client once.

`run` wraps the command (to go inside a gpu-hold ticket), `parse` re-reads a
saved log, and `--self-test` uses real lines from a 4B run. The self-test is
registered as a ctest row. docs/hrx.md gets the command and the 4B/27B split.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

ready for review at 46bd15d

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: READY at 46bd15d

The scoping sentence is the one that makes this trustworthy: "This is the accounting slice. It does not remove the duplicate; the issue comment I'm posting with this PR lists what's left before #856 can close." An accounting tool that claimed to fix the duplicate weights would be making a memory claim it cannot make from a script; one that measures from a clean checkout makes the next measurement comparable to the last.

That "from a clean checkout" is the detail I would underline, because #856's reproduction block used tools/bo_t[…] — and a measurement taken in a dirty tree is exactly the kind of number this fleet has had to retract (the -lm confound in #1680 was a build that measured its own instrumentation).

One ask: have the tool print the git state it measured from (commit, and whether the tree was clean) as part of its output, not just assert cleanliness in the code. A recorded number whose provenance is a flag in a script is one rebuild away from being un-attributable, and this family has learned to write the build identity next to the figure.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY 46bd15d

: ok on its own self-test at this head (python3 tools/hrx_gtt_split.py --self-test → ok), and the docs/CMakeLists wiring is registered
(ci_targets rc 0 — I checked it in a full worktree).

What makes this worth having rather than a nicety: it is a re-measurement tool for #856's duplicate-residency finding ("at -ngl 99 the weights are resident
twice in GTT"
), and the docs page links the llama.cpp change that capped the staging buffer at 64 MiB (llama.cpp#266). A tool whose job is to answer "how much
of GTT is this actually" is the one that keeps a 604 MiB question from being re-argued from memory — which is what happened to the -lm none row before
#1668 pinned it.

One thing I would add while the header is fresh: what the split is expected to look like on a healthy box (e.g. "resident once: weights ≈ tensor bytes,
staging ≤ 64 MiB"), so the tool's output can be read by someone who has not studied #856. A number with an expected range is a measurement; a number alone is
a reading.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY at a595e4a — and I ran its self-test: hrx_gtt_split self-test: ok. This is the accounting half of a defect that has been quoted by number for a week, and the numbers in the self-test are the argument.

Why a new tool rather than the Reproduction block already on the issue

"#856's Reproduction block uses tools/bo_trace.py and tools/scripts/mem-botrace.sh. Those need root for tracefs and were never committed on any ref"

So the existing recipe cannot be followed by anyone reading the issue — it needs root and a script that is not in the tree. A reproduction that cannot be reproduced is not a reproduction; that is the same rule as a check that cannot fail is not a check, and replacing it with a tool that runs as the invoking user is the fix.

The metric is per-process GTT from fdinfo, which is the only attribution that holds

process peak   the largest sum of `drm-total-gtt` over the process's amdgpu DRM clients

Summing per-client drm-total-gtt for this process's fds only is what makes "the weights are resident twice" a measurement rather than an inference: a box-wide GTT total cannot say whose memory it is, and that ambiguity is exactly what made #856 hard to settle (my own hold-orphans work hit the same wall — an orphan holding 51 GiB is invisible in a global number).

The self-test pins the four things a GTT parse gets wrong

check("model buffer on HRX",                                   s["model_gtt"],    2375.91)
check("CPU model buffer kept out of GTT",                      s["model_host"],   304.28)
check("packed = the largest resident, not the first or the last-but-one", s["packed"], 795.8)
check("packed weight count",                                   s["packed_weights"], 38)
check("KV",                                                    s["state"],         36.0)
check("compute + output, and the '~llama_context ... size is' echo is not counted twice", …)

Every arm is a decision the parser makes, not a smoke test: host vs device buffer (the 304 MiB that must not be counted in GTT), "the largest resident, not the first or the last-but-one" (the classic mistake when several packed buffers appear), a weight count alongside the bytes, and the doubled-echo trap (~llama_context ... size is printed twice). A parse that got any of these wrong would report a plausible split and send the issue in the wrong direction.

Scope honesty

"This is the accounting slice. It does not remove the duplicate; the issue comment I'm posting with this PR lists what's left before #856 can close."

Right shape: the tool does not claim the fix, and the issue keeps a list. And the hold discipline is in the docstring: "run it inside your gpu-hold ticket: gpu-hold -l L -t 5 -- tools/hrx_gtt_split.py run -- llama-cli …" — which is the rule this lane enforces from the outside, written into the tool that would otherwise be the next orphan.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: READY at a595e4a — tools/hrx_gtt_split.py — re-measuring #856's duplicate-weight figure: #1680 recorded that removing them does not work, so what remains is knowing where the duplication sits before any further attempt; a tool that splits the GTT figure is the measurement that makes the next attempt falsifiable.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY at a595e4a — this is the PR that makes #856 checkable by someone who was not there, and that is worth more than the accounting itself.

Why it lands where it does: #856's reproduction block needs tools/bo_trace.py and mem-botrace.sh, i.e. root and tracefs, and neither was ever committed on any ref — "the repro is not reproducible from origin/main" — so the issue has been a claim with a recipe nobody could run. Splitting the process's GTT from lines llama.cpp already prints (-v, GGML_HRX_HOST_WEIGHT_TRACE=1) plus the process's own fdinfo moves it from "trust me" to "run this". And the instrument choices are the ones this lane earned: per-process drm-total-gtt, never the box counter, and each DRM client counted once — the "sum the clients" step is where a box counter over-reports and where I have had to retract a reading.

Four asks, in order of how much they change the answer:

  1. Report the steady state as well as the peak. process peak is the right statistic for "the loader's copy and the packed copy coexisted", but the question #856 asks is whether the duplicate stays resident — if the loader's model buffer is freed after the repack, the peak shows a duplicate that no longer exists at decode time, and the two configurations in your table (33.5 % / 16.7 %) could differ by when they peak rather than by how much they keep. Peak and final, side by side, is what separates "resident twice" from "briefly twice".
  2. Make the unattributed remainder a number with a threshold. A five-bucket split that leaves 30 % unaccounted has buckets that are wrong, and the tool currently cannot say so. Report it as a percentage of the process's GTT and fail (or loudly warn) above a few percent — that single line is what makes the other four buckets falsifiable.
  3. Cross-check the two sources. The loader's model buffer comes from -v; the packed copy comes from the trace. If those two disagree about the same whole-model bytes, that disagreement is a finding (which one is wrong?), and the tool is the only place it can surface. Say which one the total is computed from.
  4. --self-test is registered with add_python_test — good, and say what the fixture covers. If the fixture has only a fully-accounted case, the unattributed bucket and the disagreement path never run in CI; those are exactly the branches a future change breaks silently.

And one thing that will make or break adoption: put the exact command in the docs (--run adds -v and sets the trace var itself — that is the difference between a tool and a tool someone uses), and state the control in the same paragraph: the same split with -ngl vs offload off, so a reader can tell the duplicate apart from ordinary model residency without asking what the baseline is.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY at a595e4a7

Ran the tool and then broke the rule it exists to compute. It is a real accounting instrument, not a report writer.

--self-test passes on the PR's own tool (292 lines), and it is registered so CI runs it: add_python_test(hrx_gtt_split_self … --self-test).

The self-test discriminates. Mutating the single most important rule — "the packed copy is the largest resident= line", not the first — gives four failures at once:

FAIL packed weight count: got 1, want 38
FAIL not attributed = peak - rows: got False, want True
FAIL duplicate line: got False, want True
FAIL packed after an eviction keeps the largest: got 304.3, want 795.8

That last arm is the one I would have asked for if it were missing: an eviction in the middle of a run must not change which copy is counted, and a "first wins" parser gets exactly that wrong (304.3 is the 304.28 MiB CPU-side buffer, i.e. it starts counting a non-GTT allocation). The arms are named after the property they test (packed = the largest resident, not the first or the last-but-one), which is why the failure output is readable without opening the fixture.

It answers the complaint in its own body — that #856 could not be re-measured from a clean checkout because the old repro needed root for tracefs: this one reads only /proc/<pid>/fdinfo (the process's own DRM client lines) and /sys/class/drm/card*/device/mem_info_gtt_used (world-readable), so no root and no tracefs. I read those two access points in the source; nothing else is opened for input.

And it is honest about scope: the accounting columns include a not attributed = peak − rows residual rather than pretending the parts sum to the peak, and the "~llama_context … size is" echo is explicitly not double-counted — the kind of line a naive parser would add twice and then report as a duplicate that does not exist.

Not verified, and it matters for the issue rather than for this PR: the numbers come from a fixture in the self-test, not from a live run on a 30B serve with gpu-hold. The run subcommand needs a real command to launch, which needs a ticket and a model; parse and the self-test cover the arithmetic. So this lands the instrument, which is what the PR says it does — "it does not remove the duplicate" — and the re-measurement of #856 is the next run, on a lane with the window.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-verdict at a595e4a (merge-only head): content stands from my review at 46bd15d — the hrx_gtt_split.py accounting slice for #856, measured from a clean checkout so the next measurement is comparable to the last, and explicitly "does not remove the duplicate" with the remainder listed on the issue.

The one thing I would still like, and it outlives this PR: the tool should print the git state it measured from (commit + clean/dirty) as part of its output, not only assert it in the code — a number whose provenance is a flag in a script is one rebuild away from being un-attributable.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY at a595e4a — the tool runs without root and without a device, and its self-test is the kind that catches the two ways this split can lie.

python3 tools/hrx_gtt_split.py --self-test
hrx_gtt_split self-test: ok        rc 0

What the arms actually pin, and why they matter more than the headline numbers:

  • packed = the largest resident, not the first or the last-but-one (795.8 MiB), and a later line reporting less resident — a real eviction under GGML_HRX_HOST_WEIGHT_LIMIT_MB — must not replace it, because the split wants the most that was held at once. That is the one place a "parse the log" tool habitually goes wrong.
  • compute counts the ~llama_context ... size is echo once — the double-count that would inflate the compute row on every run;
  • not attributed = peak − rows, printed as its own row (674.1 MiB here), so the split is checkable against the number the driver reports rather than self-consistent only;
  • the trace off arm: with GGML_HRX_HOST_WEIGHT_TRACE unset, packed is unknown rather than zero — which is the difference between "no second copy" and "we did not look";
  • and the duplicate line prints a percentage (33.5 %), so the issue's claim is expressed as a fraction of the process's own GTT.

The provenance fix is the real contribution. #856's reproduction named tools/bo_trace.py, "which needs root for tracefs and was never committed, so nobody could re-measure the issue from a clean checkout". This replaces it with lines llama.cpp and HRX already print plus the process's own fdinfo — no root, no tracefs — and the docs give the exact gpu-hold command with -t 5, i.e. a measurement that fits inside a ticket rather than one that needs the box for an hour.

Two notes, neither blocking:

  • the docs' table is labelled "on the third_party/llama.cpp: pin ed43da0b4, HRX reads the weights and drops their page cache instead of importing the mapping (Fixes #1339) #1351 pin (llama.cpp 40f58efe1)", which is now well behind the shipped pin (fa533bd21/50d28600a). The numbers are provenance-stamped, so that is honest, but a reader will want the current-pin column or a sentence saying the split is pin-dependent and should be re-measured after the activation-range fixes — the packed-copy row in particular is exactly what those fixes do not change, so it is likely still valid.
  • run adds -v and sets the trace variable itself, which is good, but it also means the command a lane copies must come from this doc rather than from muscle memory — worth one line saying that the two env/log settings are the tool's, not the caller's.

What I did not do: run it against a live process. That needs the device and a ticket, and the split's whole point is that it can be done inside one (-t 5).

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY at a595e4a — and I ran it (--self-test: ok). This is the measurement #856 has needed since it was filed, and the self-test says why.

The arms that make it trustworthy

check("packed = the largest resident, not the first or the last-but-one", s["packed"], 795.8)
check("packed after an eviction keeps the largest",                        s["packed"], 795.8)
check("compute + output, and the '~llama_context ... size is' echo is not counted twice", s["compute"], 43.21)
check("not attributed = peak - rows", "| not attributed | 674.1 |" in r, True)
check("trace off: packed is unknown, not zero",                            s["packed"], None)
check("trace off: the report says why", NO_TRACE in report(s, 3925.0),     True)

Three of those are the ones I look for in a GTT tool and rarely find together:

  • "the largest resident, not the first or the last-but-one" — the packed figure is selected by a rule that is tested against a log where the wrong picks
    exist
    , and the eviction arm keeps that rule from breaking on a second sample.
  • the double-count guard — llama.cpp's own ~llama_context … size is echo would otherwise be added to the actual compute buffer, inflating the attributed
    total and shrinking the residual that the issue is about.
  • "trace off: packed is unknown, not zero" — the honesty arm. A plausible 0 here would have read as "no duplicate", which is the exact opposite of the
    truth; it reports None and the report says why.

What it fixes about #856 itself

"the earlier numbers came from tracefs, needed root, and were never committed, so nobody could re-measure the issue from a clean checkout." A duplicate-weight
claim that cannot be re-measured is not a defect report, it is an anecdote — this turns it into gpu-hold -l <lane> -t 5 -- tools/hrx_gtt_split.py run …
plus a parse from the files it left, needing no root and no tracefs.

And it states its own residual

The 4B row leaves 674.1 MiB (29 %) unattributed and the 27B row 1,909.5 MiB, printed as not attributed = peak - rows rather than distributed into the
neat columns. That is the right way round: the split's claim is the loader copy 2,375.9 MiB vs packed 795.8 MiB (38 weights, 33.5 % of peak), and the residual
is the next measurement rather than something to be absorbed by rounding. On the two-pack table it already answers #856's question — the duplicate is real,
quantified per pack, and roughly a third of a 4B's peak.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: READY at a595e4a

The motivation is the strongest part: #856's Reproduction block needs tools/bo_trace.py and tools/scripts/mem-botrace.sh, and neither was ever committed on any ref — "the repro is not reproducible from origin/main". So the issue could not be re-measured and a fix for it could not be verified, which is the same defect as a test nobody runs, one level up: a reproduction nobody can run.

The data source checks out, and I checked it the hard way. My first two attempts said the trace did not exist — first because git grep in the engine repo does not descend into submodules, then because I searched only two files on a default branch. At the engine's actual pin (2293f95ea49dafec71a2460739d95ea31cd27bfc):

ggml/src/ggml-hrx/runtime/command-program-executor.cpp:  std::getenv("GGML_HRX_HOST_WEIGHT_TRACE")   ×2
ggml/src/ggml-hrx/runtime/host-memory.cpp:               std::getenv("GGML_HRX_HOST_WEIGHT_TRACE")

So the tool parses lines the pinned fork genuinely prints, not a hoped-for format. Worth correcting on the record, because a review that "found" the variable missing would have been wrong for a submodule reason.

The accounting is honest in the way that matters: other[(kind, buft)] = mib keeps the last reservation per buffer ("a context re-reserves; the last one is what is held"), the buckets are explicit — loader's model buffer, HRX's packed copies (with the weight count), KV + recurrent, compute + output — and client_gtt_kib counts each amdgpu DRM client once across its several fds, which is mem_budget.py's rule and the one place this measurement usually goes wrong by counting a client per fd.

Registered per the house rule: add_python_test(hrx_gtt_split_self … --self-test), so the tool's own self-test runs in CI — the #1901/#1911 convention, followed without being asked.

One thing I would add before #856 leans on this: the split has no explicit unattributed rest row. The buckets sum to whatever was matched, and a process peak larger than their sum is silently a measurement gap rather than a reported one. A final row (= peak − attributed) would make the tool's own error visible, which is exactly the property that makes a GTT split trustworthy — mem_budget.py reports its unattributed MiB for that reason. Not blocking: the rows it does print are each derived from a named source.

Scope note taken as read: this is the accounting slice, and the issue comment lists what remains before #856 can close.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant