perf(prover): pull-cursor CPU scheduler instead of fixed chunks, plus… - #901
Open
diegokingston wants to merge 1 commit into
Open
perf(prover): pull-cursor CPU scheduler instead of fixed chunks, plus…#901diegokingston wants to merge 1 commit into
diegokingston wants to merge 1 commit into
Conversation
… tests
Two follow-ups to the CPU-aware scheduler.
**Keep the pull cursor.** The CPU `run_admitted` batched `order` into
`chunks(workers)` and ran each chunk to completion, so every table waited for
the slowest member of its chunk — the behaviour the doc comment directly above
`run_admitted` says `heaviest_first` plus the cursor were introduced to remove.
Sorting heaviest-first hides this while the weight estimate is accurate: chunks
then hold similar-weight work. It bites when the estimate mispredicts, which is
plausible here since the weight is `estimate_table_vram_bytes` — a device
footprint used as a proxy for host time.
This keeps the original CUDA scheduler's shape (atomic cursor, heaviest-first,
concurrency bounded by `workers`) but spawns the workers as Rayon tasks rather
than OS threads. That preserves the fix this branch is for — nested per-table
Rayon work stays inside the pool and work-steals — while a worker that finishes
a small table pulls the next one immediately. `TABLE_PARALLELISM` keeps its
meaning and the number of simultaneously live per-table working sets is
unchanged. Blocking a pool worker is only acceptable because, unlike the CUDA
path, there is no admission gate to wait on.
**Add coverage.** `run_admitted` is now three cfg-gated implementations with no
tests. These are cfg-agnostic, so they exercise whichever one the build selects:
* results land in the slot named by their own index, not by position in
`order` (those differ once the order is heaviest-first);
* a batch does not serialize when the estimate mispredicts — verified to FAIL
at 565ms against the chunked implementation and pass at ~130ms here;
* `heaviest_first` is a descending permutation.
Not addressed here, to keep the diff reviewable: the CPU staging block is a
near-duplicate of the `debug-checks` one, and `VramGate` is still constructed
and threaded as `_gate` on CPU rather than not built.
Collaborator
|
/bench |
Benchmark — real block (
|
| Metric | main | PR | Δ |
|---|---|---|---|
| Peak heap | 47024 MB | 50014 MB | +2990 MB (+6.4%) 🔴 |
| Prove time | 157.521s | 142.752s | -14.769s (-9.4%) 🟢 |
❓ -9.4% — beyond what 3 runs resolve. Use
/bench-abbafor a paired test of the same block (default 12 pairs, ~72 min, resolves ~1%).Prove-time spread 0.5% (142.929s / 142.752s / 142.221s)
Commit: 0f2f293 · Baseline: cached · Runner: self-hosted bench
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
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.
… tests
Two follow-ups to the CPU-aware scheduler.
Keep the pull cursor. The CPU
run_admittedbatchedorderintochunks(workers)and ran each chunk to completion, so every table waited for the slowest member of its chunk — the behaviour the doc comment directly aboverun_admittedsaysheaviest_firstplus the cursor were introduced to remove.Sorting heaviest-first hides this while the weight estimate is accurate: chunks then hold similar-weight work. It bites when the estimate mispredicts, which is plausible here since the weight is
estimate_table_vram_bytes— a device footprint used as a proxy for host time.This keeps the original CUDA scheduler's shape (atomic cursor, heaviest-first, concurrency bounded by
workers) but spawns the workers as Rayon tasks rather than OS threads. That preserves the fix this branch is for — nested per-table Rayon work stays inside the pool and work-steals — while a worker that finishes a small table pulls the next one immediately.TABLE_PARALLELISMkeeps its meaning and the number of simultaneously live per-table working sets is unchanged. Blocking a pool worker is only acceptable because, unlike the CUDA path, there is no admission gate to wait on.Add coverage.
run_admittedis now three cfg-gated implementations with no tests. These are cfg-agnostic, so they exercise whichever one the build selects:order(those differ once the order is heaviest-first);heaviest_firstis a descending permutation.Not addressed here, to keep the diff reviewable: the CPU staging block is a near-duplicate of the
debug-checksone, andVramGateis still constructed and threaded as_gateon CPU rather than not built.