diff --git a/.gitignore b/.gitignore index e830d9ef..639bd723 100644 --- a/.gitignore +++ b/.gitignore @@ -176,3 +176,5 @@ benches/results/** # devcontainer files .devcontainer/devcontainer-lock.json +# local HPC job scripts, tools and measurement logs (see hpc/deucalion/README.md) +/hpc/ diff --git a/AGENTS.md b/AGENTS.md index a932c4ab..40f17a94 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -52,7 +52,12 @@ Key files: agree, or dispatch routes at a template the bindings never instantiated. - `CMakePresets.json`: the single source of truth for the supported C++ unit-test build/run entry points. The presets adopt the scikit-build-core trees generated by `uv sync`; regenerate the tree - with `uv sync`, then use the matching `skbuild-*` preset to build or run CTest. + with `uv sync`, then use the matching `skbuild-*` preset to build or run CTest. `skbuild-tsan` + adopts the `monoprop_ENABLE_TSAN=ON` (ThreadSanitizer) tree — see + `docs/content/docs/building.mdx`. That option is opt-in, composes with any build type, and is the + only machine check on the hand-written memory orderings in + `cpp/monoprop/detail/mpi/PartitionBarrier.h`; `cpp/tests/tsan.supp` suppresses third-party + reports only, never anything under `cpp/monoprop/`. - `benches/report.py` and `benches/bmf.py`: the two renderers of a benchmark run's artifacts — `REPORT.md` for humans, Bencher Metric Format JSON for the `bench_main.yml` continuous-benchmarking workflow. Benchmark names are Bencher's history key, so renaming or moving a `bench_*` test @@ -73,6 +78,21 @@ Key files: for the mutating/collecting paths, which run on the partitions' own pinned masters; `sum_partitions_`, `fold_partitions_`, `first_partition_` for reads off quiescent partitions) rather than hand-rolling a `run_on_all` loop — the declarations record which helper is legal where. +- **Placement must not divide an already-divided machine.** `enumerate_physical_cores` reports only cores + inside the calling thread's affinity mask, so when a launcher has given each co-located rank its own + disjoint slice (`srun --cpu-bind=cores`), the slice *is* the rank's share. Passing the node-wide + ranks-per-node through as `group_count` then asks for `group_count × n` cores out of a list that only + held `n`, `placement_order` correctly refuses, and every rank silently runs unpinned — which also costs + the two-level barrier its domains, because `cpuset_domains` derives them from the placement. Measured on + Deucalion at 8 ranks × 16 partitions: 437 µs/sync against 15.5 µs/sync placed. `PartitionGroup` therefore + allgathers the masks over its node-local communicator and `classify_node_mask` **measures** disjointness; + a `NodeMask::PerRank` result collapses `group_count` to 1. Mask *width* cannot substitute for this — "8 + ranks holding 16 cores each" and "8 ranks sharing one 16-core mask" both leave a rank seeing 16 of 128, + and they need opposite placement. Collapse in the wrong direction and every co-located rank pins to the + *same* cores, so `Shared` is the default and the safe error. This regressed once already, when topology + discovery was rewritten onto hwloc, because the guard lives in the placement policy rather than in + discovery: any rework of that layer must re-check it. `cpu_topology_policy_per_rank_slice_starves_without_collapse` + pins the mechanism without needing live hardware. ### Environment Management @@ -110,6 +130,29 @@ mp = MajoranaPropagator(operator, initial_state, cutoff=4) - Fixture msgpack schema is documented in `tests/data/README.md` - Tests validate against exact solutions for small systems - Heavy use of `@parametrize_with_cases` decorators +- **pytest's default capture is fd-level**, so it hides C++ diagnostics: it replaces fd 2 for each + test and discards the buffer when the test passes, while the engine writes straight to fd 2 (the + `COMMPROF` line from `monoprop_COMM_PROFILE` comes out of a transport destructor). Reading those + under pytest requires `-s`; the `just bench` recipes pass it for this reason. +- **Assert the count of what an instrument should emit, at the point of collection.** "The two arms + measured the same" and "the instrument never fired" are otherwise the same observation, and that + reads as a result rather than a failure. For the same reason, never diagnose by comparing two + zeros — get a positive control that is known to emit before concluding anything from silence. +- **A slow CTest run on an MPI build is `MPI_Init`, not slow tests.** CTest runs each Boost case as + its own process, so each pays a full `MPI_Init`, which initialises every fabric device present + whether or not the process will send a message — 8.8 s against 0.2 s of user CPU on a login node + with 8 HCAs. The tell is wall time with no CPU behind it. `monoprop_ENABLE_MPI` builds default to + `monoprop_TEST_EXCLUDE_MPI_FABRIC=ON`, which skips fabric init for the per-case tests (8.8 s → 1.9 s; + suite 34 min → 5.4). +- **That exclusion is scoped to the `serial` variants and must stay that way.** A per-case launch is + one process — world size 1, `*_World` cases skip themselves, everything else is `MPI_COMM_SELF` — so + no transport is used and the fabric can only cost startup time. The multi-rank variants exchange + real messages: `OMPI_MCA_pml=^ucx` makes a 2-rank run of the suite hang indefinitely on a case that + otherwise passes in 29 ms, on `main` as well, so it is component selection rather than engine code. + `discover_tests`' `SERIAL_ENVIRONMENT` argument exists for exactly this split. +- Use exclusions (`^…`), never a positive component list — naming a component that must exist breaks + on the next machine, since `vader` became `sm` in Open MPI 5 and `OMPI_MCA_btl=self,vader` there + silently reduces to `self` alone. ## Key Dependencies & Integration @@ -149,5 +192,15 @@ When changing behavior, APIs, build/test workflows, paths, or developer conventi - Check `build/*/compile_commands.json` for compilation flags - Use `rm -rf build` to clear environment-specific builds - Verify `monoprop_MAX_NUM_MODES` matches your use case (default: 250) +- **`uv sync` does not relink the C++ test binary.** It builds in a temporary directory and installs + only the wheel, and `bin/monoprop_unit_tests.x` is not a wheel target — so an edited test can leave + a stale binary that passes, or that reports `no test cases matching filter` for a case you just + wrote. Compare the binary's mtime against the source's before trusting either outcome. +- **The editable tree cannot be reconfigured in place**: its cache pins the build-isolation + interpreter scikit-build-core created and deleted, so `cmake --build build/editable/` fails + regenerating `build.ninja`, and the `skbuild-*` presets inherit that because they only adopt the + tree. To iterate on C++ tests, configure a standalone tree with an explicit `-Dnanobind_DIR=…` + (nanobind is a build-isolation-only dependency, absent from `.venv`) — recipe in + `docs/content/docs/building.mdx`. This is a sophisticated scientific computing project requiring careful attention to template instantiation, build system configuration, and the C++/Python boundary. diff --git a/CMakeLists.txt b/CMakeLists.txt index 2c4bd4f8..252ef6e6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -98,6 +98,7 @@ message( message(STATUS " User-appended : ${EXTRA_CXXFLAGS}") message(STATUS " MPI parallelization : ${monoprop_ENABLE_MPI}") +message(STATUS " ThreadSanitizer : ${monoprop_ENABLE_TSAN}") message(STATUS " Wide term index : ${monoprop_WIDE_TERM_INDEX}") message(STATUS " Max simulable modes : ${monoprop_MAX_NUM_MODES}") message(STATUS " C++ unit tests : ${monoprop_ENABLE_CXX_UNIT_TESTS}") diff --git a/CMakePresets.json b/CMakePresets.json index 115f246e..3f7d3b0f 100644 --- a/CMakePresets.json +++ b/CMakePresets.json @@ -14,6 +14,13 @@ "description": "Adopt the Debug build tree generated by 'uv sync --config-settings=cmake.build-type=Debug' (the 'Install in Debug mode' task). Do NOT reconfigure this preset from CMake Tools; regenerate the tree with uv/scikit-build-core, then use this preset only to build and run the C++ CTest suite.", "binaryDir": "${sourceDir}/build/editable/Debug", "generator": "Ninja" + }, + { + "name": "skbuild-tsan", + "displayName": "scikit-build-core (ThreadSanitizer, adopt existing tree)", + "description": "Adopt the ThreadSanitizer tree generated by 'uv sync --all-extras --reinstall-package monoprop --no-cache --config-settings-package=\"monoprop:cmake.build-type=RelWithDebInfo\" --config-settings-package=\"monoprop:cmake.define.monoprop_ENABLE_TSAN=ON\"'. RelWithDebInfo, not Debug: the partition threading layer is only worth auditing at the optimization level it ships with, and -g is added by monoprop_ENABLE_TSAN regardless. Do NOT reconfigure this preset from CMake Tools; regenerate the tree with uv/scikit-build-core, then use this preset only to build and run the C++ CTest suite.", + "binaryDir": "${sourceDir}/build/editable/RelWithDebInfo", + "generator": "Ninja" } ], "buildPresets": [ @@ -36,6 +43,16 @@ "monoprop_unit_tests.x" ], "jobs": 4 + }, + { + "name": "skbuild-tsan", + "displayName": "scikit-build-core (ThreadSanitizer) - Build C++ unit tests", + "description": "Build the C++ unit test executable in the ThreadSanitizer tree. Requires the tree to already exist (see the skbuild-tsan configure preset).", + "configurePreset": "skbuild-tsan", + "targets": [ + "monoprop_unit_tests.x" + ], + "jobs": 4 } ], "testPresets": [ @@ -58,6 +75,16 @@ "outputOnFailure": true }, "inheritConfigureEnvironment": true + }, + { + "name": "skbuild-tsan", + "displayName": "scikit-build-core (ThreadSanitizer) - Run C++ unit tests", + "description": "Run the C++ CTest suite under ThreadSanitizer. cpp/tests/CMakeLists.txt attaches TSAN_OPTIONS (including cpp/tests/tsan.supp) to every discovered test, so no extra environment is needed. Expect a large slowdown; a race makes the owning test exit non-zero.", + "configurePreset": "skbuild-tsan", + "output": { + "outputOnFailure": true + }, + "inheritConfigureEnvironment": true } ] } diff --git a/README.md b/README.md index 9bf340f0..a464b7f2 100644 --- a/README.md +++ b/README.md @@ -107,6 +107,10 @@ uv sync --all-extras -v ctest --test-dir build/editable/Release ``` +Note that `uv sync` builds in a temporary directory and installs only the wheel, so +it does **not** relink the test binary: after editing a C++ test, compare the +binary's mtime against the source's rather than trusting the result. + Full instructions — prerequisites, MPI options, and running the example executable — are in the [building guide](https://docs.monoprop.algorithmiq.tech/building). In particular, from-source builds require `hwloc` and `pkg-config` so CMake can @@ -120,6 +124,16 @@ just test-mpi # Python + C++ tests under MPI just test-wide # Python + C++ unit tests with a 64-bit TermIndex ``` +On an MPI build, CTest runs each case as its own process and so pays a full +`MPI_Init` per case — which initialises every fabric device present even though a +single-process test never sends a message. `monoprop_TEST_EXCLUDE_MPI_FABRIC` (on by +default) skips that for the per-case tests only, leaving the multi-rank variants on the +full component set; see the [testing guide](https://docs.monoprop.algorithmiq.tech/testing). + +To audit the partition threading layer for data races, build with +`cmake.define.monoprop_ENABLE_TSAN=ON` and run CTest against that tree; see the +[building guide](https://docs.monoprop.algorithmiq.tech/building). + See the [testing guide](https://docs.monoprop.algorithmiq.tech/testing) for the with/without-MPI details and the rank matrix. diff --git a/cmake/compiler_flags/CXXFlags.cmake b/cmake/compiler_flags/CXXFlags.cmake index 2fa94331..98d6578f 100644 --- a/cmake/compiler_flags/CXXFlags.cmake +++ b/cmake/compiler_flags/CXXFlags.cmake @@ -24,9 +24,14 @@ # list, *e.g.* to override previous compiler flags without touching the CMake # scripts. Default is empty. # +# Opt-in sanitizer flags are appended to ``CMAKE_CXX_FLAGS`` by +# ``Sanitizers.cmake``; see that file for why they do not go into +# ``monoprop_CXX_FLAGS``. +# # Variables used:: # # monoprop_ENABLE_ARCH_FLAGS +# monoprop_ENABLE_TSAN # EXTRA_CXXFLAGS # # Variables modified:: @@ -178,3 +183,5 @@ set(monoprop_CXX_FLAGS "") include(${CMAKE_CURRENT_LIST_DIR}/GNU.CXX.cmake) include(${CMAKE_CURRENT_LIST_DIR}/Intel.CXX.cmake) include(${CMAKE_CURRENT_LIST_DIR}/Clang.CXX.cmake) +# Last, so the sanitizer flags cannot be clobbered by a per-compiler `set()`. +include(${CMAKE_CURRENT_LIST_DIR}/Sanitizers.cmake) diff --git a/cmake/compiler_flags/Sanitizers.cmake b/cmake/compiler_flags/Sanitizers.cmake new file mode 100644 index 00000000..ad0b1777 --- /dev/null +++ b/cmake/compiler_flags/Sanitizers.cmake @@ -0,0 +1,54 @@ +#.rst: +# +# Opt-in sanitizer configuration. +# +# Sanitizers are wired in here rather than as a build type because they are +# orthogonal to optimization level: the partition threading layer is only worth +# auditing at the optimization level it ships with, so +# ``monoprop_ENABLE_TSAN=ON`` composes with any ``CMAKE_BUILD_TYPE`` instead of +# replacing it. +# +# The flags are appended to ``CMAKE_CXX_FLAGS`` (not to ``monoprop_CXX_FLAGS``, +# which reaches ``target_compile_options`` only) because ``-fsanitize=thread`` +# must appear on the *link* line as well as every compile line; CMake passes +# ``CMAKE_CXX_FLAGS`` to both. The explicit linker-flag appends below cover +# link steps that a toolchain file may drive without ``CMAKE_CXX_FLAGS``. +# +# Variables used:: +# +# monoprop_ENABLE_TSAN +# +# Variables modified:: +# +# CMAKE_CXX_FLAGS +# CMAKE_EXE_LINKER_FLAGS +# CMAKE_SHARED_LINKER_FLAGS +# CMAKE_MODULE_LINKER_FLAGS + +option( + monoprop_ENABLE_TSAN + "Build with ThreadSanitizer (-fsanitize=thread); opt-in, never on by default" + OFF +) + +if(monoprop_ENABLE_TSAN) + if(NOT CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang") + message( + FATAL_ERROR + "monoprop_ENABLE_TSAN requires GCC or Clang. Detected: ${CMAKE_CXX_COMPILER_ID}" + ) + endif() + + # -g is forced even in Release: without line tables a TSan report names only + # addresses, and a report you cannot attribute to a memory ordering is not an + # audit. -fno-omit-frame-pointer is already in monoprop_CXX_FLAGS, but repeat + # it here so the setting survives an EXTRA_CXXFLAGS override of that list. + set( + monoprop_TSAN_FLAGS + "-fsanitize=thread -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -g" + ) + string(APPEND CMAKE_CXX_FLAGS " ${monoprop_TSAN_FLAGS}") + string(APPEND CMAKE_EXE_LINKER_FLAGS " -fsanitize=thread") + string(APPEND CMAKE_SHARED_LINKER_FLAGS " -fsanitize=thread") + string(APPEND CMAKE_MODULE_LINKER_FLAGS " -fsanitize=thread") +endif() diff --git a/cpp/monoprop/detail/EnvConfig.h b/cpp/monoprop/detail/EnvConfig.h index 5fae134a..11766581 100644 --- a/cpp/monoprop/detail/EnvConfig.h +++ b/cpp/monoprop/detail/EnvConfig.h @@ -24,6 +24,14 @@ // monoprop_NUM_THREADS positive int (1..1e6), else ignored → num_threads // monoprop_PARTITION_PINNING bool, default ON; 0/false disables per-core pinning → partition_pinning // monoprop_PARTITIONS int N | "auto" | "off"; parsed where it is used (resolve_partition_count_) +// monoprop_COMM_PROFILE bool, default OFF; per-partition collective accounting to stderr → comm_profile +// monoprop_SPIN_BUDGET_US positive int us, default kDefaultSpinBudgetUs; barrier on-core spin before +// yielding. Exists to be swept against a real workload → spin_budget_us +// monoprop_BARRIER_GROUPING bool, default ON; 0/false forces the flat barrier while LEAVING PINNING ON +// → barrier_grouping. Without this the two-level barrier cannot be measured: +// its domains come from the cpusets, so turning pinning off to get a flat +// barrier also unpins, and every before/after confounds the two. Exists so +// "grouped vs flat, both pinned" is a run rather than an argument. namespace monoprop::config { @@ -57,6 +65,9 @@ inline auto parse_positive_int(const char *text) -> std::optional { struct Settings { std::optional num_threads; bool partition_pinning = true; + bool comm_profile = false; + std::optional spin_budget_us; // nullopt ⇒ the barrier's own default + bool barrier_grouping = true; }; // Parse the environment once; the Settings are cached and shared across TUs. @@ -65,6 +76,9 @@ inline auto get() -> const Settings & { Settings s; s.num_threads = detail::parse_positive_int(std::getenv("monoprop_NUM_THREADS")); s.partition_pinning = detail::parse_flag(std::getenv("monoprop_PARTITION_PINNING"), true); + s.comm_profile = detail::parse_flag(std::getenv("monoprop_COMM_PROFILE"), false); + s.spin_budget_us = detail::parse_positive_int(std::getenv("monoprop_SPIN_BUDGET_US")); + s.barrier_grouping = detail::parse_flag(std::getenv("monoprop_BARRIER_GROUPING"), true); return s; }(); return settings; diff --git a/cpp/monoprop/detail/evolution/layer_build/Engine.h b/cpp/monoprop/detail/evolution/layer_build/Engine.h index c48e2c63..fe60544d 100644 --- a/cpp/monoprop/detail/evolution/layer_build/Engine.h +++ b/cpp/monoprop/detail/evolution/layer_build/Engine.h @@ -382,7 +382,16 @@ struct LayerBuildEngine { auto resp = resolve_incoming(inc_q, local_op, R, is_leader_pass, matched, combined_size, sink); std::vector resp_recv = response_recv_counts(); std::vector> inc_r; - mpi::begin_alltoallv(resp, comm, /*skip_self=*/false, &resp_recv).wait_into(inc_r); + // The answers travel the query exchange's legs backwards, one per query, so the hybrid transport + // reuses that exchange's offset tables; nothing may collectively intervene between the two calls. + // Sink::kStride is the query leg's words per query, the ratio between the two legs' counts. + mpi::begin_alltoallv(resp, + comm, + /*skip_self=*/false, + &resp_recv, + /*reverse_of_previous=*/true, + /*forward_stride=*/static_cast(Sink::kStride)) + .wait_into(inc_r); process_responses(inc_r, src_idx_r, queries_r, R, my_rank, sink); } diff --git a/cpp/monoprop/detail/mpi/CMakeLists.txt b/cpp/monoprop/detail/mpi/CMakeLists.txt index c8226a41..650b43ad 100644 --- a/cpp/monoprop/detail/mpi/CMakeLists.txt +++ b/cpp/monoprop/detail/mpi/CMakeLists.txt @@ -6,6 +6,7 @@ target_sources( FILES "CheckedCount.h" "Comm.h" + "CommProfile.h" "CpuRelax.h" "Exchange.h" "HybridComm.h" diff --git a/cpp/monoprop/detail/mpi/CommProfile.h b/cpp/monoprop/detail/mpi/CommProfile.h new file mode 100644 index 00000000..cfd391d9 --- /dev/null +++ b/cpp/monoprop/detail/mpi/CommProfile.h @@ -0,0 +1,141 @@ +// Copyright 2026 Algorithmiq +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#pragma once + +#include +#include +#include +#include +#include +#include + +// Opt-in accounting for where a partitioned collective's wall time goes (monoprop_COMM_PROFILE=1). Off by +// default and never allocated then, so the hot path pays one null check per instrumented region. +// +// The split that matters is table_p0 vs table_par: the offset/count tables a HybridComm collective +// rebuilds are O(R*S^2), and whether one partition fills them alone (table_p0, the other S-1 parked in a +// barrier) or every partition fills its own slice (table_par) is the difference between a serial and a +// parallel protocol. Barrier wait is attributed per partition to make that asymmetry visible. table_move +// separates payload memcpy, which a better protocol does not shrink, from bookkeeping, which it does. + +namespace monoprop::mpi { + +class CommProfile { +public: + using Clock = std::chrono::steady_clock; + + // One cache-line-isolated accumulator per partition: every counter is written only by its own + // partition thread, so profiling adds no coherence traffic of its own to what it measures. + struct alignas(64) Slot { + uint64_t barrier_ns = 0; + uint64_t table_p0_ns = 0; // fills executed under `if (local_partition == 0)` + uint64_t table_par_ns = 0; // fills every partition runs on its own slice + uint64_t table_move_ns = 0; // payload memcpy (pack into / scatter out of staging), not bookkeeping + uint64_t mpi_ns = 0; // time inside MPI itself + uint64_t n_barriers = 0; + uint64_t n_verbs = 0; + }; + + explicit CommProfile(int n_partitions, int mpi_rank) + : slots_(static_cast(n_partitions)), + mpi_rank_(mpi_rank) {} + + // Locality domains the transport's barrier grouped its partitions into (< 2 ⇒ the flat barrier ran). + int barrier_groups = 0; + + // How many partitions actually got pinned, out of `partitions`. Reported because barrier_groups = 0 has + // two legitimate causes that are otherwise indistinguishable from outside the process: nothing was + // pinned, or every partition landed in one locality domain and so has nothing to fan in across. + // -1 means the transport was never told (no PartitionGroup owns it, e.g. a bare-transport unit test). + int pinned = -1; + + auto slot(int partition) -> Slot & { return slots_[static_cast(partition)]; } + + // Aggregate over partitions, plus partition 0 broken out. Written to stderr at teardown, one + // block per MPI rank; a driver greps `COMMPROF` and sums/compares across ranks. + // + // noexcept because the only caller is a transport destructor, which may run while an exception + // unwinds: a diagnostic print that fails must be dropped, never escalated to a terminate. + auto dump() const noexcept -> void { + try { + dump_(); + } + catch (...) { // std::print can throw (formatting, or a write error on stderr) + } + } + +private: + auto dump_() const -> void { + Slot total; + for (const Slot &s : slots_) { + total.barrier_ns += s.barrier_ns; + total.table_p0_ns += s.table_p0_ns; + total.table_par_ns += s.table_par_ns; + total.table_move_ns += s.table_move_ns; + total.mpi_ns += s.mpi_ns; + total.n_barriers += s.n_barriers; + total.n_verbs += s.n_verbs; + } + const Slot &p0 = slots_.front(); + // Peer barrier wait is the cost of the master's serial phases seen from the other side. + const uint64_t peer_barrier_ns = total.barrier_ns - p0.barrier_ns; + std::print(stderr, + "COMMPROF rank={} partitions={} barrier_groups={} pinned={} verbs={} barriers={} " + "table_p0_s={:.3f} table_par_s={:.3f} table_move_s={:.3f} mpi_s={:.3f} " + "barrier_p0_s={:.3f} barrier_peers_s={:.3f} barrier_per_sync_us={:.2f}\n", + mpi_rank_, + slots_.size(), + barrier_groups, + pinned, + p0.n_verbs, + p0.n_barriers, + to_s(p0.table_p0_ns), + to_s(total.table_par_ns) / static_cast(slots_.size()), + to_s(total.table_move_ns) / static_cast(slots_.size()), + to_s(p0.mpi_ns), + to_s(p0.barrier_ns), + to_s(peer_barrier_ns) / static_cast(slots_.size() > 1 ? slots_.size() - 1 : 1), + total.n_barriers == 0 + ? 0.0 + : (static_cast(total.barrier_ns) / static_cast(total.n_barriers)) / 1000.0); + std::fflush(stderr); + } + + static auto to_s(uint64_t ns) -> double { return static_cast(ns) / 1e9; } + + std::vector slots_; + int mpi_rank_; +}; + +// Adds the scope's duration to one counter. Held by value in the instrumented region; `target` must +// outlive it (it is a field of the comm's own CommProfile). +class ScopedNs { +public: + explicit ScopedNs(uint64_t *target) : target_(target), start_(CommProfile::Clock::now()) {} + ScopedNs(const ScopedNs &) = delete; + auto operator=(const ScopedNs &) -> ScopedNs & = delete; + ~ScopedNs() { + if (target_ != nullptr) { + *target_ += static_cast( + std::chrono::duration_cast(CommProfile::Clock::now() - start_).count()); + } + } + +private: + uint64_t *target_; + CommProfile::Clock::time_point start_; +}; + +} // namespace monoprop::mpi diff --git a/cpp/monoprop/detail/mpi/CpuRelax.h b/cpp/monoprop/detail/mpi/CpuRelax.h index 42ec2fd0..ce6dd8a6 100644 --- a/cpp/monoprop/detail/mpi/CpuRelax.h +++ b/cpp/monoprop/detail/mpi/CpuRelax.h @@ -35,8 +35,38 @@ inline auto cpu_relax() noexcept -> void { #endif } -// cpu_relax() iterations a barrier spinner burns before donating its timeslice. PAUSE is ~140 cycles on -// Sapphire Rapids ⇒ 2048 iters ≈ 0.1 ms, past a balanced exchange's arrival gaps; longer waits yield. -inline constexpr int kSpinPauseIters = 2048; +// How long a barrier spinner stays on-core before it starts yielding, in TIME rather than in cpu_relax() +// iterations. An iteration count does not carry across architectures: cpu_relax() is one PAUSE on x86 and +// one YIELD on aarch64, and those differ by more than an order of magnitude, so a count calibrated on one +// spends a wholly different budget on the other. On aarch64 the previous 2048-iteration budget came to +// roughly a microsecond -- the on-core spin was effectively skipped and every wait fell to sched_yield. +// +// 30 us is that old budget's equivalent on this Zen 2 part, measured rather than derived: a barrier stress +// at S=32 over 4 and 8 CPUs reproduces the old wall time at 30 us (0.130 s vs 0.153 s, 0.136 s vs 0.228 s) +// and drifts off it by 60 us. PAUSE here is therefore ~30 cycles, not the ~140 of Sapphire Rapids the old +// constant's comment was reasoning about. +// +// Which means one honest caveat: a single time budget cannot match the old spend on every x86 part, +// because the old spend itself varied with PAUSE latency -- that variance was the bug. On a part where +// PAUSE really is ~140 cycles the old constant bought ~100 us, so 30 us is a genuine shortening there, not +// a like-for-like port. It is calibrated where it was measured and where the benchmarks run; elsewhere +// monoprop_SPIN_BUDGET_US is the adjustment, and a part with a long PAUSE is the first place to reach for +// it. +// +// Retuning is a separate question, deliberately left to the cluster A/B: the same sweep has shorter +// budgets winning by 5-10x once partitions outnumber cores (10 us gives 0.028 s and 0.022 s), because a +// spinner holds the core its late peer needs to arrive on. Under the pinned one-partition-per-core layout +// the design targets, the trade runs the other way and staying on-core is the point -- +// monoprop_SPIN_BUDGET_US exists so that can be swept on real workloads without a rebuild. +// +// Parking (sleep_for) past a second, longer budget was tried and rejected: at S=32 on 4 CPUs one +// sleeper's timer overshoot pushed its peers past their own budgets, and the cascade cost 2021 us/sync +// against 762 for plain yielding. A yielder is runnable the moment a core frees; a sleeper cannot answer +// before its timer expires. +inline constexpr int kDefaultSpinBudgetUs = 30; + +// cpu_relax() calls between deadline checks. steady_clock::now() costs ~20 ns, which would dominate the +// pause it is meant to pace, so the check is amortized over a batch. +inline constexpr int kRelaxPerClockCheck = 64; } // namespace monoprop::mpi::detail diff --git a/cpp/monoprop/detail/mpi/HybridComm.h b/cpp/monoprop/detail/mpi/HybridComm.h index 394f9fa5..1845b458 100644 --- a/cpp/monoprop/detail/mpi/HybridComm.h +++ b/cpp/monoprop/detail/mpi/HybridComm.h @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -29,8 +30,10 @@ #include +#include "monoprop/detail/EnvConfig.h" #include "monoprop/detail/mpi/CheckedCount.h" #include "monoprop/detail/mpi/Comm.h" +#include "monoprop/detail/mpi/CommProfile.h" #include "monoprop/detail/mpi/PartitionBarrier.h" // Composes R MPI ranks x S in-process partitions into one flat P=R*S SPMD world. Global id is rank-major @@ -49,11 +52,13 @@ class MpiThreadLevelUnsupported : public std::runtime_error { class HybridComm { public: // n_local_partitions = S, identical on every rank (the facade ctor checks that before constructing). - HybridComm(MPI_Comm parent, int n_local_partitions) + // partition_l3_domains (optional, one entry per partition) makes the barrier two-level; see + // PartitionBarrier. + HybridComm(MPI_Comm parent, int n_local_partitions, const std::vector &partition_l3_domains = {}) : parent_(parent), s_(n_local_partitions), slots_(static_cast(n_local_partitions)), - barrier_(n_local_partitions) { + barrier_(n_local_partitions, partition_l3_domains) { MPI_Comm_size(parent_, &r_); MPI_Comm_rank(parent_, &mpi_rank_); int provided = MPI_THREAD_SINGLE; @@ -73,104 +78,165 @@ class HybridComm { mpi_recv_displs_.resize(static_cast(r_)); pack_off_.resize(rss); scatter_off_.resize(rss); + const size_t rs = static_cast(r_) * static_cast(s_); + col_send_.resize(rs); + col_recv_.resize(rs); + tpre_send_.resize(rs); + tpre_recv_.resize(rs); + rev_send_counts_.resize(static_cast(r_)); + rev_send_displs_.resize(static_cast(r_)); + rev_recv_counts_.resize(static_cast(r_)); + rev_recv_displs_.resize(static_cast(r_)); + mask_words_ = pad_to_line_(static_cast((r_ + 63) / 64)); + send_mask_.assign(static_cast(s_) * mask_words_, 0); + run_stride_ = pad_to_line_(static_cast(r_)); + run_scratch_.assign(static_cast(s_) * run_stride_, 0); + if (config::get().comm_profile) { + prof_ = std::make_unique(s_, mpi_rank_); + prof_->barrier_groups = barrier_.group_count(); + } } HybridComm(const HybridComm &) = delete; auto operator=(const HybridComm &) -> HybridComm & = delete; + // Profiling is opt-in, so the common case destroys nothing and prints nothing. + ~HybridComm() { + if (prof_ != nullptr) { + prof_->dump(); + } + } + auto size() const -> int { return r_ * s_; } auto global_rank(int local_partition) const -> int { return mpi_rank_ * s_ + local_partition; } + // Once partition 0 -- this rank's only participant on parent_ -- is inside a collective, the peer ranks + // are committed: their partition-0 threads enter theirs and block inside MPI with no timeout. So a + // rank-local failure here must MPI_Abort with the underlying error rather than throw, which would + // hang the job. Single-rank partitioned runs use ShmComm, not HybridComm, and keep their exceptions. + template + auto guard_partition0_(int local_partition, const char *verb, Body &&body) -> decltype(body()) { + if (local_partition != 0) { + return body(); + } + try { + return body(); + } + catch (const std::exception &e) { + abort_rank_(verb, e.what()); + } + catch (...) { + abort_rank_(verb, "unknown error"); + } + } + auto alltoall_counts(int local_partition, const int *send_counts /*[P]*/, int *recv_counts /*[P]*/) -> void { - guard_partition0_(local_partition, "alltoall_counts", [this, local_partition, send_counts, recv_counts] { + guard_partition0_(local_partition, "alltoall_counts", [&] { alltoall_counts_impl_(local_partition, send_counts, recv_counts); }); } // See AlltoallvArgs for the send-buffer lifetime and the element-vs-byte convention; `dt` is the MPI // datatype whose extent is args.elem, and it stays a separate argument because the bundle is shared - // with the non-MPI-capable transport. + // with the non-MPI-capable transport. The bundle is unpacked here rather than threaded through the + // implementation, which keeps the per-argument signature the barrier reasoning is written against. auto alltoallv(int local_partition, const AlltoallvArgs &args, MPI_Datatype dt) -> void { - guard_partition0_(local_partition, "alltoallv", [this, local_partition, &args, dt] { - alltoallv_impl_(local_partition, args, dt); + guard_partition0_(local_partition, "alltoallv", [&] { + alltoallv_impl_(local_partition, + args.send, + args.send_counts, + args.send_displs, + args.recv, + args.recv_counts, + args.recv_displs, + args.elem, + dt); }); } - // See AlltoallvResolveArgs: the recv side is an output, and args.recv is resized here. + // See AlltoallvResolveArgs: the recv side is an output and args.recv is resized here. Element bytes + // are sizeof(T) by construction, so unlike alltoallv they are derived rather than carried. template auto alltoallv_resolve(int local_partition, const AlltoallvResolveArgs &args, MPI_Datatype dt) -> void { // `args` by reference, not by value: the impl resizes args.recv and then writes through it. - guard_partition0_(local_partition, "alltoallv_resolve", [this, local_partition, &args, dt] { - alltoallv_resolve_impl_(local_partition, args, dt); + guard_partition0_(local_partition, "alltoallv_resolve", [&] { + alltoallv_resolve_impl_(local_partition, + args.send, + args.send_counts, + args.send_displs, + args.recv, + args.recv_counts, + args.recv_displs, + sizeof(T), + dt); + }); + } + + // Payload-only reverse of the immediately preceding alltoallv_resolve: same legs, opposite + // direction, one element back per element received. See alltoallv_reverse_impl_ for the contract. + auto alltoallv_reverse(int local_partition, + const void *send, + const int *send_counts /*[P]*/, + const int *send_displs /*[P]*/, + void *recv, + const int *recv_counts /*[P]*/, + const int *recv_displs /*[P]*/, + size_t elem, + MPI_Datatype dt, + int forward_stride) -> void { + guard_partition0_(local_partition, "alltoallv_reverse", [&] { + alltoallv_reverse_impl_(local_partition, + send, + send_counts, + send_displs, + recv, + recv_counts, + recv_displs, + elem, + dt, + forward_stride); }); } template auto allreduce_sum(int local_partition, T local_val) -> T { - return guard_partition0_(local_partition, "allreduce_sum", [this, local_partition, local_val] { + return guard_partition0_(local_partition, "allreduce_sum", [&] { return allreduce_sum_impl_(local_partition, local_val); }); } auto allreduce_sum_inplace(int local_partition, double *values, size_t len) -> void { - guard_partition0_(local_partition, "allreduce_sum_inplace", [this, local_partition, values, len] { + guard_partition0_(local_partition, "allreduce_sum_inplace", [&] { allreduce_sum_inplace_impl_(local_partition, values, len); }); } - auto poison() -> void { barrier_.poison(); } - auto reset() -> void { barrier_.reset(); } - -private: - struct alignas(64) Slot { - const std::byte *ptr = nullptr; // byte view of this partition's send buffer; null until published - const int *counts = nullptr; - const int *send_counts = nullptr; - const int *send_displs = nullptr; - const int *recv_counts = nullptr; - const double *vec = nullptr; - double f64 = 0.0; - uint64_t u64 = 0; - }; - - // Once partition 0 -- this rank's only participant on parent_ -- is inside a collective, the peer ranks - // are committed: their partition-0 threads enter theirs and block inside MPI with no timeout. So a - // rank-local failure here must MPI_Abort with the underlying error rather than throw, which would - // hang the job. Single-rank partitioned runs use ShmComm, not HybridComm, and keep their exceptions. - // `body` is invoked synchronously and never stored, so the callers' lambdas may capture by reference. - template - auto guard_partition0_(int local_partition, const char *verb, Body &&body) -> decltype(body()) { - if (local_partition != 0) { - return body(); - } - try { - return body(); - } - catch (const std::exception &e) { - abort_rank_(verb, e.what()); + // recv_counts[g] = amount global partition g sends to this partition. 2 barriers + one S*S-int + // MPI_Alltoall; the count matrix is filled row-per-partition before the first barrier. + auto alltoall_counts_impl_(int local_partition, const int *send_counts /*[P]*/, int *recv_counts /*[P]*/) -> void { + count_verb(local_partition); + if (local_partition == 0) { + ++tables_gen_; } - catch (...) { - abort_rank_(verb, "unknown error"); + // Each partition writes its OWN source row of the count matrix before the barrier that publishes + // it; the rows are disjoint, so this costs no extra barrier. Every element is written here and + // MPI_Alltoall fills counts_recv_ fully, so neither buffer is pre-zeroed. + { + ScopedNs timer{par_table_ns(local_partition)}; + fill_count_row_(local_partition, send_counts); } - } - - // recv_counts[g] = amount global partition g sends to this partition. 2 barriers + one S*S-int MPI_Alltoall. - auto alltoall_counts_impl_(int local_partition, const int *send_counts /*[P]*/, int *recv_counts /*[P]*/) -> void { - const size_t u = static_cast(local_partition); - slots_[u].counts = send_counts; - sync(); + sync(local_partition); if (local_partition == 0) { - pack_count_matrix_(&Slot::counts); + ScopedNs timer{mpi_ns(local_partition)}; MPI_Alltoall(counts_send_.data(), s_ * s_, MPI_INT, counts_recv_.data(), s_ * s_, MPI_INT, parent_); } - sync(); - // Partition t extracts its row: recv from (rank a, partition su) is contiguous per source rank a. + sync(local_partition); + // Partition t extracts its column: what (rank a, partition su) sends to it. const int t = local_partition; + ScopedNs timer{par_table_ns(local_partition)}; for (int a = 0; a < r_; ++a) { for (int su = 0; su < s_; ++su) { - const size_t idx = (static_cast(a) * static_cast(s_) * static_cast(s_)) - + (static_cast(t) * static_cast(s_)) + static_cast(su); - recv_counts[a * s_ + su] = counts_recv_[idx]; + recv_counts[a * s_ + su] = counts_recv_[cnt_idx_(a, su, t)]; } } // No trailing barrier: past the last sync only counts_recv_ is read, and partition 0 cannot rewrite @@ -178,28 +244,48 @@ class HybridComm { // send_counts was consumed before the last sync, so peers may free/reuse it on return. } - // Flat variable all-to-all over caller-owned buffers; see AlltoallvArgs for the conventions. - auto alltoallv_impl_(int local_partition, const AlltoallvArgs &args, MPI_Datatype dt) -> void { + // Flat variable all-to-all over caller-owned buffers (counts/displs in elements, `elem` = element bytes). + // recv_counts must already hold the transpose — same contract as MPI_Alltoallv. + auto alltoallv_impl_(int local_partition, + const void *send, + const int *send_counts /*[P]*/, + const int *send_displs /*[P]*/, + void *recv, + const int *recv_counts /*[P]*/, + const int *recv_displs /*[P]*/, + size_t elem, + MPI_Datatype dt) -> void { const size_t u = static_cast(local_partition); - Slot &me = slots_[u]; - me.ptr = args.send; - me.send_counts = args.send_counts; - me.send_displs = args.send_displs; - me.recv_counts = args.recv_counts; - sync(); // B1 - - // B2: partition 0 sizes/reallocates staging; must finish before any partition packs into stage_send_. + count_verb(local_partition); if (local_partition == 0) { - size_staging_(args.elem); + ++tables_gen_; } - sync(); // B2 + Slot &me = slots_[u]; + me.ptr = send; + me.send_counts = send_counts; + me.send_displs = send_displs; + me.recv_counts = recv_counts; + // No count matrix on this path, so the veto row phase A reads is filled on its own. + { + ScopedNs timer{par_table_ns(local_partition)}; + fill_send_mask_(local_partition, send_counts); + } + sync(local_partition); // B1 - // B3: each partition packs its own cross-rank blocks into stage_send_ (disjoint writes). - pack_send_(local_partition, args.elem); - sync(); // B3 + // Sizing is S-way parallel and carries its own two barriers (see size_staging_parallel_); on + // return every base is visible and staging is grown, so packing into stage_send_ is safe. + size_staging_parallel_(local_partition, elem); - // B4: partition 0 runs the single MPI_Alltoallv while peers park at the barrier. + // Each partition packs its own cross-rank blocks into stage_send_ (disjoint writes). + { + ScopedNs timer{move_ns(local_partition)}; + pack_send_(local_partition, elem); + } + sync(local_partition); // B3 + + // Partition 0 runs the single MPI_Alltoallv while peers park at the barrier. if (local_partition == 0) { + ScopedNs timer{mpi_ns(local_partition)}; MPI_Alltoallv(stage_send_.data(), mpi_send_counts_.data(), mpi_send_displs_.data(), @@ -210,73 +296,93 @@ class HybridComm { dt, parent_); } - sync(); // B4 + sync(local_partition); // B4 // Scatter each global source's contiguous run from stage_recv_ to recv_displs[g] (all legs, incl. - // self-rank, go through staging). Block starts come from scatter_off_: no peer slot is read past B4. - std::byte *dst = args.recv; + // self-rank, go through staging). Block starts come from the offset tables, so no peer slot is + // read after the last barrier. + ScopedNs timer{move_ns(local_partition)}; + char *dst = static_cast(recv); const int t = local_partition; for (int a = 0; a < r_; ++a) { for (int su = 0; su < s_; ++su) { const int g = a * s_ + su; - const int cnt = args.recv_counts[g]; + const int cnt = recv_counts[g]; if (cnt != 0) { - std::memcpy(dst + static_cast(args.recv_displs[g]) * args.elem, - stage_recv_.data() + scatter_off_[block_idx_(a, t, su)] * args.elem, - static_cast(cnt) * args.elem); + std::memcpy(dst + static_cast(recv_displs[g]) * elem, + stage_recv_.data() + recv_block_off_(a, t, su) * elem, + static_cast(cnt) * elem); } } } - // No trailing barrier (see alltoall_counts_impl_): past B4 only stage_recv_/scatter_off_/own buffers. + // No trailing barrier (see alltoall_counts_impl_): past B4 only stage_recv_/offset tables/own buffers. } // Fused count-resolve + payload alltoallv: folds the standalone count exchange into this verb's // B1→B2 window (4 syncs instead of 6). recv_counts / recv_displs and `recv` (resized) are outputs. // Bit-identical to alltoall_counts + alltoallv. template - auto alltoallv_resolve_impl_(int local_partition, const AlltoallvResolveArgs &args, MPI_Datatype dt) -> void { - // Typed verb: element bytes are sizeof(T) by construction, so they are derived rather than passed. - constexpr size_t elem = sizeof(T); + auto alltoallv_resolve_impl_(int local_partition, + const T *send, + const int *send_counts /*[P]*/, + const int *send_displs /*[P]*/, + std::vector &recv, + int *recv_counts /*[P]*/, + int *recv_displs /*[P]*/, + size_t elem, + MPI_Datatype dt) -> void { const size_t u = static_cast(local_partition); + count_verb(local_partition); + if (local_partition == 0) { + ++tables_gen_; + } Slot &me = slots_[u]; - // Typed here but byte-addressed in the slot: pack_send_ copies by (displ, count) in elements and - // never reconstructs T, so the slot stays type-erased for the untyped alltoallv_impl_ above. - me.ptr = reinterpret_cast(args.send); - me.send_counts = args.send_counts; - me.send_displs = args.send_displs; - // recv_counts is an output here — deliberately not published; the count Alltoall resolves it. - sync(); // B1 + me.ptr = send; + me.send_counts = send_counts; + me.send_displs = send_displs; + // recv_counts is an output here — deliberately not published; the count Alltoall resolves it. The + // count row comes from the local argument, so it rides the same barrier as the slot publish. + { + ScopedNs timer{par_table_ns(local_partition)}; + fill_count_row_(local_partition, send_counts); + } + sync(local_partition); // B1: slots and the whole send-count matrix published if (local_partition == 0) { - pack_count_matrix_(&Slot::send_counts); + ScopedNs timer{mpi_ns(local_partition)}; MPI_Alltoall(counts_send_.data(), s_ * s_, MPI_INT, counts_recv_.data(), s_ * s_, MPI_INT, parent_); - // Size staging from counts_recv_: recv of partition t from (rank, su) sits at rank*S*S + t*S + su. - size_staging_impl_(elem, [this](int t, int rank, int su) { - return counts_recv_[(static_cast(rank) * static_cast(s_) * static_cast(s_)) - + (static_cast(t) * static_cast(s_)) + static_cast(su)]; - }); } - sync(); // B2 + sync(local_partition); // B2: counts_recv_ visible to every partition + + // Size staging from counts_recv_: what partition t gets from (rank, su) is at cnt_idx_(rank, su, t). + // Carries its own two barriers and its own timers — do not wrap it in one. + size_staging_parallel_(local_partition, elem, [this](int t, int rank, int su) { + return counts_recv_[cnt_idx_(rank, su, t)]; + }); const int t = local_partition; - long long total = 0; - for (int a = 0; a < r_; ++a) { - for (int su = 0; su < s_; ++su) { - const int g = a * s_ + su; - const int c = - counts_recv_[(static_cast(a) * static_cast(s_) * static_cast(s_)) - + (static_cast(t) * static_cast(s_)) + static_cast(su)]; - args.recv_counts[g] = c; - args.recv_displs[g] = checked_mpi_count(total, "Recv displacement"); - total += c; + { + ScopedNs timer{par_table_ns(local_partition)}; + long long total = 0; + for (int a = 0; a < r_; ++a) { + for (int su = 0; su < s_; ++su) { + const int g = a * s_ + su; + const int c = counts_recv_[cnt_idx_(a, su, t)]; + recv_counts[g] = c; + recv_displs[g] = checked_mpi_count(total); + total += c; + } } + recv.resize(static_cast(checked_mpi_count(total))); } - args.recv.resize(static_cast(checked_mpi_count(total, "Total recv count"))); - - pack_send_(local_partition, elem); - sync(); // B3 + { + ScopedNs timer{move_ns(local_partition)}; + pack_send_(local_partition, elem); + } + sync(local_partition); // B3 if (local_partition == 0) { + ScopedNs timer{mpi_ns(local_partition)}; MPI_Alltoallv(stage_send_.data(), mpi_send_counts_.data(), mpi_send_displs_.data(), @@ -287,25 +393,139 @@ class HybridComm { dt, parent_); } - sync(); // B4 + sync(local_partition); // B4 - std::byte *dst = reinterpret_cast(args.recv.data()); // after the resize: it may reallocate + ScopedNs scatter_timer{move_ns(local_partition)}; + char *dst = reinterpret_cast(recv.data()); for (int a = 0; a < r_; ++a) { for (int su = 0; su < s_; ++su) { const int g = a * s_ + su; - const int cnt = args.recv_counts[g]; + const int cnt = recv_counts[g]; if (cnt != 0) { - std::memcpy(dst + static_cast(args.recv_displs[g]) * elem, - stage_recv_.data() + scatter_off_[block_idx_(a, t, su)] * elem, + std::memcpy(dst + static_cast(recv_displs[g]) * elem, + stage_recv_.data() + recv_block_off_(a, t, su) * elem, static_cast(cnt) * elem); } } } + if (local_partition == 0) { + reverse_ready_gen_ = tables_gen_; // this round's tables may be reversed exactly once + } // No trailing barrier: same discipline as alltoallv_impl_. } + // Reverses the immediately preceding alltoallv_resolve: every leg carries one element back per RECORD + // it delivered, so this round's geometry is that round's with the send/recv roles swapped and every + // count and offset divided by `forward_stride`, the forward leg's elements per record (a query is >= 2 + // words, its answer one value). Every forward count is a multiple of the stride, so the division is + // exact and no count exchange or sizing pass is needed -- that is the 3*R*S^2 table entries and one + // barrier saved. Reusing the offsets undivided would still route correctly but would stage and + // transmit `forward_stride` times the bytes. + // + // The reuse is legal because both ends see the same nesting: rank X ordered its query block to Y as + // (dest partition of Y major, source partition of X minor), and Y's recv view of it has that nesting + // and those counts, so Y answering in place produces the layout X expects back. + // + // CONTRACT: must be the next table-touching verb after an alltoallv_resolve on this comm, with + // send_counts/recv_counts the transpose of that round's. Checked on partition 0 via tables_gen_. + auto alltoallv_reverse_impl_(int local_partition, + const void *send, + const int *send_counts /*[P]*/, + const int *send_displs /*[P]*/, + void *recv, + const int *recv_counts /*[P]*/, + const int *recv_displs /*[P]*/, + size_t elem, + MPI_Datatype dt, + int forward_stride) -> void { + count_verb(local_partition); + if (local_partition == 0 && tables_gen_ != reverse_ready_gen_) { + throw std::runtime_error("HybridComm::alltoallv_reverse must directly follow the " + "alltoallv_resolve whose layout it reverses; another collective " + "has overwritten the offset tables since."); + } + sync(local_partition); // B1: the contract check is global before anyone reuses a table + + if (local_partition == 0) { + ScopedNs timer{p0_table_ns(local_partition)}; + // Per-rank counts/displs are the forward round's, roles swapped and scaled down by the + // stride. Kept in their own scratch so the forward tables stay intact for the scatter below. + long long send_total = 0; + long long recv_total = 0; + for (int b = 0; b < r_; ++b) { + const size_t i = static_cast(b); + rev_send_counts_[i] = mpi_recv_counts_[i] / forward_stride; + rev_recv_counts_[i] = mpi_send_counts_[i] / forward_stride; + rev_send_displs_[i] = checked_mpi_count(send_total); + rev_recv_displs_[i] = checked_mpi_count(recv_total); + send_total += rev_send_counts_[i]; + recv_total += rev_recv_counts_[i]; + } + grow_(stage_send_, static_cast(send_total) * elem); + grow_(stage_recv_, static_cast(recv_total) * elem); + reverse_ready_gen_ = ~0ULL; // one reverse per resolve + } + sync(local_partition); // B2: staging sized + + // Pack into the query round's RECV geometry: partition t answers (rank a, partition su) at the + // very offset that query block occupied. + { + ScopedNs timer{move_ns(local_partition)}; + const int t = local_partition; + const char *src = static_cast(send); + for (int a = 0; a < r_; ++a) { + for (int su = 0; su < s_; ++su) { + const int g = a * s_ + su; + const int cnt = send_counts[g]; + if (cnt != 0) { + const size_t off = recv_block_off_(a, t, su) / static_cast(forward_stride); + std::memcpy(stage_send_.data() + off * elem, + src + static_cast(send_displs[g]) * elem, + static_cast(cnt) * elem); + } + } + } + } + sync(local_partition); // B3 + + if (local_partition == 0) { + ScopedNs timer{mpi_ns(local_partition)}; + MPI_Alltoallv(stage_send_.data(), + rev_send_counts_.data(), // send side = the query round's recv side / stride + rev_send_displs_.data(), + dt, + stage_recv_.data(), + rev_recv_counts_.data(), // recv side = the query round's send side / stride + rev_recv_displs_.data(), + dt, + parent_); + } + sync(local_partition); // B4 + + // Scatter from the query round's SEND geometry: partition u collects the answers to the queries + // it sent, block (rank b, dest partition t) sitting where it packed that query. + ScopedNs timer{move_ns(local_partition)}; + const int u = local_partition; + char *dst = static_cast(recv); + for (int b = 0; b < r_; ++b) { + const size_t base = static_cast(b) * static_cast(s_); + for (int t = 0; t < s_; ++t) { + const int g = b * s_ + t; + const int cnt = recv_counts[g]; + if (cnt != 0) { + const size_t off = (tpre_send_[base + static_cast(t)] + pack_off_[block_idx_(b, t, u)]) + / static_cast(forward_stride); + std::memcpy(dst + static_cast(recv_displs[g]) * elem, + stage_recv_.data() + off * elem, + static_cast(cnt) * elem); + } + } + } + } + template auto allreduce_sum_impl_(int local_partition, T local_val) -> T { + count_verb(local_partition); Slot &me = slots_[static_cast(local_partition)]; if constexpr (std::is_floating_point_v) { me.f64 = static_cast(local_val); @@ -313,8 +533,9 @@ class HybridComm { else { me.u64 = static_cast(local_val); } - sync(); + sync(local_partition); if (local_partition == 0) { + ScopedNs timer{mpi_ns(local_partition)}; if constexpr (std::is_floating_point_v) { double local = 0.0; for (int s = 0; s < s_; ++s) { @@ -330,7 +551,7 @@ class HybridComm { MPI_Allreduce(&local, &red_u64_, 1, MPI_UINT64_T, MPI_SUM, parent_); } } - sync(); + sync(local_partition); T out{}; if constexpr (std::is_floating_point_v) { out = static_cast(red_f64_); @@ -345,55 +566,74 @@ class HybridComm { // In-place element-wise allreduce-sum across the flat P-world, slice-partitioned across partitions in // ascending order (bit-identical to a sequential sum). auto allreduce_sum_inplace_impl_(int local_partition, double *values, size_t len) -> void { + count_verb(local_partition); slots_[static_cast(local_partition)].vec = values; - sync(); // all inputs published + sync(local_partition); // all inputs published if (local_partition == 0) { grow_(red_vec_, len); } - sync(); // red_vec_ sized + sync(local_partition); // red_vec_ sized constexpr size_t kLine = 64 / sizeof(double); const size_t lines = (len + kLine - 1) / kLine; const size_t per = (lines + static_cast(s_) - 1) / static_cast(s_); const size_t lo = std::min(len, static_cast(local_partition) * per * kLine); const size_t hi = std::min(len, lo + per * kLine); - for (size_t k = lo; k < hi; ++k) { - double acc = 0.0; - for (int s = 0; s < s_; ++s) { - acc += slots_[static_cast(s)].vec[k]; + { + ScopedNs timer{move_ns(local_partition)}; + for (size_t k = lo; k < hi; ++k) { + double acc = 0.0; + for (int s = 0; s < s_; ++s) { + acc += slots_[static_cast(s)].vec[k]; + } + red_vec_[k] = acc; // disjoint line-rounded slices: no two partitions store to one line } - red_vec_[k] = acc; // disjoint line-rounded slices: no two partitions store to one line } - sync(); // local reduction complete + sync(local_partition); // local reduction complete if (local_partition == 0) { + ScopedNs timer{mpi_ns(local_partition)}; MPI_Allreduce(MPI_IN_PLACE, red_vec_.data(), static_cast(len), MPI_DOUBLE, MPI_SUM, parent_); } - sync(); // global result in red_vec_ + sync(local_partition); // global result in red_vec_ std::memcpy(values, red_vec_.data(), len * sizeof(double)); // No trailing barrier: red_vec_ is rewritten only inside a future verb's barriered phases. } - // Flat index of the (rank, dest partition, source partition) block in the R*S*S offset/count tables. + auto poison() -> void { barrier_.poison(); } + auto reset() -> void { barrier_.reset(); } + + // Told once by the owning PartitionGroup after its masters are up; a no-op when profiling is off. + // See CommProfile::pinned for why the count is reported at all. + auto note_pinned(int n) -> void { + if (prof_) { + prof_->pinned = n; + } + } + +private: + struct alignas(64) Slot { + const void *ptr = nullptr; + const int *send_counts = nullptr; + const int *send_displs = nullptr; + const int *recv_counts = nullptr; + const double *vec = nullptr; + double f64 = 0.0; + uint64_t u64 = 0; + }; + + // Flat index of the (rank, dest partition, source partition) block in the R*S*S offset tables. auto block_idx_(int b, int t, int u) const -> size_t { return (static_cast(b) * static_cast(s_) + static_cast(t)) * static_cast(s_) + static_cast(u); } - // Pack the S*S count matrix per dest rank into counts_send_, dest-partition-major (t) then - // source-partition-minor (su), ready for the one S*S-int MPI_Alltoall. `counts` selects which slot - // field to read, the only difference between the standalone count exchange (Slot::counts) and the - // fused resolve (Slot::send_counts). - // - // Partition 0 only, and only inside a barriered window: it reads every peer partition's published - // count pointer. Every element of counts_send_ is written here and MPI_Alltoall fills counts_recv_ - // fully, so neither buffer is pre-zeroed. - auto pack_count_matrix_(const int *Slot::*counts) -> void { - for (int b = 0; b < r_; ++b) { - for (int t = 0; t < s_; ++t) { - for (int su = 0; su < s_; ++su) { - counts_send_[block_idx_(b, t, su)] = (slots_[static_cast(su)].*counts)[b * s_ + t]; - } - } - } + // Flat index into the exchanged count matrix, SOURCE-partition-major within each rank's block, so + // partition u owns the contiguous run [cnt_idx_(b,u,0), cnt_idx_(b,u,s_)) and fills it itself: the + // O(R*S^2) transpose partition 0 used to run alone becomes R*S contiguous writes per partition with + // no false sharing. Dest-partition-major would put s_ partitions on every cache line instead. + // MPI_Alltoall is indifferent: `b` stays outermost, and both sides index through this one helper. + auto cnt_idx_(int b, int u, int t) const -> size_t { + return (static_cast(b) * static_cast(s_) + static_cast(u)) * static_cast(s_) + + static_cast(t); } template @@ -403,77 +643,169 @@ class HybridComm { } } - // Partition 0 only; recv counts come from partition t's published recv_counts. - auto size_staging_(size_t elem) -> void { - size_staging_impl_(elem, [this](int t, int rank, int su) { - return slots_[static_cast(t)].recv_counts[rank * s_ + su]; - }); + // Round a row of 8-byte entries up to whole cache lines, so one partition's row never shares a line + // with a peer's. + static auto pad_to_line_(size_t entries) -> size_t { + constexpr size_t kPerLine = 64 / 8; + return ((entries + kPerLine - 1) / kPerLine) * kPerLine; } - // recv_count(t, rank, su) yields the count partition t on this rank receives from (rank, source partition su). - template - auto size_staging_impl_(size_t elem, RecvCountFn recv_count) -> void { + // Partition u's veto row: bit b set iff u sends at least one element to rank b. `mask_words_` is the + // padded row stride; only the first ceil(R/64) words carry bits. + auto mask_row_(int u) -> uint64_t * { return send_mask_.data() + static_cast(u) * mask_words_; } + static auto mask_test_(const uint64_t *row, int b) -> bool { + return (row[static_cast(b) >> 6] & (1ULL << (static_cast(b) & 63U))) != 0; + } + auto mask_empty_(const uint64_t *row) const -> bool { + const size_t live = static_cast((r_ + 63) / 64); + for (size_t w = 0; w < live; ++w) { + if (row[w] != 0) { + return false; + } + } + return true; + } + + // Phase A's per-rank running offsets, one padded row per partition (r_ can be in the hundreds, so + // this is a member and not a stack array). + auto run_row_(int t) -> size_t * { return run_scratch_.data() + static_cast(t) * run_stride_; } + + // Partition u summarizes its own send counts into its veto row, from its own argument and before the + // publishing barrier, so it costs no barrier and reads no peer. + auto fill_send_mask_(int local_partition, const int *send_counts /*[P]*/) -> void { + uint64_t *row = mask_row_(local_partition); + std::fill(row, row + mask_words_, uint64_t{0}); for (int b = 0; b < r_; ++b) { - long long send_sum = 0; - long long recv_sum = 0; + const int *counts = send_counts + b * s_; for (int t = 0; t < s_; ++t) { - for (int su = 0; su < s_; ++su) { - send_sum += slots_[static_cast(su)].send_counts[b * s_ + t]; - recv_sum += recv_count(t, b, su); + if (counts[t] != 0) { + row[static_cast(b) >> 6] |= 1ULL << (static_cast(b) & 63U); + break; } } - mpi_send_counts_[static_cast(b)] = checked_mpi_count(send_sum, "Per-rank send count"); - mpi_recv_counts_[static_cast(b)] = checked_mpi_count(recv_sum, "Per-rank recv count"); } - long long send_running = 0; - long long recv_running = 0; - for (int b = 0; b < r_; ++b) { - mpi_send_displs_[static_cast(b)] = checked_mpi_count(send_running, "Send displacement"); - mpi_recv_displs_[static_cast(b)] = checked_mpi_count(recv_running, "Recv displacement"); - send_running += mpi_send_counts_[static_cast(b)]; - recv_running += mpi_recv_counts_[static_cast(b)]; - } - const size_t total_send = static_cast(checked_mpi_count(send_running, "Total send count")); - const size_t total_recv = static_cast(checked_mpi_count(recv_running, "Total recv count")); - // Grow-only, no zero-fill: pack_send_'s blocks tile [0, total_send) exactly and MPI_Alltoallv - // fills every live byte of stage_recv_, so stale bytes past a prior high-water mark are never read. - grow_(stage_send_, total_send * elem); - grow_(stage_recv_, total_recv * elem); - // Precompute each (rank b, dest partition t, source partition u) block start in elements, dest-major/ - // source-minor to match staging: pack/scatter become O(1) lookups that read no peer slot past B4 - // (re-summing peer count matrices instead would be O(R*S^3)). + } + + // Partition u writes its own source row of the exchanged count matrix. Disjoint per u and + // contiguous in t, so it needs no barrier of its own and no coherence ping-pong. + auto fill_count_row_(int local_partition, const int *send_counts /*[P]*/) -> void { + const int u = local_partition; for (int b = 0; b < r_; ++b) { - size_t cur = static_cast(mpi_send_displs_[static_cast(b)]); + const int *row = send_counts + b * s_; + int *dst = counts_send_.data() + cnt_idx_(b, u, 0); for (int t = 0; t < s_; ++t) { - for (int u = 0; u < s_; ++u) { - pack_off_[block_idx_(b, t, u)] = cur; - cur += static_cast(slots_[static_cast(u)].send_counts[b * s_ + t]); - } + dst[t] = row[t]; } } - for (int a = 0; a < r_; ++a) { - size_t cur = static_cast(mpi_recv_displs_[static_cast(a)]); - for (int t = 0; t < s_; ++t) { + fill_send_mask_(local_partition, send_counts); + } + + // Non-resolve alltoallv path: recv counts come from partition t's own published recv_counts, so + // in phase A below every partition reads only its own slot. + auto size_staging_parallel_(int local_partition, size_t elem) -> void { + size_staging_parallel_(local_partition, elem, [this](int t, int rank, int su) { + return slots_[static_cast(t)].recv_counts[rank * s_ + su]; + }); + } + + // Replaces the O(R*S^2) serial sizing pass with a parallel prefix. recv_count(t, rank, su) is the + // count partition t on this rank receives from (rank, source partition su). + // + // The offset of block (b, t, u) in the staged message decomposes as + // mpi_send_displs_[b] + sum_{t' < t} col_send_[b][t'] + sum_{u' < u} c[u'][b][t] + // \------------- tpre_send_[b][t], one O(R*S) scan -----/ \--- pack_off_[b][t][u] ---/ + // The right-hand term is an independent scan per (b,t) pair, so partition t owns every pair with its + // own t: R*S work each, S-way parallel, covering the same R*S^2 entries. Only the middle term needs + // global knowledge, and at O(R*S) it is small enough to leave on partition 0. + // + // The two barriers this costs (phase A complete before partition 0 reduces it, bases visible before + // anyone packs) buy S-way parallelism over what measured 84% of wall time at S=112. + template + auto size_staging_parallel_(int local_partition, size_t elem, RecvCountFn recv_count) -> void { + const int t = local_partition; + { + ScopedNs timer{par_table_ns(local_partition)}; + // Send half, SOURCE-partition-outer so each peer's veto row is loaded once for all R ranks + // instead of probing its count array R times on R separate cache lines -- the dominant cost + // of this phase once the serial fill was gone. Skipped blocks leave a stale pack_off_ entry, + // which is safe: an offset is read only where the matching count is nonzero. + size_t *run = run_row_(t); + std::fill(run, run + r_, size_t{0}); + for (int u = 0; u < s_; ++u) { + const uint64_t *mask = mask_row_(u); + if (mask_empty_(mask)) { + continue; + } + const int *counts = slots_[static_cast(u)].send_counts; + for (int b = 0; b < r_; ++b) { + if (!mask_test_(mask, b)) { + continue; // u sends nothing to rank b, so it contributes 0 to every (b,t) scan + } + pack_off_[block_idx_(b, t, u)] = run[b]; + run[b] += static_cast(counts[b * s_ + t]); + } + } + for (int b = 0; b < r_; ++b) { + col_send_[static_cast(b) * static_cast(s_) + static_cast(t)] = run[b]; + size_t got = 0; for (int su = 0; su < s_; ++su) { - scatter_off_[block_idx_(a, t, su)] = cur; - cur += static_cast(recv_count(t, a, su)); + scatter_off_[block_idx_(b, t, su)] = got; + got += static_cast(recv_count(t, b, su)); + } + col_recv_[static_cast(b) * static_cast(s_) + static_cast(t)] = got; + } + } + sync(local_partition); // phase A complete on every partition + + if (local_partition == 0) { + ScopedNs timer{p0_table_ns(local_partition)}; + long long send_running = 0; + long long recv_running = 0; + for (int b = 0; b < r_; ++b) { + long long send_sum = 0; + long long recv_sum = 0; + const size_t base = static_cast(b) * static_cast(s_); + for (int k = 0; k < s_; ++k) { + send_sum += static_cast(col_send_[base + static_cast(k)]); + recv_sum += static_cast(col_recv_[base + static_cast(k)]); + } + mpi_send_counts_[static_cast(b)] = checked_mpi_count(send_sum); + mpi_recv_counts_[static_cast(b)] = checked_mpi_count(recv_sum); + mpi_send_displs_[static_cast(b)] = checked_mpi_count(send_running); + mpi_recv_displs_[static_cast(b)] = checked_mpi_count(recv_running); + // tpre_*_ folds the per-rank base in, so pack/scatter add exactly two numbers. + size_t cur_s = static_cast(send_running); + size_t cur_r = static_cast(recv_running); + for (int k = 0; k < s_; ++k) { + tpre_send_[base + static_cast(k)] = cur_s; + cur_s += col_send_[base + static_cast(k)]; + tpre_recv_[base + static_cast(k)] = cur_r; + cur_r += col_recv_[base + static_cast(k)]; } + send_running += mpi_send_counts_[static_cast(b)]; + recv_running += mpi_recv_counts_[static_cast(b)]; } + // Grow-only, no zero-fill: pack_send_'s blocks tile [0, total_send) exactly and MPI_Alltoallv + // fills every live byte of stage_recv_, so stale bytes past a prior high-water mark are never read. + grow_(stage_send_, static_cast(checked_mpi_count(send_running)) * elem); + grow_(stage_recv_, static_cast(checked_mpi_count(recv_running)) * elem); } + sync(local_partition); // bases and staging visible to every packer } auto pack_send_(int local_partition, size_t elem) -> void { const int u = local_partition; - // Own slot only — no peer's published send buffer is read here, which is what lets every - // partition pack concurrently in the B2→B3 window. - const std::byte *src = slots_[static_cast(u)].ptr; + const char *src = static_cast(slots_[static_cast(u)].ptr); const int *my_send_counts = slots_[static_cast(u)].send_counts; const int *my_send_displs = slots_[static_cast(u)].send_displs; for (int b = 0; b < r_; ++b) { + const size_t base = static_cast(b) * static_cast(s_); for (int t = 0; t < s_; ++t) { const int cnt = my_send_counts[b * s_ + t]; if (cnt != 0) { - std::memcpy(stage_send_.data() + pack_off_[block_idx_(b, t, u)] * elem, + // Absolute start = per-(b,t) base (partition 0) + within-(b,t) scan (partition t). + const size_t off = tpre_send_[base + static_cast(t)] + pack_off_[block_idx_(b, t, u)]; + std::memcpy(stage_send_.data() + off * elem, src + static_cast(my_send_displs[b * s_ + t]) * elem, static_cast(cnt) * elem); } @@ -481,6 +813,12 @@ class HybridComm { } } + // Absolute start of the block partition t receives from (rank a, source partition su). + auto recv_block_off_(int a, int t, int su) const -> size_t { + return tpre_recv_[static_cast(a) * static_cast(s_) + static_cast(t)] + + scatter_off_[block_idx_(a, t, su)]; + } + [[noreturn]] auto abort_rank_(const char *verb, const char *what) -> void { std::print(stderr, "monoprop: rank {} cannot complete the collective '{}' ({}). Its peer ranks are " @@ -493,7 +831,31 @@ class HybridComm { std::abort(); // MPI_Abort is not marked [[noreturn]]; unreachable in practice } - auto sync() -> void { barrier_.sync(); } + // Barrier wait is attributed to the partition that waited, which is the whole point: when the + // master monopolises a serial phase, its own wait stays near zero while every peer's grows. + auto sync(int local_partition) -> void { + if (prof_ == nullptr) { + barrier_.sync(local_partition); + return; + } + CommProfile::Slot &sl = prof_->slot(local_partition); + ++sl.n_barriers; + ScopedNs t{&sl.barrier_ns}; + barrier_.sync(local_partition); + } + + // nullptr target ⇒ ScopedNs is inert, so instrumented regions need no #ifdef or duplicate code. + auto p0_table_ns(int u) -> uint64_t * { return prof_ == nullptr ? nullptr : &prof_->slot(u).table_p0_ns; } + auto par_table_ns(int u) -> uint64_t * { return prof_ == nullptr ? nullptr : &prof_->slot(u).table_par_ns; } + auto move_ns(int u) -> uint64_t * { return prof_ == nullptr ? nullptr : &prof_->slot(u).table_move_ns; } + auto mpi_ns(int u) -> uint64_t * { return prof_ == nullptr ? nullptr : &prof_->slot(u).mpi_ns; } + auto count_verb(int u) -> void { + if (prof_ != nullptr) { + ++prof_->slot(u).n_verbs; + } + } + + std::unique_ptr prof_; MPI_Comm parent_; int s_; @@ -510,12 +872,32 @@ class HybridComm { std::vector mpi_send_displs_; std::vector mpi_recv_counts_; std::vector mpi_recv_displs_; - // [R*S*S] block starts (elements) in the staging buffers. + // [R*S*S] within-(rank, dest partition) exclusive scans over the SOURCE partition, filled by the + // owning dest partition; absolute starts are these plus tpre_*_ (see size_staging_parallel_). std::vector pack_off_; std::vector scatter_off_; + // [R*S] per-(rank, dest partition) column totals and their prefix, the only globally-reduced part. + std::vector col_send_; + std::vector col_recv_; + std::vector tpre_send_; + std::vector tpre_recv_; + // [S * mask_words_] one cache-line-padded veto row per source partition, and [S * run_stride_] + // per-partition scratch for phase A's per-rank running offsets. Both written only by their owner. + std::vector send_mask_; + std::vector run_scratch_; + size_t mask_words_ = 8; + size_t run_stride_ = 8; // Aggregated MPI payload staging, HWM-sized. std::vector stage_send_; std::vector stage_recv_; + // [R] reverse-round per-rank counts/displs (the forward round's, swapped and divided by the stride). + // Only partition 0 touches these and tables_gen_/reverse_ready_gen_, inside a barriered window. + std::vector rev_send_counts_; + std::vector rev_send_displs_; + std::vector rev_recv_counts_; + std::vector rev_recv_displs_; + uint64_t tables_gen_ = 0; + uint64_t reverse_ready_gen_ = ~0ULL; double red_f64_ = 0.0; uint64_t red_u64_ = 0; std::vector red_vec_; diff --git a/cpp/monoprop/detail/mpi/MPICompat.h b/cpp/monoprop/detail/mpi/MPICompat.h index fb5c7fd3..c6429cb0 100644 --- a/cpp/monoprop/detail/mpi/MPICompat.h +++ b/cpp/monoprop/detail/mpi/MPICompat.h @@ -116,7 +116,8 @@ auto alltoall_counts(const int *send_counts, int *recv_counts, int n, Comm comm) // recv_counts is valid on return from begin_alltoallv; wait_into completes the payload transfer (a // no-op on the synchronous Shm / single-process paths) and unpacks by source. template -struct PendingAlltoallv { +struct [[nodiscard("complete the transfer with wait_into(); dropping the handle frees buffers MPI still " + "owns")]] PendingAlltoallv { int num_ranks = 0; std::vector send_counts; std::vector send_displs; @@ -128,13 +129,46 @@ struct PendingAlltoallv { MPI_Request request = MPI_REQUEST_NULL; // set only on the Kind::Mpi async path #endif - auto wait_into(std::vector> &recv_data) -> void { + // Move-only and self-completing, for the same reason mpi::Ticket is: the posted MPI_Ialltoallv writes + // directly into this handle's own send_buffer/recv_buffer, so a copy would hand two owners the same + // request (and wait on it twice), and a handle destroyed without wait_into -- what an exception or an + // early return between post and unpack does -- would free those buffers while MPI is still writing + // into them. + PendingAlltoallv() = default; + PendingAlltoallv(const PendingAlltoallv &) = delete; + auto operator=(const PendingAlltoallv &) -> PendingAlltoallv & = delete; + PendingAlltoallv(PendingAlltoallv &&other) noexcept { *this = std::move(other); } + auto operator=(PendingAlltoallv &&other) noexcept -> PendingAlltoallv & { + if (this != &other) { + wait(); // never drop a request this handle already owns + num_ranks = other.num_ranks; + send_counts = std::move(other.send_counts); + send_displs = std::move(other.send_displs); + recv_counts = std::move(other.recv_counts); + recv_displs = std::move(other.recv_displs); + send_buffer = std::move(other.send_buffer); + recv_buffer = std::move(other.recv_buffer); +#ifdef monoprop_ENABLE_MPI + request = other.request; + other.request = MPI_REQUEST_NULL; +#endif + } + return *this; + } + ~PendingAlltoallv() { wait(); } + + // Idempotent; a no-op on the synchronous Shm / single-process paths and in non-MPI builds. + auto wait() -> void { #ifdef monoprop_ENABLE_MPI if (request != MPI_REQUEST_NULL) { MPI_Wait(&request, MPI_STATUS_IGNORE); request = MPI_REQUEST_NULL; } #endif + } + + auto wait_into(std::vector> &recv_data) -> void { + wait(); recv_data.resize(static_cast(num_ranks)); for (int i = 0; i < num_ranks; ++i) { const auto lo = recv_buffer.begin() + recv_displs[static_cast(i)]; @@ -148,11 +182,18 @@ struct PendingAlltoallv { // skip_self: do not send the self slot (the caller handles self inline) — self send/recv = 0. // known_recv_counts: recv counts already known (e.g. the transpose of the query counts), so skip the // count exchange. The self slot is also zeroed when skip_self is set. +// reverse_of_previous: this exchange is the answer leg of the immediately preceding begin_alltoallv on +// the same comm, and forward_stride is that leg's elements per record (a query is several words, its +// answer one value). Only the hybrid transport acts on it, reusing that round's offset tables instead of +// rebuilding them (see HybridComm::alltoallv_reverse); it requires known_recv_counts, and no other +// collective may intervene on that comm. template inline auto begin_alltoallv(const std::vector> &send_data, Comm comm, bool skip_self = false, - const std::vector *known_recv_counts = nullptr) -> PendingAlltoallv { + const std::vector *known_recv_counts = nullptr, + bool reverse_of_previous = false, + int forward_stride = 1) -> PendingAlltoallv { const int num_ranks = size(comm); if (static_cast(send_data.size()) != num_ranks) { throw CollectiveArgumentError( @@ -255,7 +296,24 @@ inline auto begin_alltoallv(const std::vector> &send_data, } #ifdef monoprop_ENABLE_MPI else if (comm.kind == Comm::Kind::Hybrid) { - comm.hyb->alltoallv(comm.shm_rank, flat, datatype::get()); + // Both arms read the same bundle, taken after the recv_buffer resize above. The reverse verb + // keeps its per-argument signature: it needs forward_stride, which is not part of the shared + // bundle because only the hybrid transport's reverse leg has a notion of it. + if (reverse_of_previous) { + comm.hyb->alltoallv_reverse(comm.shm_rank, + flat.send, + flat.send_counts, + flat.send_displs, + flat.recv, + flat.recv_counts, + flat.recv_displs, + flat.elem, + datatype::get(), + forward_stride); + } + else { + comm.hyb->alltoallv(comm.shm_rank, flat, datatype::get()); + } } #endif else { diff --git a/cpp/monoprop/detail/mpi/PartitionBarrier.h b/cpp/monoprop/detail/mpi/PartitionBarrier.h index 1f31a482..c0ef5054 100644 --- a/cpp/monoprop/detail/mpi/PartitionBarrier.h +++ b/cpp/monoprop/detail/mpi/PartitionBarrier.h @@ -14,10 +14,15 @@ #pragma once +#include #include +#include +#include #include #include +#include +#include "monoprop/detail/EnvConfig.h" #include "monoprop/detail/mpi/CpuRelax.h" namespace monoprop::mpi { @@ -32,38 +37,108 @@ class ShmCommPoisoned : public std::runtime_error { // word gets a private cache line: if `gen_` shared `arrived_`'s line, spinners' reloads would miss to // L3 on every peer arrival — O(S) coherence bounces (measured top hotspot at S=112). // +// Given a group id per participant (its locality domain, from CpuTopology: the deepest shared cache, or +// the NUMA node) the barrier becomes two-level: fan in within a domain, then across domains, then release +// within the domain. Both the arrival fetch_add and the release store then cost O(S/G) coherence +// transactions instead of O(S), and stay inside one cache slice. No group ids (pinning off, or /sys +// unreadable), a single domain, all-singleton domains, or monoprop_BARRIER_GROUPING=0 degrade to the +// flat barrier. That last one exists because the domains come from the cpusets: without it the only way +// to get a flat barrier is to unpin, so "grouped vs flat" and "pinned vs unpinned" could never be +// separated, and the second level's value could not be measured on a real workload. +// +// The second level is not a free win. It replaces one fetch_add with two sequential hops, so it pays +// only when there is contention to relieve: at high partition counts, or when arrivals are skewed. On a +// collective short enough that the barrier is most of it, and with partitions arriving together, the +// extra hop is pure latency. +// // The acquire/release/relaxed orderings below are load-bearing, not an oversight: the release store to // `gen_` is what publishes the preceding relaxed reset of `arrived_`. Promoting them to seq_cst would // put a full barrier in the spin loop of that same hotspot, so cpp:S8417 is suppressed for this file -// in sonar-project.properties. Do not "simplify" them to the default ordering. +// in sonar-project.properties. Do not "simplify" them to the default ordering, and do not weaken one +// either: the monoprop_ENABLE_TSAN build (see docs/content/docs/building.mdx) is the check that stands +// in for the suppressed rule, and demoting any single one of the five -- either `gen_` store, the +// `group_gen_` store, the arrival fetch_adds, or the spin load -- makes it report the published data as +// a race. Every ordering here is individually necessary, and none is stronger than it needs to be. class PartitionBarrier { public: - explicit PartitionBarrier(int participants) : participants_(participants) {} + // `spin_budget` overrides how long a waiter stays on-core before yielding; unset takes + // monoprop_SPIN_BUDGET_US, else kDefaultSpinBudgetUs. Injectable because config::get() caches on + // first call, so a test cannot reach the env path in-process, and because sweeping the budget is + // how its default is justified. + explicit PartitionBarrier(int participants, + const std::vector &group_of = {}, + std::optional spin_budget = std::nullopt) + : spin_budget_(spin_budget.value_or( + std::chrono::microseconds{config::get().spin_budget_us.value_or(detail::kDefaultSpinBudgetUs)})), + participants_(participants) { + if (static_cast(group_of.size()) != participants || participants <= 0 || !config::get().barrier_grouping) { + return; // flat + } + // Compact the domain ids to 0..G-1 in first-seen order, and count each group. + std::vector domains; + group_of_.resize(static_cast(participants)); + for (int p = 0; p < participants; ++p) { + const auto it = std::find(domains.begin(), domains.end(), group_of[static_cast(p)]); + if (it == domains.end()) { + group_of_[static_cast(p)] = static_cast(domains.size()); + domains.push_back(group_of[static_cast(p)]); + } + else { + group_of_[static_cast(p)] = static_cast(it - domains.begin()); + } + } + // One domain has nothing to fan in across. So does a set of all-singleton domains, which is what + // an unreadable topology produces: every participant would be its own representative and they + // would all still meet at the root, i.e. the flat barrier plus a cache line and a release store + // per participant. Reporting both as flat also stops group_count() from claiming a level that is + // not actually running. + if (domains.size() < 2 || domains.size() == static_cast(participants)) { + group_of_.clear(); + return; // flat + } + groups_ = static_cast(domains.size()); + group_arrived_ = std::vector(domains.size()); + group_gen_ = std::vector(domains.size()); + group_size_.assign(domains.size(), 0); + for (int p = 0; p < participants; ++p) { + ++group_size_[static_cast(group_of_[static_cast(p)])]; + } + } PartitionBarrier(const PartitionBarrier &) = delete; auto operator=(const PartitionBarrier &) -> PartitionBarrier & = delete; - auto sync() -> void { - const unsigned g = gen_.load(std::memory_order_acquire); - if (arrived_.fetch_add(1, std::memory_order_acq_rel) + 1 == participants_) { - arrived_.store(0, std::memory_order_relaxed); - gen_.store(g + 1, std::memory_order_release); + // `participant` selects the caller's group; ignored on the flat path. + auto sync(int participant) -> void { + if (groups_ < 2) { + const unsigned g = gen_.load(std::memory_order_acquire); + if (arrived_.fetch_add(1, std::memory_order_acq_rel) + 1 == participants_) { + arrived_.store(0, std::memory_order_relaxed); + gen_.store(g + 1, std::memory_order_release); + } + else { + spin_until_(gen_, g); + } } else { - // Bounded on-core spin first (pinned partitions ⇒ the release store lands in the pause window, - // no syscall); only long waits (imbalance, oversubscription) fall to yield. - int spins = 0; - while (gen_.load(std::memory_order_acquire) == g) { - if (poisoned_.load(std::memory_order_acquire)) { - throw ShmCommPoisoned(); - } - if (spins < detail::kSpinPauseIters) { - ++spins; - detail::cpu_relax(); + const size_t gi = static_cast(group_of_[static_cast(participant)]); + const unsigned seen = group_gen_[gi].v.load(std::memory_order_acquire); + if (group_arrived_[gi].v.fetch_add(1, std::memory_order_acq_rel) + 1 == group_size_[gi]) { + group_arrived_[gi].v.store(0, std::memory_order_relaxed); + // The domain's last arriver represents it upstream, and releases the domain only after + // the root barrier, so passing the domain word still implies everyone arrived. + const unsigned root_seen = gen_.load(std::memory_order_acquire); + if (arrived_.fetch_add(1, std::memory_order_acq_rel) + 1 == groups_) { + arrived_.store(0, std::memory_order_relaxed); + gen_.store(root_seen + 1, std::memory_order_release); } else { - std::this_thread::yield(); + spin_until_(gen_, root_seen); } + group_gen_[gi].v.store(seen + 1, std::memory_order_release); + } + else { + spin_until_(group_gen_[gi].v, seen); } } if (poisoned_.load(std::memory_order_acquire)) { @@ -71,20 +146,84 @@ class PartitionBarrier { } } + // Locality domains the participants were grouped into; < 2 means the flat barrier is in use. Reported by + // CommProfile so a run states which barrier it ran instead of leaving it inferred from the timing. + auto group_count() const -> int { return groups_; } + + // The on-core spin budget actually in force, so a test can observe the resolution rather than + // re-computing it, and so a sweep can report what it swept. + auto spin_budget() const -> std::chrono::microseconds { return spin_budget_; } + // Signal that this participant is unwinding (e.g. an engine exception), releasing peers spinning in a // barrier. Idempotent. auto poison() -> void { poisoned_.store(true, std::memory_order_release); } // Must be called only when every participant is quiescent (between rounds), so a poison-aborted round - // leaves no dirty state. `gen_` deliberately stays monotonic: each participant re-reads it at its next - // barrier. + // leaves no dirty state. The generations deliberately stay monotonic: each participant re-reads its + // own at the next barrier. auto reset() -> void { poisoned_.store(false, std::memory_order_relaxed); arrived_.store(0, std::memory_order_relaxed); + for (auto &c : group_arrived_) { + c.v.store(0, std::memory_order_relaxed); + } } private: + // Wait until `word` leaves `seen`, backing off through the two phases CpuRelax.h describes: on-core + // relax for a bounded time, then yield indefinitely. Both phases re-read `poisoned_` on every + // iteration, so an unwinding peer releases us with an exception rather than a hang and poison() needs + // no wakeup channel of its own. There is deliberately no third parking phase: sleeping past the yield + // budget measured 2021 vs 762 us/sync when oversubscribed, because one sleeper's timer overshoot + // delays every participant behind the barrier (see CpuRelax.h). + auto spin_until_(const std::atomic &word, unsigned seen) const -> void { + // The deadline is computed lazily, after the first load fails. The overwhelmingly common case is + // that the release store has already landed, and a clock read per arrival is pure overhead on the + // path this barrier exists to keep cheap. + std::chrono::steady_clock::time_point spin_deadline{}; + bool have_deadline = false; + bool spinning = true; + int until_check = detail::kRelaxPerClockCheck; + while (word.load(std::memory_order_acquire) == seen) { + if (poisoned_.load(std::memory_order_acquire)) { + throw ShmCommPoisoned(); + } + if (!have_deadline) { + spin_deadline = std::chrono::steady_clock::now() + spin_budget_; + have_deadline = true; + } + if (spinning) { + if (--until_check <= 0) { + until_check = detail::kRelaxPerClockCheck; + spinning = std::chrono::steady_clock::now() < spin_deadline; + } + detail::cpu_relax(); + } + else { + std::this_thread::yield(); + } + } + } + + // Per-domain words, each on its own cache line for the same reason the flat pair is split. + struct alignas(64) Count { + std::atomic v{0}; + }; + struct alignas(64) Gen { + std::atomic v{0}; + }; + + // Resolved once per barrier, not once per spin: monoprop_SPIN_BUDGET_US is a sweep knob, and the spin + // loop is the measured hotspot. + std::chrono::microseconds spin_budget_; + int participants_; + int groups_ = 0; // < 2 ⇒ flat: arrived_/gen_ count participants, not domains + std::vector group_of_; + std::vector group_size_; + std::vector group_arrived_; + std::vector group_gen_; + // Flat barrier, or (two-level) the root barrier the domains' last arrivers meet at. alignas(64) std::atomic arrived_{0}; alignas(64) std::atomic gen_{0}; alignas(64) std::atomic poisoned_{false}; diff --git a/cpp/monoprop/detail/mpi/ShmComm.h b/cpp/monoprop/detail/mpi/ShmComm.h index 4ddc7092..c917ab1d 100644 --- a/cpp/monoprop/detail/mpi/ShmComm.h +++ b/cpp/monoprop/detail/mpi/ShmComm.h @@ -24,8 +24,10 @@ #include #include +#include "monoprop/detail/EnvConfig.h" #include "monoprop/detail/mpi/CheckedCount.h" #include "monoprop/detail/mpi/Comm.h" +#include "monoprop/detail/mpi/CommProfile.h" #include "monoprop/detail/mpi/PartitionBarrier.h" // In-process shared-memory SPMD transport: S partition-master threads each call the same collective @@ -38,21 +40,41 @@ namespace monoprop::mpi { class ShmComm { public: - explicit ShmComm(int n) : n_(n), slots_(static_cast(n)), barrier_(n) {} + // partition_l3_domains (optional, one entry per partition) makes the barrier two-level; see + // PartitionBarrier. + explicit ShmComm(int n, const std::vector &partition_l3_domains = {}) + : n_(n), + slots_(static_cast(n)), + barrier_(n, partition_l3_domains) { + if (config::get().comm_profile) { + prof_ = std::make_unique(n, /*mpi_rank=*/-1); // -1 marks the single-rank transport + prof_->barrier_groups = barrier_.group_count(); + } + } ShmComm(const ShmComm &) = delete; auto operator=(const ShmComm &) -> ShmComm & = delete; + ~ShmComm() { + if (prof_ != nullptr) { + prof_->dump(); + } + } + auto size() const -> int { return n_; } // recv_counts[s] = what rank s sends to me (the transpose of the send-count matrix). auto alltoall_counts(int rank, const int *send_counts, int *recv_counts) -> void { + count_verb(rank); slots_[static_cast(rank)].counts = send_counts; - sync(); - for (int s = 0; s < n_; ++s) { - recv_counts[s] = slots_[static_cast(s)].counts[rank]; + sync(rank); + { + ScopedNs timer{par_table_ns(rank)}; + for (int s = 0; s < n_; ++s) { + recv_counts[s] = slots_[static_cast(s)].counts[rank]; + } } - sync(); + sync(rank); } // Variable all-to-all over caller-owned flat buffers, addressed as raw bytes (counts/displs stay in @@ -66,24 +88,28 @@ class ShmComm { const int *recv_counts, const int *recv_displs, size_t elem) -> void { + count_verb(rank); Slot &me = slots_[static_cast(rank)]; me.ptr = send; me.displs = send_displs; - sync(); - auto *dst = recv; - for (int s = 0; s < n_; ++s) { - const Slot &src = slots_[static_cast(s)]; - const auto count = static_cast(recv_counts[s]); - // Bail before offsetting src: a source that sends us nothing need not have published a - // buffer at all, and its displs[rank] may then point past the end of one. - if (count == 0) { - continue; + sync(rank); + { + ScopedNs timer{move_ns(rank)}; + auto *dst = recv; + for (int s = 0; s < n_; ++s) { + const Slot &src = slots_[static_cast(s)]; + const auto count = static_cast(recv_counts[s]); + // Bail before offsetting src: a source that sends us nothing need not have published a + // buffer at all, and its displs[rank] may then point past the end of one. + if (count == 0) { + continue; + } + std::memcpy(dst + static_cast(recv_displs[s]) * elem, + src.ptr + static_cast(src.displs[rank]) * elem, + count * elem); } - std::memcpy(dst + static_cast(recv_displs[s]) * elem, - src.ptr + static_cast(src.displs[rank]) * elem, - count * elem); } - sync(); + sync(rank); } // Fused count-resolve + payload all-to-all in one round (2 syncs vs 4). Same contiguous @@ -97,28 +123,35 @@ class ShmComm { me.ptr = reinterpret_cast(args.send); me.displs = args.send_displs; me.counts = args.send_counts; - sync(); // B1: send buffers published - long long total = 0; - for (int s = 0; s < n_; ++s) { - const int c = slots_[static_cast(s)].counts[rank]; // what s sends to me - args.recv_counts[s] = c; - args.recv_displs[s] = checked_mpi_count(total, "Recv displacement"); - total += c; + count_verb(rank); + sync(rank); // B1: send buffers published + { + ScopedNs timer{par_table_ns(rank)}; + long long total = 0; + for (int s = 0; s < n_; ++s) { + const int c = slots_[static_cast(s)].counts[rank]; // what s sends to me + args.recv_counts[s] = c; + args.recv_displs[s] = checked_mpi_count(total, "Recv displacement"); + total += c; + } + args.recv.resize(static_cast(checked_mpi_count(total, "Total recv count"))); } - args.recv.resize(static_cast(checked_mpi_count(total, "Total recv count"))); - auto *dst = reinterpret_cast(args.recv.data()); // after the resize: it may reallocate - for (int s = 0; s < n_; ++s) { - const Slot &src = slots_[static_cast(s)]; - const auto count = static_cast(args.recv_counts[s]); - // See alltoallv: an unpublished source must not be offset by its displacement. - if (count == 0) { - continue; + { + ScopedNs timer{move_ns(rank)}; + auto *dst = reinterpret_cast(args.recv.data()); // after the resize: it may reallocate + for (int s = 0; s < n_; ++s) { + const Slot &src = slots_[static_cast(s)]; + const auto count = static_cast(args.recv_counts[s]); + // See alltoallv: an unpublished source must not be offset by its displacement. + if (count == 0) { + continue; + } + std::memcpy(dst + static_cast(args.recv_displs[s]) * sizeof(T), + src.ptr + static_cast(src.displs[rank]) * sizeof(T), + count * sizeof(T)); } - std::memcpy(dst + static_cast(args.recv_displs[s]) * sizeof(T), - src.ptr + static_cast(src.displs[rank]) * sizeof(T), - count * sizeof(T)); } - sync(); // B2: peers finished reading our send buffer before the caller may reuse it + sync(rank); // B2: peers finished reading our send buffer before the caller may reuse it } template @@ -130,7 +163,8 @@ class ShmComm { else { me.u64 = static_cast(local_val); } - sync(); + count_verb(rank); + sync(rank); T acc{}; for (int s = 0; s < n_; ++s) { const Slot &src = slots_[static_cast(s)]; @@ -141,30 +175,34 @@ class ShmComm { acc += static_cast(src.u64); } } - sync(); + sync(rank); return acc; } // Safe in place: each element is read then overwritten by its single slice owner, and slices are // cache-line-rounded. auto allreduce_sum_inplace(int rank, double *values, size_t len) -> void { + count_verb(rank); slots_[static_cast(rank)].vec = values; - sync(); - constexpr size_t kLine = 64 / sizeof(double); - const size_t lines = (len + kLine - 1) / kLine; - const size_t per = (lines + static_cast(n_) - 1) / static_cast(n_); - const size_t lo = std::min(len, static_cast(rank) * per * kLine); - const size_t hi = std::min(len, lo + per * kLine); - for (size_t k = lo; k < hi; ++k) { - double acc = 0.0; - for (int s = 0; s < n_; ++s) { // ascending rank order - acc += slots_[static_cast(s)].vec[k]; - } - for (int s = 0; s < n_; ++s) { // publish the same bits into every rank's buffer - slots_[static_cast(s)].vec[k] = acc; + sync(rank); + { + ScopedNs timer{move_ns(rank)}; + constexpr size_t kLine = 64 / sizeof(double); + const size_t lines = (len + kLine - 1) / kLine; + const size_t per = (lines + static_cast(n_) - 1) / static_cast(n_); + const size_t lo = std::min(len, static_cast(rank) * per * kLine); + const size_t hi = std::min(len, lo + per * kLine); + for (size_t k = lo; k < hi; ++k) { + double acc = 0.0; + for (int s = 0; s < n_; ++s) { // ascending rank order + acc += slots_[static_cast(s)].vec[k]; + } + for (int s = 0; s < n_; ++s) { // publish the same bits into every rank's buffer + slots_[static_cast(s)].vec[k] = acc; + } } } - sync(); // peers write into our buffer (and read from it) until here + sync(rank); // peers write into our buffer (and read from it) until here } // See PartitionBarrier::poison / ::reset for when each is legal to call. @@ -172,6 +210,14 @@ class ShmComm { auto reset() -> void { barrier_.reset(); } + // Told once by the owning PartitionGroup after its masters are up; a no-op when profiling is off. + // See CommProfile::pinned for why the count is reported at all. + auto note_pinned(int n) -> void { + if (prof_) { + prof_->pinned = n; + } + } + private: // One cache-line-isolated publish slot per rank. A rank writes only its own slot, and reads peers' // slots only between the two barriers of a collective. @@ -184,8 +230,28 @@ class ShmComm { uint64_t u64 = 0; }; - auto sync() -> void { barrier_.sync(); } + // Same per-partition attribution as HybridComm::sync. Every non-barrier phase here is already + // parallel, so a run's whole floor lands in barrier_ns and reads off as cost-per-sync. + auto sync(int rank) -> void { + if (prof_ == nullptr) { + barrier_.sync(rank); + return; + } + CommProfile::Slot &sl = prof_->slot(rank); + ++sl.n_barriers; + ScopedNs t{&sl.barrier_ns}; + barrier_.sync(rank); + } + + auto par_table_ns(int rank) -> uint64_t * { return prof_ == nullptr ? nullptr : &prof_->slot(rank).table_par_ns; } + auto move_ns(int rank) -> uint64_t * { return prof_ == nullptr ? nullptr : &prof_->slot(rank).table_move_ns; } + auto count_verb(int rank) -> void { + if (prof_ != nullptr) { + ++prof_->slot(rank).n_verbs; + } + } + std::unique_ptr prof_; int n_; std::vector slots_; PartitionBarrier barrier_; diff --git a/cpp/monoprop/detail/partition/CpuTopology.cpp b/cpp/monoprop/detail/partition/CpuTopology.cpp index dd019d8e..5a3813a8 100644 --- a/cpp/monoprop/detail/partition/CpuTopology.cpp +++ b/cpp/monoprop/detail/partition/CpuTopology.cpp @@ -15,6 +15,7 @@ #include "monoprop/detail/partition/CpuTopology.h" #include +#include #include #include @@ -220,10 +221,17 @@ auto enumerate_physical_cores() -> std::vector { /* ── partition_cpusets ─────────────────────────────────────────────────────── */ -auto partition_cpusets(size_t n, size_t group_index, size_t group_count) -> std::vector { +auto partition_cpusets(size_t n, size_t group_index, size_t group_count, NodeMask mask) -> std::vector { if (!config::get().partition_pinning) { return {}; } + /* A PerRank mask is already this rank's own slice, so dividing it again by group_count would + * overflow placement_order()'s capacity guard and disable pinning entirely. See the note on + * partition_cpusets() in the header for the measurement behind this. */ + if (mask == NodeMask::PerRank) { + group_index = 0; + group_count = 1; + } const auto cores = enumerate_physical_cores(); const auto order = topo_detail::placement_order(cores, n, group_index, group_count); @@ -236,22 +244,84 @@ auto partition_cpusets(size_t n, size_t group_index, size_t group_count) -> std: /* ── pin_this_thread ───────────────────────────────────────────────────────── */ -auto pin_this_thread(const CpuSet &set) -> void { +auto pin_this_thread(const CpuSet &set) -> bool { if (set.pu < 0) { - return; + return false; } const auto topo = get_topology(); if (!topo) { - return; + return false; } hwloc_cpuset_t cpuset = hwloc_bitmap_alloc(); if (!cpuset) { - return; + return false; } hwloc_bitmap_only(cpuset, static_cast(set.pu)); - /* Errors are intentionally ignored: pinning is performance-only, not a correctness requirement. */ - hwloc_set_cpubind(topo, cpuset, HWLOC_CPUBIND_THREAD | HWLOC_CPUBIND_STRICT); + /* A failure is not an error -- pinning is performance-only -- but it is reported rather than + * swallowed, so `barrier_groups = 0` can be told apart from "nothing was pinned". */ + const bool ok = hwloc_set_cpubind(topo, cpuset, HWLOC_CPUBIND_THREAD | HWLOC_CPUBIND_STRICT) == 0; hwloc_bitmap_free(cpuset); + return ok; +} + +/* ── Node-mask classification ──────────────────────────────────────────────── */ + +auto this_thread_cpumask() -> CpuMask { + CpuMask mask; + const auto topo = get_topology(); + if (!topo) { + return mask; + } + const hwloc_cpuset_t allowed = effective_allowed_cpuset(topo); + if (!allowed) { + return mask; + } + int pu = hwloc_bitmap_first(allowed); + while (pu >= 0) { + cpumask_set(mask, static_cast(pu)); + pu = hwloc_bitmap_next(allowed, pu); + } + hwloc_bitmap_free(allowed); + return mask; +} + +auto classify_node_mask(const std::vector &masks) -> NodeMask { + if (masks.size() < 2) { + return NodeMask::Shared; // nobody to be disjoint from + } + for (size_t a = 0; a < masks.size(); ++a) { + if (cpumask_count(masks[a]) == 0) { + return NodeMask::Shared; // a mask we could not read ⇒ cannot tell + } + for (size_t b = a + 1; b < masks.size(); ++b) { + for (size_t w = 0; w < masks[a].words.size(); ++w) { + if ((masks[a].words[w] & masks[b].words[w]) != 0) { + return NodeMask::Shared; // shares a PU ⇒ not a per-rank split + } + } + } + } + return NodeMask::PerRank; +} + +/* ── cpuset_domains ────────────────────────────────────────────────────────── */ + +auto cpuset_domains(const std::vector &sets) -> std::vector { + if (sets.empty()) { + return {}; + } + const auto cores = enumerate_physical_cores(); + std::vector domains; + domains.reserve(sets.size()); + for (const CpuSet &set : sets) { + const auto it = + std::find_if(cores.begin(), cores.end(), [&](const PhysicalCore &c) { return c.cpu == set.pu; }); + if (it == cores.end()) { + return {}; // a token naming no known core ⇒ flat barrier rather than a wrong grouping + } + domains.push_back(it->l3_domain); + } + return domains; } } // namespace monoprop::detail::partition diff --git a/cpp/monoprop/detail/partition/CpuTopology.h b/cpp/monoprop/detail/partition/CpuTopology.h index 1b60359e..d2b8ca06 100644 --- a/cpp/monoprop/detail/partition/CpuTopology.h +++ b/cpp/monoprop/detail/partition/CpuTopology.h @@ -24,13 +24,77 @@ #pragma once +#include +#include #include +#include #include #include "monoprop/detail/EnvConfig.h" namespace monoprop::detail::partition { +/*! + * @brief How the launcher divided this host's CPUs among the ranks sharing it. + * + * Mask *size* alone cannot tell the two apart — four ranks holding 32 cores each and four ranks + * sharing one 32-core mask both report 32 — and they need opposite placement. So the caller that + * owns a node-local communicator measures this by exchanging masks rather than inferring it. + * See partition_cpusets() and classify_node_mask(). + */ +enum class NodeMask { + Shared, //!< Every co-located rank sees the same mask (`mpirun --bind-to none`, or a lone process). + PerRank, //!< Each co-located rank was given its own disjoint slice (`srun --cpu-bind=cores`). +}; + +/*! + * @brief Number of PU indices a CpuMask can represent. + * + * Larger than any current machine's PU count, and generous relative to glibc's @c CPU_SETSIZE of + * 1024, because a PU index beyond the mask is silently invisible to classify_node_mask() — which + * would misread a per-rank split as shared. + */ +inline constexpr size_t kCpuMaskBits = 4096; + +/*! + * @brief A fixed-size affinity mask, as a trivially copyable POD. + * + * Deliberately not an hwloc bitmap: the node-local peer exchange in PartitionGroup ships this + * straight through @c MPI_Allgather as @c MPI_BYTE, so it must have the same size and layout in + * every rank and own no heap. Keeping it hwloc-free also lets classify_node_mask() be unit-tested + * without live hardware. + */ +struct CpuMask { + std::array words{}; //!< Bit @c i of word @c i/64 ⇒ PU @c i is allowed. +}; + +/*! + * @brief Set the bit for PU @p pu, ignoring an index beyond @c kCpuMaskBits. + */ +inline auto cpumask_set(CpuMask &mask, size_t pu) -> void { + if (pu < kCpuMaskBits) { + mask.words[pu / 64] |= (uint64_t{1} << (pu % 64)); + } +} + +/*! + * @brief Whether the bit for PU @p pu is set. + */ +[[nodiscard]] inline auto cpumask_test(const CpuMask &mask, size_t pu) -> bool { + return pu < kCpuMaskBits && ((mask.words[pu / 64] >> (pu % 64)) & uint64_t{1}) != 0; +} + +/*! + * @brief Number of PUs the mask names. + */ +[[nodiscard]] inline auto cpumask_count(const CpuMask &mask) -> size_t { + size_t total = 0; + for (const uint64_t word : mask.words) { + total += static_cast(std::popcount(word)); + } + return total; +} + /*! * @brief One physical CPU core the process may use, tagged with its L3 cache domain. * @@ -98,21 +162,75 @@ auto enumerate_physical_cores() -> std::vector; * @param n Number of partitions to place. * @param group_index This rank's 0-based index among the co-located ranks on the host. * @param group_count Total number of co-located ranks on the host. + * @param mask How the launcher divided the host, as measured by classify_node_mask(). Under + * @c NodeMask::PerRank, @p group_index / @p group_count are ignored — see below. * @returns Vector of @p n CpuSet tokens, or empty when @c monoprop_PARTITION_PINNING is disabled, - * hwloc is unavailable, or the host cannot provide @p group_count × @p n distinct cores. + * hwloc is unavailable, or the host cannot provide the requested distinct cores. + * + * @note enumerate_physical_cores() reports only cores inside this process's affinity mask, so under + * a @c PerRank mask the launcher has *already* handed each co-located rank a disjoint slice. + * Dividing by @p group_count a second time splits an already-split machine: @p group_count × + * @p n exceeds the share, topo_detail::placement_order() reads that as "host too small", and + * every rank silently runs unpinned — taking the two-level barrier's domains with it, because + * cpuset_domains() derives them from these sets. Measured on Deucalion at 8 ranks × 16 + * partitions: 437 µs/sync unpinned against 15.5 µs/sync placed. So a @c PerRank mask + * collapses to a single group and the whole slice is partitioned, leaving the cross-rank split + * to whoever imposed it. The default is @c Shared because that is both the pre-existing + * behaviour and the safe error direction: collapsing a genuinely shared mask would point every + * co-located rank at the *same* cores, breaking the disjointness invariant above, whereas + * dividing a @c PerRank mask only costs pinning. + */ +auto partition_cpusets(size_t n, size_t group_index = 0, size_t group_count = 1, NodeMask mask = NodeMask::Shared) + -> std::vector; + +/*! + * @brief This thread's current affinity mask, for a caller that compares it against its peers'. + * + * @returns The allowed-PU mask hwloc reports for the calling thread, or an all-zero mask when the + * topology or the cpubind query is unavailable — which classify_node_mask() reads as + * "cannot tell" ⇒ @c Shared. + */ +auto this_thread_cpumask() -> CpuMask; + +/*! + * @brief Classify how the launcher divided the host, given every co-located rank's mask. + * + * The caller gathers the masks because it owns the node-local communicator and this header stays + * free of MPI. Pure and hwloc-free, so it is testable without live hardware. + * + * @param masks One mask per rank sharing the host, in any order. + * @returns @c PerRank when the masks are pairwise disjoint and all non-empty; otherwise @c Shared — + * identical masks, partial overlap, or a mask that could not be read all land there, which + * is the conservative answer (see partition_cpusets()). + */ +auto classify_node_mask(const std::vector &masks) -> NodeMask; + +/*! + * @brief The locality domain each placement token lands in, in partition_cpusets() order. + * + * What a two-level PartitionBarrier groups by. Derived from the tokens rather than from the + * placement logic, so the two cannot drift apart. + * + * @param sets Placement tokens as returned by partition_cpusets(). + * @returns One L3-domain id per token, or empty (⇒ flat barrier) when @p sets is empty or one token + * names no PU that enumerate_physical_cores() knows. */ -auto partition_cpusets(size_t n, size_t group_index = 0, size_t group_count = 1) -> std::vector; +auto cpuset_domains(const std::vector &sets) -> std::vector; /*! * @brief Bind the calling thread to the PU identified by @p set. * * Allocates a temporary hwloc bitmap, sets the single bit for @c set.pu, and calls - * @c hwloc_set_cpubind with @c HWLOC_CPUBIND_THREAD | @c HWLOC_CPUBIND_STRICT. The call is - * best-effort: hwloc errors are silently ignored because only performance, not correctness, - * depends on successful pinning. + * @c hwloc_set_cpubind with @c HWLOC_CPUBIND_THREAD | @c HWLOC_CPUBIND_STRICT. * * @param set Placement token as returned by partition_cpusets(). A token with @c pu == -1 * is a no-op. + * @returns Whether the affinity actually took. Correctness never depends on pinning, so a failure + * is not an error — but it must not be invisible: @c barrier_groups = 0 has two legitimate + * causes (nothing was pinned, or each rank was confined to a single locality domain and so + * has nothing to fan in across), and without this they are indistinguishable from outside + * the process. CommProfile reports the count so a run states what happened instead of + * leaving it inferred from the timing. */ -auto pin_this_thread(const CpuSet &set) -> void; +[[nodiscard]] auto pin_this_thread(const CpuSet &set) -> bool; } // namespace monoprop::detail::partition diff --git a/cpp/monoprop/detail/partition/PartitionGroup.h b/cpp/monoprop/detail/partition/PartitionGroup.h index fc3f64fd..1fed5bad 100644 --- a/cpp/monoprop/detail/partition/PartitionGroup.h +++ b/cpp/monoprop/detail/partition/PartitionGroup.h @@ -57,9 +57,11 @@ class PartitionGroup { parent_(parent), partitions_(static_cast(n_partitions)), errs_(static_cast(n_partitions)) { - make_transport_(); + // Placement is decided before the transport, because the transport's barrier is grouped by the + // locality domain each partition will be pinned to. discover_node_peers_(); cpusets_ = topo_partition_cpusets(n_, node_rank_, node_size_); + make_transport_(); start_masters_(); // The masters are already running, so a ctor throw must not escape: ~PartitionGroup would never run, // and destroying joinable threads during unwinding calls std::terminate. @@ -70,6 +72,7 @@ class PartitionGroup { stop_and_join_(); throw; } + publish_pinned_count_(); } // Fresh transport and threads over the same parent; each partition is deep-copied on its new master, then @@ -79,10 +82,13 @@ class PartitionGroup { parent_(src.parent_), node_rank_(src.node_rank_), node_size_(src.node_size_), + // Copied, not re-measured: discover_node_peers_() is collective over the parent and this ctor is + // not. Losing it would silently place the clone differently from the group it was copied from. + node_mask_(src.node_mask_), partitions_(static_cast(src.n_)), errs_(static_cast(src.n_)) { - make_transport_(); cpusets_ = topo_partition_cpusets(n_, node_rank_, node_size_); + make_transport_(); // after cpusets_, as in the primary ctor start_masters_(); try { // see the primary ctor: a throw past live masters would std::terminate run_on_all([&](int r) { @@ -95,6 +101,7 @@ class PartitionGroup { stop_and_join_(); throw; } + publish_pinned_count_(); } auto operator=(const PartitionGroup &) -> PartitionGroup & = delete; @@ -146,15 +153,17 @@ class PartitionGroup { } // Free-function wrapper so the header compiles on non-Linux (where partition_cpusets returns {}). - static auto topo_partition_cpusets(int n, int group_index, int group_count) + auto topo_partition_cpusets(int n, int group_index, int group_count) const -> std::vector { return monoprop::detail::partition::partition_cpusets(static_cast(n), static_cast(group_index), - static_cast(group_count)); + static_cast(group_count), + node_mask_); } // Under an MPI parent, find how many ranks share this host and which we are, so each co-located rank - // pins to a disjoint core block (see partition_cpusets). Collective over `parent`; clones copy the result. + // pins to a disjoint core block (see partition_cpusets), and whether the launcher already sliced the + // host per rank. Collective over `parent`; clones copy the result. auto discover_node_peers_() -> void { #ifdef monoprop_ENABLE_MPI if (parent_.kind == mpi::Comm::Kind::Mpi && mpi::size(parent_) > 1) { @@ -162,19 +171,50 @@ class PartitionGroup { MPI_Comm_split_type(parent_.mpi, MPI_COMM_TYPE_SHARED, 0, MPI_INFO_NULL, &node); MPI_Comm_rank(node, &node_rank_); MPI_Comm_size(node, &node_size_); + // Exchange the raw affinity masks while the node communicator is open. Measured rather than + // inferred because mask *size* cannot distinguish "8 ranks holding 16 cores each" from "8 ranks + // sharing one 16-core mask": both leave a rank seeing 16 of the host's 128 CPUs, and the two + // need opposite placement. Guessing it wrong in the collapsing direction points every + // co-located rank at the same cores, which is worse than not pinning at all. + if (node_size_ > 1) { + const auto mine = monoprop::detail::partition::this_thread_cpumask(); + std::vector all(static_cast(node_size_)); + MPI_Allgather(&mine, sizeof(mine), MPI_BYTE, all.data(), sizeof(mine), MPI_BYTE, node); + node_mask_ = monoprop::detail::partition::classify_node_mask(all); + } MPI_Comm_free(&node); } #endif } + // Called once the first run_on_all has returned, which is the earliest point every master has passed + // its pin attempt: masters pin before taking any job, so the count is final and needs no extra + // synchronisation of its own. Reporting it is what makes `barrier_groups = 0` readable -- unpinned and + // one-domain-per-rank are both legitimate causes of it and are otherwise indistinguishable. + auto publish_pinned_count_() -> void { + const int pinned = pinned_count_.load(std::memory_order_relaxed); +#ifdef monoprop_ENABLE_MPI + if (hyb_) { + hyb_->note_pinned(pinned); + return; + } +#endif + if (shm_) { + shm_->note_pinned(pinned); + } + } + auto make_transport_() -> void { + // Empty unless the partitions are pinned and hwloc reported a topology ⇒ flat barrier (see + // PartitionBarrier); cpusets_ must already be set. + const std::vector domains = monoprop::detail::partition::cpuset_domains(cpusets_); #ifdef monoprop_ENABLE_MPI if (parent_.kind == mpi::Comm::Kind::Mpi && mpi::size(parent_) > 1) { - hyb_ = std::make_unique(parent_.mpi, n_); + hyb_ = std::make_unique(parent_.mpi, n_, domains); return; } #endif - shm_ = std::make_unique(n_); + shm_ = std::make_unique(n_, domains); } auto comm_for_(int r) -> mpi::Comm { #ifdef monoprop_ENABLE_MPI @@ -219,8 +259,14 @@ class PartitionGroup { } auto master_loop_(int rank) -> void { - if (!cpusets_.empty()) { - pin_this_thread(cpusets_[static_cast(rank)]); + // Relaxed is sufficient, but not because ordering does not matter here -- it is because the + // ordering comes from somewhere else. Each master increments once before it takes any job, so + // every increment happens-before that master's job completion, and job completion is already + // published to the facade thread through m_/done_count_. publish_pinned_count_ reads the counter + // only after run_on_all has returned, i.e. after acquiring that same mutex. Do not "strengthen" + // this to acq_rel expecting it to carry the edge itself; the edge is the dispatch mutex's. + if (!cpusets_.empty() && pin_this_thread(cpusets_[static_cast(rank)])) { + pinned_count_.fetch_add(1, std::memory_order_relaxed); } unsigned seen = 0; for (;;) { @@ -250,9 +296,12 @@ class PartitionGroup { } int n_; - mpi::Comm parent_; // enclosing communicator (size R) — decides the transport - int node_rank_ = 0; // this rank's index among the ranks sharing the host - int node_size_ = 1; // how many parent ranks share the host (1 unless MPI R>1) + mpi::Comm parent_; // enclosing communicator (size R) — decides the transport + int node_rank_ = 0; // this rank's index among the ranks sharing the host + int node_size_ = 1; // how many parent ranks share the host (1 unless MPI R>1) + // How the launcher divided the host among those ranks, measured in discover_node_peers_(). Shared until + // measured, which is both the single-rank truth and the safe default (see partition_cpusets). + monoprop::detail::partition::NodeMask node_mask_ = monoprop::detail::partition::NodeMask::Shared; std::unique_ptr shm_; // set iff R == 1 #ifdef monoprop_ENABLE_MPI std::unique_ptr hyb_; // set iff R > 1 @@ -261,6 +310,9 @@ class PartitionGroup { std::vector errs_; std::vector cpusets_; std::vector masters_; + // Incremented by each master that actually pinned. Relaxed throughout: written once per master before + // it takes work, read once after they are all up (see publish_pinned_count_). + std::atomic pinned_count_{0}; // Job dispatch: the facade thread publishes one job and waits for all masters to complete it. std::mutex m_; diff --git a/cpp/tests/CMakeLists.txt b/cpp/tests/CMakeLists.txt index fe5c7c0b..b60247d2 100644 --- a/cpp/tests/CMakeLists.txt +++ b/cpp/tests/CMakeLists.txt @@ -51,10 +51,83 @@ target_link_libraries( include(${CMAKE_CURRENT_LIST_DIR}/boost-test.cmake) +# Under monoprop_ENABLE_TSAN every test process needs the suppressions file, so it +# is attached as a test property rather than left to the caller's shell. +# history_size raises the shadow-memory depth so a report about a barrier word +# can still name the thread that wrote the previous generation; the spin loops +# retire far more accesses between two interesting ones than the default 2 keeps. +set(_monoprop_test_env_entries) +if(monoprop_ENABLE_TSAN) + list( + APPEND _monoprop_test_env_entries + "TSAN_OPTIONS=suppressions=${CMAKE_CURRENT_LIST_DIR}/tsan.supp:history_size=4:second_deadlock_stack=1" + ) +endif() + +# CTest runs every Boost case as its own process, so each one pays a full MPI_Init -- +# and MPI_Init initialises every fabric device it finds whether or not the process will +# ever send a message. Measured on a Deucalion login node (8 Mellanox HCAs): 8.8 s per +# process, against 0.2 s of user CPU, so 224 cases spend over half an hour opening +# adapters for single-process tests. Excluding the fabric drops that to 1.9 s. +# +# A per-case launch is ONE process: its world size is 1, the `*_World` cases skip +# themselves, and every remaining communicator is MPI_COMM_SELF. So the fabric cannot +# affect the outcome, only the startup cost -- which is why this is scoped to the +# `serial` variants through SERIAL_ENVIRONMENT and must never reach the MPI ones. On +# this node `OMPI_MCA_pml=^ucx` makes a 2-rank run of +# zero_cutoff_upper_atol_zero_is_exact_World hang indefinitely where it otherwise +# passes in 29 ms; the same hang reproduces on the pre-branch commit, so it belongs to +# the component selection, not to the engine. +# +# Exclusions (`^`), never a positive component list -- naming a component that must +# exist is how this breaks on the next machine: `vader` became `sm` in Open MPI 5, and +# `OMPI_MCA_btl=self,vader` there silently reduces to `self` alone. Unknown OMPI_MCA_* +# variables are ignored by other MPIs, so this is inert under MPICH. +option( + monoprop_TEST_EXCLUDE_MPI_FABRIC + "Skip fabric init in the single-process (serial) test variants to cut MPI_Init cost" + ON +) +set(_monoprop_serial_env_entries) +if(monoprop_TEST_EXCLUDE_MPI_FABRIC AND monoprop_ENABLE_MPI) + list( + APPEND _monoprop_serial_env_entries + "OMPI_MCA_pml=^ucx" + "OMPI_MCA_btl=^openib,ofi,uct" + ) +endif() + +# ENVIRONMENT takes ONE value holding every VAR=value, so the separators must be +# ESCAPED semicolons. A plain "${list}" does not work: a CMake list *is* a +# semicolon-joined string, so it flattens straight back out and every entry after the +# first is read as the name of the next property -- verified by inspecting +# `ctest --show-only=json-v1`, where the second entry vanished from ENVIRONMENT and +# reappeared as a stray property. Keep the escape, and re-check that output if this +# block is ever edited. +set(_monoprop_test_env) +if(_monoprop_test_env_entries) + string( + REPLACE ";" + "\;" + _monoprop_test_env_joined + "${_monoprop_test_env_entries}" + ) + set( + _monoprop_test_env + ENVIRONMENT + "${_monoprop_test_env_joined}" + ) +endif() + # Automatic discovery of unit tests. # Default CTest run includes per-case serial tests plus suite-level MPI variants # for monoprop_MPI_TEST_PROCS. -discover_tests(monoprop_unit_tests.x +discover_tests( + monoprop_unit_tests.x PROPERTIES - LABELS "unit" + LABELS + "unit" + ${_monoprop_test_env} + SERIAL_ENVIRONMENT + ${_monoprop_serial_env_entries} ) diff --git a/cpp/tests/boost-test.cmake b/cpp/tests/boost-test.cmake index 4f686b2b..1b54d3fe 100644 --- a/cpp/tests/boost-test.cmake +++ b/cpp/tests/boost-test.cmake @@ -21,12 +21,16 @@ if(NOT _monoprop_mpiexec_numproc_flag) set(_monoprop_mpiexec_numproc_flag "-n") endif() +# SERIAL_ENVIRONMENT holds VAR=value entries applied to the per-case `serial` variants ONLY, +# never to the MPI ones. The two groups differ in a way that matters for MPI: a per-case launch is +# one process, so its world size is 1 and no inter-process transport is ever used, whereas the MPI +# variants exchange real messages and need every component the fabric offers. function(discover_tests TARGET) cmake_parse_arguments( "" "" "WORKING_DIRECTORY" - "EXTRA_ARGS;PROPERTIES" + "EXTRA_ARGS;PROPERTIES;SERIAL_ENVIRONMENT" ${ARGN} ) @@ -66,6 +70,7 @@ function(discover_tests TARGET) "TEST_EXECUTABLE=$" -D "TEST_WORKING_DIR=${_WORKING_DIRECTORY}" -D "TEST_EXTRA_ARGS=${_EXTRA_ARGS}" -D "TEST_PROPERTIES=${_PROPERTIES}" -D + "TEST_SERIAL_ENVIRONMENT=${_SERIAL_ENVIRONMENT}" -D "TEST_LIST=${_TEST_LIST}" -D "CTEST_FILE=${ctest_tests_file}" -D "TEST_ENABLE_MPI_VARIANTS=${_enable_mpi_variants}" -D "TEST_MPI_NUMPROCS=${monoprop_MPI_TEST_PROCS}" -D diff --git a/cpp/tests/boostAddTests.cmake b/cpp/tests/boostAddTests.cmake index f0fd71b2..74b5ff00 100644 --- a/cpp/tests/boostAddTests.cmake +++ b/cpp/tests/boostAddTests.cmake @@ -39,6 +39,7 @@ endif() set(extra_args ${TEST_EXTRA_ARGS}) set(properties ${TEST_PROPERTIES}) +set(serial_env ${TEST_SERIAL_ENVIRONMENT}) set(script) set(tests) @@ -205,6 +206,8 @@ foreach(LINE ${LINES}) ${extra_args} LABELS serial + ENVIRONMENT + ${serial_env} ) endif() endforeach() diff --git a/cpp/tests/cpu_topology_tests.cpp b/cpp/tests/cpu_topology_tests.cpp index 5cda9980..9bb1bc80 100644 --- a/cpp/tests/cpu_topology_tests.cpp +++ b/cpp/tests/cpu_topology_tests.cpp @@ -58,7 +58,10 @@ BOOST_AUTO_TEST_CASE(cpu_topology_enumerate_and_place) { if (!one.empty()) { // A placement only comes back when topology discovery succeeded and pinning is enabled. BOOST_CHECK(!cores.empty()); - partition::pin_this_thread(one.front()); + // Not asserted: pinning is best-effort, and a restrictive cgroup or seccomp policy can refuse + // it without that being a defect. The return value exists so the outcome is reportable (see + // CommProfile's pinned count), not so a test can require success. + static_cast(partition::pin_this_thread(one.front())); // guard restores affinity on scope exit } // When topology discovery succeeds, a non-empty core list must produce a non-empty placement. @@ -173,3 +176,162 @@ BOOST_AUTO_TEST_CASE(cpu_topology_policy_uneven_domains) { BOOST_CHECK_EQUAL(order[1], 2); BOOST_CHECK_EQUAL(order[2], 4); } + +/* ── Node-mask classification and the per-rank collapse ───────────────────── */ + +// The mechanism the collapse in partition_cpusets exists for, stated without needing live hardware. +// Under a per-rank launcher split, enumerate_physical_cores() already reports only this rank's slice, +// so passing the node-wide group_count through asks placement_order for group_count x n cores out of +// a list that only ever held n -- and it correctly refuses. Refusing means unpinned, which also +// costs the two-level barrier its domains: measured at 437 us/sync against 15.5 us/sync placed. +BOOST_AUTO_TEST_CASE(cpu_topology_policy_per_rank_slice_starves_without_collapse) { + // One rank's slice under `srun --cpu-bind=cores`: 2 cores of a 16-core host, one L3 domain. + const std::vector slice = {{6, 0}, {7, 0}}; + + // Told the node-wide truth (8 ranks x 2 partitions), the request cannot be met from a slice. + BOOST_CHECK(placement_order(slice, 2, /*group_index=*/3, /*group_count=*/8).empty()); + + // Collapsed to a single group -- what NodeMask::PerRank does -- the same slice places fully. + const auto collapsed = placement_order(slice, 2, /*group_index=*/0, /*group_count=*/1); + BOOST_REQUIRE_EQUAL(collapsed.size(), 2u); + BOOST_CHECK_EQUAL(collapsed[0], 6); + BOOST_CHECK_EQUAL(collapsed[1], 7); +} + +// Two ranks holding disjoint masks are a per-rank split; identical masks are a shared one. Nothing else +// distinguishes them, which is the whole reason this is measured rather than inferred: mask *width* +// cannot tell "8 ranks holding 16 cores each" from "8 ranks sharing one 16-core mask", and the two need +// opposite placement. +BOOST_AUTO_TEST_CASE(cpu_topology_classify_node_mask_disjoint_vs_identical) { + partition::CpuMask a; + partition::CpuMask b; + partition::cpumask_set(a, 0); + partition::cpumask_set(a, 1); + partition::cpumask_set(b, 2); + partition::cpumask_set(b, 3); + BOOST_CHECK(partition::classify_node_mask({a, b}) == partition::NodeMask::PerRank); + BOOST_CHECK(partition::classify_node_mask({a, a}) == partition::NodeMask::Shared); + + // Partial overlap is pathological; Shared is the conservative answer because dividing never + // double-books a core within a rank, whereas collapsing points every rank at the same cores. + partition::CpuMask c = a; + partition::cpumask_set(c, 2); + BOOST_CHECK(partition::classify_node_mask({a, c}) == partition::NodeMask::Shared); + + // An unreadable mask arrives empty and must not be read as "disjoint from everything". + const partition::CpuMask empty; + BOOST_CHECK(partition::classify_node_mask({a, empty}) == partition::NodeMask::Shared); + // A lone rank has nobody to be disjoint from. + BOOST_CHECK(partition::classify_node_mask({a}) == partition::NodeMask::Shared); +} + +// Disjointness must be tested per PU, not by popcount or by first/last index: two masks of equal size +// in different words are disjoint, and two overlapping in one word are not. +BOOST_AUTO_TEST_CASE(cpu_topology_classify_node_mask_spans_word_boundaries) { + partition::CpuMask low; + partition::CpuMask high; + partition::cpumask_set(low, 5); + partition::cpumask_set(high, 200); // a different 64-bit word + BOOST_CHECK(partition::classify_node_mask({low, high}) == partition::NodeMask::PerRank); + BOOST_CHECK_EQUAL(partition::cpumask_count(low), 1u); + + partition::CpuMask also_high; + partition::cpumask_set(also_high, 200); + BOOST_CHECK(partition::classify_node_mask({high, also_high}) == partition::NodeMask::Shared); + + // A PU index past the mask is dropped rather than wrapping onto an unrelated bit. + partition::CpuMask overflow; + partition::cpumask_set(overflow, partition::kCpuMaskBits + 7); + BOOST_CHECK_EQUAL(partition::cpumask_count(overflow), 0u); + BOOST_CHECK(!partition::cpumask_test(overflow, 7)); +} + +// This thread's mask must be non-empty on any host where hwloc found cores, and must agree with the +// cores enumerate_physical_cores() reports -- the two are read from the same hwloc cpuset, so a +// disagreement means the mask exchange would classify against a different machine than the placement. +BOOST_AUTO_TEST_CASE(cpu_topology_this_thread_cpumask_covers_enumerated_cores) { + const auto cores = partition::enumerate_physical_cores(); + const auto mine = partition::this_thread_cpumask(); + if (cores.empty()) { + return; // no topology (hwloc unavailable); nothing to agree with + } + BOOST_CHECK(partition::cpumask_count(mine) > 0u); + for (const auto &core : cores) { + BOOST_CHECK(partition::cpumask_test(mine, static_cast(core.cpu))); + } +} + +/* ── Live placement under a restricted mask ───────────────────────────────── */ + +#if defined(__linux__) + +// A Slurm-style per-rank confinement, end to end: narrow this thread's affinity to two cores, then ask +// for both of them while passing the node-wide group_count a launcher would report. Nothing asserted +// this before the collapse landed, which is why the bug surfaced only through monoprop_COMM_PROFILE. +BOOST_AUTO_TEST_CASE(cpu_topology_per_rank_mask_still_places) { + const auto full = partition::enumerate_physical_cores(); + if (full.size() < 2) { + return; // need two cores to confine to + } + + const AffinityGuard guard; + + cpu_set_t confined; + CPU_ZERO(&confined); + CPU_SET(full[0].cpu, &confined); + CPU_SET(full[1].cpu, &confined); + if (sched_setaffinity(0, sizeof(confined), &confined) != 0) { + return; // not permitted here (seccomp, restrictive cgroup); nothing to assert + } + + // n = the whole share, and a group_count as if seven sibling ranks shared the node. + const auto sets = + partition::partition_cpusets(/*n=*/2, /*group_index=*/3, /*group_count=*/8, partition::NodeMask::PerRank); + BOOST_REQUIRE_EQUAL(sets.size(), 2u); + for (const auto &set : sets) { + // Never pin outside the mask the launcher gave us. + BOOST_CHECK(set.pu == full[0].cpu || set.pu == full[1].cpu); + } + // The two partitions must not land on the same core, and each must carry a domain for the barrier. + BOOST_CHECK(sets[0].pu != sets[1].pu); + BOOST_CHECK_EQUAL(partition::cpuset_domains(sets).size(), 2u); +} + +// The invariant the PerRank collapse most endangers: under a genuinely Shared mask, two co-located +// ranks must still deal themselves DIFFERENT cores. Collapsing unconditionally on "the mask is narrower +// than the machine" pointed every rank at the same cores, because mask width cannot distinguish a +// per-rank slice from a shared one -- which is why the caller measures it instead. +BOOST_AUTO_TEST_CASE(cpu_topology_shared_mask_keeps_co_located_ranks_disjoint) { + const auto full = partition::enumerate_physical_cores(); + if (full.size() < 4) { + return; // need two cores per rank for two ranks + } + + const AffinityGuard guard; + + // A shared mask narrower than the host: four cores that both ranks can see. + cpu_set_t shared; + CPU_ZERO(&shared); + for (size_t i = 0; i < 4; ++i) { + CPU_SET(full[i].cpu, &shared); + } + if (sched_setaffinity(0, sizeof(shared), &shared) != 0) { + return; + } + + const auto rank0 = + partition::partition_cpusets(/*n=*/2, /*group_index=*/0, /*group_count=*/2, partition::NodeMask::Shared); + const auto rank1 = + partition::partition_cpusets(/*n=*/2, /*group_index=*/1, /*group_count=*/2, partition::NodeMask::Shared); + BOOST_REQUIRE_EQUAL(rank0.size(), 2u); + BOOST_REQUIRE_EQUAL(rank1.size(), 2u); + for (const auto &a : rank0) { + for (const auto &b : rank1) { + // Two ranks sharing a core would have each one's busy-polling collectives starve the other's + // barrier spins -- the failure this whole placement path exists to avoid. + BOOST_CHECK(a.pu != b.pu); + } + } +} + +#endif // __linux__ diff --git a/cpp/tests/env_config_tests.cpp b/cpp/tests/env_config_tests.cpp index 77ff0955..595a8d4c 100644 --- a/cpp/tests/env_config_tests.cpp +++ b/cpp/tests/env_config_tests.cpp @@ -14,9 +14,12 @@ #include +#include #include #include "monoprop/detail/EnvConfig.h" +#include "monoprop/detail/mpi/CpuRelax.h" +#include "monoprop/detail/mpi/PartitionBarrier.h" using monoprop::config::detail::parse_flag; using monoprop::config::detail::parse_positive_int; @@ -59,6 +62,27 @@ BOOST_AUTO_TEST_CASE(env_config_parse_positive_int_range) { BOOST_CHECK(parse_positive_int("1000000") == std::optional(1'000'000)); // inclusive upper bound } +// Observe the budget the barrier actually resolved, rather than recomputing +// `parse_positive_int(...).value_or(kDefault)` here: an assertion of that shape passes even if the +// barrier is never wired to the setting at all, which is the only thing worth checking. config::get() +// caches on first call, so the env path is unreachable in-process; the injectable override is not. +BOOST_AUTO_TEST_CASE(env_config_spin_budget_reaches_the_barrier) { + using namespace std::chrono_literals; + BOOST_CHECK_GT(monoprop::mpi::detail::kDefaultSpinBudgetUs, 0); + + // Default construction lands on the configured value, else the compiled-in default -- never on a + // zero-length spin, which would send every waiter straight to sched_yield. + const monoprop::mpi::PartitionBarrier configured(2); + const auto expected = std::chrono::microseconds{ + monoprop::config::get().spin_budget_us.value_or(monoprop::mpi::detail::kDefaultSpinBudgetUs)}; + BOOST_CHECK(configured.spin_budget() == expected); + BOOST_CHECK_GT(configured.spin_budget().count(), 0); + + // An explicit override wins over both, which is what lets the default be swept and justified. + const monoprop::mpi::PartitionBarrier overridden(2, {}, 25us); + BOOST_CHECK(overridden.spin_budget() == 25us); +} + BOOST_AUTO_TEST_CASE(env_config_settings_cached_singleton) { const auto &a = monoprop::config::get(); const auto &b = monoprop::config::get(); diff --git a/cpp/tests/hybrid_comm_tests.cpp b/cpp/tests/hybrid_comm_tests.cpp index 5f82d266..60c042ed 100644 --- a/cpp/tests/hybrid_comm_tests.cpp +++ b/cpp/tests/hybrid_comm_tests.cpp @@ -327,4 +327,131 @@ BOOST_AUTO_TEST_CASE(hybrid_comm_poison_releases_waiters) { } } +// The cases above give every destination the SAME count per source, which cannot tell the +// (rank, dest partition, source partition) index order the offset tables are built from from its +// transpose. So drive counts that depend on source AND destination, and tag every element with both ends +// of its leg: a swapped index or a wrong base then misroutes payload instead of yielding a +// coincidentally-correct total. S=5 (> the 3 used above) leaves R*S^2 room to go wrong. +BOOST_AUTO_TEST_CASE(hybrid_comm_alltoallv_resolve_asymmetric_counts) { + if (world_size() < 2) { + return; + } + const int R = world_size(); + const int S = 5; + const int P = R * S; + const int rounds = 12; + // Deliberately not symmetric under swapping src/dst, and hits 0 for some legs. + const auto leg_len = [](int src, int dst, int round) { return (src * 7 + dst * 3 + round) % 5; }; + const auto tag = [](int src, int dst, int j) { return ((src * 1000 + dst) * 100) + j; }; + std::atomic failures{0}; + auto errs = run_hybrid(S, [&](HybridComm &hyb, int u) { + Comm c = Comm::make_hybrid(&hyb, u); + const int g = monoprop::mpi::rank(c); + std::vector recv; + std::vector rc(static_cast(P)), rd(static_cast(P)); + for (int round = 0; round < rounds; ++round) { + std::vector send; + std::vector sc(static_cast(P)), sd(static_cast(P)); + int off = 0; + for (int d = 0; d < P; ++d) { + const int len = leg_len(g, d, round); + sc[static_cast(d)] = len; + sd[static_cast(d)] = off; + for (int j = 0; j < len; ++j) { + send.push_back(tag(g, d, j)); + } + off += len; + } + hyb.alltoallv_resolve(u, + {.send = send.data(), + .send_counts = sc.data(), + .send_displs = sd.data(), + .recv = recv, + .recv_counts = rc.data(), + .recv_displs = rd.data()}, + monoprop::mpi::datatype::get()); + int expected_total = 0; + for (int src = 0; src < P; ++src) { + const int len = leg_len(src, g, round); + if (rc[static_cast(src)] != len || rd[static_cast(src)] != expected_total) { + failures.fetch_add(1); + } + for (int j = 0; j < len; ++j) { + if (recv[static_cast(expected_total + j)] != tag(src, g, j)) { + failures.fetch_add(1); + } + } + expected_total += len; + } + if (static_cast(recv.size()) != expected_total) { + failures.fetch_add(1); + } + } + }); + for (const auto &e : errs) { + BOOST_CHECK(e == nullptr); + } + BOOST_CHECK_EQUAL(failures.load(), 0); +} + +// Same asymmetric legs through the count-exchange + flat-alltoallv pair, which uses the other +// sizing entry point (recv counts published per partition rather than resolved from the wire). +BOOST_AUTO_TEST_CASE(hybrid_comm_alltoall_counts_then_alltoallv_asymmetric) { + if (world_size() < 2) { + return; + } + const int R = world_size(); + const int S = 4; + const int P = R * S; + const auto leg_len = [](int src, int dst) { return (src * 5 + dst * 11) % 4; }; + const auto tag = [](int src, int dst, int j) { return ((src * 1000 + dst) * 100) + j; }; + std::atomic failures{0}; + auto errs = run_hybrid(S, [&](HybridComm &hyb, int u) { + const int g = hyb.global_rank(u); + std::vector sc(static_cast(P)), sd(static_cast(P)); + std::vector rc(static_cast(P)), rd(static_cast(P)); + std::vector send; + int off = 0; + for (int d = 0; d < P; ++d) { + const int len = leg_len(g, d); + sc[static_cast(d)] = len; + sd[static_cast(d)] = off; + for (int j = 0; j < len; ++j) { + send.push_back(tag(g, d, j)); + } + off += len; + } + hyb.alltoall_counts(u, sc.data(), rc.data()); + int total = 0; + for (int src = 0; src < P; ++src) { + if (rc[static_cast(src)] != leg_len(src, g)) { + failures.fetch_add(1); + } + rd[static_cast(src)] = total; + total += rc[static_cast(src)]; + } + std::vector recv(static_cast(total)); + hyb.alltoallv(u, + monoprop::mpi::FlatAlltoallvArgs{.send = send.data(), + .send_counts = sc.data(), + .send_displs = sd.data(), + .recv = recv.data(), + .recv_counts = rc.data(), + .recv_displs = rd.data()} + .bytes(), + monoprop::mpi::datatype::get()); + for (int src = 0; src < P; ++src) { + for (int j = 0; j < rc[static_cast(src)]; ++j) { + if (recv[static_cast(rd[static_cast(src)] + j)] != tag(src, g, j)) { + failures.fetch_add(1); + } + } + } + }); + for (const auto &e : errs) { + BOOST_CHECK(e == nullptr); + } + BOOST_CHECK_EQUAL(failures.load(), 0); +} + #endif // monoprop_ENABLE_MPI diff --git a/cpp/tests/mpi_distributed_layer_equivalence.cpp b/cpp/tests/mpi_distributed_layer_equivalence.cpp index 152eb2b7..05a86e72 100644 --- a/cpp/tests/mpi_distributed_layer_equivalence.cpp +++ b/cpp/tests/mpi_distributed_layer_equivalence.cpp @@ -200,4 +200,134 @@ BOOST_AUTO_TEST_CASE(hybrid_mpi_partition_energy_and_size_equivalence) { BOOST_CHECK_EQUAL(n_serial, n_hybrid_global); } +// An emit gate that keeps one half of a matched rotation pair and drops the other is what makes the +// cross-rank leader/follower exchange asymmetric, and nothing in the suite exercised that with real +// ranks. Both atol arguments are nullopt in every case above; exact_upper_atol_rescue.cpp and +// mpi_fresh_insert_equivalence.cpp pass upper_atol = 0, which rescues every truncated term and so +// restores the symmetry they appear to test. The only asymmetric-gate case with more than one partition +// is partition_equivalence_tests.cpp's Pauli energy case, which runs over the in-process ShmComm and +// never crosses a rank boundary. A protocol defect that appears only when the leader emits and the +// follower does not would therefore pass the entire suite. +// +// graph_path selects which sink resolves the exchange: build_graph goes through GraphSink, which +// pre-sizes a response slot per incoming query, while propagate goes through ContractSink, which writes +// a half-rotation record instead. They fail differently, so both are covered. +// LihFixture (n=12), not the random_exact fixture the rest of this file uses. That one truncates +// nothing at any setting -- partition_equivalence_tests.cpp already records this -- and the first run of +// this case proved it from the other direction: the ungated term count was **3**, so no coefficient +// threshold could drop anything and the case passed while exercising none of the asymmetry it exists to +// cover. A gate that cannot bite is the same defect as no gate at all. +template +auto run_gated(const CaseData& data, + MPI_Comm comm, + std::optional lower_atol, + std::optional only_rotate_len_k, + bool graph_path) -> std::pair { + // only_rotate_len_k is a gate-application argument, not a construction one -- the constructor's + // ninth parameter is basis_change. Passing it here instead is a compile error, which is the useful + // kind of mistake to make. + MonomialPropagator sim(data.hamiltonian, + kCutoff, + data.initial_state, + std::nullopt, + comm, + lower_atol, + std::nullopt, + CutoffType::Length); + double e = 0.0; + if (graph_path) { + sim.build_graph(data.majoranas, + data.param_inds, + data.gen_coeffs, + std::nullopt, + std::nullopt, + only_rotate_len_k); + auto fn = sim.expectation_value_functional(); + e = fn(data.parameters); + } + else { + sim.propagate(data.majoranas, data.param_inds, data.gen_coeffs, data.parameters, only_rotate_len_k); + e = sim.expectation_value({}); + } + // size() is per-rank -- the facade holds only its local partitions -- so the comparable quantity is + // the sum over the propagator's own communicator. Over MPI_COMM_SELF that is the identity, which is + // what lets the serial reference and the distributed run share this helper. + return {e, mpi::allreduce_sum(sim.size(), comm)}; +} + +BOOST_AUTO_TEST_CASE(rank_count_matches_under_an_asymmetric_emit_gate) { + if (mpi::size(MPI_COMM_WORLD) < 2) { + BOOST_TEST_MESSAGE("Skipping asymmetric-gate case: requires at least 2 ranks."); + return; + } + constexpr size_t kLihModes = LihFixture::n_modes; + const LihFixture lih; + const CaseData& data = lih.data; + + // Ungated reference count. Without it this case could pass while every atol below was too small to + // drop anything -- that is, while exercising none of the asymmetry it exists to cover, which is the + // hole being closed. Checked once at the end, since one biting value is enough. + const auto [e_ref, n_ungated] = run_gated(data, MPI_COMM_SELF, std::nullopt, std::nullopt, true); + static_cast(e_ref); + bool gate_bit = false; + // Tracked separately from gate_bit. A single flag OR-ed across every combination is satisfied by the + // propagate cases alone, which would leave the GraphSink path -- where a mis-sized or uninitialised + // response slot would live -- covered in name only. lower_atol turns out not to reduce build_graph's + // term count at all, so the length cap is what has to bite there. + bool graph_gate_bit = false; + + for (const double atol : {1e-2, 1e-3, 1e-6}) { + for (const bool graph_path : {true, false}) { + const auto [e_serial, n_serial] = run_gated(data, MPI_COMM_SELF, atol, std::nullopt, graph_path); + const auto [e_world, n_world] = run_gated(data, MPI_COMM_WORLD, atol, std::nullopt, graph_path); + gate_bit = gate_bit || n_serial < n_ungated; + // Reported, not just asserted: BOOST_TEST_CONTEXT only prints on failure, so without this a + // passing run cannot show how much asymmetry it actually exercised -- and "the gate bit" is + // the one fact that decides whether this case is worth anything. + BOOST_TEST_MESSAGE("lower_atol=" << atol << (graph_path ? " build_graph" : " propagate") + << " ungated=" << n_ungated << " gated=" << n_serial + << " dropped=" << (n_ungated - n_serial)); + BOOST_TEST_CONTEXT("lower_atol=" << atol << (graph_path ? " build_graph" : " propagate") + << " n_serial=" << n_serial << " n_world=" << n_world) { + // Energy is tolerance-equal across rank counts, never bit-equal: the reduction order + // differs. The term count, by contrast, must agree exactly -- a rotation dropped or + // double-counted by the asymmetric path shows up here and essentially nowhere else. + BOOST_TEST(near(e_serial, e_world)); + BOOST_CHECK_EQUAL(n_serial, n_world); + } + } + } + + // A length cap gates on the generator's length rather than on a coefficient, so it drives the same + // protocol with a different survivor set. + for (const size_t cap : {size_t{2}, size_t{3}}) { + for (const bool graph_path : {true, false}) { + const auto [e_serial, n_serial] = run_gated(data, MPI_COMM_SELF, 1e-3, cap, graph_path); + const auto [e_world, n_world] = run_gated(data, MPI_COMM_WORLD, 1e-3, cap, graph_path); + if (graph_path) { + graph_gate_bit = graph_gate_bit || n_serial < n_ungated; + } + BOOST_TEST_MESSAGE("cap=" << cap << (graph_path ? " build_graph" : " propagate") << " ungated=" << n_ungated + << " gated=" << n_serial << " dropped=" << (n_ungated - n_serial)); + BOOST_TEST_CONTEXT("only_rotate_len_k=" << cap << (graph_path ? " build_graph" : " propagate") + << " n_serial=" << n_serial << " n_world=" << n_world) { + BOOST_TEST(near(e_serial, e_world)); + BOOST_CHECK_EQUAL(n_serial, n_world); + } + } + } + + BOOST_CHECK_MESSAGE(gate_bit, + "no lower_atol reduced the term count below the ungated " << n_ungated + << ": the gate never bit, so this " + "case exercised no asymmetry"); + // Measured on LiH/n=12: lower_atol drops 208-232 of 866 on the propagate path and *nothing* on the + // build_graph path, while the length cap drops 32 on both. So this second check is what keeps + // GraphSink covered, and it fails independently of the one above -- if a fixture or default change + // ever makes the cap stop biting, the loss of coverage is reported rather than silent. + BOOST_CHECK_MESSAGE(graph_gate_bit, + "no gate reduced build_graph's term count below the ungated " + << n_ungated << ": the GraphSink path was not exercised asymmetrically"); +} + } // namespace diff --git a/cpp/tests/partition_barrier_tests.cpp b/cpp/tests/partition_barrier_tests.cpp new file mode 100644 index 00000000..b556381c --- /dev/null +++ b/cpp/tests/partition_barrier_tests.cpp @@ -0,0 +1,175 @@ +// Copyright 2026 Algorithmiq +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include + +#include +#include +#include +#include +#include + +#include "monoprop/detail/mpi/PartitionBarrier.h" + +using monoprop::mpi::PartitionBarrier; +using monoprop::mpi::ShmCommPoisoned; + +namespace { + +// Every participant stamps its own slot with the round number; after the barrier every slot must read as +// stamped, so anyone released early sees a stale one. Tested directly because the two-level path needs +// pinning and >=2 L3 domains to engage through a real ShmComm, which no test host is guaranteed to have. +auto stamp_rounds(int n, const std::vector &groups, int rounds) -> std::vector { + PartitionBarrier barrier(n, groups); + std::vector slots(static_cast(n), -1); + std::vector errs(static_cast(n)); + std::atomic mismatches{0}; + std::vector threads; + threads.reserve(static_cast(n)); + for (int p = 0; p < n; ++p) { + threads.emplace_back([&, p] { + try { + for (int round = 0; round < rounds; ++round) { + slots[static_cast(p)] = round; + barrier.sync(p); + for (int q = 0; q < n; ++q) { + if (slots[static_cast(q)] != round) { + ++mismatches; + } + } + barrier.sync(p); // peers must finish reading before the next stamp overwrites + } + } + catch (...) { + errs[static_cast(p)] = std::current_exception(); + } + }); + } + for (auto &t : threads) { + t.join(); + } + BOOST_CHECK_EQUAL(mismatches.load(), 0); + return errs; +} + +auto no_errors(const std::vector &errs) -> void { + for (const auto &e : errs) { + BOOST_CHECK(e == nullptr); + } +} + +} // namespace + +// No group ids ⇒ the flat barrier, the path every unpinned run takes. +BOOST_AUTO_TEST_CASE(partition_barrier_flat_releases_together) { + for (const int n : {1, 2, 3, 8}) { + no_errors(stamp_rounds(n, {}, 20)); + } +} + +// Two-level: same semantics, and the group sizes are deliberately uneven and not powers of two, so a +// barrier that mixed up "last in group" with "last overall" would deadlock or release early. +BOOST_AUTO_TEST_CASE(partition_barrier_grouped_releases_together) { + no_errors(stamp_rounds(8, {0, 0, 0, 0, 1, 1, 1, 1}, 20)); + no_errors(stamp_rounds(7, {0, 0, 0, 1, 1, 2, 2}, 20)); + no_errors(stamp_rounds(6, {3, 9, 3, 9, 3, 9}, 20)); // interleaved, non-contiguous domain ids + no_errors(stamp_rounds(5, {0, 1, 1, 1, 1}, 20)); // a domain of one +} + +// One domain is no reason to pay for a root barrier: it must behave exactly like the flat path. +BOOST_AUTO_TEST_CASE(partition_barrier_single_group_is_flat) { + no_errors(stamp_rounds(4, {2, 2, 2, 2}, 20)); +} + +// A group id list that does not cover every participant is a programming error upstream, not a reason +// to hang: it falls back to flat rather than indexing out of range. +BOOST_AUTO_TEST_CASE(partition_barrier_short_group_list_falls_back) { + no_errors(stamp_rounds(4, {0, 1}, 20)); +} + +// All-singleton domains are what an unreadable cache/NUMA topology produces. The two-level path would +// then have every participant represent itself at the root -- the flat barrier plus a cache line and an +// extra release store each -- so it must report and behave as flat, and group_count() must not claim a +// level that is not running. +BOOST_AUTO_TEST_CASE(partition_barrier_all_singleton_groups_are_flat) { + const PartitionBarrier singletons(4, {0, 1, 2, 3}); + BOOST_CHECK_LT(singletons.group_count(), 2); + no_errors(stamp_rounds(4, {0, 1, 2, 3}, 20)); + + // A grouping with real fan-in still engages, so the guard has not disabled the optimization wholesale. + const PartitionBarrier grouped(4, {0, 0, 1, 1}); + BOOST_CHECK_EQUAL(grouped.group_count(), 2); +} + +// poison() must release waiters in BOTH levels: a peer that throws mid-round leaves the others parked +// on their domain's word, not the root's. +BOOST_AUTO_TEST_CASE(partition_barrier_poison_releases_grouped_waiters) { + constexpr int kN = 6; + const std::vector groups{0, 0, 0, 1, 1, 1}; + PartitionBarrier barrier(kN, groups); + std::atomic poisoned_count{0}; + std::vector threads; + threads.reserve(kN - 1); + for (int p = 0; p < kN - 1; ++p) { // participant kN-1 never arrives + threads.emplace_back([&, p] { + try { + barrier.sync(p); + } + catch (const ShmCommPoisoned &) { + ++poisoned_count; + } + }); + } + // Let the arrivers reach the barrier before poisoning, so this exercises the release of parked + // waiters rather than the post-barrier poison check. + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + barrier.poison(); + for (auto &t : threads) { + t.join(); + } + // The last arriver of the complete domain parks at the root; the rest park on their domain word. + BOOST_CHECK_EQUAL(poisoned_count.load(), kN - 1); +} + +// reset() after an aborted round must leave no partial arrival count behind, in any group. +BOOST_AUTO_TEST_CASE(partition_barrier_reset_clears_partial_arrivals) { + constexpr int kN = 4; + const std::vector groups{0, 0, 1, 1}; + PartitionBarrier barrier(kN, groups); + std::thread lone([&] { + try { + barrier.sync(0); // arrives alone, then is released by the poison below + } + catch (const ShmCommPoisoned &) { + } + }); + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + barrier.poison(); + lone.join(); + barrier.reset(); + + std::atomic through{0}; + std::vector threads; + threads.reserve(kN); + for (int p = 0; p < kN; ++p) { + threads.emplace_back([&, p] { + barrier.sync(p); + ++through; + }); + } + for (auto &t : threads) { + t.join(); + } + BOOST_CHECK_EQUAL(through.load(), kN); +} diff --git a/cpp/tests/tsan.supp b/cpp/tests/tsan.supp new file mode 100644 index 00000000..e5653562 --- /dev/null +++ b/cpp/tests/tsan.supp @@ -0,0 +1,35 @@ +# ThreadSanitizer suppressions for the monoprop C++ unit tests +# (monoprop_ENABLE_TSAN=ON). Wired in as TSAN_OPTIONS=suppressions=... by +# cpp/tests/CMakeLists.txt, so a `ctest` run picks it up with no extra setup. +# +# Everything here is a *third-party* entry. There is deliberately no suppression +# for anything under cpp/monoprop/: the point of the TSan configuration is to +# machine-check PartitionBarrier's memory orderings, and a suppression there +# would silently retract that check. If a monoprop access is benign, say so in a +# comment at the access and, where the tool cannot see the ordering, annotate it +# — do not hide it here. + +# --------------------------------------------------------------------------- +# MPI (monoprop_ENABLE_MPI=ON builds only). +# +# OpenMPI/PMIx are not built with instrumentation, and their progress engine +# races on its own state by design (lock-free free lists, opal_progress +# callbacks, the async event thread). TSan cannot see their synchronization, so +# every report from inside them is noise. `called_from_lib` needs the shared +# object's soname, which is what OpenMPI 5.x installs. +# --------------------------------------------------------------------------- +called_from_lib:libmpi.so +called_from_lib:libopen-pal.so +called_from_lib:libopen-rte.so +called_from_lib:libpmix.so +called_from_lib:libevent_core.so +called_from_lib:libevent_pthreads.so +race:^ompi_ +race:^opal_ +race:^mca_ +race:^pmix +race:^PMIx +race:^orte_ +# The MPI_Init_thread/MPI_Finalize pair leaks a progress thread whose teardown +# races with the runtime's own atexit work; not monoprop's memory. +thread:^opal_progress_ diff --git a/cspell.json b/cspell.json index 4f86409b..b3ab6776 100644 --- a/cspell.json +++ b/cspell.json @@ -17,8 +17,13 @@ "backend", "barriered", "bitstring", + "btl", + "CCX", "coeffs", "conj", + "cpumask", + "cpuset", + "cpusets", "ctest", "cutoff", "inplace", @@ -39,20 +44,30 @@ "Fock", "Hamiltonian", "Hartree", + "HCAs", + "hwloc", "majorana", "Majorana", "Majoranas", "majoranic", + "Mellanox", "memray", "microbenchmarks", "mpiexec", + "ofi", + "openib", "oversubscribe", "Pauli", "Paulis", + "pml", + "popcount", "qubit", "qubits", "Remigio", "tracemalloc", + "uct", + "ucx", + "vader", "schrodinger", "simulable", "unnormalized", diff --git a/docs/content/docs/building.mdx b/docs/content/docs/building.mdx index aa0fe949..48f16d95 100644 --- a/docs/content/docs/building.mdx +++ b/docs/content/docs/building.mdx @@ -110,6 +110,67 @@ uv sync --all-extras -v --config-settings=cmake.build-type=Debug ctest --test-dir build/editable/Debug ``` +### ThreadSanitizer tree + +`monoprop_ENABLE_TSAN=ON` builds with `-fsanitize=thread`. It is opt-in and +composes with any build type rather than replacing one, because the partition +threading layer — the pinned partition masters and the sense-reversing +`PartitionBarrier` they synchronise on — is only worth auditing at the +optimization level it ships with. `-g` is added regardless of build type, since a +report without line tables cannot be attributed to a memory ordering. + +```bash +uv sync --all-extras -v --reinstall-package monoprop --no-cache \ + --config-settings-package="monoprop:cmake.build-type=RelWithDebInfo" \ + --config-settings-package="monoprop:cmake.define.monoprop_ENABLE_TSAN=ON" +ctest --test-dir build/editable/RelWithDebInfo --output-on-failure -L serial +``` + +`cpp/tests/CMakeLists.txt` attaches `TSAN_OPTIONS` — including the +`cpp/tests/tsan.supp` suppressions — to every discovered test, so no extra +environment is needed. A reported race makes the owning test exit non-zero. +Expect a large slowdown, so prefer running the threading suites on their own: + +```bash +ctest --test-dir build/editable/RelWithDebInfo --output-on-failure \ + -R 'partition_barrier_|shm_comm_|partition_' +``` + +`tsan.supp` holds third-party entries only (the MPI runtime, whose progress +engine TSan cannot see the synchronisation of). Nothing under `cpp/monoprop/` is +suppressed: a suppression there would silently retract the check the +configuration exists to provide. + +### Iterating on a C++ test + +`uv sync` builds in a temporary directory and installs only the wheel, and the +test binary is not a wheel target — so editing a test and re-syncing leaves +`bin/monoprop_unit_tests.x` as it was. A stale binary does not announce itself: it +passes, or it reports `no test cases matching filter` for a case you just wrote. +Compare the binary's mtime against the source's before trusting either outcome. + +`cmake --build build/editable/` cannot repair that tree, because its cache +pins the build-isolation interpreter that scikit-build-core created and deleted, so +regenerating `build.ninja` fails. The `skbuild-*` presets adopt an existing tree +and set no cache variables of their own, so they inherit the same failure. To +iterate without a full `uv sync` per edit, configure a standalone tree instead — +nanobind is a build-isolation-only dependency and is absent from `.venv`, so it has +to be supplied by hand: + +```bash +uv pip install --python .venv/bin/python --target /tmp/nbdeps nanobind +cmake -S . -B /tmp/mp-tests -G Ninja -DCMAKE_BUILD_TYPE=Release -DSKBUILD=2 \ + -DSKBUILD_PROJECT_VERSION_FULL=0.0.0 -DSKBUILD_SABI_COMPONENT= \ + -Dnanobind_DIR=/tmp/nbdeps/nanobind/cmake \ + -DPython_EXECUTABLE="$PWD/.venv/bin/python" \ + -Dmonoprop_MAX_NUM_MODES=32 # narrow the templates; much faster to build +cmake --build /tmp/mp-tests +ctest --test-dir /tmp/mp-tests --output-on-failure -L serial +``` + +This leaves the editable install alone, so the Python side keeps working while you +rebuild the tests. Add `-Dmonoprop_ENABLE_MPI=ON` to get the `mpi`-labelled cases. + ### Related workflows - Use `just test-wide` for the 64-bit `monoprop_WIDE_TERM_INDEX` configuration. diff --git a/docs/content/docs/features/parallelism.mdx b/docs/content/docs/features/parallelism.mdx index bf57b046..3c99c014 100644 --- a/docs/content/docs/features/parallelism.mdx +++ b/docs/content/docs/features/parallelism.mdx @@ -22,6 +22,20 @@ gives each partition a pinned worker thread that runs it serially. Each partitio keeps its own small, cache-resident term index; a gate is applied to all partitions at once, synchronised by a lightweight barrier, and anticommuting terms whose partner lives in another partition are resolved through a per-gate exchange. +When the partitions are pinned and span more than one locality domain, that barrier +fans in within each domain and then across domains, so both the arrival counter and +the release store stay inside one cache slice instead of crossing the socket +interconnect. A domain is the deepest cache level that more than one core shares — +an L3 slice or CCX on x86 — falling back to the NUMA node on parts that report no +shared cache at all. + +Placement respects whatever CPU mask the process was launched under. When several +ranks share a node, they compare masks with each other to decide how the node was +divided: given a mask each (`srun --cpu-bind=cores --cpus-per-task=N`), every rank +partitions the cores it was actually given, and given one mask between them +(`mpirun --bind-to none`), they divide it so that no two ranks land on the same +core. The two cases look identical from a single rank — each sees fewer CPUs than +the machine has — which is why they are measured rather than guessed. ## Runtime environment variables @@ -30,6 +44,9 @@ whose partner lives in another partition are resolved through a per-gate exchang | `monoprop_NUM_THREADS` | one partition per physical core | Caps the number of partitions. Set it to run fewer partitions than cores. | | `monoprop_PARTITIONS` | `auto` | `auto` = one partition per core (capped by `monoprop_NUM_THREADS`); an integer `N` = exactly `N` partitions; `off` = one partition holding the whole operator. | | `monoprop_PARTITION_PINNING` | `on` | `0`/`false`/`no` disables pinning each partition to a core. Supported on platforms where hwloc can bind threads. | +| `monoprop_COMM_PROFILE` | `off` | `1`/`true`/`yes` makes each transport print one `COMMPROF` line per rank to stderr when it is destroyed, accounting for where its collectives' wall time went. The line is written to file descriptor 2 from the destructor, so under `pytest` it needs `-s` to survive the default fd-level capture, and the counters accumulate over the whole process lifetime — compare them against the sum of the operations that ran, not against one. Diagnostic only; off costs one branch per collective phase. | +| `monoprop_SPIN_BUDGET_US` | `30` | Microseconds a partition spends spinning on-core at a barrier before it starts yielding. Longer suits a pinned one-partition-per-core layout; shorter suits an oversubscribed one, where a spinner holds the core its late peer needs. A tuning knob — the default is calibrated, not arbitrary. | +| `monoprop_BARRIER_GROUPING` | `on` | `0`/`false`/`no` forces the flat barrier while leaving pinning on. The two-level barrier takes its domains from the cpusets, so disabling pinning to get a flat barrier also unpins — which makes "is the second level worth it?" unanswerable from the outside. This knob separates the two so it can be measured on a real workload. Diagnostic and tuning; the second level is a win at high partition counts and a small loss on collectives short enough for the extra hop to dominate. | ```bash # Run 8 partitions instead of one-per-core: diff --git a/docs/content/docs/testing.mdx b/docs/content/docs/testing.mdx index 2485508c..4bc8dac8 100644 --- a/docs/content/docs/testing.mdx +++ b/docs/content/docs/testing.mdx @@ -37,6 +37,14 @@ SKBUILD_CMAKE_ARGS="-Dmonoprop_ENABLE_MPI=ON" \ mpiexec --allow-run-as-root -n 2 uv run --no-sync python -m pytest tests --with-mpi ``` +The C++ engine writes its diagnostics straight to file descriptor 2 — the +`COMMPROF` line from `monoprop_COMM_PROFILE` comes out of a transport destructor, +see [Parallelism and distribution](/features/parallelism). pytest's default +capture is fd-level: it replaces fd 2 for each test and discards the buffer when +the test passes, so under pytest those lines need `-s` to survive. Count the ones +you expect rather than reading the log for changes — an instrument that never +fired and an instrument reporting no change look exactly alike. + ### C++ unit tests The C++ tests run through CTest. The build tree is produced by `uv sync` (via @@ -73,6 +81,32 @@ A 64-bit `TermIndex` build is the only configuration that compiles the just test-wide # rebuilds via uv sync with monoprop_WIDE_TERM_INDEX=ON, then runs CTest ``` +The partition threading layer synchronises with hand-written atomic memory +orderings, which only a ThreadSanitizer build checks. Build with +`monoprop_ENABLE_TSAN=ON` and run the threading suites against that tree — see +the [building guide](/building#threadsanitizer-tree) for the `uv sync` +invocation and what `cpp/tests/tsan.supp` does and does not suppress. + +Because CTest runs each case as its own process (see below), an MPI-enabled build pays a +full `MPI_Init` per case — and `MPI_Init` initialises every fabric device it finds whether +or not the process will ever send a message. On a login node with eight host adapters that +is 8.8 s per process against 0.2 s of user CPU: it looks like slow tests, but it is wall +time with no CPU behind it. So MPI builds default to +`monoprop_TEST_EXCLUDE_MPI_FABRIC=ON`, which skips fabric init for the per-case tests +(8.8 s to 1.9 s each, and this suite from 34 minutes to 5.4). + +The exclusion applies **only** to the per-case `serial` variants, which are single-process: +their world size is 1, the `*_World` cases skip themselves, and every other communicator is +`MPI_COMM_SELF`, so no transport is used and the fabric can only cost startup time. The +multi-rank variants keep the full component set, because they exchange real messages. Set +the option to `OFF` to put the per-case tests back on the fabric too: + +```bash +uv sync --all-extras -v --reinstall-package monoprop --no-cache \ + --config-settings-package="monoprop:cmake.define.monoprop_ENABLE_MPI=ON" \ + --config-settings-package="monoprop:cmake.define.monoprop_TEST_EXCLUDE_MPI_FABRIC=OFF" +``` + CTest registers every Boost case individually as a `serial` variant. When the build has MPI enabled and a launcher is found, it also registers the whole suite once per rank count in `monoprop_MPI_TEST_PROCS`, labelled `mpi` and `mpi-` — diff --git a/justfile b/justfile index 7063c523..4dc23656 100644 --- a/justfile +++ b/justfile @@ -124,12 +124,19 @@ docs-install: # just bench serial # monoprop_NUM_THREADS=10 just bench serial-t10 --num-modes 64 --bench-rounds 10 +# The `-s` below is load-bearing, not cosmetic. pytest's default capture is +# fd-level: it replaces fd 2 for each test and discards the buffer when the test +# passes. monoprop_COMM_PROFILE=1 writes its COMMPROF line straight to fd 2 from a +# transport destructor, so without `-s` a passing benchmark reports no profile at +# all -- and "the two runs cost the same" is then indistinguishable from "the +# instrument never fired". + # Run the suite (timing + memory) for one LABEL; extra args go to pytest. bench LABEL *ARGS: @mkdir -p "{{ bench_results }}" label="$1"; shift; \ monoprop_BENCH_LABEL="$label" monoprop_BENCH_RESULTS="{{ bench_results }}" \ - uv run --no-sync python -m pytest benches -o filterwarnings=default \ + uv run --no-sync python -m pytest benches -o filterwarnings=default -s \ --benchmark-json="{{ bench_results }}/time-$label.json" "$@" uv run --no-sync python benches/report.py "{{ bench_results }}" @@ -146,7 +153,7 @@ bench-mpi LABEL RANKS *MPIARGS: monoprop_BENCH_LABEL="$label" monoprop_BENCH_RESULTS="{{ bench_results }}" \ uv run --no-sync mpiexec -n "$ranks" \ -x monoprop_BENCH_LABEL -x monoprop_BENCH_RESULTS "$@" \ - python -m pytest benches -o filterwarnings=default \ + python -m pytest benches -o filterwarnings=default -s \ --benchmark-json="{{ bench_results }}/time-$label.json" uv run --no-sync python benches/report.py "{{ bench_results }}" @@ -162,7 +169,7 @@ bench-build-mpi: bench-smoke: @mkdir -p "{{ bench_results }}" monoprop_BENCH_LABEL=smoke monoprop_BENCH_RESULTS="{{ bench_results }}" \ - uv run --no-sync python -m pytest benches -o filterwarnings=default \ + uv run --no-sync python -m pytest benches -o filterwarnings=default -s \ --benchmark-json="{{ bench_results }}/time-smoke.json" \ -m "not slow" --num-generators 8 --num-modes 8 --cutoff 6 --obs-terms 16 uv run --no-sync python benches/report.py "{{ bench_results }}"