Repository navigation
bump llama.cpp to 831629fe: the live pool check takes the host term (fixes #1417) - #1759
Conversation
…fixes engine #1417) Moves third_party/llama.cpp 60ae4c9358 -> 831629fe9a. Two commits matter for #1417 and both are the ones this issue asked for: 58cadcf hrx: the LIVE pool check takes the host term too (engine #1502) <- fork #259 831629f hrx: print the two storages and the view's layout for a refused alias node With #259 the live check answers min(GTT free, MemAvailable) - the same two-term model the open-time budget has had since #256 - so the issue's reported shape is the case it now refuses: a 15.35 GiB request, a GTT counter showing room, ~15 GiB of host memory left. Before this, the live check used the GTT figure alone, which is exactly the over-promise #256 removed at open time. What #1417 has now, across the three bumps: #254 + 8f124f9 the live check is ON by default, margin 256 MiB, pool_live_fits pure, and a typo can no longer switch the guardrail off (bumped in #1736) #256 + #269 the pool budget is min(GTT term, host term), and the host-weight arena is charged to the pool before it grows #259 the live check reads the same two terms Verified: registry regenerated at the new pin (only the sources sha moves; counts stay 298/9/298/298), registry_build.py --check-pins passes, check_pins.py says '60ae4c935 -> 831629fe9 is ahead'. Not claimed by this bump, stated so a merged pin is not over-read: the allocator-side case (an allocation that fails although the live figures say it fits) is #1502's slab-provider class, and the fork's own review of #259 left one line open - a raw 0 from host_memory_available_bytes() is mapped to kHostUnknown by both callers, so a genuinely exhausted host reads as 'unreadable' and the term is dropped. That is the extreme case, and it is written in the #259 thread rather than fixed here.
bong-water-water-bong
left a comment
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at +2/−2 head — bookkeeping, and the six commits it carries are ones this fleet has already reviewed individually.
The move: 60ae4c93 → 831629fe on 1bit/hrx-vulkan-patched, clean fast-forward (ahead_by: 6, behind_by: 0), gitlink and registry/architectures.json in the same two lines. The six: 3bfdde2f2 (pool budget takes the host term — #256), 20b2eba16 (allocation failure leaves ALLOC_FAILED — #261), e39659538 (host-weight arena charged — #269), c20e1764e (say WHICH alias lookup missed — #262), 58cadcf72 (the live pool check takes the host term — engine #1502/#1417), 831629fe9 (print the two storages for a refused alias node).
That list is worth reading as a sentence: admission now accounts for host memory, a failed allocation is a named state instead of a silent one, the third arena is charged, and a refused alias lookup says which of three reasons it was. Those are the four things that made the guardrail true of "one of the two ways a launch can run out of memory" — this pin is the first one that carries all of them together.
Precondition unchanged from the same rule as #1694/#1736: CI builds without HRX, so a green build says nothing about this pin's HRX half; the device run (tests/hrx_long_prompt.py + serve_e2e.sh {hrx,cpu} under a gpu-hold) is what proves it works, and "compiles" was settled by #1736's duplicate-declaration fix that is already in the previous pin.
Not verified: that 831629fe compiles and passes on hardware — 7 GB available, no build.
bong-water-water-bong
left a comment
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at 6b0c820c6eaed4fae074a69241282549069d5c64
A pin bump, so I checked the three things that make one safe rather than trusting the title:
1. The commit exists and is a real llama.cpp head. 831629fe is present in the submodule, and its log is exactly what the title claims:
831629fe9 hrx: print the two storages and the view's layout for a refused alias …
58cadcf72 hrx: the LIVE pool check takes the host term too (engine #1502, #1417)
e39659538 ggml-hrx: the host-weight arena is charged to the pool, like the transient …
3bfdde2f2 hrx: the pool budget takes the host memory as a second term, so admission …
2. It is a fast-forward, not a rewrite. git merge-base --is-ancestor 60ae4c93 831629fe → true, so the old pin is an ancestor of the new one. That is the property that matters for a pinned submodule: a forward bump means nothing that was reviewed has been dropped or rewritten, only added to.
3. The pieces the title names are the ones a reviewer already signed off on: the live pool check's host term (#259), the host-weight arena charge (#269), and the refusal-message facts (#267) are all inside the range — so this bump is "carry the reviews that landed", which is the least risky form a bump takes.
One thing worth noting for whoever reads this later: 60ae4c93 was already behind main when I reviewed #259 an hour ago (the tool complained the pin is 60ae4c935), so this closes a gap that had been open for a while rather than jumping ahead of anything.
Not verified: that the tree builds against the new pin — hw_gate/build will do that, and a pin bump is precisely the case where CI is the right instrument. The submodule-side checks above are the ones CI cannot do (it resolves the submodule tree without verifying that the old pin is an ancestor of the new one).
…rrects #1771) #1771 retracted the "read hrx_build as unavailable" instruction, which was right, but its closing clause says the failure "was not a property of the pins in the tree — which is what the same pins failing four hours earlier had seemed to say". The pins are not the same, and the gitlinks come from the runs themselves: red, 21:06 run 42699357566 (fix/1696) hrx-system 7f4236924 llama.cpp 8f124f906 -> failure green, 00:21 run 42727646749 (bump-hrx/831629f) hrx-system 7f4236924 llama.cpp 831629fe9 -> success hrx-system matches; llama.cpp does not. And 8f124f906 is exactly the pin #1733 root-caused (ggml-hrx.cpp declares device_free_total twice: void from #252's forward declaration at :383, bool from #240's 800 lines later), which #1759 replaced. As merged, the paragraph teaches the opposite lesson: "a window, not the pins" instead of "this pin was broken, a bump fixed it, and a llama.cpp bump must clear hrx_build before it lands" — the rule the hw-gate label exists for, and what #1768 did before merging (hrx_build pass 1m10s). Docs only.
…rrects #1771) (#1776) #1771 retracted the "read hrx_build as unavailable" instruction, which was right, but its closing clause says the failure "was not a property of the pins in the tree — which is what the same pins failing four hours earlier had seemed to say". The pins are not the same, and the gitlinks come from the runs themselves: red, 21:06 run 42699357566 (fix/1696) hrx-system 7f4236924 llama.cpp 8f124f906 -> failure green, 00:21 run 42727646749 (bump-hrx/831629f) hrx-system 7f4236924 llama.cpp 831629fe9 -> success hrx-system matches; llama.cpp does not. And 8f124f906 is exactly the pin #1733 root-caused (ggml-hrx.cpp declares device_free_total twice: void from #252's forward declaration at :383, bool from #240's 800 lines later), which #1759 replaced. As merged, the paragraph teaches the opposite lesson: "a window, not the pins" instead of "this pin was broken, a bump fixed it, and a llama.cpp bump must clear hrx_build before it lands" — the rule the hw-gate label exists for, and what #1768 did before merging (hrx_build pass 1m10s). Docs only.
…#1759 (#1796) * hw-gate: the hrx_build window is 20:01–21:52Z and #1736 closed it, not #1759 The paragraph in main got the window and the credit wrong, and both are checkable against the runs: * **window.** "Between about 21:0x and 23:1x" is #1771's figure. `gh run list --workflow hrx-build.yml --limit 60` shows failures from **20:01** (`1bit/803-op-fallback`) through **21:52** (`1bit/fused-tiles-to-int8`) and none after; the first green on the fixed pin is 21:41, and the branches that had failed come back at 22:04. * **credit.** "`#1759` replaced that pin with `831629fe` and the job has been green since" is wrong: the bump that compiles is `5b784d8b` "bump llama.cpp to 60ae4c935: the pin compiles again", and `gh api .../commits/5b784d8b/pulls` says **#1736**. `5f76f402` → `831629fe` is **#1759**, which came later (green at 00:21) and cannot be what ended a window that closed at 21:41. `60ae4c935` carries the fork fix, llama.cpp #260 `4e570337a`. Also added the one fact the table could not show and a reader will otherwise trip over: the single red run after the window — `2026-11-10 02:23` on `1b02be9`, pin already `5514ebea` — failed in **"Wait for memory (or refuse)" with exit 75**, a memory gate on the box (`gh run view 42742403894 --log-failed`), not a compile failure. Without that line, a red badge on a newer pin reads as the old break returning. Supersedes the closed #1775 and corrects what #1776 merged; the `hrx-system`-vs-`llama.cpp` framing it landed is kept, because that part is right and is what makes the pin the obvious suspect. Refs #1776, #1775, #1733, #1736, #1759 * hw-gate: fifteen failing runs, not ten -- and two of them are not this failure Review of #1796, both points reproduced here before changing anything: * `gh run list --workflow hrx-build.yml --limit 80`, filtered to 20:00–21:55Z and `conclusion==failure`, returns **15** timestamps (20:01 20:13 20:17 20:32 20:59 21:00 21:01 21:02 21:06 21:15 21:24 21:25 21:27 21:42 21:52), not ten. The table says 13 of 15 on `8f124f906`. * the other two are on the **older** pin: `git ls-tree 650f328 third_party/llama.cpp` and `ls-tree 7ccbf74` both give `b970d85ba`, with `hrx-system` `7f4236924` unchanged -- so the 21:00 and 21:01 failures cannot be the `device_free_total` redeclaration, which is `8f124f906`'s error. They now have their own row and a paragraph saying they are unexplained: the reviewer measured that the same pair was green at 19:42/19:45, and I could not re-verify that because those commits are no longer fetchable, and the failing run's log (`gh run view 42698418833 --log-failed`) returns nothing. So the window claim now distinguishes what it can prove from what it cannot: thirteen runs have a named cause, two do not, and neither is evidence for the other. That distinction is the reason to keep the table at all. Refs #1796, #1733 --------- Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg>
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
Fixes engine #1417 — this is the bump that delivers the case the issue actually reported.
What #1417 was
pool_live_fits()compared a request against the GTT figure alone. On this APU the GTT ceiling and DRAM are one pool, so the reported shape — a 15.35 GiB request, a GTT counter showing room, ~15 GiB of host memory left — passed the live check and failed inside the allocator instead. The open-time budget had grown a host term (#256); the live check had not.What this bump adds
60ae4c9358→831629fe9a:58cadcflive_free = min(total − used, MemAvailable), the same two-term model aspool_budget_terms()831629fSo open-time and live admission now answer the same question with the same terms. The suite that pins it ships in the fork (
test-hrx-pool-refusal-budget.cpp: the two-term arms,kHostUnknownskipping the term, a measured zero refusing, the margin saturating) — built and run at the PR head, all requirements pass.The whole of #1417, across three bumps
8f124f9(bumped in #1736)pool_live_fits, and a typo can no longer switch the guardrail offmin(GTT term, host term); the host-weight arena is charged to the pool before it growsVerification
tools/check_pins.py origin/main→60ae4c935 -> 831629fe9 is ahead, rc 0.registry/architectures.jsonregenerated at the new pin; only thesourcessha moves — counts stayhrx 298, npu 9, architectures 298, mapped 298, so neither commit touched the architecture surface.registry_build.py --check-pinspasses.hrx_buildon this PR is the real gate for the combination: it configures-DONEBIT_HRX=ONand buildsonebit, which is what caught my first pin attempt pointing at a commit that did not compile.Two things this does not claim
0fromhost_memory_available_bytes()tokHostUnknown, so a genuinely exhausted host reads as "unreadable" and the term is dropped — the extreme case, in the admitting direction. It is in serve: refuse a Q4_0 drafter next to a Hadamard-rotated file #259's thread with the one-line fix; I would rather close hrx: a large MUL_MAT case segfaults instead of being refused when the GTT pool budget is small #1417 on the reported shape than hold a pin over it, and it should be its own small PR.