Repository navigation
fix(omp): size the OpenMP team in the four engines that never did - #1517
Conversation
1440370 to
ddcf71b
Compare
|
Correction, and it is a real one — thank you @huppiflupp for catching it in #1518.
if not env.get("COLI_NO_OMP_TUNE"):
if arch != "deepseek_v4":
env.setdefault("OMP_NUM_THREADS", str(physical_cpu_count()))
I should have said which launch path I measured on, and I did not. Every number in
What the numbers still support, and what they do not:
On overlap with #1534: that one is |
omp_tune.h has carried the physical-core sizing since JustVugg#718, where it measured +2.3x on a 16C/32T part from the thread count alone. colibri.c, inkling.c, kimi_k3.c and olmoe.c call it. glm53.c, qwen36.c, qwen38.c and deepseek_v41.c were written afterwards and never did, and nothing failed when that happened -- the symptom is not an error, it is one thread per logical CPU. On a 208-logical-CPU host that is not a rounding error. perf on deepseek-v41 at the shipped default puts 93% of all cycles inside libgomp.so.1.0.0 (+0x256c0 and +0x258a0, plus 1.24% in native_queued_spin_lock_slowpath), because the engine issues ~720 parallel regions per decoded token -- three per expert application, 240 applications a token -- and each is only a few thousand rows wide. The barrier is the work. The sweep, on the released 510.3 GB DeepSeek-V4.1-Flash container, cold cache, every run token-exact 16/16 (same 25-token prompt and same 16 reference ids): threads disk matmul attention decode tok/s 16 16.11 3.68 2.54 13.89 1.15 32 16.12 3.16 1.91 13.24 1.21 104 16.53 10.45 2.61 21.05 0.76 208 20.32 185.26 108.62 247.23 0.06 <- the default The default decodes 18.7x slower than 32 threads and 11.7x slower than the 104 physical cores the shared helper selects. Shipped default, before and after: decode 247.2 s -> 18.6 s, whole turn 409.9 s -> 43.5 s, 0.06 -> 0.86 tok/s, identical tokens, three cold runs each way. The fix is the call the four working engines already make. OMP_NUM_THREADS still wins and COLI_NO_OMP_TUNE=1 still disables it, both verified. The Makefile prerequisites gain omp_tune.h, which tests/test_makefile_deps.py requires and caught. Also: tests/test_omp_engine_wiring.py replaces the inkling-only source gate with a table over every engine (a defect that four had was guarded for one), and it strips comments first -- the test it replaces would have passed a call that had been commented out, which I confirmed by commenting one out in two engines. Raw per-run stderr and the perf summary: docs/experiments/ dsv41-omp-team-2026-09-15-raw.txt.
Both the record and the manifest name the commit in the first commit of this branch, so a reader can go from the claim to the tree that produced it. The manifest passes c/experiment_manifest.py (OMP_TEAM_SIZING, improvement).
ddcf71b to
96dc9a5
Compare
… it ran in The A/B in this record was measured with the engine's default OpenMP team, which on a 208-logical-CPU host is pathological: 93% of cycles inside libgomp and a turn that takes 409.9 s instead of 37.2 s (PR JustVugg#1517). It concluded that the disk phase is 5% of a turn and that this work is worth ~2.5%. That conclusion was wrong, and the correction is larger than the result it corrects. Re-run with the team sized (OMP_NUM_THREADS=32), same container, same prompt, same 16 reference ids, cold cache, arms interleaved, 3 runs each, every run token-exact 16/16: arm disk rate turn decode P one NVMe 16.07 s 6.17 GB/s 37.2 s 12.86 s M3 mirror, 2nd controller 12.66 s 7.83 GB/s 34.0 s 11.00 s M2 both copies on one RAID0 16.36 s 6.06 GB/s 37.9 s 12.96 s D0 buffered reads 31.14 s 3.18 GB/s 52.7 s 21.01 s So O_DIRECT is -48% of the disk phase and +63% of the decode rate, and the split is -21% and +17%. The engine now reaches the device's own ceiling -- 6.17 GB/s against the 6.34 GB/s iobench measures one nvme0n1 at depth 4 -- and M3 clears it at 7.83, which is only possible if the split is really using two devices. M2 stays a null, which is the control that says the variable is the number of independent filesystems, not the number of drives. The 208-thread numbers are kept, because they are real measurements of a real configuration and because the trap is that the phase breakdown looks entirely reasonable either way. Both manifests are regenerated from the team-sized runs and validated; V41_DIRECT +41% whole-turn, COLI_MODEL_MIRROR +12%.
|
Reproduced on a much smaller SMT host before merging, so the effect is not confined to 208-thread machines. Ryzen 7 PRO 8700GE (8 cores, 16 threads), current dev binaries launched directly, OMP_NUM_THREADS set by hand to what the helper would pick versus the OpenMP default, two runs per arm, every run token-identical: DeepSeek V4.1 Flash, cold page cache, 26 prompt + 24 generated, cap 8:
Qwen3.6 int4-g64, fully resident (cap 256), 128 generated tokens, per-token phase timers:
7.5x on the V4.1 turn and about 40x per token on qwen36, both from the team size alone. One note for the record rather than for the change: |
Summary
omp_tune.hhas carried the physical-core team sizing since #718, where it measured+2.3x on a 16C/32T part from the thread count alone. Four engines call it --
colibri.c,inkling.c,kimi_k3.c,olmoe.c. Four were written afterwards andnever did:
glm53.c,qwen36.c,qwen38.c,deepseek_v41.c.Nothing failed when that happened, which is how it lasted. The symptom is not an error:
the engine runs one thread per logical CPU, and where the logical/physical ratio is
large the per-region barrier cost swallows the arithmetic. On a 208-logical-CPU host
deepseek-v41spent 93% of its cycles inside libgomp and decoded 18.7x slowerthan it does with a sane team.
The fix is the one the four working engines already make --
#include "omp_tune.h"pluscoli_omp_tune_threads("<engine>")as the first statement ofmain-- in each of thefour.
OMP_NUM_THREADSstill wins;COLI_NO_OMP_TUNE=1still disables it.Evidence
Full record:
docs/experiments/dsv41-omp-team-2026-09-15.md. Raw per-run stderr:docs/experiments/dsv41-omp-team-2026-09-15-raw.txt. Validated manifest:docs/experiments/dsv41-omp-team.json.Released
deepseek-ai/DeepSeek-V4.1-Flash, 510.3 GB, 25 prompt tokens, 16 generated,cap=8, cold cache per run, every run token-exact 16/16 -- the team size changesscheduling, not results.
The profile, before anything was changed (
perf record, 208 threads):libgomp.so.1.0.0+0x256c0libgomp.so.1.0.0+0x258a0[k] native_queued_spin_lock_slowpath93% of cycles in the OpenMP runtime, not in the math. The engine issues ~720 parallel
regions per decoded token (three per expert application, 240 applications a token) and
each is a few thousand rows wide, so the barrier is the work.
The sweep (one variable, same container/prompt/reference, all 16/16):
OMP_NUM_THREADSShipped default, before and after:
13.3x on decode, 9.4x on the turn, 14.3x on tokens/second, identical tokens.
Two things I want to be precise about rather than tidy:
doubling and defers to the user". 32 threads beat the 104 physical cores by 1.6x
on this host. Hard-coding a smaller number here would be the same mistake in the
other direction, so the change is exactly the shared helper's documented behaviour.
208 spinning threads steal the memory bandwidth the expert reads need. The pathology
is not confined to the arithmetic it appears in.
It also invalidates a ratio that is easy to measure on this engine
While measuring the V4.1 expert-read path I concluded that the disk phase is ~5% of a
turn and that multi-drive streaming is therefore worth ~1%. That was measured at 208
threads: with the team sized, the disk is 16.5 s of a 43.5 s turn. The I/O code did not
change; what it is worth did, by an order of magnitude. Anyone who has measured this
engine (or the other three) on a high-core-count host should check which regime they
were in before trusting a ratio -- the phase breakdown looks entirely reasonable either
way, which is what makes it worth a line in the docs.
Validation
make -C c check-- full suite, including the new Python gate[OMP] <engine>: 104 physical-core threads instead of 208 logical CPUsline, verified by running each with an unusableSNAPOMP_NUM_THREADS=7produces no override line (the user still wins)COLI_NO_OMP_TUNE=1produces no line (the kill switch still works)deepseek_v41token-exact against the tiny oracle at caps 1/8/49commented out, and passes when it is present
The gate
tests/test_omp_engine_wiring.pyreplacestests/test_inkling_omp_source.py, whichguarded one engine against a defect that four had. The new one is table-driven over
every engine, records the four labels, and has a second category for
deepseek_v4.c-- which sizes its own team (loading-worker reservation) rather thancalling the shared helper, and must keep doing something so the same defect cannot
arrive through the side door.
It strips comments before matching, because the test it replaces would have passed a
call that had been commented out. I verified that by commenting the call out in
qwen38.candglm53.cand watching it fail.Compatibility
omp_tune.his header-only andalready in the tree, and the helper is a no-op without
_OPENMPomp_set_num_threadsis a runtimecall, so unlike GLM's spin-wait knobs this needs no re-exec
physical >= logical, which is why the four engines that had it never noticed a
problem
Not in this change
deepseek_v4.cis deliberately left alone. It already resizes its team(
omp_set_num_threads(logical - COLI_V4_EXPERT_LOADER_COUNT)), so it is not the sameomission; whether that reservation should also be physical-core sizing is a separate
question that needs its own measurement.