perf: byte-identical CPU/startup optimizations — RoPE inv_freq cache, parallel prefill KV writes, grammar compile cache, analyze_model cache, route stack buffers, LTO - #1058
Conversation
zh-Processor
commented
Aug 16, 2026
|
Reviewed this properly. The three algorithmic optimizations are good and I want them — the reasoning is sound in each case and, unusually, you proved it instead of asserting it:
And the tests are not decorative. 15,326,112 float pairs with zero bit mismatches across 4×3×200k combinations is a differential test I'd hold up as an example, and running the grammar-cache ownership cases under ASan+UBSan across nine control-flow paths (including compile-fail) is exactly where a cached-pointer design goes wrong. The The Two things before I merge it. 1. Please make LTO opt-in rather than default
The reason is specific to this project: LTO enables cross-translation-unit inlining, which can change floating-point contraction (FMA formation). Token-exactness is the property colibrì sells, and we got a live demonstration today — #1044 found that olmoe's IDOT path quantizes activations, and #1024's "bit-identical" turned out to mean "bit-identical to that path" because FMA plus two accumulator chains changed the rounding (I measured 915/1024 outputs differing from stock). Your three differential tests cover rope, grammar and the analysis cache. None of them covers what LTO does to the arithmetic, and by construction they can't — it's a whole-program property, and it will differ by compiler and version. Defaulting it on means every user silently gets a different optimization regime than the one the oracles were validated under. As 2. Please also drop
|
a981413 to
4ef9a99
Compare
a981413 to
f0d0717
Compare
|
All three points addressed in the updated commit (now a single commit on current 1. LTO is opt-in. 2. 3. Numbers. Ryzen 3700X (8C/16T), 32 GB, Windows 11, MinGW gcc 15.2.0,
Two harness fixes surfaced by this PR's own CI (test-only, engine untouched):
CI green on all four jobs. |
|
Heads-up, and an apology: this went #1063 landed on A rebase on current Two things worth knowing while you're in there:
Sorry for the churn. Landing the registry before your branches was the right call for the project, but it does mean the cost of it fell on the people with open work. |
- colibri.c: cache RoPE inv_freq keyed on (theta,qk) instead of rerunning half x powf per position; parallelize the prefill RoPE/KV-write loop on CPU-only builds (iterations write distinct KV rows, rope cache is _Thread_local; CUDA/Vulkan shadow-shrink keeps GPU builds serial); cache the compiled grammar per serve slot when the identical schema text is resent (grammar_reset == fresh setup end state) - deepseek_v4.c: stack routing buffers (<=512 experts, malloc fallback beyond); COLI_MODEL_DIRS multi-directory shard lookup via st_init_multi - resource_plan.py: sidecar cache for analyze_model (atomic tmp+replace writes, (size,mtime) signature self-invalidation; the non-serializable resolved_family registry object is rebuilt from a cheap resolve_model() on cache hits, with only the scan-derived indexer flag persisted) - download_fp8.py: --dest / $GLM_DEST instead of a hardcoded I:\ path - Makefile: opt-in LTO=1 (off by default: cross-TU inlining can change FP contraction and the token-exactness oracles are validated against the default build's regime, JustVugg#1044/JustVugg#1024) - tests: test_rope_invfreq (15.3M float pairs, 0 bit diffs; built with -ffp-contract=off so the reference-vs-engine compare does not depend on contraction luck), test_grammar_cache (ASan/UBSan ownership), test_analysis_cache (temp dir resolved for macOS/Windows canonical paths)
|
No apology needed — the registry is the right structure, and the rebase was One thing turned out to be more than mechanical, exactly along the lines of The rebased run's Windows leg then caught a real race in my cache writer: Re-measured the one number whose code path changed (the warm hit now does
Tests on this tree: |
|
Heads-up: the |
|
Merged — and thank you for a first contribution that reads like it was written by someone who has been here for months: LTO kept opt-in with the cross-TU contraction risk spelled out, a dedicated bit-exactness test for the RoPE cache, and byte-identical discipline throughout. That is exactly the bar this repo tries to hold. (The Lavapipe delay was a hung runner on our side, not your PR — a cancel+rerun fixed it.) Welcome aboard. |
|
Thanks for the thorough review — learned a lot. |