Conversation
Upstream took the whole ternary engine (b5b892e and friends, mine) but not the HTTP wiring that reaches it: `larql_inference::ternary` exports `predict_bitnet`, `infer_bitnet_walk` and `generate_streaming_bitnet`, and before this commit nothing in larql-server called any of them -- only larql-cli did. The engine landed; the server could not serve it. The wiring existed on feat/bitnet-streaming-walk and was then lost: merge 918a397 on the fork's main dropped `is_bitnet()` / `get_or_load_bitnet()` from state.rs while keeping two callers in routes/openai/. That branch has not compiled since (E0599); f11d8a1 fixed the E0428s from the same merge and missed this. Ported here onto current upstream rather than merged, since state.rs and the openai routes have both since become modules. state/loaded_model.rs `bitnet_model: OnceLock<RwLock<BitnetModel>>` + `bitnet_init` guard, mirroring the existing `weights` / `weights_init` single-flight pattern; `is_bitnet()`, `is_dense_only()`, `get_or_load_bitnet()`, `force_load_bitnet_model()`. bootstrap/mod.rs Eager-load the ternary path instead of the dense one for BitNet containers (~5 GB of dense allocation saved on a 2 B BitNet), and exclude them from the startup memcheck -- `estimate_resident_bytes` models the dense path and over-counts a container that allocates no dense BitLinear tensors. routes/infer.rs Native-ternary /v1/infer, checked *before* the `has_model_weights` gate: a --keep-quant container carries ternary artifacts instead of the dense manifest that gate looks for, so it would otherwise be refused as weightless. Walk-mode goes through residual capture + KNN override, following upstream's current session-resolution idiom (`sessions.get(sid).and_then(|s| s.patched())`, reader not writer). routes/openai/{completions,chat/stream}.rs SSE streaming for both OpenAI surfaces via the ternary path, which also skips the dense `lock_weights_for_gen()` write lock that serialises all generation. Chat refuses tools / constrained generation rather than ignoring them: both need masked logits over the dense path, and answering a tool request with prose looks like a model that declined to call the tool. Adapted to upstream rather than copied: * chat streaming uses upstream's `TokenTap` for stop-string handling instead of the branch's hand-rolled buffering, so both paths share one implementation rather than two that must agree; * `pick_template` now requires `&ModelWeights`, which this path deliberately never loads -- the template comes from `ChatTemplate::for_family(&config.family)`, the same string `weights.arch.family()` would have produced; * both branches record a `GenerationTally` (`add_v3`), else /v1/stats reports BitNet traffic as zero throughput; * `FINISH_REASON_*` constants, not the branch's literal "stop"/"length"; * no inline SSE_DONE -- the response stream already chains it, so the branch's version would have emitted it twice. Also `is_dense_only()`: a --dense-only container has no gate vectors, so walk-mode runs against an empty KNN store and returns nothing useful. /v1/infer defaults mode to walk when a client omits it (pg_infer's remote backend posts {prompt, top} with no mode), which silently produced garbage. Walk/compare now coerce to dense on such containers. Verified with the pinned 1.98.0, not the ambient nix toolchain: clippy -p larql-server --all-targets -- -D warnings: clean (--all-targets matters -- 7 LoadedModel literals in tests/ need the two new fields and `cargo build` alone does not see them) cargo test -p larql-server --no-fail-fast: 1123 passed, 0 failed + 2 new tests (is_dense_only_detects_empty_gate_layers, bitnet_model_not_loaded_by_default) 16 test *binaries* SIGSEGV under --no-fail-fast. Pre-existing and not from this change: the identical 16 crash on unmodified 23a56db (verified by stashing). Untouched here.
CI's coverage policy flagged the four files the BitNet serving commit
touched. Two responses, split by what is actually testable.
Testable, so tested — state/loaded_model.rs gains two tests beside the
two already there:
* bitnet_guards_refuse_a_dense_vindex_with_a_useful_message —
`get_or_load_bitnet()` on a dense container must name *why* it
refused ("no bitnet_layout ... not a --keep-quant build") rather
than surfacing a load error for a file that was never going to
exist.
* force_load_bitnet_model_is_a_noop_when_infer_disabled —
`bootstrap::serve` calls this unconditionally for every model, so it
has to stay quiet on `--no-infer` even when the container *is*
BitNet-shaped. Eagerly loading ternary weights into a process that
refuses to infer would spend exactly the memory a --no-infer
operator asked not to spend.
Not testable yet, so baselined — routes/infer.rs (75.0),
routes/openai/completions.rs (79.5), routes/openai/chat/stream.rs (66.0),
at the values CI measured. Every ternary arm sits behind
`LoadedModel::is_bitnet()`, which needs a container carrying
`bitnet_layout` plus the `bitnet/` I2_S artifacts, and `synthetic_vindex`
builds a dense V2 container. So those ~300 lines are structurally
unreachable from the fixtures that exist, in the same way this policy
already documents for the V2 per-token emit closures and the tool-success
path.
The real fix is a synthetic BitNet fixture, and it is deliberately not
attempted here: it needs packed I2_S bytes plus per-row scales in the
kernel's contiguous layout, not just a config flag, so it is a piece of
work in its own right rather than a line in this commit. The policy note
records that, so the baselines read as debt with a named discharge
condition instead of as a lowered bar. Ratchet them when the fixture
lands.
`loaded_model.rs` deliberately gets no baseline: the four guard tests
should carry it over the 90% default, and if they do not, that is a real
gap worth seeing rather than papering over.
fmt, clippy --all-targets -D warnings, and 595 lib tests pass on the
pinned 1.98.0. Local full-suite coverage is not measurable on this
machine — its integration binaries SIGSEGV on unmodified main too — so
the baselines are CI's numbers, not mine.
…-> 90) The previous commit's two tests moved loaded_model.rs from 88.63% to 88.99%, 1.01 short of the 90% default floor. This covers the remaining reachable branch rather than adding a baseline for it. bitnet_load_failure_names_the_container: a container that *claims* to be BitNet (bitnet_layout present) but has no bitnet/ artifacts on disk must fail with the load error, not the "not a --keep-quant build" refusal. Those are different operator problems -- the first means "this vindex is the wrong kind", the second means "this vindex is the right kind and is incomplete" -- and reporting the wrong one sends someone to rebuild a container that only needs its files restored. Reachable with no weights: the fixture's path points at no bitnet/ directory, which is exactly the on-disk state of a truncated or partially-copied container. That drives `load_bitnet_model` far enough to return its error, which was the last uncovered branch in `ensure_bitnet_cell` short of a real ternary load. Also asserts a failed load leaves the cell empty, so a later attempt -- after the operator restores the files -- tries again rather than caching the failure for the process lifetime. That is a property of the OnceLock-set-after-success ordering worth pinning, not just a coverage line. fmt, clippy --all-targets -D warnings, 596 lib tests pass.
Found by verifying against the real microsoft/bitnet-b1.58-2B-4T, which
is the only thing that could have found it: the synthetic fixture is a
dense V2 container and *has* the weight files whose absence this is about.
The three non-streaming generation paths — the `/v1/completions` batch
loop, `chat/handler.rs`, and `responses/engine.rs` — all take a
`&mut ModelWeights` for the duration of generation, so they call
`lock_weights_for_gen()`. On a BitNet `--keep-quant` container there are
no dense weights to lock, so `ensure_weights_cell` reached for a manifest
that does not exist and the request came back as:
503 "failed to load model weights: IO error: No such file or
directory (os error 2)"
which tells an operator nothing about the actual situation: the model is
loaded and working, just not through that path.
Guarded in `lock_weights_for_gen()` rather than at the three call sites.
Every non-streaming path funnels through this one method, so one check
covers all of them instead of three that have to stay in agreement — and
the streaming paths are unaffected because they test `is_bitnet()` and
return before they ever reach the lock (completions.rs:381 before :461,
chat/stream.rs:56 before :148).
Refused rather than silently rerouted to the ternary engine: these callers
hold a `&mut ModelWeights` across generation and there is no dense
`ModelWeights` to hand them. The message names the paths that do work
(`POST /v1/infer`, or either OpenAI surface with `"stream": true`), since
the capability exists and only the route is wrong.
Real-model verification, microsoft/bitnet-b1.58-2B-4T (1.2 GB I2_S GGUF
-> `--keep-quant --dense-only --f16 --level inference`, 210 I2_S tensors,
30 layers, hidden 2560):
/v1/infer, no `mode` field -> Paris 0.9494, mode=bitnet
/v1/infer, mode=dense -> Paris 0.9494, mode=bitnet
/v1/infer, mode=walk -> Paris 0.9494, mode=bitnet (coerced)
All three agree to 4dp, which is the point: `is_dense_only()` coerces
walk to dense rather than answering from an empty KNN store. 0.9494
matches the 94.5% the original work measured on the June tree.
Also verified end to end on the real model: eager ternary pre-load
("Pre-loaded BitNet model for 'bitnet2b' in 3.3s" — the ternary path, not
the dense one); `/v1/completions` and `/v1/chat/completions` SSE both
stream coherent text with exactly one `[DONE]` (the duplicate I removed
during the port stayed removed) and `finish_reason: length`; chat refuses
tools with the intended message; `/v1/runtime` reports
`decode_tokens_per_second: 0.98`, i.e. the GenerationTally added during
the port is reaching the stats surface instead of reporting zero.
Throughput on 32 vCPU x86_64, A/B alternated, 3 reps, medians:
infer_short 4.757s (spread 0.008)
infer_long 24.785s (spread 0.100)
gen_8tok 11.629s (0.69 tok/s)
gen_32tok 32.661s (0.98 tok/s)
~1 tok/s is expected here rather than a regression: `ternary_matvec` has
a NEON path under `cfg(target_arch = "aarch64")` and no x86 SIMD
equivalent, so x86_64 runs the scalar kernel. An AVX2/AVX-512 ternary
kernel is the obvious follow-up and is not attempted here.
clippy -p larql-server --all-targets -- -D warnings: clean.
cargo test -p larql-server --no-fail-fast: 1283 passed, 0 failed, 0
crashes (+1 test: lock_weights_for_gen_refuses_bitnet_with_an_actionable_message).
`describe()` on a real BitNet 2B browse container took 288 ms (12 layers)
and 648 ms (30 layers), i.e. ~21.6 ms/layer, which works out to ~3.3 GB/s
of effective bandwidth on hardware that streams 200+. Two causes, both in
the gate scan.
1. The transpose defeated BLAS.
`gemv` was `matmul_transb(vec.reshape(1, hidden), gate)` -> `a.dot(&b.t())`
with `a` shaped [1, hidden]. ndarray only dispatches to BLAS when the
operand layouts qualify, and that shape does not reach `sgemv`.
Measured on one real layer (6912 features x 2560 dims, f32):
a.dot(&b.t()) 15.5 ms/layer 4.6 GB/s <- was
gate.dot(&vec) 3.3 ms/layer 21.4 GB/s <- is
manual row dot 9.8 ms/layer 7.2 GB/s
`Array2::dot(&Array1)` is ndarray's gemv entry and reaches
`cblas_sgemv` for a standard-layout operand, which the gate view is.
4.7x on the kernel, from deleting a reshape.
2. f16 layers cloned the whole gate matrix per query.
`gate_knn_mmap_fast` only handled f32, so every f16 layer fell through
to `resolve_gate`, which ends in `cache[layer].as_ref().unwrap().clone()`
-- a full f32 copy of the layer. At this shape that is ~71 MB cloned per
layer per call, so a 12-layer describe() spent ~850 MB on allocation and
memcpy before scoring a feature. The *decode* was already cached; it was
purely the copy. Now scores out of the cache through a view.
The lock is held across `gemv`. Deliberate: the alternative is cloning to
release it early, which is the cost being removed. Contention is
per-layer and a walk visits layers in sequence. Noted in a `ponytail:`
comment that the upgrade path is `Arc<Vec<f32>>` if high concurrency
shows it.
Measured end to end on a real microsoft/bitnet-b1.58-2B-4T browse vindex
(30 layers x 6912 features), via pg_infer on PG 18.6:
describe (12L) 288 ms -> 96 ms 3.0x
describe (30L) 648 ms -> 171 ms 3.8x
walk 677 ms -> 198 ms 3.4x
similar_to 639 ms -> 198 ms 3.2x
nearest_to 51.3 ms -> 35.5 ms 1.4x
concurrency (describe, c=16): 41 qps -> 111 qps
p50 under load: 268 ms -> 110 ms
Correctness unchanged: identical edges, same scores to 2dp (`Zone` 332.80
L22 top for 'France' before and after), and 93 concurrent queries still
return exactly one distinct answer.
cargo test -p larql-vindex --lib: 4736 passed, 0 failed.
clippy --all-targets -- -D warnings: clean. fmt: clean.
Not addressed here: the scan is still single-threaded (~15% of a 32-vCPU
box under 8 concurrent requests) and still O(all features). Parallelising
across layers with rayon, and an ANN index that beats a full scan at this
feature count, are both larger changes than this one.
…eiling
`PatchedVindex::walk` scanned layers in sequence. Each layer's `gate_knn`
is an independent gemv over that layer's gate matrix with no ordering or
data dependency, so `par_iter` over them is straightforward. `map` on a
parallel iterator preserves order, so the trace stays layer-ordered.
This required changing `f16_decode_cache` from
`Mutex<Vec<Option<Vec<f32>>>>` to `Mutex<Vec<Option<Arc<Vec<f32>>>>>`.
The previous commit's f16 fast path held the mutex across `gemv`, which was
fine when layers were sequential and fatal once they are not: parallel
layers would have serialised on a single lock, cancelling the change
exactly. Taking an `Arc` handle and releasing the lock before scoring costs
a refcount bump; cloning the buffer instead would reinstate the ~71 MB per
layer copy the previous commit removed. Applied to the batch path in
`scores_batch.rs` for the same reason.
A/B on a real BitNet 2B browse vindex (30 layers x 6912 features),
alternating binaries within each rep so drift cannot masquerade as a
difference, 5 reps, medians:
SINGLE-QUERY LATENCY
band=knowledge (12 layers) 72.3 ms -> 13.0 ms 5.56x
band=all (30 layers) 177.1 ms -> 14.4 ms 12.29x
THROUGHPUT (qps)
c=1 13.6 -> 73.6 5.40x
c=4 52.9 -> 156.6 2.96x
c=16 157.4 -> 199.1 1.27x
c=32 195.7 -> 210.2 1.07x
The latency win is real and large. The throughput speedup **decays to 1.0**
as concurrency rises, which is the point worth recording: both binaries
converge on ~200 qps. That ceiling is memory bandwidth, and parallelism
cannot move it -- rayon fills idle cores when queries are few, and at c=32
there are no idle cores to fill. 200 qps x 849 MB/query is ~170 GB/s, at
or past what this instance class sustains.
So: use this for interactive latency, and do not expect it to raise
saturated throughput. Raising that requires touching fewer bytes per query
(f16 scoring in place would be ~2x; sparse/ANN retrieval over the top-K
features rather than a full scan is the only route to an order of
magnitude), not more threads.
Correctness: identical top edges on both bands (`French` 755.70 for
band=all, `Zone` 332.80 for band=knowledge) across all 5 reps of both
binaries. Verified through pg_infer on PG 18.6 as well -- describe() 13.5 ms,
walk() 17.6 ms, describe_many() over 10 entities 55.8 ms.
cargo test -p larql-vindex --lib: 4736 passed, 0 failed.
clippy --all-targets -- -D warnings: clean. fmt: clean.
Contributor
Author
Added: rayon across layers (12× latency), and where the ceiling actually isSecond commit on this branch. This required changing A/B on a real BitNet 2B browse vindex, alternating binaries within each rep so drift can't masquerade as a difference, 5 reps, medians: Single-query latency
Throughput (qps)
The latency win is large and real. The throughput speedup decays to 1.0 as concurrency rises, and that's the part worth recording: both binaries converge on ~200 qps. That ceiling is memory bandwidth — 200 qps × 849 MB/query ≈ 170 GB/s, at or past what this instance class sustains. Rayon fills idle cores when queries are few; at c=32 there are none to fill. So this is a latency optimisation, and the PR text says so rather than quoting the 5.40× from c=1 as if it were a throughput number. Raising the ceiling needs fewer bytes per query, not more threads. f16 scoring in place would be ~2×. Sparse/ANN retrieval over top-K features instead of a full scan is the only route to an order of magnitude — and CorrectnessIdentical top edges on both bands (
|
CI caught this; my local runs did not. `ingestion_closure.rs` walks every source file and asserts the set of `record`-family call sites matches `ingestion_record_sites.json` exactly — a deliberate ledger, so a new recording route cannot appear without someone naming its owner. The two new sites are the `GenerationTally` recordings in the BitNet ternary arms of `stream_chat_completion` and `stream_completions`, added by 964166b's parent work so `/v1/stats` would not report BitNet traffic as zero throughput. Both owners were already in the ledger with one `record` each; the ternary arm gives each a second. Not a defect in this branch's perf work — the calls predate it. It surfaced here because `ingestion_closure` is a larql-vindex test and its workflow is path-filtered: the branch that introduced the calls (chrishayuk#480) touches only larql-server, so the test never ran there and chrishayuk#480 shows 16/16 green. This branch touches larql-vindex, so it ran. Worth noting for the reviewer of chrishayuk#480: that PR is green for a path-filter reason, not because the ledger agrees with it. Insertion only — the file stays sorted by (file, owner, call) and no existing entry moved (diff is +10 lines, nothing removed). cargo test -p larql-vindex --test ingestion_closure: 2 passed. clippy --all-targets -- -D warnings: clean. fmt: clean. (The lib tests SIGSEGV intermittently on my local box — a known artifact of that machine, not this change; CI runs the same 4736 tests green on ubuntu, macos and windows.)
CI's coverage policy flagged `gate_store.rs` at 86.46% against its 89%
floor: the f16 arm added to `gate_knn_mmap_fast` is the hot path of the
whole gate scan and had no direct test. Covered rather than baselined --
this is new code on the query path, not pre-existing debt.
Three tests, each asserting something that would be a real defect:
f16_fast_path_scores_without_cloning_the_layer
The arm is reached at all (returns Some, so f16 layers no longer fall
through to `resolve_gate`), the scores are right against the identity
fixture, and a second call -- a cache hit rather than a decode --
returns identical numbers.
f16_fast_path_agrees_with_the_resolve_gate_slow_path
Two routes to the same numbers: score in place out of the Arc'd cache,
versus `resolve_gate`'s owned copy multiplied by the caller. They must
agree. This is the test that matters -- the failure mode worth
guarding is not "slower than hoped" but "the optimisation changed
answers".
f16_cache_hands_out_arc_handles_not_copies
`Arc::ptr_eq` on two handles to one layer. The cache holds
`Arc<Vec<f32>>` specifically so a reader can release the mutex before
scoring; if a future change reverts to cloning the buffer, the
parallel-layer walk silently serialises again and only this asserts it.
Reuses the existing `f16_mmap_index` fixture in the same module, so no new
test scaffolding.
cargo test -p larql-vindex --lib gate_cache_lru_tests: 8 passed.
clippy --all-targets -- -D warnings: clean. fmt: clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
describe()on a real BitNet 2B browse container ran at ~21.6 ms/layer — about 3.3 GB/s of effective bandwidth on a box that streams 200+. Two causes, both in the gate scan, both fixed here.1. A transpose defeated BLAS
gemvwasmatmul_transb(vec.reshape(1, hidden), gate)— i.e.a.dot(&b.t())withashaped[1, hidden]. ndarray only dispatches to BLAS when operand layouts qualify, and that shape does not reachsgemv.Timed on one real layer (6912 features × 2560 dims, f32 cached):
a.dot(&b.t())(was)gate.dot(&vec)(is)4.7× from deleting a reshape. The diagnostic tell: a naive scalar loop beat the original "BLAS" call. If BLAS is slower than a
forloop, it isn't being called.2. f16 layers cloned the whole gate matrix per query
gate_knn_mmap_fastonly handled f32, so every f16 layer fell through toresolve_gate, which ends incache[layer].as_ref().unwrap().clone()— a full f32 copy of the layer. At this shape that's ~71 MB per layer per call, so a 12-layerdescribe()spent ~850 MB on allocation and memcpy before scoring a single feature. The decode was already cached; this was purely the copy.Now scores out of the cache through a view. The lock is held across
gemv— deliberate, since the alternative is cloning to release it early, which is the cost being removed. Contention is per-layer and a walk visits layers in sequence; there's aponytail:note that the upgrade path isArc<Vec<f32>>if high concurrency ever shows it.Measured end to end
Real
microsoft/bitnet-b1.58-2B-4Tbrowse vindex (30 layers × 6912 features), via pg_infer on PG 18.6:describe(12 layers)describe(30 layers)walksimilar_tonearest_toConcurrency (
describe, c=16): 41 → 111 qps, p50 under load 268 → 110 ms.The queries that don't scan gate vectors (
show_layers,show_features,show_relations) did not move — which is the check that this fix hit what it claims rather than something else drifting.Correctness
Identical output:
Zone332.80 L22 is still the top edge forFrance, same scores to 2dp. 93 concurrent queries still return exactly one distinct answer.cargo test -p larql-vindex --lib: 4736 passed, 0 failed.clippy --all-targets -- -D warningsclean.fmtclean.Not addressed
The scan is now bandwidth-sane but still single-threaded (~15% of a 32-vCPU box under 8 concurrent requests) and still O(all features).
rayonacross layers looks like most of another order of magnitude. HNSW is not the answer at this shape —cli.rssays it's "break-even or net loss for dense ≤ 10K-feature models" and this one has 6912 — so a different index would be needed. Both are larger changes than this.