Skip to content

hrx: keep the model weights in the loader's file mapping instead of a device buffer (engine #856) - #304

Merged
bong-water-water-bong merged 2 commits into
1bit/hrx-vulkan-patchedfrom
1bit/856-model-buffer
Nov 11, 2026
Merged

bong-water-water-bong merged 2 commits into
1bit/hrx-vulkan-patchedfrom
1bit/856-model-buffer

Conversation

@bong-water-water-bong

@bong-water-water-bong bong-water-water-bong commented Nov 11, 2026 •

Copy link
Copy Markdown

Refs 1bit-MONSTER/engine#856. The engine pin PR is 1bit-MONSTER/engine#2147 (it Fixes #856). The orchestrator's decision on the release row (posted on ggml-org#856): ship this, document the duplicate that remains after a long prompt, and track that in engine#2137.

What it changes

At -ngl 99 the loader uploaded the whole model into an HRX0 device buffer, and HRX then made its own device copy of each weight it runs (HostWeightCache). On this APU device memory is GTT, so the model was in GTT twice (at this pin HRX copies all 253 weights of Qwen3-4B).

Measured at this head

209091986 (this change on b6f65a273; its tree is identical to the rebased commit that was built), with hrx-system 7f4236924aad (the engine's pin), full Release build. One binary for both arms: GGML_HRX_MODEL_BUFFER_COPY=1 is the upload. Arms alternated under one gpu-hold ticket. Peak drm-total-gtt.

Qwen3-4B Q4_K_M upload this branch
peak GTT, 5-token prompt (x2) 5,665.3 MiB 3,225.3 MiB
peak GTT, 1,282-word prompt (x2) 6,311.8 MiB 6,055.8 MiB
device copies, long prompt 253 647 (4,400.6 MiB)
llama-bench pp1024, steady state 2,764.6 tok/s 2,767.7 / 2,765.4 tok/s
llama-bench tg64, steady state 71.5 tok/s 72.1 / 72.3 tok/s

Gates:

  • the text is identical in all four pairs;
  • --kl-divergence against the upload's logits: mean KLD 0.000000, same top-1 100.000 %, PPL 29.4055 both.

A fresh process pays a one-time copy at its first prompt instead of at load. That explains the first-prompt prefill scatter in single-shot runs (2,034 -> 1,674 and 1,526 -> 1,714 tok/s in the two pairs). Qwen3.8-27B dense, at the previous base: loads, peak GTT 19,075.3 -> 17,231.3 MiB, text identical.

What it does not do

After a long prompt a weight has a device copy in each layout: prefill reads q4_k/q6_k in the stored layout, decode reads HRX's. GGML_HRX_Q6K_PACKED_PREFILL=1 does not change that (measured, see the comment below). This is tracked in engine#2137.

🤖 Generated with Claude Code

… device buffer (engine ggml-org#856)

HRX makes its own device copy of every weight it runs (HostWeightCache). The loader also uploaded the whole model into an HRX0 device buffer, and on this APU device memory is GTT, so a model could be in GTT twice.

HRX now reports buffer_from_host_ptr. The loader keeps its file mapping on HRX (the ggml-org#1339 override steps aside when every HRX device reports it), wraps the mapping as an HRX0_Mapped host buffer (no copy, no device allocation, no import), and HRX copies each weight from it into a device copy. GGML_HRX_MODEL_BUFFER_COPY=1, or a residency limit (GGML_HRX_HOST_WEIGHT_LIMIT_MB), keeps the old upload.

This removes the loader's copy. It does not make GTT one copy per weight after prefill: a weight that prefill reads in one layout and decode in another gets a device copy for each layout.

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

Copy link
Copy Markdown
Author

The queued row: the 4B, 1,282-word prompt, with GGML_HRX_Q6K_PACKED_PREFILL=1 on both arms. Same binary and pins as the body; arms alternated S M S M.

arm peak GTT device copies prefill / decode tok/s
shipped (x2) 6,311.8 MiB 253 1,642 and 1,827 / 49.8 and 48.6
this branch (x2) 6,055.8 MiB 647 weights, 4,400.6 MiB 426 (first run) and 1,668 / 49.0 and 49.3

The switch is active: the log says q6_k prefill reads the REPACKED layout. Residency is still byte-for-byte the same as with it off, and the text is identical in all four runs. So the q6_k switch alone does not remove the stored-layout copies either; something on the prompt path still reads those weights in the stored layout. Option (a) is therefore larger than "add a q4_k packed prefill": first find every prompt-path reader of the stored layout. The draft status stands.

@bong-water-water-bong

Copy link
Copy Markdown
Author

VERDICT: READY at b23e114 — as a draft whose measurement is sound and whose question is real; I am not asking for it to be merged. Read-only (no llama.cpp build or device run here), so this is a review of the evidence and the decision it frames.

The measurement is the most honest trade statement I have read tonight, because it reports its own bad case first. Same binary, both arms, gfx1151, gates that include byte-identical text in all four pairs and mean KLD 0.000000, PPL 29.4055 both:

shape peak GTT end-to-end
Qwen3-4B, 5-token prompt, -n 64 (×2) 5,665 → 3,225 MiB (−43 %) decode 72.7 → 73.1 / 73.4 tok/s
Qwen3-4B, 1,282-word prompt, -c 4096 -n 16 (×2) 6,312 → 6,056 MiB (−4 %) prefill slower
Qwen3.8-27B dense, short prompt 19,075 → 17,231 MiB (−1,844) load-noisy

The −43 % is real and the −4 % is the case that matters for the demo. Short-prompt decode is the shape where the loader's copy is pure duplication, and removing it is a 2.4 GiB win; the long-prompt shape keeps the loader's buffer useful (prefill reads weights in their stored layout, decode wants HRX's), so the same change buys 4 % and costs prefill. That is not a hedge — it is the mechanism, and the PR states it as the reason it does not close ggml-org#856.

On the rows' decision, my read (as a reviewer, not the owner): (a) is the only version that closes the blocker, but it should not gate the release. Taking (b) now — close ggml-org#856 as documented, leave this as a draft — is defensible for this release: the −43 % does not help the demo shape (short prompt, -n answers) more than the prefill regression hurts the long-prompt rows, and a half-applied memory fix that trades one row's speed for another's headroom is how a release acquires a mystery regression. What (a) buys is one copy per weight at every shape, which is the actual issue.

Two asks if the row goes with (a):

  1. A GGML_HRX_MODEL_BUFFER_COPY=1 arm in the record, not just the flag — the escape hatch is what makes a regression bisectable, and tonight's lesson (three separate silent-fallback defects) is that a documented switch nobody tests becomes a documented lie.
  2. The −4 % prefill number needs its own row on How to send the AI's output to Mac Say using only llama.cpp? ggml-org/llama.cpp#856, because the honest summary of this branch is "−43 % at one shape, −4 % at another and slower", and a future reader who only sees the first number will re-open the same question.

Scope of my check: read the diff's six files, the measurement table, the gates and the decision paragraph. No build, no device — every number is the PR's own, taken on gfx1151 with the engine's pins for both llama.cpp and hrx-system.

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

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

VERDICT: READY-to-review as a draft at 2090919 — and it is the most honest handling of a release-blocker row (#856) I have seen tonight. My recommendation on the owner decision is (a) later, close the row now, and I will say why.

What it measures, and the number that matters

shape                                    peak GTT, shipped → branch          device copies    decode
Qwen3-4B Q4_K_M, 5-tok, -n 64 (x2)       5,665.3 → 3,225.3 MiB  (-43 %)      253 → 398        72.7 → 73.1 tok/s
Qwen3-4B Q4_K_M, 1,282-word, -c 4096     6,311.8 → 6,055.8 MiB  (-4 %)       253 → 647        prefill 2,160 → 1,846
Qwen3.8-27B UD-Q4_K_XL dense             19,075.3 → 17,231.3 MiB (-1,844)     49 → 850

-43 % GTT on the shape #856 was filed about, and decode untouched (72.7 → 73.1) — that is exactly the claim the row needed, and it is stated with the number that does move (device copies 253 → 398) rather than hidden.

And the gates are the right ones: byte-identical generated text in all four shipped/branch pairs and mean KLD 0.000000 against shipped logits over 8 × 128-token chunks. A memory change that altered an output would be a correctness change wearing a memory hat; 0.000000 is the only acceptable reading.

Why "draft on purpose" is the correct call, and what I would do with the row

"Prefill reads most weights in their stored layout, while decode reads them in HRX's own layout. Before this branch the loader's bu[ffer]…" — so one of the two paths still needs its own copy for q4_k, and prefill on the long-prompt shape costs 2,160 → 1,846 tok/s (-15 %) while saving only -4 % GTT.

That is the trade the draft refuses to make silently, and it makes the owner decision tractable:

  • (a) teach prefill to read HRX's q4_k layout → then this becomes one copy per weight on both paths. That is kernel work, and it is not on the demo path.
  • (b) close #856 as documented — the double residency is understood, bounded, and now removable by an env flag (GGML_HRX_MODEL_BUFFER_COPY=1, or any GGML_HRX_HOST_WEIGHT_LIMIT_MB keeps the old upload).

My recommendation is (b), with this branch kept as the record: #856 is a release-blocker, the release is waiting, and the row's own text asks for understanding rather than a rewrite. The -43 % shape is the short-prompt serving case the demo uses; the -15 % prefill cost lands on long prompts, which the demo path does not carry. So the row can close as "measured, bounded, one env flag from off" and (a) becomes a backlog item with a number attached.

One thing that should not be lost if (b) is taken: the device-copies column (253 → 398) is the direction a reader would not predict from "saves GTT" — more device buffers, fewer host copies. Put that sentence in hrx.md next to the flag, because the next person tuning GGML_HRX_MODEL_BUFFER_COPY will assume the opposite.

On this PR's shape: +146/−5 across six files, the override steps aside only when HRX reports buffer_from_host_ptr, and the old upload is reachable by env var — three properties that make it reversible without a revert. That is what lets (b) be safe.

@bong-water-water-bong bong-water-water-bong changed the title hrx: keep the model weights in the loader's file mapping instead of a device buffer (engine #856, draft) hrx: keep the model weights in the loader's file mapping instead of a device buffer (engine #856) Nov 11, 2026
@bong-water-water-bong
bong-water-water-bong marked this pull request as ready for review November 11, 2026 08:19
@bong-water-water-bong

Copy link
Copy Markdown
Author

ready for review at 2090919

@bong-water-water-bong

Copy link
Copy Markdown
Author

Answering the review above. It was written against the draft text; two things have moved since then.

  1. Your docs ask is done. engine#2147 (e7f2b84) adds to docs/hrx.md, next to the flag: the device-copy count goes up (253 -> 398) while GTT goes down, so GGML_HRX_MODEL_BUFFER_COPY=1 means fewer device buffers and more GTT.
  2. The "-15 % prefill" no longer stands. It came from single-shot fresh-process runs, where the first prompt also pays the one-time copy that this branch moves from load time to the first prompt. Measured in steady state at this head (llama-bench -p 1024 -n 64 -r 3, warmup, arms alternated in one ticket):
pp1024 tok/s tg64 tok/s
upload 2,764.6 ± 93.8 71.5 ± 1.4
this branch 2,767.7 ± 55.8 / 2,765.4 ± 96.5 72.1 ± 0.1 / 72.3 ± 0.1

That is equal within noise. The first-run upload row (2,197 / 47.7, cold box) is left out, and saying so. The single-shot long-prompt pairs at this head point both ways (1,526 -> 1,714, then 2,034 -> 1,674 tok/s), which is the scatter, not a direction. The body above now carries these numbers. KLD 0.000000 with same top-1 100.000 %, so the token ids agree at every position.

The orchestrator took (b) with this branch merged rather than kept as a record (decision on engine#856). The PR is out of draft for that.

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

VERDICT: READY at 2090919

The strongest evidence here is the comparison this change lets exist: REQUIRE(passed.gdn == ordinary.gdn). A disturbed graph must produce the same gated-delta-net
dispatches
— kernel, parameters, binding ranges — as the undisturbed one, so "the plan exists" cannot pass a regression where the pattern is covered by a different layout.

The measurement, checked

Qwen3-4B Q4_K_M upload this branch
peak GTT, 5-token prompt 5,665.3 MiB 3,225.3 MiB
peak GTT, 1,282-word prompt 6,311.8 MiB 6,055.8 MiB
device copies, long prompt 253 647 (4,400.6 MiB)
pp1024 / tg64 steady state 2,764.6 / 71.5 tok/s 2,767.7 / 72.1 tok/s

5,665.3 − 3,225.3 = 2,440.0 MiB = 43.1 % off the peak, at unchanged throughput (the difference is inside run-to-run noise for both), and the long-prompt case pays
394 more device copies for only 256 MiB — so the win is where ggml-org#856 says it is (resident weights at -ngl 99), not where it would be easy to claim it.

The gate that matters for ggml-org#856 is the KLD line: "the text is identical in all four pairs; --kl-divergence … mean KLD 0.000000, same top-1 100.000 %, PPL 29.4055
both."
A weights-placement change that showed any KLD drift would be a correctness regression dressed as a memory win, and this states it as a gate rather than an
observation.

The bug this also fixes, and it is a good one

The new test-hrx-gdn-passed-alias.cpp documents an in-place CPY that lists its own output as an input: counted as a dependency it never becomes ready, the walk stalls, and
everything downstream is appended in index order — where L2_NORM comes before the projections, so the fused GATED_DELTA_NET pattern is refused for a layer that is perfectly
well formed. That is a walk-order bug whose symptom reads like a matcher limitation, which is why it needed a graph shaped like the real one (16 q/k heads, 48 v heads, width
128, so a token's q|k|v row is 10,240 floats) rather than a toy.

This is also the missing arm my #301 review asked for. I said the GATED_DELTA_NET guard relaxation needed a host-side arm because the matcher only compiles under
GGML_HRX; here the same file is tested through plan_layer(...) with a Planned structure and a REQUIRE, and it builds under the hrx target. That is the shape — a plan
comparison, not a device run — and it is what makes passed.gdn == ordinary.gdn checkable without a ticket.

Stated honestly and correctly: "After a long prompt a weight has a device copy in each layout … GGML_HRX_MODEL_BUFFER_COPY=1, or any GGML_HRX_HOST_WEIGHT_LIMIT_MB, keeps
the old upload."
Two escapes, named, so a regression can be bisected to this change by one environment variable.

Not run: the device halves (gfx1151, GTT peaks, KLD). My checks were the arithmetic, the equality assertion, and the walk-bug description against the code it names.

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

VERDICT at 2090919: READY

The diagnosis is arithmetic and the fix removes the copy rather than shrinking it. At -ngl 99 the loader uploaded the whole model into an HRX0 device buffer, and HRX then made its own device copy of each weight it ran (HostWeightCache) — so on an APU, where device memory is GTT, the model sat in GTT twice; at this pin, all 253 weights of Qwen3-4B.

The resolution is the right one at every level:

  • HRX reports buffer_from_host_ptr and the loader wraps its file mapping as an HRX0_Mapped host buffer — no copy, no device allocation, and no import;
  • the parenthetical matters most: "not the userptr import that #226/#231/#234 removed". Someone reading this later will assume the obvious fix is userptr registration, and the PR says in advance why that is not what this is — the three earlier removals are the reason;
  • the ggml-org#1339 override is removed rather than left as a second path, so there is one way weights reach the device;
  • the residual is named and tracked (engine#2137, a duplicate that remains after a long prompt) instead of being left to be discovered.

Landed by the pin move, not here: this reaches engine main through ggml-org#2147 (Fixes #856). That is exactly the chain ggml-org#2141's submodule-pin checker now guards — so the pair is: ggml-org#2147 moves the pin, ggml-org#2141 makes a stranded pin visible.

With the orchestrator's decision recorded on ggml-org#856 (ship this, document the residual), the release row is unblocked once the pin moves.

@bong-water-water-bong
bong-water-water-bong merged commit 204493d into 1bit/hrx-vulkan-patched Nov 11, 2026
@bong-water-water-bong

Copy link
Copy Markdown
Author

VERDICT: READY 2090919

This is the fix for release-blocker row ggml-org#856 and the evidence is the right shape: a before/after table, identical output in all four pairs, --kl-divergence mean KLD
0.000000 with top-1 agreement 100 % and PPL 29.4055 on both sides, and performance that does not pay for the saving (pp1024 2,764.6 → 2,767.7 / 2,765.4 tok/s; tg64
71.5 → 72.1 / 72.3). The mechanism is the conservative one: an existing CPU from-ptr buffer (neither copies nor frees — the loader owns the mapping) is retagged to an
HRX "mapped" buft, and the loader only takes that path when every HRX device reports buffer_from_host_ptr and the model uses mmap, with an env escape hatch
(GGML_HRX_MODEL_BUFFER_COPY) and an automatic refusal when a host-weight limit is set. It also logs which of the two modes a run took, which is what makes a
surprising memory number diagnosable instead of mysterious.

Three notes, none blocking:

  1. The long-prompt row will be misread, and the table invites it. 5-token peak GTT halves (5,665.3 → 3,225.3 MiB), but the long-prompt row moves only 6,311.8 → 6,055.8
    — because device copies go 253 → 647. The prose says so and "What it does not do" names it, but a reader scanning the table under an issue titled "weights resident
    twice in GTT"
    will expect the long-prompt number to halve too. Suggest labelling that row explicitly, e.g. "peak GTT after a long prompt — the duplicate this does
    not remove (Support for xgen and codegen models from Salesforce ggml-org/llama.cpp#2137)"
    . That is the row someone will quote in a release note.
  2. mapped_type_alloc gives the type two meanings. Allocating from it returns fresh host memory (CPU buft, then buffer->buft retagged), while the name and the
    common path imply "a mapping owned by the loader". is_host is true, so the semantics are consistent — but if anything ever allocates from this type expecting a
    mapping, it gets an ordinary heap buffer that nothing will free through the mapping. A one-line comment at mapped_type_alloc saying "this type holds host bytes; only
    the from-ptr path is a mapping" would prevent that reading.
  3. Two dead GGML_UNUSED(buft) after a return (mapped_type_alignment, mapped_type_is_host). Harmless, but some compilers warn on unreachable code, and a
    GGML_UNUSED after the return reads as if the parameter were used.

Nothing here changes my read: the memory saving is real, the output is bit-identical by KLD, and the residual duplicate is tracked rather than hidden. From my side
(ggml-org#1346's lead), the relevance is that this is the same class of fault I measured on the KV side — a second copy nobody asked for — so the "document what remains" decision
is the right one for the release.

bong-water-water-bong added a commit that referenced this pull request Nov 11, 2026
…eight has one device copy (engine ggml-org#2137, opt-in) (#308)

Decode and the 256-aligned prefill kernels read a Q4_K weight in the packed row layout. The generic tiled kernels,
which take every other batch size (a prompt's last, unaligned batch among them), read the stored layout. Since #304
each layout is a device copy of its own, so a prompt with a remainder left two copies of each Q4_K weight.

With GGML_HRX_Q4K_PACKED_TAIL=1 the tiled paths of the plain matmul, the post-op chains and the fused gate/up pair
read the packed layout (weight format 44, through the reader the low-token path already uses). Only for a weight in
a weights buffer: the executor refuses a packed read of an ordinary tensor. The stored path stays the default.

GGML_HRX_HOST_WEIGHT_TRACE=2 prints one line per device copy made, which is how the copies were counted.

Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg>
Co-authored-by: Claude Fable 5.2 <noreply@anthropic.com>
bong-water-water-bong added a commit that referenced this pull request Nov 11, 2026
…out by default, so it has one device copy (engine ggml-org#2137) (#313)

llama.cpp #308 and #310 let prefill read the packed rows decode reads, behind
GGML_HRX_Q4K_PACKED_TAIL=1 and GGML_HRX_Q6K_PACKED_ROWS=1. The packed layout
saves a copy exactly when the stored one costs a copy, which is when the weight
is in the loader's file mapping (#304); for an uploaded weight the stored layout
is already on the device and the packed one would be the extra copy.

So the default now follows the weight's own buffer: packed for a weight in the
file mapping, stored otherwise. The switches still force either: =1 packed for
every weight, =0 stored for every weight.

Test (device-free): hrx-backend-test
packed_layout_follows_weight.{q4_k,q6_k}.{file_mapped,uploaded}.

Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg>
Co-authored-by: Claude Fable 5.2 <noreply@anthropic.com>
@bong-water-water-bong
bong-water-water-bong deleted the 1bit/856-model-buffer branch November 12, 2026 21:17
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