perf(mpi): ⚡ exchange gate payloads point-to-point over reachable peers - #352
Draft
diagonal-hamiltonian wants to merge 7 commits into
Draft
diagonal-hamiltonian wants to merge 7 commits into
diagonal-hamiltonian wants to merge 7 commits into
Conversation
At R = 2 a peer plan can only ever resolve one peer, so every `for k in [0, f)` in the sparse transport this branch adds stays a single iteration and the multi-peer paths are unreachable however many cases are written for them. CI MPI test time roughly doubles: the suite-level variants now run at 2 and 4. Assisted-by: ClaudeCode:claude-opus-5
Under GF(2)-linear routing a generator's queries all land on one peer rank, so the destinations a round can reach are that rank's S partitions -- one contiguous run of the P = R*S world instead of all of it. `SlotWindow` names the run and `WindowIndex` is its re-based index, a distinct type because a flat slot used as one stays in bounds and addresses the wrong peer. `PeerPlan` carries the two states routing::Router has, and its `window` derives the run in one expression per field: dense is the count == P value of the same expression, not a second case. `Router::bits_for` answers the resolved rank-bit count for a geometry alone. It IS the private constructor, so the resolution and the non-power-of-two throw cannot drift from a router's; `linear_bits_for` wraps it in the environment's mode for callers that gate a transport on the number rather than route with it. No caller yet. Assisted-by: ClaudeCode:claude-opus-5
HybridComm was 636 lines carrying two unrelated jobs: the barrier discipline that lets S partition threads drive one rank's MPI, and the staging tables that tile their blocks onto the wire. `HybridStaging` takes the second -- the count matrices, the per-rank layout, the two grow-only payload buffers and every sweep over them -- and the comm keeps the verbs, the reductions and the abort. `size_staging_send_` and `size_staging_recv_` were the same function twice, the sum-prefix-base layout over a per-slot column with different arrays and different overflow messages. They fold into one `layout_peers_`, which the two sizers now call after preparing their column; the labels travel as a struct of three literals so the shared sweep still names the side it is laying out and nothing is formatted on the success path. The two recv-column fillers likewise differ only in where they read a count from -- the rows published before B1, or the count blocks just exchanged -- so they become one `fill_recv_col` taking that reader. Pure relocation otherwise: same phases, same barriers, same block order. 334/334 ctest, including both MPI variants. Assisted-by: ClaudeCode:claude-opus-5
Linear routing makes the destination RANK of a query predictable, but nothing
used that: the exchange still called MPI_Alltoall on the counts and
MPI_Alltoallv on the payload, so the message count stayed R*(R-1) whatever the
data looked like.
`mpi::PeerPlan` carries the structure from where it is known (build_layer, which
holds the generator) to where it is spent (HybridStaging, and the plain-MPI path
for S == 1). Its accessors are written so that a dense plan degenerates exactly:
`peer(k) == k` and `count == ranks`, so every loop walks all ranks in the old
order and the verbs take their collective path unchanged.
What the plan buys, in the order the campaign's evidence ranks them:
* the SERIAL sweeps shrink. pack_count_matrix, size_send's two passes,
fill_recv_col and the scatter are all O(R*S^2) and all run on partition 0
while the other S-1 park at a barrier -- work nothing overlaps. Restricted
to the f = plan.count(R) reachable ranks they become O(f*S^2), so R times
less at fanout 1, and narrowing them matters as much as the message count.
* the messages shrink. One MPI_Alltoall + one MPI_Alltoallv become f
Isend/Irecv pairs; a self peer (a generator whose rank shift is zero) is a
memcpy and costs no message at all.
`sparse_pairwise` in the new Pairwise.h is the one driver for all of it, and
Pairwise.h also holds every tag in one place, since one thread per rank calls
MPI and the tag is all that keeps a count round in flight from being matched by
a payload receive. The fused resolve now POSTS its count round in B1->B2 and
drains it in B3->B4, so every partition's pack runs underneath it; the dense arm
is blocking and completes on post regardless.
`derive_wire_bits` lets alltoallv narrow its own wire where the caller cannot:
only partition 0 reaches MPI, and its own row may be the empty one while a
sibling partition holds the rank's only traffic, so the peer set is read off the
recv rows every partition published before B1.
Ranks that all agree on a WRONG shift stay symmetric and never hang; they drop
the blocks outside the peer set in silence. pack_count_matrix asserts the
non-peer remainder is empty, and begin_alltoallv asserts the same of the blocks
a caller hands outside the window.
345/345 ctest, at 1, 2 and 4 ranks.
Assisted-by: ClaudeCode:claude-opus-5
The build path now routes GF(2)-linearly and exchanges over its one peer, but replay still posted MPI_Ialltoallv over all R ranks -- so propagate(), which Hubbard calls 29 times per build_graph, paid a full collective to move one peer's worth of doubles. post_flat_alltoallv takes a `wire_bits` gate and, when it is positive, posts Irecv/Isend pairs over the legs that carry a payload. No plan and no count round are needed: derive_layer_exchange already hands both sides the same array, so what a rank sends a peer IS that peer's recv count and both ends drop the same legs on the same value. sparse_pairwise drives it with a DENSE plan walking [0, R) and posting only the non-zero legs; the self slot stays a memcpy. Its request vector moves into the Ticket, which drains it in wait() the way the collective's single request is drained -- resized once and indexed, never push_back'ed, because MPI holds those pointers until the wait. The branch must be rank-uniform or it deadlocks, so it is NOT a predicate over a rank's own row: layer_exchange_wire_bits reads routing::linear_bits_for, which depends on the environment and the rank count alone, and PR #2's check_routing_agreement already allreduces that configuration at construction. Fanout 1 is both why the other legs are empty and why no rank can straddle. splitmix routing, or any non-power-of-two rank count, resolves to 0 bits and keeps the collective. Ticket::in_flight() reports what wait() still has to drain, which is what flat_exchange_tests uses to assert the transport rather than infer it. 351/351 ctest at 1, 2 and 4 ranks. Assisted-by: ClaudeCode:claude-opus-5
The transport was in no ledger at all. `d_wire_staging_bytes` reports what a rank's transport holds that never shrinks: HybridComm's grow-only payload staging plus its (R, S)-fixed count, displacement and offset tables, and ShmComm's publish slots. The payload buffers are sized to the widest message the run has needed and never shrunk, so on a run whose widest gate comes early they are resident for the whole of it with nothing resting able to name them. Per RANK, not per partition: one transport serves all S partitions, so only partition 0 reports it and the facade's sum over partitions counts it once. A pure-MPI rank reports 0, and that is the figure rather than a gap -- there the payload buffers belong to the in-flight exchange handle and die with the gate that opened the round. A diagnostic: outside total_bytes(), because it is the comm's memory and not the operator's, and accumulated by operator+= like the other d_ fields. Reads only; no allocation size or order moves. Assisted-by: ClaudeCode:claude-opus-5
The `### Rank routing` section derived the one-peer structure and stopped there. Two paragraphs now say what it is spent on -- the plan a gate carries, the Isend/Irecv pair that replaces the two collectives, the memcpy when the shift is zero, and the serial O(R S^2) staging sweeps that shrink with it, which under the hybrid is the larger win -- and why the transport choice cannot be a predicate on a rank's own traffic: rows vary, any threshold on one straddles, and a straddle is a deadlock. Also that a wrong-but-agreed shift drops blocks silently rather than hanging, which is what the debug asserts are for. `cpp/tests/README.md` names the new `flat_exchange_tests.cpp` and what the sparse hybrid cases cover; `AGENTS.md` records that the MPI CTest variants now run at 2 and 4 ranks, and why 2 alone is not enough. 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 |
github-actions
Bot
added
documentation
|
Docs preview: https://pr-352.monoprop-docs.pages.dev |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## perf/stack-2-router #352 +/- ##
====================================================
Coverage 97.70% 97.70%
====================================================
Files 14 14
Lines 742 742
Branches 98 98
====================================================
Hits 725 725
Misses 12 12
Partials 5 5
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
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
1 participant
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
Main's hybrid comm stages and exchanges a gate's payload densely across every rank in the
partition, regardless of how many of them actually hold reachable terms for the router's geometry.
This PR narrows the exchange to point-to-point over the peers the router can reach: a
PeerPlan(threaded through a new
HybridStaging.h) drives sparsealltoallvvariants and a dedicatedPairwise.hfor the two-rank case, so a gate's transport cost scales with its peer count instead ofthe partition size.
HybridComm.his split into a ~427-line comm header and the newHybridStaging.h(~550 lines) thatcarries payload staging; pair-exchange logic (~190 lines) stays out of scope here and is PR 7's.
monoprop_MPI_TEST_PROCSgains rank count 4 alongside 2 as this PR's headcommit, so CI exercises the multi-peer paths the sparse transport adds.
This is PR 3 of 7, based on
perf/stack-2-router. It is transport only: no value, index, orordering change. The next PR (
perf/stack-4-term-table) replaces main's hash index with apersistent term table, independent of this transport change.
Changes
Engine
cpp/monoprop/detail/mpi/Comm.h(new, 207 lines):WindowIndex,SlotWindow,PeerPlan.cpp/monoprop/detail/mpi/Pairwise.h(new, 116 lines): tags,PeerLayout,sparse_pairwise.cpp/monoprop/detail/mpi/HybridComm.h(636 → 427 lines): split;size_staging_send_/_recv_folded into
layout_peers_; the twofill_recv_col_from_*folded intofill_recv_col(value).cpp/monoprop/detail/mpi/HybridStaging.h(new, 550 lines): the staging half of the split, withthe
PeerPlanthreaded throughfill_peers/peers/zero_peer_slots_/derived_wire_plan/narrowing_is_lossless,post/wait/exchange_count_blocks, and the sparseexchange_payloadarm.
cpp/monoprop/detail/mpi/Exchange.h,Evolution.cpp: the replay path(
active_leg_count/Ticket/in_flight()/post_flat_alltoallv(wire_bits)).cpp/monoprop/detail/mpi/MPICompat.h/.cpp:begin_alltoallvgains a window + plan, and asparse
Kind::Mpiarm;alltoall_countsgains a sparse arm.cpp/monoprop/detail/mpi/ShmComm.h,Routing.h: small supporting additions.cpp/monoprop/detail/evolution/layer_build/Engine.h: aplanmember, derived inbuild_layer.cpp/monoprop/detail/operator/MPOperator.h:wire_staging_bytesledger field(
HybridComm::staging_bytes/ShmComm::staging_bytes), filled only atcomm_.shm_rank == 0(partition 0) to avoid double-counting.
Tests
cpp/tests/boost-test.cmake(head commit):monoprop_MPI_TEST_PROCS"2"→"2;4".cpp/tests/hybrid_comm_tests.cpp(+668 lines, 9 cases) andflat_exchange_tests.cpp(new, 230lines).
cpp/tests/routing_tests.cpp: threeSlotWindow/PeerPlanwindow cases.cpp/tests/README.md,tests/test_monoprop_smoke.py:d_wire_staging_byteskey.Docs
docs/content/docs/features/parallelism.mdx: point-to-point exchange and rank-uniform-gateparagraphs of "### Rank routing".
AGENTS.md: the MPI CTest-at-2-and-4 Notes bullet.Measurements
Gated multiset vs origin/main c5e88c8's raw bits (PR 2's routing already moved the row layout at
P > 1; this transport change adds no further move but inherits the requirement).gates.shrecord md5
cbbe395e5090b3f8ee442a76847bd343(rebased onto PR 2 at19bd012, tip812b1bc).3 interleaved reps, ratios only:
R=1)R=1)R=1)R=4×P=4, 250 M terms)The first three rungs run at
R=1, where this PR changes nothing observable — pauli's 1.028 isnoise (p = 0.25 on a rung this PR does not touch). At
R=4(L2b), where the sparse exchange shouldshow its effect, this PR is stated plainly: on one node the 4-rank cell is shared memory, so the
point-to-point exchange shows no separate time effect vs. its predecessor (0.951 vs. 0.957 — the
~5% gain over main at L2b is PR 1's store effect, carried through, not this PR's transport). The
ranks-summed peak is 0.998 vs. main, essentially unchanged from PR 2's 0.994: at 250 M terms the
store's own gain (PR 1) is eaten by pool and coefficient slack, exactly as PR 1's body states, and
this PR's own
wire_staging_bytescontribution (80.7 MiB) is small next to that. A time effect fromnarrowing the exchange to reachable peers needs more than one node to show, which the single-node
measurements here cannot provide.
Notes for reviewers
FusedScanResult's arrays, the engine'squeries_r/src_idx_r/src_val_r/combined_qv_, the probe'sgoff, the resolver'sresponses, andbegin_alltoallv's sweepsWindowVec-typed — andWindowVecis PR 7's. Without it, shorteningthose arrays with raw offsets would be exactly the unsafe re-basing
WindowVecexists to prevent,so this PR takes only the transport half:
PendingAlltoallvgainswindow, andbegin_alltoallvderives it from the plan and masks the caller's flat
[P]arrays through it. The per-slot arraysstay
[P]-sized here.monoprop_MPI_TEST_PROCS"2" → "2;4" (head commit) because atR=2a peer plan can onlyever resolve one peer, so every multi-peer path in the sparse transport goes untested; cross-rank
cases self-skip below two ranks, so the same test files still run in the serial variant. This
roughly doubles CI's MPI ctest time.
observed — 351/351 ctest including
mpiexec -n 2and-n 4, and--with-mpipytest atn=1/2/4under both default andsplitmixrouting.wire_staging_bytesis new (see Changes);indexing_bytesis untouched by this PR.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.