perf(mpi): ⚡ exchange a gate's records over one pairwise round when it names one peer - #356
Draft
diagonal-hamiltonian wants to merge 4 commits into
Draft
diagonal-hamiltonian wants to merge 4 commits into
diagonal-hamiltonian wants to merge 4 commits into
Conversation
…ndow Under linear routing a gate reaches one peer rank's partitions, so every per-slot array it builds carried R - S empty blocks. mpi::WindowVec is the array over a SlotWindow: at_slot() is the single re-basing door, and operator[] takes a WindowIndex so a flat slot cannot compile as a raw index. The scan's queries/sent/sent_c0, the collective's receive buffer, round 2's responses and the decoded records' offsets all move onto it; begin_alltoallv and wait_into take either shape, so the tests keep handing whole [P] arrays. The scan's self reserve now fires on window.contains(my_rank) rather than a one-slot world: at R > 1 roughly one gate in R has a zero rank shift, and those staged their whole self side from an empty vector. Assisted-by: ClaudeCode:claude-opus-5
Under linear routing every partition has exactly one peer partition per generator, so a gate's records need no all-to-all: each participant hands over its S sub-streams and takes the peer rank's S back, in ascending source order. In-rank that is ONE barrier and no copy (peers read the publisher's buffers in place through PairSlots' descriptor table, which is double-buffered by call parity); across ranks it is two barriers and one message each way, sent in place through a hindexed datatype whose leading block is the S*S word counts, with the receive sized by MPI_Mprobe -- so a silent side still posts exactly one message and there is no count round to keep symmetric. The copy-free gather defers the second barrier rather than dropping it: a send buffer may be reused only from the gate after next, which is the caller's half of the contract and the whole lifetime rule in PairExchange.h. pair_exchange_tests takes that permission literally -- it scribbles a poison word over the released buffer set the instant the next gate returns, so a view that aliased it fails on a value instead of reading a plausible earlier gate. A debug-only generation assert in PairSlots catches the same fault from the other side. Assisted-by: ClaudeCode:claude-opus-5
… one rank pair Under linear routing a gate's window is one rank's partitions, so its records need no collective: the engine hands its per-slot blocks to mpi::pair_exchange and takes the peer rank's back. That removes the count round, the flat send-buffer copy every alltoallv makes of the whole wire volume, and one of the two in-rank barriers, on both rounds of the protocol. Round 2's inserts move ahead of the exchange rather than under it, the verb being synchronous; their order against apply_responses is unchanged, so the result is not. The buffers pair_exchange publishes must outlive the gate that filled them (PairExchange.h rule 1), so the round-1 records move out of the scan result into a two-slot scratch pool the gate parity alternates -- one buffer per round rather than a shared pool, since a shared slot would leave the response staging as wide as the query staging -- and `reuse_wire` applies the engine's usual 4x release rule to a pool that no gate boundary can free. Bit-identity, measured rather than assumed: at four partitions the whole 35-cell golden dump is positionally BIT-IDENTICAL to the same tree without the pair path, and at two and four MPI ranks every rank's coefficient bits agree cell for cell. evolution_detail_tests forces both arms on one gate of a two-partition world and compares the halves bit for bit. Assisted-by: ClaudeCode:claude-opus-5
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Docs preview: https://pr-356.monoprop-docs.pages.dev |
… end `MissStage` and `IncomingRecords` are per-gate: their size is proportional to that gate's |Anti(G)|, and nothing can read them once the gate is over. They live in `GateScratch` because the decode and the join write them through out-parameters -- not so that their storage outlives the gate, which is what it was doing: no caller was left for the 4x release rule they were introduced under (the last one went with `reuse_incoming_wire`, when the collective receive buffer became gate-local), so the widest gate of a call rested in the partition's footprint for the remainder of it. Releasing them where the gate ends is what they cost when the engine held them by value. Nothing about sizing, ordering or arithmetic changes, and the gate stamp still prices them at their widest instant, which is the figure a per-gate slab would have to serve. Measured on the 9.26 M-term cell at P=16 (`w_repro.py 12 7 1e-6`), against the campaign branch this stack re-lands: `gate_scratch_bytes` 46,055,498 -> 18,870,488 B (campaign 18,866,392), kernel `VmHWM` 561,712 -> 540,296 kB (campaign 538,088), `d_gate_buffers_hwm_bytes` 65,383,416 -> 59,306,826 B. The 4 KB that remain above the campaign's ledger are the two descriptor arrays (`wire_spans`, `slot_views`) which this `memory_bytes()` counts and that one does not. At the 250 M-term rung the same retention was +215.6 MiB of ledger and 0.824 against 0.815 of main's peak RSS. Also deletes `release_if_oversized`, which has had no caller since then, and retargets the `reuse_wire` comment's cross-reference at `TableJoin::begin_queries`, the rule's live definition. Bit-identity unchanged: `golden_raw.py` over 35 cells (6,024,567 terms) at P=1 is positionally 0 ULP against `golden/x2-raw.npz` -- bitne 0, msetne 0, monoseq 0, every scalar column 0. Assisted-by: ClaudeCode:claude-opus-5
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.
🤖 AI text below 🤖
Summary
Under linear routing (PR 2), a gate's window reaches exactly one peer rank's partitions once the rank
count is above one — so every per-slot array the collective exchange built carried
R - Semptyblocks, and the exchange itself paid a count round and a full send-buffer copy for a peer set of one.
This PR adds
mpi::pair_exchange, a direct pairwise exchange for that case: each participant handsover its
Ssub-streams and takes the peer rank'sSback, in ascending source order. In-rank that isone barrier and no copy — peers read the publisher's buffers in place through
PairSlots' descriptortable, double-buffered by call parity; across ranks it is two barriers and one message each way, sent
in place with the receive sized by
MPI_Mprobe, so a silent side still posts exactly one message.Getting there needed the per-slot arrays re-based onto the peer window first (
mpi::WindowVec,deferred here from PR 3):
at_slot()is the one re-basing door, andoperator[]takes aWindowIndexso a flat slot index cannot compile as a raw one. The engine's two exchange rounds then move onto
pair_exchangewhen the gate's window names one rank pair, and onto the existing collectiveotherwise; round 2's inserts move ahead of the exchange (the verb is synchronous) with no change to
their order against
apply_responses.The buffers
pair_exchangepublishes must outlive the gate that filled them — a send buffer may bereused only from the gate after next — so round-1's records move out of the scan result into a
two-slot scratch pool the gate parity alternates, and
reuse_wireapplies the engine's usual 4×release rule to a pool no gate boundary can free.
This is PR 7 of 7, based on
perf/stack-6-pair-once. It targets peak memory (the count round andsend-buffer copy this PR removes) and time, both at
R > 1, where PR 5's protocol currently costs8-9% time on Hubbard (see PR 5's Measurements). The whole stack's tip is measured against
origin/mainin a longer, higher-rep ladder once this PR is gated; those numbers are quoted separately, not in this
body.
Changes
Engine
cpp/monoprop/detail/mpi/Comm.h(252 lines):WindowVec<T>(
reset/rewindow/at_slot/operator[](WindowIndex)/capacity).cpp/monoprop/detail/mpi/PairSlots.h(new, 175 lines): the peer descriptor table, plus adebug-only generation assert that catches a reader taking a descriptor from another gate.
cpp/monoprop/detail/mpi/PairExchange.h(new, 181 lines):pair_exchange, with the lifetimecontract written down as a numbered
@warning(a published buffer set must outlive the gate afternext).
cpp/monoprop/detail/mpi/HybridComm.h(567 lines),ShmComm.h(218 lines):pair_member,pair_exchange/_impl_/_with_rank_/pair_views_,staging_bytesover both the collective andpairwise paths.
cpp/monoprop/detail/mpi/MPICompat.h(425 lines):SlotBlockValue/slot_window_of/slot_block/reset_slotsoverload pairs;wait_into/begin_alltoallvtemplated on the block array.cpp/monoprop/detail/mpi/Pairwise.h(122 lines):kPairExchangeTag, one message per gate.cpp/monoprop/detail/evolution/layer_build/Engine.h(515 lines):windowmember (default = wholeworld),
pair_path/pair_shiftderived frommpi::geometry(comm)and the window; both rounds takeeither the pairwise or the collective arm; the collective's receive buffer is now gate-local rather
than a scratch member, since only a published buffer must outlive its gate.
cpp/monoprop/detail/evolution/layer_build/Scan.h(675 lines):FusedScanResult::windowandWindowVec-typedqueries/sent/sent_c0; self reserve now fires onwindow.contains(my_rank)rather than
rank_count == 1(O8) — atR > 1roughly one gate inRhas a zero rank shift, andthose previously staged their whole self side from an empty vector.
cpp/monoprop/detail/evolution/layer_build/Resolve.h(421 lines):slot_streams/decode_incoming_records/join_incoming/apply_responses/absence_passtake an explicit windowargument.
cpp/monoprop/detail/evolution/layer_build/GateScratch.h(320 lines): the wire pool(
wire_q[2],wire_r,wire_gate,wire_spans,slot_views,wire_queries(two_rounds),wire_bytes()) replacesincoming_wire/reuse_incoming_wire.cpp/monoprop/detail/evolution/layer_build/Common.h(159 lines):kWireFloorWords,reuse_wire(WindowVec, SlotWindow).Tests
cpp/tests/pair_exchange_tests.cpp(new, 502 lines): five in-rank patterns, six cross-rank, aback-to-back-gates case, argument refusal, a poison-release check, hybrid and raw kinds, plus
pair_exchange_shm_views_never_alias_a_released_buffer_set.cpp/tests/evolution_detail_tests.cpp(1024 lines):one_gate_pair_and_collective_arms_agree_bitwise— two partitions of an in-process
ShmComm, one gate, run twice (eng.pair_pathforced off for thesecond arm),
HalfRotationReccompared field by field withv_partnerbit-cast.cpp/tests/sparse_resolve_tests.cpp,mpi_utils_tests.cpp,pair_once_tests.cpp: thewindow_base=16/=6arms restored onWindowVec;check_bucket_ownershipand the per-slotsent-ordinal lists over window indices.
Docs
docs/content/docs/features/parallelism.mdx: the pair exchange's cost per gate, and why the singlein-rank barrier is a deferred second barrier rather than a dropped one (two send-buffer sets,
descriptor parity).
Measurements
Gated positional against the x2 raw-bit reference, the same reference PR 5's tail and PR 6 gate against:
gate record md5
f03de2e9ac158dc6e40f752b5145330b(df6d9af2…before the last commit).Paired A/B, 3 interleaved reps against origin/main c5e88c8 and the predecessor, ratios only:
The in-place pair exchange recovers the P=16 time PR 5 cost (1.076 → 1.012 at M2a); the P=1 Hubbard
rung keeps PR 5's cost, which the tip ladder below quotes against main at full size.
Tip ladder, 7 interleaved reps per rung against origin/main c5e88c8 (
**= 7/7 agreement, sign test p = 0.016):The last commit (
c796bcd) closes the one exact difference the ladder showed against the same engine measuredearlier as one branch: the gate's mint and decoded-record stages were members of
GateScratchwith no release,so the widest gate's storage stayed resident in every partition for the rest of the call. They are now handed
back at the end of
exchange_and_join, matching per-gate ownership. Sizing only, bit-identical by construction(re-gated positionally). Re-measured at M2a-hubbard (R=1, P=16, 3 reps):
gate_scratch_bytes103 → 70 MiB,d_gate_buffers_hwm_bytes39 → 6 MiB, ledger 0.733 → 0.721 of main, peak 0.764 → 0.759, time within noise.Notes for reviewers
pair_pathrequiresR > 1, and thepositional gate above runs at
monoprop_PARTITIONS=1. It is still the right gate for what it covers(the collective arm, and everything PR 5/PR 6 already established), but the pairwise transport's own
bit-identity needs separate evidence, which the implementer produced and is quoted here rather than
re-derived:
monoprop_PARTITIONS=4(every gate takes the pair path): the full 35-cell golden dump comparedpositionally against PR 6's own
P=4dump (the collective path, same partitioning) isbit-identical on every column. The same
P=4dump compared against theP=1reference isvalue-identical (
msetne 0) with the layout and gradient reduction order moved, as expected of apartition-count change.
n=2,n=4): a per-rank, per-cell sha256 of the local coefficient array's IEEE-754bits and the expval bits, over 21 cells (7 fixtures ×
c4/c6_la1e-6/c6_schro), is identicalbetween PR 6 and this PR at both rank counts.
pair_exchange's buffers may be reusedfrom the gate after next;
pair_exchange_shm_views_never_alias_a_released_buffer_setscribbles apoison word over the just-released buffer set the instant the next gate returns, so a view that
aliased it fails on a value rather than reading a plausible earlier gate. A mutation check (forcing
gather_in_rankonto the released parity) confirmed the test actually catches this: it failed on3/19 value assertions before the fix was reverted. A debug-only generation assert in
PairSlotscatches the same fault from the publisher's side.
PendingAlltoallv::staging_bytes()is not carried here — it belongs to the buffers-HWM work alreadylanded in PR 4/PR 5, and
Engine's own wire-byte accounting covers the same instant.scratch.incoming_records) and its misses/join insidethe scratch, rather than by-value records and an engine-owned miss stage: that refactor is outside
this PR's commits and does not change the result.
Checklist
docs/,CONTRIBUTING.md) if neededCHANGELOG/ release notes updated if applicable (n/a — the repository has noCHANGELOG)AI/LLM disclosure
Important
By opening this PR I confirm that I have read CONTRIBUTING.md and I agree to the terms of the Contributor License Agreement.
Warning
If you're contributing on behalf of your employer, contact cla@algorithmiq.fi to arrange a Corporate CLA.