pilot: multi-worker PILOT_REAL prefetch (PILOT_WORKERS) — byte-identical base for the #441 hardware A/B#480
pilot: multi-worker PILOT_REAL prefetch (PILOT_WORKERS) — byte-identical base for the #441 hardware A/B#480cdhdt wants to merge 1 commit into
Conversation
1b711a1 to
7c2d9ee
Compare
|
This is the exact base we want for the #441 question. To move it out of draft and merge: keep |
|
Thanks @JustVugg — that's the plan exactly. State of the branch for whoever runs the A/B:
Single-binary A/B for the enterprise-NVMe / big-RAM folks (#389 / #161), baseline arm first: Report the Written with AI assistance (Claude). |
|
Status check on this one — it's marked draft, and it conflicts with The framing in the title is the part I want to keep: byte-identical base for the #441 hardware A/B. A prefetch change that can't alter output is exactly the kind that can be evaluated on speed alone, and that's the only honest way to compare a prefetch strategy — otherwise a quality difference hides inside a throughput number. Two things worth knowing before you spend more time on it, because the ground moved under this feature recently:
If you rebase and it's ready to leave draft, I'll approve the workflows and review it. If you'd rather park it until the #441 A/B has hardware behind it, that's reasonable too — just say which, so it isn't sitting here ambiguous. |
|
Still on it — thanks for the #497/#490 heads-up. You're right that my original guard over-dropped, so any numbers from before that fix sit on a broken baseline. I'll rebase this onto current |
…o the corrected JustVugg#497 guard Rebase of the multi-worker pilot onto current dev, which now carries the CORRECTED LFRU eviction guard (JustVugg#497, fixing the JustVugg#490 regression my original JustVugg#474 formula caused). The branch's own copy of the old guard is gone: both speculative sites now use dev's narrowed test — protect the victim only when it is genuinely warm (heat >= 2) AND clearly hotter than the speculation by tier_pick_lfru's 25%+4-freq hysteresis. Contents (unchanged in intent from the pre-rebase branch): - SPMC pilot ring (CAS claim, test_pilot_ring proves exactly-once across 8 workers) - PILOT_WORKERS env, default 1 = today's behaviour; clamped [1,16]; URING path stays single-consumer by construction (pilot_uring_batch drains a whole batch) - visible -(eid+2) reservations on the blocking loader, so concurrent workers never pick each other's in-flight slots as eviction victims nor double-load the same eid - tests/test_pilot_ring.c + Makefile rule Two fixes folded in during this rebase: - the guard stays INSIDE the eviction branch in pilot_uring_batch. Fresh ecache slots are calloc'd (eid == 0), so a guard placed on the grow path would read slot 0 as if it held expert 0 and could drop the speculation for no reason. - dead `isnew` removed: with visible reservations the slot is published (ecn bumped) at claim time, not after the pread. Draft base for the JustVugg#441 good-hardware A/B; not for merge until it shows a win. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
7c2d9ee to
165f5a2
Compare
|
Rebased onto current The rebaseThe branch now sits on the corrected #497 guard, not my broken #474 one. Both speculative sites (
Re-measured on the corrected base — and a correction to my July-21 tableSame protocol as before: frozen Output is byte-identical across all 11 runs (sha256 of the generated text: PILOT off ×2 / workers 1|8 × guard on|off × 2 reps). Placement-only contract holds on the new base.
(rep1 / rep2. Wall clock is the untrustworthy column here: the rep-1 runs spread 66.9–86.6 s with no ordering that matches any arm, i.e. session drift dominates it. Hit rate, MB/token and felt-wait reproduce tightly across the two reps — those are the numbers I'd base anything on. The rep-2 runs and the two PILOT-off runs all landed in the same tight 55.7–57.4 s band, which is the only stretch where wall clock says anything at all.) The correction. My July-21 table on #441 had One more thing the table shows, offered as tuning feedback on #497 rather than a complaint: even corrected, the guard still costs a lot of hit rate on a small cache — 49.7% guard-ON vs 67.8% guard-OFF at workers=1, and felt-wait 23.8 s vs 17.5 s, both reproducible across reps. At cap 8/layer essentially every resident is "warm", so Why "park"On the PR's actual question — does queue depth > 1 pay? — the answer here is still no, on a corrected baseline:
And the thing this box cannot show hasn't changed: at cap 8/layer there are no genuinely cold slots for extra queue depth to fill. That still needs enterprise NVMe with a QD8–32 sweet spot, or a big-RAM host where the drive is idle during compute. So: I'm not asking for review or merge, and I don't want it sitting in your queue re-conflicting every week. It's rebased, green, and byte-identical, so it's a ready-to-build A/B vehicle for whoever has the hardware (#389 / #161). Keep it as a draft if that's useful to you; if you'd rather close it, close it — the branch stays on my fork, #441 links to it, and I'll rebase on demand the day someone volunteers a box. Either way it shouldn't block anything. On your #403 pointChecked it, and it's cleaner than I expected: every prefetch buffer is slot-owned ( Rebased, reviewed, and measured with AI assistance (Claude). The July-21 correction above is mine, not the tool's — I had the #490 evidence in my own table and read it wrong. |
|
One more reason for "park" that I left out above, and it's the one that actually decides it for me — I owe it to you since you offered to review if I took it out of draft.
So merging this doesn't ship a dormant knob — it puts new concurrency code on the path of every The counter-argument is real and I'll state it fairly: merged with default 1, anyone with the right box could run the A/B on a stock build instead of compiling my branch, which is exactly the friction currently blocking this. If you judge that discoverability is worth the code-path change, say so and I'll take it out of draft the same day — you know the maintenance cost of this tree better than I do. Left to me, it stays parked until a number exists. Written with AI assistance (Claude). |
What this is
Part-1 of #441: multi-thread the blocking
PILOT_REALprefetcher so speculative loads run atqueue-depth N instead of 1. SPMC ring (
pilot_ring_claim, CAS claim, read-payload-before-claim),PILOT_WORKERSenv (default 1 = today's exact behavior), the URING path's visible-(eid+2)reservation ported to the blocking loader, and the LFRU eviction guard re-applied on top of the new
SPMC slot-selection — in its corrected #497 form since the 2026-07-26 rebase. Rebased onto
current
dev.Why it's a draft, not a merge request
On every drive I can test it regresses or no-ops, and your own gate on #441 is the right one:
multi-worker needs (a) LFRU-aware eviction (merged as #474, corrected in #497) and (b) a good-hardware A/B
before it ships even opt-in. This PR is (a)-done + the vehicle for (b). It should merge only if
the enterprise-NVMe / big-RAM A/B shows a real win.
What I measured (and why it isn't a win here)
Controlled 2×2 on my QLC / 62 GB box —
PILOT_WORKERS {1,8}×PILOT_EVICT_GUARD {0,1}, frozen.coli_usagerestored before every run,CAP_RAISE=0(cap 8/layer everywhere), one native build,interleaved, 2 reps in reversed order,
DIRECT=1, greedy, 12-token decode. Output byte-identicalacross all 11 runs (sha256 of the generated text):
(rep1 / rep2. Wall clock is drift-dominated in rep 1 and should not be read across reps; hit rate,
MB/token and felt-wait reproduce tightly and are the trustworthy columns.)
workers=8 still doesn't pay here. Guard ON it costs +1.0/+1.5 s felt wait for +0-1% bytes;
guard OFF it costs +11/+15 s. There are no cold slots at cap 8/layer for extra queue depth to
fill, so this box structurally can't show the upside — that needs a host with a real cache
(big RAM) + a drive with a QD8-32 sweet spot / idle bandwidth during compute.
Output-preserving
Cache placement + prefetch scheduling only — never a value. The
moe()per-layer barrier fencesevery layer against in-flight pilot loads; reservations (
-(eid+2)) and loading slots (eid=-1)are never matmul'd; a dropped/late speculation is just demand-loaded. sha256-identical across
PILOT off / workers 1 / workers 8 / guard on / guard off on the full GLM-5.2 744B model (11/11 runs
on the rebased base, 9/9 before it).
What's in it
PILOT_WORKERS(default 1), visible-(eid+2)reservations on the blocking pathtests/test_pilot_ring.c(N-thread ring stress: each item claimed exactly once, no loss/dup/torn)(
PILOT_EVICT_GUARD, default on)rss_guardeviction UAF this surfaced already shipped separately as rss_guard: free the evicted expert slab under g_pilot_mx (fixes a use-after-free with the pilot prefetcher) #449.
How to run the A/B (single binary)
Report
PROF=1's byte counter (MB/token) + felt-wait (and tok/s).PILOT_EVICT_GUARD=0isolatesthe eviction-thrash component.
Validation
make -C c check— 111/111, native + portable, 0 warningsrebased base)
Compatibility
PILOT_WORKERS=1= current behavior; no default-config user is affectedImplemented, reviewed, and measured with AI assistance (Claude), per the project's AI-contribution
precedent (#428, #398). Kept as a draft on purpose — it should not merge without the good-hardware
win it's built to let you measure.