Repository navigation
perf(cpu-ep): stop the half GEMM forking the pool for work too small to repay it (3.15x on small shapes) - #1149
Conversation
…to repay it `gemm_impl` split every half GEMM across the pool with no work guard, so it spent 0.14 ms of fork/join overhead to multiply an 8x64 by a 64x64 -- work one core finishes in 0.046 ms. Splitting was 3.1x *slower* than not splitting. That is now the common case, not a corner: since the f16 prefill gate (#1140) this kernel only ever sees the small shapes that decline widening, so the unguarded fork was mis-sized for every shape it still serves. Add the same guard its siblings already apply (`half_gemv::PARALLEL_MIN_WORK`, `accelerate_gemm`'s `k*n` bound), with the crossover measured here rather than copied: this kernel forks per row-block instead of per stripe, so it needs a larger operand to repay the split. Measured `serial/parallel`, interleaved rep-by-rep, p50 of 9, pinned, two runs per thread count: m*k*n T=4 T=16 32_768 0.70x 0.32x 262_144 0.91x/0.92x 0.93x/0.74x 524_288 1.00x/1.00x 1.15x/0.96x 786_432 1.04x/1.04x 1.28x/1.22x 1_048_576 1.06x/1.06x 1.36x/1.37x 786_432 is the smallest size that wins at every thread count in every run. The guard changes scheduling only, so both routes stay bit-identical. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1149 +/- ##
==========================================
- Coverage 80.53% 80.48% -0.06%
==========================================
Files 368 371 +3
Lines 161313 162859 +1546
Branches 161313 162859 +1546
==========================================
+ Hits 129921 131083 +1162
- Misses 26659 27025 +366
- Partials 4733 4751 +18
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
🔴 Benchmark Regression DetectedComparison of criterion micro-benchmarks: PR head vs merge-base, measured on the same runner in the same job (base first → PR second).
Visual flags: Host infoWhat this cannot catch
|
…unt, and never fork for one block Opus review found the fixed 786_432 threshold was tuned for T=16 and forced serial in a band where T=2/T=8 genuinely benefit from splitting -- a real regression against the pre-PR always-fork code at partial pool sizes. Re-swept m*k*n across T=2/4/8/16/32, two runs each. The crossover does move with the pool, but *not monotonically*: T=4 wants the highest threshold and T=8 the lowest, because for small m the block count is pinned by m rather than by the pool. A thread-scaled formula would fit that noise, so keep one constant and lower it to 524_288 -- the smallest size that wins at every measured thread count in every run (worst case 1.01x). 262_144 stays declined: it wins at T=2/T=8 but loses at T=4 (0.92x) and T=32 (0.80x). Also decline a "split" that yields a single block: it cannot use more than one thread, so it is pure fork overhead however large the operand. m == 1 always lands there -- a 1x1024 by 1024x1024 GEMV cleared the work bound and forked for one block. Tests for the three review findings: - exact-boundary test, so swapping >= for > is caught (it previously was not) - single-block test - bf16 parity, rather than assuming it inherits f16's guarantee Document that m*k*n is a proxy: the split is over rows of C, so speedup is bounded by block count, and the parallel route re-packs B per row-block. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…d build actually shows `codegen-units = 1` for `onnx-runtime-ep-cpu` (#1174) makes the *serial* route materially faster, so the point where splitting starts to repay the fork moves up. Re-ran `bench_half_gemm_parallel_threshold` at RAYON 2/4/8/16 with that pin in place, two runs per thread count: | m*k*n | T=2 | T=4 | T=8 | T=16 | |-----------|------|-----------|------|-----------| | 262_144 | 1.20 | 0.92/0.91 | 1.52 | 0.64/0.95 | | 393_216 | 1.28 | 0.99/0.96 | 1.66 | 0.80/1.10 | | 524_288 | 1.32 | 0.99/0.98 | 1.81 | 1.03/1.19 | | 786_432 | 1.33 | 1.03/0.92 | 1.35 | 0.97/1.25 | | 1_048_576 | 1.37 | 1.05/1.05 | 1.46 | 1.31/1.34 | The rule is unchanged -- the smallest size that wins at *every* measured thread count in *every* run -- but the answer is now `1_048_576`, not `524_288`: `524_288` is a wash at T=4 (0.99/0.98) and `786_432` regresses there (0.92) and at T=16 (0.97). Below the threshold the loss is still the 0.32-0.37x the guard exists to stop, so the guard itself is unaffected. Boundary tests are moved with the constant so they keep testing the boundary: the "must split" shape becomes 8x512x384, the exact-threshold shape becomes 8x512x256, and the one-block shape becomes 1x1024x2048. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Reviewed and re-measured on this box (AMD EPYC 9V74, 16 physical cores, AVX2+FMA+F16C, The guard is right; the constant is stale as of #1174. #1174 pins
Applying your own rule — smallest size that wins at every measured thread count in every run — the answer on the pinned build is Nothing about the guard's premise changed: below the threshold the loss is still the 0.32–0.37x at T=16 that this PR exists to stop, and the one-block decline ( Pushed Merge order: this must land after #1174. The |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
`target-leon/weight-offload-tests/qmoe-1179131-8.bin` is a generated test artifact that a local run dropped into the tree and a `git add -A` swept up. It has nothing to do with the half-GEMM fork threshold. Two siblings of it are already tracked on `main`; untracking those is a separate cleanup and does not belong in a perf PR. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This branch merged `main` after #1200 landed a trailing blank line in `pipeline/mod.rs`, which fails the required `Rust quality` check. #1204 fixes it on `main`; the identical one-line deletion here lets this PR go green without waiting, and merges as a no-op once #1204 lands. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Re-measured, re-derived and armed for merge. Since #1174 pinned Two housekeeping items also carried here:
Marked ready with squash auto-merge armed. Ordering constraint stands: this must land after #1174 ( |
Reviewer verification — reproduced (and then some), mergingValidated on Intel Core i7-13800H (14C/20T), Windows 11, stacked on the just-merged #1183.
Merging by squash on this evidence. |
… (1.66x -> 1.36x vs ORT) (#1218) ## What this changes `erf_ps`'s big-branch `R` polynomial and the `exp_ps` polynomial it feeds are switched from Horner's rule to **Estrin's scheme**. Horner is a six-deep dependent FMA chain; Estrin regroups it as `a^3·(c0 a^3 + c1 a^2 + c2 a + c3) + (c4 a^2 + c5 a + c6)`, three deep for two extra multiplies, whose halves the out-of-order engine runs in parallel. Only the two chains actually on `erf`'s critical path are converted; the small branch stays Horner (it runs in parallel and is off-path). Rebased onto current `main` so it **preserves #1227** (`map_ps`'s `#[target_feature(enable = "avx2,fma")]`). The original branch predated #1227 and its raw diff would have moved that attribute back onto `map_bias_ps`; that hunk is dropped. Only `simd_activations.rs`'s erf/exp polynomials and one accuracy test change here. ## Numerics — measured, not assumed (reviewer) Estrin reassociates f32 FMAs, so it is **not** bit-identical to Horner by construction. The squad reviewer measured the actual error with a two-build dump-and-diff (base Horner build vs this Estrin build, identical input grid of 404,045 points spanning `[-6,6]` densely, the `0.921875` split boundary, large `|x|`, denormals, and NaN/±inf), on **Intel Core i7-13800H, Windows 11**: - **Estrin vs Horner: max difference = exactly 1 ULP.** 4,401 of 404,043 finite points (1.09%) differ, every one of them by a single ULP; max absolute diff 5.96e-8. Nothing moves by more than one ULP anywhere in the domain. - **vs f64 `erf` truth:** max |Horner − erf| = 5.77e-8, max |Estrin − erf| = 6.55e-8 (worst at x≈0.933, just past the split). Both within ~1 ULP of the correctly rounded result; Estrin is at most ~0.8e-8 looser than Horner and never worse than 1 ULP from it. - **Special values are all correct and identical between the two:** large `|x|` (7,8,10,20,50,100,1000,1e30,f32::MAX) saturate to ±1.0; denormals produce bit-identical tiny outputs; NaN → NaN; +inf → +1.0; −inf → −1.0. This is a conscious, documented 1-ULP change, not a hand-wave. The accuracy gate `erf_reassociation_costs_no_accuracy` (worst scaled error pinned at 5.97e-8 = 2^-24 over `[-6,6]`) **passes** on this box, so a future regrouping that widens the envelope fails instead of being absorbed by the looser `ERF_BOUND`. ## Performance — measured on this box, ORT-independent The original body quoted "1.66× → 1.36× vs ORT" (≈ −17.7% kernel time). I could not reproduce that specific figure: no pinned ORT reference on this Windows box, and it is different hardware. Instead I timed `erf_avx2` directly, same machine, two builds, single-thread, min-of-200 × 9 rounds over a 1,048,576-element mixed-range buffer: | build | per-round min (ms) | best | | --- | --- | ---: | | Horner (base) | 0.6346–0.6348 (8/9 rounds) | 0.6346 | | **Estrin (this PR)** | **0.6042–0.6043 (8/9 rounds)** | **0.6042** | The ranges do not overlap, so the direction is solid: **Estrin is ~5.0% faster (1.05×)** on the erf kernel here. That is a real win but smaller than the −17.7% originally claimed — reported honestly as the measured kernel-internal delta rather than an unverified ORT ratio. The mechanism (a latency-bound chain freed by shorter dependency depth) is consistent with a positive but hardware-dependent magnitude. ## Validation (reviewer) - `cargo test -p onnx-runtime-ep-cpu --lib` — **1353 passed, 0 failed, 17 ignored** (base 1345 + #1183/#1149 + this test), including the accuracy gate. - `cargo clippy -p onnx-runtime-ep-cpu --all-targets -- -D warnings` — clean. - No MLAS feature involved; `map_ps`'s target_feature (#1227) verified intact in the merged result. Co-authored-by: justinchuby <223556219+Copilot@users.noreply.github.com> Co-authored-by: resch <resch@users.noreply.github.com>
…-ORT headline) (#1259) ## What Records the correction of record for **#1218** (erf's Estrin-scheme reassociation), appended as **§32** to the CPU-EP-vs-ORT ledger — the same document and style where the #1226 (§30.2) and #1230 withdrawals live. #1218 landed as *"1.66x -> 1.36x vs ORT"* (~17.7% kernel-time reduction). On the review host that ratio is **unreproducible** — there is no pinned ORT reference build here, and the original number came from different hardware — so it is **withdrawn**. Stated plainly: it was **neither reproduced nor refuted**, because it was measured against a reference we do not run. ## What the doc now records - **The withdrawal**, with the reason (no ORT arm on this box), and a note that the overstated figure reached **no benchmark table, README, or source comment** — only the (unrewritable) squash-commit subject. - **The measured number:** ORT-independent same-binary A/B on `erf_avx2`, single-thread, min-of-200 × 9 rounds, 1M elements, on **Intel Core i7-13800H** — Horner 0.6346 ms → Estrin 0.6042 ms, non-overlapping ranges → **~5.0% (1.05x)**. Real, but a fraction of the implied ~17.7%. - **The durable correctness fact, stated in as many words:** this reassociation is **NOT bit-identical**. Estrin differs from Horner by **at most exactly 1 ULP** (1.09% of 404k points, max abs 5.96e-8); vs f64 truth Horner 5.77e-8 / Estrin 6.55e-8 (both within ~1 ULP of correctly-rounded erf); large |x| → ±1.0, denormals bit-identical, NaN → NaN, ±inf → ±1.0. Names the gate test **`erf_reassociation_costs_no_accuracy`** as the tripwire. ## Validation Documentation only — no source, no benchmarks re-run. No markdown linter/build exists in-tree, so there is nothing to build; the change is prose appended to an existing report. Refs #1218. Companion to the separate threshold-tuning issue filed for #1149. Co-authored-by: justinchuby <223556219+Copilot@users.noreply.github.com>
What this changes
half_gemm::gemm_implforked every half GEMM across the rayon pool with nowork guard, so it paid fork/join overhead to multiply tiny operands a single core
finishes far faster. Since the f16 prefill gate (#1140) this kernel only receives
the small shapes that decline widening, so the unguarded fork was mis-sized for
essentially every shape it still serves.
Two guards, scheduling-only (arithmetic is untouched):
PARALLEL_MIN_WORK = 1_048_576onm*k*n— stay serial below it.m.div_ceil(split_mc) == 1);it cannot use more than one thread, so it is pure overhead however large the
operand.
m == 1always lands here.Measurement (reviewer re-measurement)
Re-measured by the squad reviewer on Intel Core i7-13800H (14C/20T), Windows 11,
via the in-repo
bench_half_gemm_parallel_thresholdA/B harness (m = 8, serial vspool-split interleaved rep-by-rep, p50 of 9), at
RAYON_NUM_THREADS = 4/8/16. Thelast column is
serial/par— >1 means splitting wins.Smallest shapes (the ones this guard sends serial) — splitting is catastrophic, so
declining it is the win:
m*k*nSo the worst shape (8×64×64) goes from a forked path to serial and is
~10–12× faster on this box (par 0.39/0.25/0.42 ms → serial 0.033/0.025/0.037 ms
at T=4/8/16). That is even larger than the 3.15× the original body reported on its
Linux reference box, because this laptop's relative fork/join overhead is higher.
Around the threshold and above:
m*k*nHonest hardware caveat. On this box the crossover where splitting starts to
pay sits higher than
1_048_576— around 2–3M MACs, not at ~1M. The originalconstant was derived on a 16-core Linux box. This does not make it unsafe here:
the guard only ever moves below-threshold work to serial, and leaves everything
>= PARALLEL_MIN_WORKsplitting exactly asmaindoes today. So relative to thecurrent unguarded base there is zero regression at any shape — every shape either
improves (below threshold) or is byte-for-byte the same code path (at/above). The
only imperfection on this box is that shapes in ~[1M, 2.5M] still fork and lose
0.68–0.98× as they already do on
main; the guard leaves that (pre-existing) moneyon the table rather than creating a new regression.
I deliberately did not retune the constant to this laptop: it is a defensible,
evidence-backed value pinned by a test and an evidence table, and overfitting it to a
single dev box would be worse than a conservative shared default. If the CPU-EP owner
wants the constant re-derived on the reference hardware, that is a separate,
non-blocking follow-up.
No large-shape regression
Explicitly checked, because the risk of a "don't fork below N" heuristic is that N is
wrong for some other shape: every shape
>= PARALLEL_MIN_WORKtakes the identicalsplit path as base (the guard's
parallelbranch is unchanged), so nothing large gotslower. Confirmed above — the 1.5M–3.1M rows behave as they do on
main.Correctness
The guards change scheduling only, never arithmetic, so both routes must agree
bit-for-bit (compared on
to_bits()), for f16 and bf16. All pass on this box:both_routes_agree_bit_for_bit/bf16_routes_agree_bit_for_bit— spansm > MAX_MCso the serial multi-block indexing is actually exercised.half_gemm_declines_to_split_work_below_the_crossover— proves the route via athread-local counter, both directions.
the_crossover_itself_splits_and_one_mac_below_it_does_not— exact boundary.a_single_block_is_never_split_however_large_the_operand.half_gemm_parallel_threshold_matches_the_measured_crossover— pins the constant.Validation (reviewer)
cargo test -p onnx-runtime-ep-cpu --lib— 1352 passed, 0 failed, 17 ignored(stacked on perf(cpu-ep): ship the decode f32 GEMV that #1091 left switched off (6.85x -> 1.15x vs ORT) #1183; the 6 new guard tests account for the delta).
cargo clippy -p onnx-runtime-ep-cpu --all-targets -- -D warnings— clean.Scope
This does not move the
K=N=2048plugin benchmarks (those shapes are ≥4.2M MACs,far above the threshold). The win is confined to half GEMMs below ~1M MACs, which is
the range this kernel actually serves post-#1140.