pipe: blocking pipe_wait (COLI_PIPE_BLOCK=1) + PIPE_WORKERS implies PIPE=1

[ebootheee]

What this is now

Rewritten against current dev (post #274/#293) and narrowed to the piece flagged as worth keeping: the pipe blocking-wait and the PIPE_WORKERS ⇒ PIPE implication. One commit — 39 lines in glm.c plus a test.

• COLI_PIPE_BLOCK=1: pipe_wait parks on a condvar instead of the sched_yield spin (~5 µs wake against 0.5–3 ms expert reads; no lost wakeup — workers release-store ready[] before taking the mutex to broadcast, and the waiter re-checks under the lock). Default OFF = byte-identical spin. Pthread pool only: the URING backend has no waiter spin to replace.
• PIPE_WORKERS>0 without PIPE implies PIPE=1 with a stderr note — sizing the pool declares the intent to use it (an entire benchmark campaign once ran with PIPE_WORKERS=16 and the pipe silently off). Adapted to current dev: it's a no-op on Windows where PIPE now defaults to 1, PIPE_WORKERS=0/empty does not enable a clamped 1-worker pipe (hardening carried over from the adversarial review of the v1 series), and an explicit PIPE=0 always wins. The rule lives in pipe_workers_imply_pipe() so the table is unit-testable.
• tests/test_pipe_block.c (in TEST_BINS, all three platforms): pins the implication truth table, then drives the pool through 200 generations under each waiter against an on-disk expert fixture — spin as the control arm, condvar arm alternating parked and fast-path waits — verifying identical slot bytes. Temp file is CWD-relative per test_stops' warning about the Windows job.

Numbers

Measured on the v1 branch (2×5090 + Gen5 NVMe, GLM-5.2 744B int4, CPU decode): 1.98 → 2.16 tok/s at 192 tokens (expert-disk service 44.6 s → 33.5 s). The ported logic is identical, but I have not re-benchmarked on current dev. On Metal/GPU decode, @michael-denyer's M5 Pro A/B in this thread measured no change — consistent with the mechanism: the win comes from freeing the core the spinner steals from the CPU matmul team, and on GPU decode with idle cores nothing scarce is being stolen. Default stays OFF.

Dropped from this PR

COLI_CUDA_BATCH and COLI_CUDA_SYNC are out. #274's pipe2 resident pipeline and #293's profiling rework moved the ground they stood on; I'll re-evaluate and re-benchmark against the new code and, if anything still earns its keep, bring it back as a separate focused PR.

[JustVugg]

Heads-up: this now conflicts with dev after #274 + #293 landed (the pipe2 resident-pipeline rework + the profiling/t_emm split touch the same expert-dispatch region your COLI_CUDA_BATCH/COLI_CUDA_SYNC changes sit in). Since you have "allow edits from maintainers" on, I'm happy to rebase it onto current dev for you rather than have you redo it — just say the word, or push a rebase yourself if you'd prefer. The blocking-wait + PIPE_WORKERS implies PIPE parts look good and worth keeping.

[JustVugg]

Could you rewrite this against current dev? I'm keeping it open rather than closing it, but it can't land in its present shape.

It's 167 commits behind dev. On glm.c, which changes several times a day, that isn't a rebase — it's a rewrite, and doing it for you would mean rewriting your design decisions without understanding why you made them. So I'd rather ask.

The PIPE/CUDA area moved hard: #274 (pipe2 resident pipeline, device-count-aware gate) and #293 (COLI_CUDA_MTP off by default) both landed. Some of your delta may now be redundant, and some may conflict with the new gating.

What changed under you, which makes a rewrite cheaper than it sounds:

• CI now exists (ci: make check on ubuntu / windows (MSYS2 UCRT64) / macos — and it already caught a Windows test bug (#140) #143 + ci: GitHub Actions — engine, web, Python, CUDA syntax (free for public repos) #144, merged today): make check runs on ubuntu/windows/macos and the engine/web/python/CUDA-syntax jobs on Linux, for every PR. You'll get a verdict in ~2 minutes instead of waiting on a maintainer who can't test your platform. It caught a bug in my own test on its first Windows run.
• All PRs now target dev, not main. main is locked to admins.
• The backlog is being cleared (31 → 17 today). Your PR sat long enough to rot, and that's this repo's fault, not yours — two contributors independently rebuilt the same Windows CUDA loader this week because nobody triaged in time.

What would help it land fast: the smallest version that does one thing. A 300–700 line PR touching glm.c needs a maintainer who can run it, and for most platforms here that maintainer doesn't exist. A focused change with a test the CI can execute gets reviewed in an hour.

If you'd rather not, say so and I'll close it with thanks — no hard feelings either way. And if you think I've misjudged and it should go in as-is, push back: I've been wrong twice today already and both times a contributor caught it.

[michael-denyer]

Datapoint for commit 1 (COLI_PIPE_BLOCK) from a different regime: Apple M5 Pro 64GB, Metal decode with COLI_METAL=1 DIRECT=1 PIPE=1 PIPE_WORKERS=8, GLM-5.2 REAP-pruned E=168 int4, greedy 128 tokens, --ram 50, TOPP=0.7.

These measurements are noisy on a shared box, so the method matters. Three interleaved SPIN/BLOCK pairs on one binary, with the env knob as the only variable. I restored a snapshot of .coli_usage before every run so autopin warming could not drift the arms; hit rate came out at exactly 57.0% in all six runs. Each run was bracketed by AC-power, load, and foreign-process checks, and dirty runs were discarded.

run	tok/s	disk-wait	expert-matmul	attention
SPIN1	1.44*	42.7	23.9	18.2
BLOCK1	2.51	28.7	11.4	7.4
SPIN2	2.44	28.6	12.5	8.0
BLOCK2	2.45	28.6	12.2	7.8
SPIN3	2.29	30.3	13.7	8.7
BLOCK3	2.46	28.7	12.1	7.7

*ambient contention hit this slot and doubled its GPU-phase walls, so I treat it as an outlier.

Throughput verdict on this hardware: null. BLOCK means 2.47 vs SPIN 2.37 with the outlier excluded, inside this machine's noise floor. I read that as consistent with your mechanism, not against it. Your win was on CPU decode, where the spinning core competes with the matmul team. Here decode compute runs on the GPU and there are idle cores, so parking instead of spinning frees nothing scarce.

One thing I noticed but can't prove at n=3: the BLOCK arm had the tightest spread I've measured on this box, 2.4% across three runs, and the only badly contention-hit run in the sweep was a SPIN one. The blocking wait may be more robust when something else grabs the machine. I can rerun under deliberate background load if that would help decide the default.

[ebootheee]

@JustVugg — rebased and narrowed as you asked. This is now just the blocking-wait + the PIPE_WORKERS implication: one commit on today's dev, 39 lines in glm.c, with a test in TEST_BINS (implication truth table + a two-arm pool stress against a disk fixture — spin as control, condvar as treatment). make check passes locally on Linux; the Windows temp-file trap documented in test_stops is accounted for. Body rewritten to match; the old full-series description is in the edit history.

You were right about #274/#293 moving the ground under COLI_CUDA_BATCH/COLI_CUDA_SYNC — those are out. I'll re-benchmark against pipe2 and bring back only what still earns its keep, as a separate focused PR.

@michael-denyer — thank you for the disciplined A/B; snapshot-restoring .coli_usage between arms so autopin can't drift is exactly the right method. A null on Metal is what the mechanism predicts (decode compute is on the GPU, cores are idle, so parking frees nothing scarce), and your result is now cited in the PR body. The tighter spread under BLOCK is intriguing, but since the default stays OFF there's no need for the background-load rerun on my account — though if you run it anyway I'd love to see the numbers.