diff --git a/cpp/monoprop/detail/EnvConfig.h b/cpp/monoprop/detail/EnvConfig.h index 5fae134a..30120417 100644 --- a/cpp/monoprop/detail/EnvConfig.h +++ b/cpp/monoprop/detail/EnvConfig.h @@ -21,22 +21,13 @@ // Single home for runtime environment configuration. Kept dependency-free by design, because it is // pulled into hot-path headers. // -// 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_NUM_THREADS positive int (1..1e6), else ignored → num_threads // monoprop_PARTITIONS int N | "auto" | "off"; parsed where it is used (resolve_partition_count_) namespace monoprop::config { namespace detail { -inline auto parse_flag(const char *value, bool default_value) -> bool { - if (value == nullptr || value[0] == '\0') { - return default_value; - } - const char c = value[0]; - return !(c == '0' || c == 'f' || c == 'F' || c == 'n' || c == 'N'); -} - inline auto parse_positive_int(const char *text) -> std::optional { if (text == nullptr) { return std::nullopt; @@ -56,7 +47,6 @@ inline auto parse_positive_int(const char *text) -> std::optional { struct Settings { std::optional num_threads; - bool partition_pinning = true; }; // Parse the environment once; the Settings are cached and shared across TUs. @@ -64,7 +54,6 @@ inline auto get() -> const Settings & { static const Settings 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); return s; }(); return settings; diff --git a/cpp/monoprop/detail/partition/CpuTopology.cpp b/cpp/monoprop/detail/partition/CpuTopology.cpp index 3eaee8f3..84ef0cdd 100644 --- a/cpp/monoprop/detail/partition/CpuTopology.cpp +++ b/cpp/monoprop/detail/partition/CpuTopology.cpp @@ -15,10 +15,14 @@ #include "monoprop/detail/partition/CpuTopology.h" #include +#include #include +#include #include #include #include +#include +#include #include #include @@ -144,8 +148,8 @@ auto placement_order(const std::vector &cores, size_t n, size_t gr if (offset + n > order.size()) { return {}; } - return std::vector(order.begin() + static_cast(offset), - order.begin() + static_cast(offset + n)); + return {order.begin() + static_cast(offset), + order.begin() + static_cast(offset + n)}; } } // namespace topo_detail @@ -153,7 +157,7 @@ auto placement_order(const std::vector &cores, size_t n, size_t gr /* ── enumerate_physical_cores ──────────────────────────────────────────────── */ auto enumerate_physical_cores() -> std::vector { - const auto topo = get_topology(); + auto *const topo = get_topology(); if (!topo) { return {}; } @@ -175,7 +179,7 @@ auto enumerate_physical_cores() -> std::vector { const unsigned num_cores = hwloc_get_nbobjs_by_depth(topo, core_depth); for (unsigned i = 0; i < num_cores; ++i) { - const hwloc_obj_t core = hwloc_get_obj_by_depth(topo, core_depth, i); + auto *const core = hwloc_get_obj_by_depth(topo, core_depth, i); if (!core || !core->cpuset) { continue; } @@ -202,7 +206,7 @@ auto enumerate_physical_cores() -> std::vector { * receive their own singleton domain so the placement algorithm can still spread across * whatever structure the topology does have. */ int domain; - const hwloc_obj_t l3 = hwloc_get_ancestor_obj_by_type(topo, HWLOC_OBJ_L3CACHE, core); + auto *const l3 = hwloc_get_ancestor_obj_by_type(topo, HWLOC_OBJ_L3CACHE, core); if (l3) { const auto [it, inserted] = l3_domain_map.emplace(l3->logical_index, next_domain_id); if (inserted) { @@ -214,7 +218,7 @@ auto enumerate_physical_cores() -> std::vector { domain = next_domain_id++; } - cores.push_back(PhysicalCore{rep, domain}); + cores.push_back(PhysicalCore{.cpu = rep, .l3_domain = domain}); } hwloc_bitmap_free(allowed); @@ -228,7 +232,7 @@ auto affinity_mask_words(uint64_t *out, size_t nwords) -> bool { return false; } std::fill_n(out, nwords, uint64_t{0}); - const auto topo = get_topology(); + auto *const topo = get_topology(); if (!topo) { return false; } @@ -276,12 +280,72 @@ auto masks_are_pairwise_disjoint(const uint64_t *masks, size_t n, size_t words) return true; } +/* ── summarize_masks ──────────────────────────────────────────────────────── */ + +// hwloc indexes a bitmap in unsigned long units, so a 64-bit word must be one of them. +static_assert(sizeof(unsigned long) == sizeof(uint64_t), "the mask word is not an hwloc bitmap unit"); + +auto summarize_masks(const uint64_t *masks, size_t n, size_t words, size_t self) -> std::optional { + if (masks == nullptr || n == 0 || words == 0 || self >= n) { + return std::nullopt; + } + auto *const row = hwloc_bitmap_alloc(); + auto *const all = hwloc_bitmap_alloc(); + if (row == nullptr || all == nullptr) { + hwloc_bitmap_free(row); + hwloc_bitmap_free(all); + return std::nullopt; + } + MaskSummary out; + bool ok = true; + for (size_t r = 0; r < n; ++r) { + hwloc_bitmap_zero(row); + for (size_t w = 0; w < words; ++w) { + hwloc_bitmap_set_ith_ulong(row, static_cast(w), masks[(r * words) + w]); + } + const int weight = hwloc_bitmap_weight(row); + if (weight <= 0) { // an all-zero row is a mask that did not fit the exchange window + ok = false; + break; + } + if (r == self) { + out.cpus = static_cast(weight); + } + hwloc_bitmap_or(all, all, row); + } + if (ok) { + out.node_cpus = static_cast(hwloc_bitmap_weight(all)); + std::array text{}; + const int need = hwloc_bitmap_list_snprintf(text.data(), text.size(), all); + out.cpu_list = text.data(); + // Truncation is stated, never silent: a cut list read as complete is a smaller machine. Cut + // back to the last whole range first, so the marker never follows a half-written CPU id. + if (std::cmp_greater_equal(need, text.size())) { + const size_t last = out.cpu_list.rfind(','); + out.cpu_list.resize(last == std::string::npos ? 0 : last + 1); + out.cpu_list += "+"; + } + } + hwloc_bitmap_free(row); + hwloc_bitmap_free(all); + return ok ? std::optional{out} : std::nullopt; +} + +auto format_place_line(int mpi_rank, int node_rank, int node_size, const char *verdict, const MaskSummary &summary) + -> std::string { + return std::format("COMMPLACE rank={} node_rank={} node_size={} masks={} cpus={} node_cpus={} cpu_list={}\n", + mpi_rank, + node_rank, + node_size, + verdict, + summary.cpus, + summary.node_cpus, + summary.cpu_list); +} + /* ── partition_cpusets ─────────────────────────────────────────────────────── */ auto partition_cpusets(size_t n, size_t group_index, size_t group_count, NodeMask mask) -> std::vector { - if (!config::get().partition_pinning) { - return {}; - } const auto cores = enumerate_physical_cores(); // A private mask IS this rank's share: the launcher already separated co-located ranks. @@ -317,7 +381,7 @@ auto pin_this_thread(const CpuSet &set) -> void { if (set.pu < 0) { return; } - const auto topo = get_topology(); + auto *const topo = get_topology(); if (!topo) { return; } diff --git a/cpp/monoprop/detail/partition/CpuTopology.h b/cpp/monoprop/detail/partition/CpuTopology.h index 2c2c1fca..e862fb51 100644 --- a/cpp/monoprop/detail/partition/CpuTopology.h +++ b/cpp/monoprop/detail/partition/CpuTopology.h @@ -20,6 +20,7 @@ * Policy: one partition per physical core, spread across L3/CCX domains, each worker thread pinned * to its representative PU. Falls back to unpinned execution when hwloc cannot load the topology or * when binding is unsupported — pinning is a performance optimisation, not a correctness requirement. + * Pinning has no runtime knob: leaving placement to the launcher measured propagate[hubbard] 2.90x slower. */ #pragma once @@ -27,10 +28,10 @@ #include #include #include +#include +#include #include -#include "monoprop/detail/EnvConfig.h" - namespace monoprop::detail::partition { /*! @@ -83,10 +84,6 @@ auto placement_order(const std::vector &cores, size_t n, size_t gr * * @returns Vector of PhysicalCore in hwloc logical-core order, or empty when hwloc cannot load * the topology or when no core passes the affinity filter. - * - * @note This function deliberately ignores @c monoprop_PARTITION_PINNING so that the auto - * partition-count heuristic (one partition per physical core) works even when pinning is - * disabled by the user. */ auto enumerate_physical_cores() -> std::vector; @@ -104,6 +101,36 @@ auto affinity_mask_words(uint64_t *out, size_t nwords) -> bool; */ [[nodiscard]] auto masks_are_pairwise_disjoint(const uint64_t *masks, size_t n, size_t words) -> bool; +//! What one host's exchanged affinity masks add up to. The union says what the JOB got, not what one rank got. +struct MaskSummary { + size_t cpus = 0; //!< CPUs in our own mask + size_t node_cpus = 0; //!< CPUs in the union over the host + std::string cpu_list = "none"; //!< that union as ascending ranges, "0-15,64-79" +}; + +/*! @brief Summarize the @p n masks of @p words words laid end to end in @p masks, row @p self being ours. + * @returns nullopt for a null/empty argument or any all-zero row: a mask that did not fit the exchange + * window cannot be summed with the others. Diagnostic only; nothing branches on the result. + */ +[[nodiscard]] auto summarize_masks(const uint64_t *masks, size_t n, size_t words, size_t self) + -> std::optional; + +/* COMMPLACE: a rank seeing 16 of a host's 128 CPUs is equally "my own 16" and "eight of us share + * these 16", and only the co-located ranks' masks separate them, so the exchange PartitionGroup + * already runs to place also reports. Report-only, unconditional; nothing branches on the line. */ + +/*! @brief One newline-terminated COMMPLACE line naming what the launcher handed this rank. + * @param verdict how the co-located masks relate: "private" (pairwise disjoint), "shared" (two ranks + * can land on one CPU), "alone" (the only rank on its host, which is NOT "private"), or + * "unknown" (a mask did not fit the exchanged window, so no verdict is sound). + * Returned, not written, so the formatting is testable without a live rank. + */ +[[nodiscard]] auto format_place_line(int mpi_rank, + int node_rank, + int node_size, + const char *verdict, + const MaskSummary &summary) -> std::string; + //! Whether the launcher has already handed this rank a private slice of the node, or the node's CPUs are shared. enum class NodeMask { Shared, PerRank }; @@ -119,8 +146,8 @@ enum class NodeMask { Shared, PerRank }; * @param group_count Total number of co-located ranks on the host. * @param mask NodeMask::PerRank only when the co-located ranks' affinity masks have been measured * pairwise DISJOINT (PartitionGroup::classify_node_masks_), so this mask is our share. - * @returns Vector of @p n CpuSet tokens, or empty when @c monoprop_PARTITION_PINNING is disabled, - * hwloc is unavailable, or fewer than @p group_count x @p n cores are visible (@p n under PerRank). + * @returns Vector of @p n CpuSet tokens, or empty when hwloc is unavailable or fewer than + * @p group_count x @p n cores are visible (@p n under PerRank). * * @note Under NodeMask::PerRank the group split is skipped: our share is already this rank's alone. */ diff --git a/cpp/monoprop/detail/partition/PartitionGroup.h b/cpp/monoprop/detail/partition/PartitionGroup.h index 31efb7ae..89ce2f93 100644 --- a/cpp/monoprop/detail/partition/PartitionGroup.h +++ b/cpp/monoprop/detail/partition/PartitionGroup.h @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -168,8 +169,10 @@ class PartitionGroup { MPI_Comm_size(node, &node_size_); classify_node_masks_(node); MPI_Comm_free(&node); + return; } #endif + report_placement_(nullptr, 0, "alone"); } #ifdef monoprop_ENABLE_MPI @@ -177,6 +180,7 @@ class PartitionGroup { auto classify_node_masks_(MPI_Comm node) -> void { node_mask_ = NodeMask::Shared; if (node_size_ <= 1) { + report_placement_(nullptr, 0, "alone"); return; // nobody to collide with; the normal split already handles group_count == 1 } constexpr size_t kMaskWords = monoprop::detail::partition::kAffinityMaskWords; @@ -189,9 +193,34 @@ class PartitionGroup { static_cast(node_size_), kMaskWords); node_mask_ = disjoint ? NodeMask::PerRank : NodeMask::Shared; + report_placement_(all.data(), static_cast(node_size_), disjoint ? "private" : "shared"); } #endif + /* COMMPLACE only, over the array MPI_Allgather already filled: no extra collective, and no + * reduction either, since every rank reads the same rows and so reaches the same verdict. `masks` + * nullptr means no peers, so measure our own mask; the verdict is then "alone", which is NOT + * evidence that a multi-rank launcher did the right thing. Reached only from the primary ctor, so + * a clone does not re-emit -- the mask belongs to the process, not the object. */ + auto report_placement_(const uint64_t *masks, size_t peers, const char *verdict) -> void { + constexpr size_t kWords = monoprop::detail::partition::kAffinityMaskWords; + std::array own{}; + if (masks == nullptr && affinity_mask_words(own.data(), kWords)) { + masks = own.data(); + peers = 1; + } + // No summary is "unknown" rather than a plausible zero: some mask did not fit the window. + const auto sum = summarize_masks(masks, peers, kWords, static_cast(node_rank_)); + std::fputs(format_place_line(mpi::rank(parent_), + node_rank_, + node_size_, + sum ? verdict : "unknown", + sum.value_or(MaskSummary{})) + .c_str(), + stderr); + std::fflush(stderr); + } + auto make_transport_() -> void { #ifdef monoprop_ENABLE_MPI if (parent_.kind == mpi::Comm::Kind::Mpi && mpi::size(parent_) > 1) { diff --git a/cpp/tests/cpu_topology_tests.cpp b/cpp/tests/cpu_topology_tests.cpp index 1f64526e..5b0334f9 100644 --- a/cpp/tests/cpu_topology_tests.cpp +++ b/cpp/tests/cpu_topology_tests.cpp @@ -27,13 +27,13 @@ #include #include #include +#include #include #if defined(__linux__) #include #endif -#include "monoprop/detail/EnvConfig.h" // config::get().partition_pinning -- the one licensed empty placement #include "monoprop/detail/partition/CpuTopology.h" namespace partition = monoprop::detail::partition; @@ -49,19 +49,6 @@ struct AffinityGuard { #endif }; -namespace { - -// An empty placement is licensed by pinning being off and by nothing else; "placed nothing" is the bug. -auto empty_placement_is_licensed() -> bool { - if (monoprop::config::get().partition_pinning) { - return false; - } - BOOST_TEST_MESSAGE("monoprop_PARTITION_PINNING is off; partition_cpusets places nothing"); - return true; -} - -} // namespace - /* ── Live smoke tests ─────────────────────────────────────────────────────── */ BOOST_AUTO_TEST_CASE(cpu_topology_enumerate_and_place) { @@ -76,8 +63,8 @@ BOOST_AUTO_TEST_CASE(cpu_topology_enumerate_and_place) { 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. - if (!cores.empty() && !(one.empty() && empty_placement_is_licensed())) { + // Pinning is unconditional, so a non-empty core list must produce a non-empty placement. + if (!cores.empty()) { BOOST_CHECK_EQUAL(one.size(), 1u); } @@ -119,9 +106,6 @@ BOOST_AUTO_TEST_CASE(cpu_topology_place_co_located_ranks) { /*group_index=*/1, /*group_count=*/2, partition::NodeMask::PerRank); - if (private_mask.empty() && empty_placement_is_licensed()) { - return; - } BOOST_REQUIRE_EQUAL(private_mask.size(), cores.size()); std::set placed; for (const auto &set : private_mask) { @@ -367,9 +351,6 @@ BOOST_AUTO_TEST_CASE(cpu_topology_per_rank_mask_still_places) { // What `srun --cpu-bind=cores` produces: our whole share, told there are eight sibling ranks. const auto sets = partition::partition_cpusets(/*n=*/k, /*group_index=*/3, /*group_count=*/8, partition::NodeMask::PerRank); - if (sets.empty() && empty_placement_is_licensed()) { - return; - } BOOST_REQUIRE_EQUAL(sets.size(), k); for (const auto &set : sets) { // Never pin outside the mask the launcher gave us. @@ -406,9 +387,6 @@ BOOST_AUTO_TEST_CASE(cpu_topology_shared_mask_keeps_co_located_ranks_disjoint) { /*group_index=*/1, /*group_count=*/2, partition::NodeMask::Shared); - if (rank0.empty() && rank1.empty() && empty_placement_is_licensed()) { - return; - } BOOST_REQUIRE_EQUAL(rank0.size(), per_rank); BOOST_REQUIRE_EQUAL(rank1.size(), per_rank); for (const auto &a : rank0) { @@ -420,3 +398,49 @@ BOOST_AUTO_TEST_CASE(cpu_topology_shared_mask_keeps_co_located_ranks_disjoint) { } #endif // __linux__ + +/* ── summarize_masks and the COMMPLACE line ───────────────────────────────── */ + +BOOST_AUTO_TEST_CASE(cpu_topology_summarize_masks) { + constexpr size_t kWords = partition::kAffinityMaskWords; + + // Two ranks holding one CPU each and two ranks sharing the same CPUs differ in exactly node_cpus. + const auto private_ = packed_masks({{0, 1}, {2, 3}}, kWords); + const auto priv = partition::summarize_masks(private_.data(), 2, kWords, 0); + BOOST_REQUIRE(priv.has_value()); + BOOST_CHECK_EQUAL(priv->cpus, 2U); + BOOST_CHECK_EQUAL(priv->node_cpus, 4U); + BOOST_CHECK_EQUAL(priv->cpu_list, "0-3"); + + const auto shared = packed_masks({{0, 1}, {0, 1}}, kWords); + const auto shd = partition::summarize_masks(shared.data(), 2, kWords, 1); + BOOST_REQUIRE(shd.has_value()); + BOOST_CHECK_EQUAL(shd->cpus, 2U); + BOOST_CHECK_EQUAL(shd->node_cpus, 2U); + + // A run crossing a 64-bit word boundary is ONE run: a per-word loop would print "63,64". + const auto cross = packed_masks({{63, 64}}, kWords); + BOOST_CHECK_EQUAL(partition::summarize_masks(cross.data(), 1, kWords, 0).value().cpu_list, "63-64"); + + // No summary rather than a plausible zero: an all-zero row is a mask that did not fit the window. + const auto with_empty = packed_masks({{0, 1}, {}}, kWords); + BOOST_CHECK(!partition::summarize_masks(with_empty.data(), 2, kWords, 0).has_value()); + BOOST_CHECK(!partition::summarize_masks(nullptr, 1, kWords, 0).has_value()); + BOOST_CHECK(!partition::summarize_masks(private_.data(), 2, kWords, 2).has_value()); // self out of range +} + +// The formatter returns a string rather than writing one, so the line is testable without a live rank. +BOOST_AUTO_TEST_CASE(cpu_topology_place_line_reports_every_field) { + const partition::MaskSummary sum{.cpus = 16, .node_cpus = 128, .cpu_list = "0-127"}; + // Whole line, not field lookups: a reordered or unterminated line has to fail too. + BOOST_CHECK_EQUAL(partition::format_place_line(5, 2, 8, "private", sum), + "COMMPLACE rank=5 node_rank=2 node_size=8 masks=private cpus=16 node_cpus=128 " + "cpu_list=0-127\n"); +} + +// The state summarize_masks refuses to classify must SAY unknown rather than print a plausible zero. +BOOST_AUTO_TEST_CASE(cpu_topology_place_line_unknown_is_not_a_verdict) { + BOOST_CHECK_EQUAL(partition::format_place_line(0, 0, 1, "unknown", partition::MaskSummary{}), + "COMMPLACE rank=0 node_rank=0 node_size=1 masks=unknown cpus=0 node_cpus=0 " + "cpu_list=none\n"); +} diff --git a/cpp/tests/env_config_tests.cpp b/cpp/tests/env_config_tests.cpp index 77ff0955..fe46afe7 100644 --- a/cpp/tests/env_config_tests.cpp +++ b/cpp/tests/env_config_tests.cpp @@ -18,30 +18,8 @@ #include "monoprop/detail/EnvConfig.h" -using monoprop::config::detail::parse_flag; using monoprop::config::detail::parse_positive_int; -BOOST_AUTO_TEST_CASE(env_config_parse_flag_default_when_unset_or_empty) { - BOOST_CHECK_EQUAL(parse_flag(nullptr, true), true); - BOOST_CHECK_EQUAL(parse_flag(nullptr, false), false); - BOOST_CHECK_EQUAL(parse_flag("", true), true); - BOOST_CHECK_EQUAL(parse_flag("", false), false); -} - -BOOST_AUTO_TEST_CASE(env_config_parse_flag_falsey_first_char) { - // Only the first character decides, so "0abc" is falsey too. - for (const char *v : {"0", "f", "F", "n", "N"}) { - BOOST_CHECK_MESSAGE(parse_flag(v, true) == false, v); - } - BOOST_CHECK_EQUAL(parse_flag("0abc", true), false); -} - -BOOST_AUTO_TEST_CASE(env_config_parse_flag_truthy_first_char) { - for (const char *v : {"1", "t", "T", "y", "Y", "on", "true", "anything"}) { - BOOST_CHECK_MESSAGE(parse_flag(v, false) == true, v); - } -} - BOOST_AUTO_TEST_CASE(env_config_parse_positive_int_null_and_malformed) { BOOST_CHECK(parse_positive_int(nullptr) == std::nullopt); BOOST_CHECK(parse_positive_int("") == std::nullopt); @@ -64,5 +42,5 @@ BOOST_AUTO_TEST_CASE(env_config_settings_cached_singleton) { const auto &b = monoprop::config::get(); BOOST_CHECK_EQUAL(&a, &b); // Touch a field so the Settings aggregate is actually read. - BOOST_CHECK(a.partition_pinning == true || a.partition_pinning == false); + BOOST_CHECK(a.num_threads == std::nullopt || *a.num_threads >= 1); } diff --git a/docs/content/docs/features/parallelism.mdx b/docs/content/docs/features/parallelism.mdx index bf57b046..e9e56ed5 100644 --- a/docs/content/docs/features/parallelism.mdx +++ b/docs/content/docs/features/parallelism.mdx @@ -29,13 +29,31 @@ 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. | ```bash # Run 8 partitions instead of one-per-core: export monoprop_NUM_THREADS=8 ``` +## Placement report + +Building a propagator writes one `COMMPLACE` line per rank to stderr, naming the +CPUs the launcher gave that rank and whether co-located ranks got disjoint masks. +It is report-only — no placement decision reads it — and there is no knob: redirect +stderr to drop it. + +```text +COMMPLACE rank=0 node_rank=0 node_size=2 masks=private cpus=64 node_cpus=128 cpu_list=0-63 +``` + +`masks` is `private` when the co-located ranks' affinity masks are pairwise +disjoint, `shared` when two ranks can land on the same CPU, `alone` when this rank +is the only one on its host (which is *not* evidence a multi-rank launcher bound +correctly), and `unknown` when a mask did not fit the exchanged window. + +Pinning each partition to a core is not configurable: leaving placement to the +launcher measured `propagate[hubbard]` 2.90x slower, so the disabled arm is gone. + ## MPI distribution (multi-node) MPI partitions the operator and graph across ranks, composing with per-rank