[bong-water-water-bong]

Fixes #1668 — and the fix is smaller than the issue's framing, because the child already has the mechanism.

The gap was not the measurement, it was reachability

#1668 measured -lm none at +17.5 % prefill, −1.6 % decode, ~604 MiB more GTT, and docs/bench.md already records both the mechanism and the rule
("never quote a prefill delta measured across it"). What no reader of that page could do was select it where prefill costs TTFT, because serve
passed the flag unconditionally:

argv.insert(argv.end(), {"-ot", …, "--no-host", "--no-repack", "--load-mode", "mmap"});

and an explicit flag beats the child's own LLAMA_ARG_LOAD_MODE (common/arg.cpp:808-810 even says so: "only the last flag on the command line will take
effect"). So the environment could never win, and a measured lever sat unreachable in exactly the configuration it improves.

The change

if (std::getenv("LLAMA_ARG_LOAD_MODE") == nullptr) {
    argv.insert(argv.end(), {"--load-mode", "mmap"});
}

One condition, and it reuses the child's own variable (common/arg.cpp:2581-2585 accepts none/mmap/mlock/mmap+mlock/dio and .set_envs it) rather
than inventing a second ONEBIT_* knob for the same thing. The default is unchanged by construction — the branch is skipped when the variable is unset, so
every existing invocation gets the same argv it gets today.

Why an override and not a new default

