milnor_gpu: drop the global device mutex (thread-local resident, per-thread streams) - #15
Draft
JoeyBF wants to merge 13 commits into
Draft
milnor_gpu: drop the global device mutex (thread-local resident, per-thread streams)#15JoeyBF wants to merge 13 commits into
JoeyBF wants to merge 13 commits into
Conversation
…hanism" This reverts commit 71a21b6.
The relaxed wavefront keeps many bidegrees in flight at once, so at any instant it is likely that some job is inside a linear-algebra critical section (ParallelGuard). The scheduler re-spawned a bounced job immediately, which just re-checked is_in_parallel, found it still busy, and bounced again — spawning a whole rayon job per re-check and pegging every core on a retry storm that does no useful work. Instead the receiver checks the flag itself (a cheap atomic load) and parks a bidegree only when the section is genuinely busy. A job acquires and releases its guards many times and spends most of its time outside them, so the section frees far more often than jobs complete; parked bidegrees are therefore re-checked via a short recv_timeout while anything is parked, and re-spawned as soon as the section frees. Incoming messages are still handled the instant they arrive. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MCUtWj6P6suSZqvATCdg6d
is_in_parallel was a global count of active par_iter critical sections, so a step_resolution job bounced whenever *any* thread was in one. Under the relaxed wavefront many bidegrees are in flight, so that flag is almost always set and nearly every job bounced, producing the retry churn the parking mitigation only softened. The priority inversion the guard exists to prevent is narrower: a worker that initiated a par_iter blocks in the join and work-steals, and if it steals another (heavy, nested-parallel) resolution step, that step stalls the section the worker is blocked on. A stolen job runs on the stealer's own OS thread, so a thread-local depth counter reports exactly whether *this* worker is a blocked guard holder. Jobs picked up by a free worker read zero and run, letting independent bidegrees resolve concurrently instead of serializing behind any single critical section. The scheduler thread never holds a guard, so it can no longer read the flag to sense saturation; park bounced bidegrees and retry them on each completion or a short recv_timeout tick. Bounces are now rare (only a genuine steal-onto-a-blocked-holder), so the parking path barely engages. The classical scheduler shares the guard and benefits the same way, so its immediate-respawn no longer storms. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MCUtWj6P6suSZqvATCdg6d
Pins the invariant the previous commit relies on — a ParallelGuard held on one thread reads as absent on another — so a future change that reverts to a shared counter fails loudly instead of silently reintroducing the retry storm. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MCUtWj6P6suSZqvATCdg6d
…-graph-d69k0s' into hpc
…ead-local
The batched multiply serialized every launch behind one RESIDENT mutex held
across the whole marshal+upload+kernel+readback section, and additionally
pinned all work to CUDA stream 0. With the relaxed dependency graph exposing
~max_s-wide bidegree parallelism, that lock collapsed a ~12-core CPU wavefront
to ~2.6 busy cores and left the GPU idle 80% of the time — making NASSAU_GPU=1
a net 1.4x slowdown over CPU-only at stem 130 (193s vs 142s).
cubecl 0.10 does not need the lock: a per-device runner thread already
serializes all server access (concurrent client calls are memory-safe), and
memory pools are per-stream. So:
- RESIDENT becomes a thread_local RefCell: each rayon worker keeps its own
admissible cache and cs/mk device handles, created and consumed only on the
thread (and thus the default per-thread CUDA stream) that owns them, so no
handle ever crosses threads and no cross-stream event sync fires.
- The GPU_STREAM{value:0}.executes pin is removed; each worker launches on its
own default stream, so independent bidegrees marshal and execute
concurrently. memory_cleanup now trims only the calling worker's pool.
Stem 130 (S_2, s<=152, 16-core H200 box): 193s/2.6 cores (old mutex GPU) and
142s/10 cores (CPU-only) -> 44-49s/5.6 cores. Verified bit-identical to the
CPU path with NASSAU_GPU_VERIFY=1 at stem 80 (MIN_WORK=0, every build) and
stem 130 (default gate, all offloaded/chunked launches, concurrent workers).
Note: concurrency raises peak host memory (concurrent marshal buffers across
workers); a 16-worker VERIFY run at stem 130 exceeded a ~48GB cgroup, while
normal runs fit comfortably. Bound RAYON_NUM_THREADS if memory-constrained.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… resident master)
The mutex-removal commit let many workers run device sections concurrently, which
exposed three unbounded memory consumers at record stems (>100GB host AND device
by stem 150, measured):
1. Unbounded launch transients: the all-rows reuse build allocated its full output
in one shot, per in-flight worker. Fixed by splitting builds into row blocks
bounded by NASSAU_GPU_BLOCK_MB (default 512MB) of output AND GPU_PAIR_CHUNK
kernel threads — one launch per block, subsuming the former pair-chunk loop
(rows are independent, so blocks concatenate exactly).
2. Unbounded stream count: every worker thread got its own CUDA stream, and each
stream's pool retains freed slabs indefinitely. Fixed by NASSAU_GPU_CONCURRENCY
(default 8) permits that double as stream slots: at most 8 device sections run
at once, on 8 fixed streams. A permit must never be held across a rayon parallel
section (par_iter chunks execute on guard-free threads that can steal a bidegree
job which then parks on acquire — observed deadlock); it is acquired only for
the strictly sequential layout+device section. Do NOT raise to 16: measured
catastrophic (>30x) slowdown from cross-stream sync churn.
3. Per-thread resident duplication: the thread-local resident store copied the
admissible-matrix master (~8.5GB at stem 150, growing with degree) once per
worker, on host and device. Fixed by re-sharing it: host master behind an
RwLock (enumeration outside the write lock), one device mirror behind a small
mutex, handles shared across threads/slots (cubecl event-syncs cross-stream
reuse). Re-uploads are needs-based — only when a launch dereferences past the
uploaded prefix — since re-uploading on mere growth serialized multi-GB copies
on nearly every frontier launch (measured 1.5x wall regression).
Stem 150 (S_2, s<=152, 16-core H200 box), verified bit-identical to CPU at
stem 80 (every build, forced multi-block) and stem 130 (all offloaded launches):
wall cores host RSS device
before this commit 682s 3.3 137 GB 140 GB (full card)
after (32 workers) 721s 3.8 65 GB 37 GB
CPU-only reference 771s 10.3 4.4 GB —
Verdict: at record stems the GPU path now merely ties CPU-only while using far
more memory — the CPU path (no row-reuse matrix, per-signature builds) is both
frugal and wavefront-parallel. Recommend CPU-only for the stem-300 production
run; the GPU path remains correct, memory-bounded, and a real win at mid stems
(3x at stem 130).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Owner
Author
|
Follow-up commit f1cc8fa bounds the GPU-path memory the mutex removal exposed (>100GB on host and device by stem 150): row-block chunking (subsumes the pair-chunk loop), 8 launch permits doubling as fixed stream slots, and a re-shared resident master with needs-based uploads. Verified bit-identical at stem 80 (every build, forced multi-block) and stem 130. Stem-150 result: 137GB/140GB -> 65GB/37GB at comparable wall time (721s vs 682s). CPU-only reference: 771s at 4.4GB. Verdict: at record stems the GPU path ties CPU-only while using 15x the memory — recommend CPU-only (drop NASSAU_GPU=1) for the stem-300 production run. GPU stays a real win at mid stems (3x at stem 130). Do not raise NASSAU_GPU_CONCURRENCY past 8 (measured >30x collapse at 16). |
Replaces the count-based launch cap (NASSAU_GPU_CONCURRENCY=8 exclusive sections) with two decoupled controls: - NASSAU_GPU_MEM_BUDGET_MB (default 4096): admission weighted by a launch's output bytes, so dozens of small low-stem launches run concurrently again (the count cap throttled exactly the region that never had a memory problem) while the frontier stays bounded to ~budget/block-size in flight. - NASSAU_GPU_STREAMS (default 8): fixed CUDA stream slots, round-robin and SHARED (small launches serialize on a stream rather than demanding an exclusive one), so stream/pool count is bounded independently of concurrency. Master device uploads are now prefix-only with doubling: a launch ships max(need, 2*uploaded) entries, not the whole master, so frontier launches (which append new high-degree R each t) no longer re-ship gigabytes of untouched tail. Stem 130 improved 187s -> 150s; stem 150 memory 65/37 -> 68/30 GB, verified bit-identical (stem 80 all-builds, stem 130 all offloaded). But a slots x budget sweep is FLAT (8/4G=150s, 16/8G=170s, 32/16G=160s, 64/32G=201s): concurrency knobs are not the ceiling. The ceiling is Amdahl — the GPU accelerates only the Milnor multiply (~17% of frontier wall time; row_reduce/signature_matrix/readback dominate and are CPU/serial through cubecl's single runner thread), so the end-to-end GPU:CPU ratio is flat ~1.13x across the 130-150 heavy bands, not widening. Widening it would require offloading row_reduce (PR SpectralSequences#274's RREF). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Owner
Author
|
Per-band analysis (S_2, s<=152, current bounded binary, w=32):
GPU crosses over above stem 130 but the ratio is flat ~1.13x, not widening. Slots×budget sweep at stem 130 is flat (8/4G=150s … 64/32G=201s), so concurrency knobs are not the ceiling. The ceiling is Amdahl: the GPU accelerates only the Milnor multiply (~17% of frontier wall time; row_reduce + signature_matrix + readback dominate and are CPU/serial through cubecl's single runner). Arithmetic intensity rises with stem but the multiply's share of wall time doesn't, so the end-to-end ratio stays flat. Revised verdict: at bounded memory the GPU path is ~1.13x on heavy bands, a loss on light ones — modest. Widening it needs row_reduce offloaded (PR SpectralSequences#274 RREF). For stem-300 production, CPU-only remains the safe default (frugal, wavefront-parallel); GPU is worth enabling only if the ~12% frontier win justifies the memory + tuning. |
…tiply path
The zero-signature image matrix (d_s applied to the zero-sig source basis,
column-masked to the zero-sig target) was the last per-bidegree Milnor multiply
still on the CPU — a serial per-row apply_to_basis_element_restricted, ~17% of
frontier wall time in the perf profile. But it is the *same* restricted multiply
as the QI-source `full_matrix` already built via restricted_partial_matrix_maybe_gpu,
just on d_s = differentials[b.s()] instead of d_{s-1}, and its target
mask/dimension are exactly the `target_mask`/`target_dim` already computed for the
bidegree (d_s and d_{s-1} share the target module modules[b.s()-1]). So route it
through the same GPU-offloaded, work-gated, already-verified path and apply the
column mask on CPU; drop the serial `signature_matrix` method. (Reinstates the
"signature_matrix offload" win from the original nassau_gpu branch, lost in the
SpectralSequences#272 relaxed-graph merge.) row_reduce stays on CPU — the signature-masked matrices
are very flat (~100 x 100000), a poor RREF target for the GPU.
Correctness: GPU Ext chart byte-identical to CPU-only through (100,152);
NASSAU_GPU_VERIFY passes at stem 130.
This shrinks the serial tail that Amdahl-capped the GPU:CPU ratio, so the
arithmetic-intensity advantage finally shows through and the gap WIDENS with stem
(S_2, s<=152, 16-core H200 box, w=32):
band GPU CPU ratio
130->140 159s 206s 1.30x
140->150 278s 423s 1.52x
cum 0->150 596s 771s 1.29x (was 723s, a near-tie)
Memory stays bounded by the same byte-budget/block machinery (this path reuses
multiply_batch_on_gpu). Next lever: the full-reuse-matrix readback.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… a host zero buffer Per-thread stack sampling at stem 145 showed the wavefront's serial stalls were a rayon worker pegged in __memcpy_ssse3 inside create_from_slice — host-side upload marshaling, NOT readback (cubecl 0.10 already does async D2H off pinned memory with the event wait on the worker thread, so the runner is free during the copy). The dominant offender: the batched multiply allocated + zeroed a host `vec![0u32; out_len]` (hundreds of MB at the frontier) and memcpy'd it up as the kernel's XOR accumulator, every launch/block. Allocate out_h uninitialized (client.empty) and zero it with a trivial on-device kernel (zero_u32), same stream as the multiply so it is ordered before it. Removes the host memset, the non-pinned host->device copy, and the transfer itself; on-device zeroing is memory-bound (microseconds on an H200). Verified: GPU Ext chart byte-identical to CPU through (100,152); NASSAU_GPU_VERIFY passes at stem 130. Bands (S_2, s<=152, 16-core H200 box, w=32), vs the prior signature-offload binary: 0->130 159 -> 141s (ties CPU-only 142; was a 0.89x loss) 0->140 318 -> 245s 130->140 marginal 104s vs CPU 206s = 1.98x (was 1.30x) peak RSS 46GB, GPU 28GB (both down). Next serial upload to check: term_pparts / the per-product record arrays. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The shared admissible master reaches ~3GB by stem 138 (cs 1.2GB + mk 1.9GB), and every launch took the RESIDENT_DEV mutex to read its device handles — with a growth-triggering launch doing a multi-GB create_from_slice re-upload *while holding that mutex*. Per-thread stack sampling showed the frontier collapsing to a single thread memcpy-ing gigabytes while every other bidegree blocked on the lock (upload byte-size instrumentation under NASSAU_GPU_DEBUG confirmed the master, not term data or the seqno table, as the giant upload). Make handle reads lock-free (RESIDENT_DEV: Mutex -> RwLock) and move the upload memcpy outside that lock, serialized only among uploaders by a separate RESIDENT_UPLOAD mutex with a re-check that coalesces a burst of growth-needing launches into one upload. A launch whose R's are already resident proceeds without ever blocking on someone else's upload. Verified GPU chart byte-identical to CPU through (100,152); NASSAU_GPU_VERIFY passes at stem 130. Removes the lock-held-across-copy stall, but occupancy only rose ~3.5->4.5 cores: the dominant limiter is upstream (thin GPU bidegrees + wavefront width), not this lock. Kept because it is correct and matters more at stem 300 where the master is larger. Also adds a per-buffer upload-size line to NASSAU_GPU_DEBUG. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…er-product alloc storm A frontier launch has ~1e5-1e6 products, and the marshal built term data as `Vec<(Vec<u16>, Vec<u32>)>` — two heap allocations per product (~1e6 tiny allocs per launch) — then extend-copied them into the flat upload buffers. Per-thread profiling of the GPU path showed this as a dominant chunk of the per-bidegree CPU "envelope" (~16% _int_malloc/_int_free plus the marshal copy) that wraps each (fast) kernel and, because the wavefront is only ~10-15 bidegrees wide, cannot be hidden — so the GPU sits idle between brief spikes. Precompute the term-count prefix sum (`term_off`), size the flat `term_pparts`/ `term_lens` once, and parallel-fill each product's disjoint slice in place (unsafe but sound: prefix-sum ranges never alias). The later layout loop just reads `term_off[pi]` for `prod_term_start` — no per-product allocation, no concat copy. Verified GPU chart byte-identical to CPU through (100,152). Same GPU results, far less allocation and marshal work per launch. (The remaining per-product `GpuProduct.term_indices: Vec<usize>` built in extract is the next alloc to flatten.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…er-splitting The row-block splitter caps a launch at GPU_PAIR_CHUNK thread-pairs (the kernel indexes threads by u32 ABSOLUTE_POS, ceiling 2^32). It was set to 1<<30 (~1.07e9), ~4x below the real ceiling — so every billion-pair giant was chopped into ~4 launches, each a separate upload + kernel + BLOCKING readback round-trip, even though its output is only ~350 MB (well under gpu_block_bytes). Debug confirmed the giants pegged at 1.07e9 pairs; this, not the byte budget, was the binding split, which is why a NASSAU_GPU_BLOCK_MB sweep was flat. Raise it to 3.9e9 (leaves ~0.39e9 headroom under 2^32; the splitter always takes >=1 row and a lone row past 2^32 still trips the per-block u32::try_from assert; grid stays ~1.5e7 cubes, far under 2^31). Giants now run as a single launch (max total_pairs observed 3.90e9), collapsing 4 round-trips to 1. GPU 0->140 (w=100): ~245-288s -> 216s. Chart byte-identical to CPU through (100,152). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See commit message: removes the RESIDENT mutex + stream-0 pin; thread-local resident store, per-thread CUDA streams. Stem-130: 193s/2.6 cores -> 44-49s/5.6 cores (CPU-only: 142s/10). Verified bit-identical (VERIFY at stem 80 all-builds and stem 130 default-gate, concurrent). Note: verify at 16 workers exceeded a ~48GB cgroup (normal runs fit); bound RAYON_NUM_THREADS if memory-constrained. Includes the local guard-relaxation commits not yet on origin/hpc.
🤖 Generated with Claude Code