Do not fork an OpenMP team to run one iteration - #806
Conversation
The DSA indexer's per-position top-k is `#pragma omp parallel for` over
`s < S`. At decode S is 1, so the region has exactly one iteration -- and
OpenMP still forks and joins the whole team to run it, once per layer, per
token. It happens whether or not the body does any work: both early
`continue` branches are taken on short contexts, and the fork precedes them.
`if(S > 1)` makes the single-iteration case run inline. Prefill (S > 1) is
untouched.
This is not a measurement-driven change -- a one-iteration parallel region
is overhead by construction, no benchmark needed to say so. But it came out
of one, and the measurement is worth recording because it points at
something larger.
Running the tiny oracle model (5 layers, hidden 128) at different team
sizes, 15 runs each, median with quartiles:
OMP_NUM_THREADS=1 17109 tok/s (p25 15573, p75 18119)
OMP_NUM_THREADS=4 10749 tok/s (p25 10650, p75 11125)
OMP_NUM_THREADS=24 1529 tok/s (p25 913, p75 1935)
The distributions do not overlap. One thread is 11x faster than 24, and the
spread widens with the team -- +-8% at one thread, +-34% at twenty-four,
which is the same tail the profiler reports as p90 27% above p50.
That is a small model on a 24-core host, so the ratio does not transfer to
a 744B run. What does transfer is the shape: colibri hands every parallel
region the full team regardless of how much work it contains, and there is
no notion of a minimum grain anywhere in the tree. PyTorch's at::parallel_for
takes a grain_size for exactly this reason, and vLLM's CPU backend carries
its own thread-count controls.
Fixing that properly means a per-region threshold, and a threshold is only
honest if it is measured on the model people actually run -- which needs the
lab machine, not a 0.015s tiny-model replay whose noise (356..3893 tok/s on
identical binaries) swamps any single-region effect. This commit takes only
the case that needs no threshold at all: a loop that cannot be parallel.
Verified identical, not just green: teacher-forcing is 32/32 positions and
the REPLAY token sequence is byte-for-byte the same before and after. Four
engines build warning-free, make test-c passes.
Mine can, and it is running your branch now. 4× RTX A6000 + EPYC 7402P (24c/48t), GLM-5.2 744B int4, Two things I can say before that, because one of them looks like it contradicts you and does not. Your tiny-model result and my 744B result point in opposite directions, and both are rightYou measured
Near-linear to the physical core count, then a collapse at SMT. The two results are the same finding seen from opposite ends of the grain axis. On So your sentence —
— is exactly right, and having the two extremes measured makes it much harder to argue with than either number alone. A fixed team size cannot be correct for both, and today there is only one. What I will report, and what I can run nextFor this PR specifically: median of 3 before/after, the On the larger question you deliberately left alone — the per-region threshold — I would rather measure than speculate, and I think the honest experiment is not a Unrelated but worth having: my #805 sets |
`make -C c bench-omp-grain` measures what a parallel region costs before its
body runs: a loop whose iterations do nothing measurable, so the number IS
the fork/join. It is the tool behind the `if(S > 1)` clause -- and behind the
claim that a one-iteration region is not free.
On this host (Core Ultra 9 285K, 24 cores, no SMT):
threads fork/join per region
4 881 ns
8 1500 ns
16 2498 ns
24 12767 ns (OMP_WAIT_POLICY=active OMP_PROC_BIND=close)
At 24 threads that is 12.8 us of overhead for a region with one iteration --
1.0 ms/token for a 78-layer model paying it once per layer, and 149x the cost
of running the same loop inline.
Why a benchmark and not just the engine: the tiny-model replay is 15 ms end
to end, and repeated runs of an UNCHANGED binary spread from 356 to 3893
tok/s. Nothing about a single parallel region survives that. This runs the
region 200k times and reports a median of samples, so the number is stable
to a few percent and comparable across machines.
One thing it is NOT evidence for. The jump between 16 and 24 threads is
steeper than the trend, and this CPU is hybrid (P-cores + E-cores) -- the
same effect omp_tune.h already avoids on Apple Silicon by counting only
performance cores (JustVugg#707, -4.2% decode from letting E-cores into the team).
The Linux path has no equivalent: it dedupes SMT via thread_siblings_list
and takes every physical core, E-cores included. That may well be the same
bug on Intel hybrid parts, but this host runs under WSL2, which exposes
neither /sys/devices/system/cpu/types/ nor cpu_capacity, so P/E membership
is not observable here and the jump cannot be attributed. Someone on bare
metal with a hybrid CPU can settle it with this benchmark in a minute.
|
Measured on the 744B, as promised. Your change is free, and your tiny-model ratio does not transfer — which I think strengthens the PR rather than weakening it. 4× RTX A6000 + EPYC 7402P, GLM-5.2 744B int4,
No measurable difference, and the Why I read this as supporting the PRYou wrote that the region "needs no benchmark to justify" and that the 11× is a shape, not a transferable ratio. Both hold up. On It is also the cheapest kind of correctness: a one-iteration parallel region cannot be right, and the measurement confirms it costs nothing to remove. Where the grain question actually bites on a real modelSince you deliberately left the general threshold alone for want of a lab machine — here is what mine says, so the next person has a number to aim at.
Orchestration is 8.2 % of wall time on a 744B run. That is the entire envelope any grain-size work has to fit inside — it bounds the prize, and it is small. The thing that is not small on this host is one line up: 19.67 GB/s of CPU-side expert reads on 8-channel DDR4-2400 that sustains ~85. I swept the team size to find out whether that was a broken parallelisation or a per-core ceiling:
Near-linear to the physical core count, then SMT collapse. So the parallelisation is fine and ~1.4 GB/s per Zen 2 core is the real ceiling — the team is not too large in general, it is too large by exactly the SMT factor. That is #805, and it is orthogonal to yours. Happy to run any specific region you want instrumented against the 744B; the harness is scripted and the host is set up for it. |
|
This is the measurement I could not make, and it lands harder than the PR did on its own. Thank you for running it properly — snapshot/restore of Taking your two results in turn. Your 744B numbers correct my framing, not just complete itYou are right that the 11× does not transfer, and I should not have let that number stand as close to the change as it did. On So the honest claim for this PR is the narrow one: a one-iteration parallel region cannot be correct, and removing it costs nothing measurable on the model people actually run. Your table is what licenses that second half. I have no objection to the PR being judged on that basis alone. On the general threshold — I think your data argues against pursuing itYou measured I would rather this be recorded as a bounded negative result than turned into work: the grain-threshold prize on a 744B decode is ≤ 8.2 % and realistically far less. If you would rather have it instrumented than argued, the one region I would still spend a counter on is the router — 1.81 s, called once per layer per token, small per call, high frequency, which is the shape most likely to be paying fork/join it cannot repay. If that one also comes back flat, the question is closed and worth closing loudly. Everything else on your profile says the same thing about where the time is: #805 is the one with the actual headroom
No conflict from my side: this PR deletes a region that should never have forked and touches nothing about team sizing. I will not go near One thing worth stating plainly for anyone reading later, since you verified it independently: token-identical across six runs at temperature 0 on 4 GPUs. That makes an output diff a real signal here rather than noise, which is what let your byte-for-byte comparison mean anything. That is a useful property to have on the record. |
2 participants
Draft.
The change
The DSA indexer's per-position top-k is
#pragma omp parallel forovers < S. At decodeSis 1 — the region has exactly one iteration, and OpenMP still forks and joins the whole team to run it. Once per layer, per token.It happens whether or not the body does any work: both early
continuebranches are taken on short contexts, and the fork precedes them.#pragma omp parallel for schedule(dynamic,1) if(S > 1)Prefill (
S > 1) is untouched. A one-iteration parallel region is overhead by construction — this needs no benchmark to justify.But it came out of one, and that part is worth reading
Tiny oracle model (5 layers, hidden 128), 15 runs per configuration, median with quartiles:
OMP_NUM_THREADSThe distributions do not overlap. One thread is 11× faster than 24, and the spread widens with the team — ±8% at one thread, ±34% at twenty-four. That is the same tail the profiler reports as p90 sitting 27% above p50.
A small model on a 24-core host, so the ratio does not transfer to a 744B run. What transfers is the shape: colibri hands every parallel region the full team regardless of how much work it contains, and there is no notion of a minimum grain anywhere in the tree. PyTorch's
at::parallel_fortakes agrain_sizefor precisely this reason; vLLM's CPU backend carries its own thread-count controls (VLLM_CPU_OMP_THREADS_BIND,VLLM_CPU_NUM_OF_RESERVED_CPU).What this PR deliberately does not do
Fix that properly. A per-region threshold is only honest if it is measured on the model people actually run, and this machine cannot do that: the tiny replay is 0.015 s, and repeated runs of an unchanged binary land anywhere from 356 to 3893 tok/s. That noise swamps any single-region effect — I tried, got a plausible-looking +135% median, and threw it away because the quartiles overlapped almost completely.
So this takes only the case that needs no threshold at all: a loop that cannot be parallel. The rest wants the lab machine and a real model.
The benchmark is included
make -C c bench-omp-grainis the tool those numbers come from, and it is in this PR. It runs a region whose body does nothing measurable 200k times and reports a median of samples, so the figure is stable to a few percent instead of the 356..3893 tok/s the engine-level replay gives on an unchanged binary.On this host (Core Ultra 9 285K, 24 cores, no SMT), with the engine's own OMP settings:
12.8 µs for a region with one iteration — 1.0 ms/token on a 78-layer model paying it once per layer, and 149× the cost of the same loop inline.
A lead this does NOT establish
The 16 → 24 jump is steeper than the trend, and this CPU is hybrid (P-cores + E-cores). That is the effect
omp_tune.halready avoids on Apple Silicon, by counting only performance cores — #707 measured E-cores costing 4.2% of decode there. The Linux path has no equivalent: it dedupes SMT viathread_siblings_listand takes every physical core, E-cores included.So Intel hybrid parts may have exactly the same bug. But this host runs under WSL2, which exposes neither
/sys/devices/system/cpu/types/norcpu_capacity, so P/E membership is not observable here and the jump cannot be attributed. I am flagging it, not claiming it. Anyone on bare metal with a hybrid CPU can settle it with the included benchmark in a minute.Verification
Identical output, not merely green:
REPLAYtoken sequence is byte-for-byte the same before and aftermake test-cpassesSide note that made this measurable at all
These numbers come from
tests/test_inefficiency.pyand itsglm_tinyfixture. Those 8 tests had never executed on any platform — they looked forc/glm.exe, a name that has not existed since theglm→colibrirename (fixed in #804). With that fix and a generated fixture, 5 of them run for the first time and pass. They are the throughput floor, the disk-wait ceiling, the PROFILE phase assertions and the determinism check — i.e. the harness this kind of work needs.