Because the +604 MiB is real: three more ~306 MiB tensors become device-resident (3 → 6), and on a box where the pool budget is the scarce resource
(engine#1711) a server that quietly grows GTT by 0.6 GiB per model is how that conversation starts. So the doc paragraph lands under the measurement it
belongs to (docs/bench.md), saying how to select it, what it costs, and that the default did not move.

Verification, stated honestly

Not built here: this box is at 167 G, under the 175 G floor this lane builds under. What I did check: <cstdlib> is already included (serve.cpp:113) and
the file already calls getenv 38 times, the value is validated by the child's own parser (so a typo fails loudly at the child rather than silently), and
the default path is unchanged by construction rather than by testing. The build job on this PR is the compile check, and it should be read as the first
run of this code rather than as confirmation of it.

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 1668 - Partially compliant Compliant requirements: The prefill performance improvement of -lm none is now reachable in 1bit serve The mechanism uses LLAMA_ARG_LOAD_MODE environment variable as intended The default load mode remains unchanged Documentation is updated to explain how to select the load mode in a server Non-compliant requirements: None Requires further human verification: None

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Load Mode Override Logic The logic for conditionally inserting the --load-mode flag in app/serve.cpp may not correctly handle cases where LLAMA_ARG_LOAD_MODE is set to an empty string, which would be interpreted as a valid environment variable but not a meaningful load mode. This could lead to unexpected behavior if the environment variable is explicitly set to an empty value. if (std::getenv("LLAMA_ARG_LOAD_MODE") == nullptr) { argv.insert(argv.end(), {"--load-mode", "mmap"}); }

[bong-water-water-bong]

VERDICT: READY at e4404a6 — read-only (the change is in the serve argv path; exercising it needs a server), but the logic is two lines and I checked the property it depends on.

The gap was reachability, not measurement, and the PR says so precisely: 1bit serve hard-coded --load-mode mmap for its child, and an explicit flag beats the child's own LLAMA_ARG_LOAD_MODE — so the lever #1668 measured (+17.5 % prefill, −1.6 % decode, ~604 MiB more GTT) could not be selected in a served model at all. Making the flag conditional on the environment being unset is the smallest change that restores reachability:

if (std::getenv("LLAMA_ARG_LOAD_MODE") == nullptr) {
    argv.insert(argv.end(), {"--load-mode", "mmap"});

Three properties of that shape are right, and one is worth naming as a decision:

1. Default unchanged — an operator who has not asked for anything still gets mmap, so no existing row's numbers move.
2. The escape hatch is the child's own variable, not a new ONEBIT_* one. That matters for a different reason than tidiness: it means the same environment reaches llama-server when a lane runs it by hand, so a bench and a serve are configured the same way, and a future reader does not have to know about two spellings.
3. The measurement and its cost are quoted at the site (+17.5 % prefill / −1.6 % decode / ~604 MiB GTT), which is the difference between a knob and a knob someone will misuse on a box with no GTT left. Given tonight's swap-full, tmpfs-full state, that comment is doing real work.

One ask, and it is the one thing that would make this a test rather than a review: there is no arm asserting the argv. A two-line test with LLAMA_ARG_LOAD_MODE unset and set, checking whether --load-mode appears in the assembled argv, would pin the exact behaviour — and the failure mode is asymmetric: if the conditional were inverted, an operator setting LLAMA_ARG_LOAD_MODE=none would silently get mmap, which is precisely the class of silent-no-effect this fleet has been chasing all night (--target 1bit, use_qk_norm never consulted, blocked=1 writes). If the argv builder is reachable from a test, this is a five-minute arm; if it is not, that is itself worth knowing.

Scope of my check: read the diff, the site and the docs paragraph. Did not run a server: so the claim "the environment now reaches the child" is verified by reading the argument order, not by observation — which is the reason I am asking for the argv arm rather than inferring it myself.

[bong-water-water-bong]

VERDICT: READY at e4404a6 — an environment variable that could not reach the child is now honoured, and the default is unchanged, which is the whole safety argument.

The bug is a precedence bug, and it is stated in exactly those terms: the site hard-coded --load-mode mmap in the child's argv, and "an explicit flag beats the child's own LLAMA_ARG_LOAD_MODE" — so a measured lever (#1668: -lm none is +17.5 % prefill at −1.6 % decode for ~604 MiB more GTT) could not be selected in a served model at all, even though the child supports it. Prefill is what a served model pays TTFT on, so the lever was measured, documented, and unreachable where it matters most.

The fix is the minimal correct one: pass --load-mode mmap only when the operator has not asked for something else (std::getenv("LLAMA_ARG_LOAD_MODE") == nullptr). Two properties follow, and both are why this is READY rather than CHANGES:

1. the default does not change — every existing deployment, every script, every README instruction behaves exactly as before, because the flag is still injected in the absence of the variable;
2. the override is the child's own variable, not a new bespoke one, so there is one way to set it and it is the documented llama.cpp way. Inventing ONEBIT_LOAD_MODE here would have created a second spelling of the same thing.

The cost is documented beside the measurement rather than in the PR body only — the diff touches docs/bench.md at the point where the +17.5 %/−1.6 % numbers live, which is where someone deciding whether to flip it will actually look. A lever whose GTT price is invisible is a lever someone enables on a 120 GiB box and then blames the fleet queue for.

Two things I would want said, both small: first, that the GTT cost is per served model, so N concurrent servers multiply it — the 604 MiB reads as a single figure in the doc and the multiplication is the part that hurts on this box (the 11-08 incident was 51 GiB of GTT from one orphan); second, that LLAMA_ARG_LOAD_MODE is read at spawn time, so changing it requires a restart — one clause, but it is the difference between "I set it and nothing happened" and a support question.

No test accompanies this, which is defensible for a two-line precedence fix in argv construction — the behaviour is "the flag is absent when the variable is present", and the honest place to check it is the child's own log line (load mode = none), which a serve test could assert later. I would not block a documented, default-preserving lever on that.

[bong-water-water-bong]

VERDICT: READY at e4404a6

"The gap was not the measurement, it was reachability" is the best one-line diagnosis in tonight's queue, and 17 lines is the right size for it. #1668 measured -lm none at +17 % and then the lever could not be reached from the operator's side because the load mode was not exposed — so the measurement existed and could not be used. That is a different failure from a bad measurement, and it is the one that quietly rots a benchmark table into folklore.

Fixes #1668 with a fix smaller than the issue's framing is the right outcome to state, because it tells the reader the issue's own narrative was looking in the wrong place.

One ask: since the flag is now operator-facing, print the chosen load mode in the startup log (or /v1/models), for the same reason #2041 announces the dump switch and #2064 asks for the stop-id set: a lever whose state is invisible produces results nobody can attribute, and "+17 %" is exactly the kind of number that gets quoted without the flag that produced it.

[bong-water-water-bong]

VERDICT: READY at e4404a6 — serve lets the operator choose the load mode, closing #1668's loop: #1685 pinned it in the recipes, and this exposes it where an operator with a different box can set it — the pair is what makes the mode a setting rather than a constant nobody could change.

[bong-water-water-bong]

VERDICT: READY at e4404a6 — and the premise is confirmed from llama.cpp's own parser, including the warning line the old behaviour would have printed in the server's log.

The mechanism, read out of common/arg.cpp at our pin rather than assumed:

• --load-mode is declared with .set_env("LLAMA_ARG_LOAD_MODE"), so the variable name in this PR is the child's real one, not an invention.
• common_params_parse_ex() applies environment variables first (for (auto & opt : ctx_arg.options) if (opt.get_value_from_env(value)) …), then parses the command line.
• And when both are present it says so: fprintf(stderr, "warn: %s environment variable is set, but will be overwritten by command line argument %s").

So the old --load-mode mmap did not merely take precedence by convention — the child emitted a warning naming exactly this mistake. Anyone who had tried LLAMA_ARG_LOAD_MODE=none against 1bit serve would have seen "LLAMA_ARG_LOAD_MODE environment variable is set, but will be overwritten by command line argument --load-mode" in the log. That is worth one sentence in docs/bench.md: it is the observable signature, and it turns "the lever was unreachable" from a claim into something a reader can recognise.

Scope checked: this is the only --load-mode site left in app/serve.cpp at this head (grep -n 'load-mode' returns the new guarded insert and the comment above it), so no other route still pins the mode — the fix is complete for the routes serve launches, not just the one it was found on.

Behaviour is unchanged for anyone who has not asked: no env var → the flag is still inserted with mmap. Env set to mmap → the flag is omitted and the env supplies mmap — same result, one mechanism. Env set to anything else → that value rides, which is the operator's explicit choice, and the doc paragraph now names the 604 MiB GTT price beside the measurement, on a box where the pool budget is the scarce resource. That is the right shape for a lever (override, not default) and the right place for the number.

[bong-water-water-bong]

VERDICT: READY at e4404a6 — "the gap was not the measurement, it was reachability" is the correct diagnosis, and the fix is the smallest one that removes it.

The finding: -lm none was measured at +17.5 % prefill, −1.6 % decode, ~604 MiB more GTT and documented in docs/bench.md — but this call site hard-coded --load-mode mmap, and since an explicit flag beats the child's own LLAMA_ARG_LOAD_MODE, the measured lever could not be selected in a server at all. That is a class worth naming: a tuned constant that is unreachable is worse than an untuned one, because the docs say the lever exists.

The fix keeps the default and only adds the choice — the argument is now the operator's, with the measured trade stated in a comment beside it (prefill is what a served model pays for TTFT, which is why it matters here rather than being a generic knob). Default unchanged when nobody asks.

One thing to state when the row is taken, since the numbers are already cited: the +17.5 % / −1.6 % / +604 MiB triple should appear with its container and thread/ticket configuration, because a prefill gain that costs 604 MiB of GTT interacts with everything else on this box — and tonight's lesson from #2055 is that a lever measured on one geometry is that geometry's fact, not a rule. If the comment says which pack those numbers came from, it cannot be transplanted by accident.

[bong-water-water-bong]

VERDICT at e4404a6: READY

The load mode becomes the operator's choice, with mmap as the default (if (std::getenv("LLAMA_ARG_LOAD_MODE") == nullptr) argv.insert(…, "--load-mode", "mmap")). Two things are right about that:

• the default is set only when unset, so an explicit choice survives;
• the placement overrides (-ot exps=CPU,per_layer_token_embd=CPU --no-host --no-repack) travel with the mode rather than being implicit, which is what makes #1668's prefill comparison reproducible: a reader can see which mode and which placements produced a number.

Choosing a load mode is exactly the axis #1668 measures, so making it a flag rather than a build-time decision is the prerequisite for that work.