fix: 🧹 use hwloc instead of custom topology discovery - #180
Conversation
The package is installed under `lib/python3.X/site-packages/monoprop` all other artifacts are installed in subfolders of it: * `libmonoprop.so` is under `lib` (or `lib64`, depending on the machine) * all headers are under `include` with the proper hierarchical folder structure * CMake helper files are under `cmake` The header files are now explicitly listed (instead of globbed) and we use `target_sources` with `FILE_SET` pento handle them sanely (instead of appending to a global list) I have dropped the distinction between private and public for now.
Signed-off-by: Roberto Di Remigio Eikås <robertodr@users.noreply.github.com>
So we don't bundle it in the wheel later on.
Needs lcov format, plus some workarounds, to merge the Python and C++ results.
There was a problem hiding this comment.
Pull request overview
This PR replaces the project’s CPU topology discovery + partition-thread pinning implementation with an hwloc-backed approach, aiming to improve portability and correctness across platforms and environments (cgroups/cpusets, macOS, etc.).
Changes:
- Introduce an
hwloc-based implementation of physical-core discovery, domain grouping (L3), partition placement, and thread pinning. - Wire
hwlocinto the build (CMake/pkg-config) and CI/dev tooling (APT/Homebrew/DevContainer/install script). - Update/reshape C++ topology tests and refresh documentation to reflect
hwloc-based pinning behavior.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/install-deps.sh | Adds optional hwloc source install path and skip flag for local dependency bootstrapping. |
| tests/cpp/cpu_topology_tests.cpp | Updates tests to validate hwloc-backed enumeration/placement/pinning behavior. |
| src/monoprop/detail/partition/CpuTopology.h | Replaces platform-specific inline implementation with exported declarations and a simplified CpuSet. |
| src/monoprop/detail/partition/CpuTopology.cpp | New hwloc-backed implementation for core discovery, placement, and pinning. |
| src/monoprop/detail/partition/CMakeLists.txt | Ensures the new CpuTopology.cpp is built into the library objects. |
| src/monoprop/CMakeLists.txt | Links PkgConfig::HWLOC so the library can compile/link the hwloc-backed implementation. |
| README.md | Notes hwloc as a source-build prerequisite for topology discovery/pinning. |
| pyproject.toml | Adds hwloc to macOS wheel build dependencies. |
| docs/content/docs/features/parallelism.mdx | Updates pinning docs from “Linux-only” to “where hwloc supports binding.” |
| docs/content/docs/building.mdx | Adds hwloc prerequisite (but currently drops CMake/Ninja). |
| CMakeLists.txt | Adds pkg-config discovery for hwloc>=2.9 via pkg_check_modules. |
| AGENTS.md | Documents hwloc as a core dependency for topology and affinity. |
| .github/workflows/test.yml | Installs hwloc on CI runners (brew/apt). |
| .github/workflows/copilot-setup-steps.yml | Installs libhwloc-dev in Copilot setup steps. |
| .devcontainer/Dockerfile | Adds libhwloc-dev and pkg-config to the devcontainer toolchain. |
Suppressed comments (1)
src/monoprop/detail/partition/CpuTopology.cpp:122
- On the early-return path,
hwloc_bitmap_free(candidate)is called even ifcandidateis null. Guarding the free avoids a potential crash when allocation fails.
const auto *const allowed = topology_context().allowed();
auto *const candidate = hwloc_bitmap_alloc();
if (allowed == nullptr || candidate == nullptr) {
hwloc_bitmap_free(candidate);
return {};
| ~Topology() { | ||
| hwloc_bitmap_free(allowed_); | ||
| if (topology_ != nullptr) { | ||
| hwloc_topology_destroy(topology_); | ||
| } | ||
| } |
| install_hwloc() { | ||
| if [ "$INSTALL_HWLOC" != true ]; then | ||
| echo "Skipping hwloc installation" | ||
| return 0 | ||
| fi | ||
|
|
||
| local hwloc_version="2.12.2" | ||
| echo "Installing hwloc $hwloc_version..." | ||
| curl -fsSL "https://download.open-mpi.org/release/hwloc/v2.12/hwloc-$hwloc_version.tar.gz" | tar -xz | ||
| cd "hwloc-$hwloc_version" | ||
| ./configure \ | ||
| --prefix="$INSTALL_PREFIX" \ | ||
| --disable-cairo \ | ||
| --disable-libxml2 \ | ||
| --disable-static \ | ||
| --enable-shared | ||
| make -j"$(nproc)" | ||
| make install | ||
| cleanup_build "hwloc-$hwloc_version" | ||
| } |
| - a C++23-compliant compiler; on Linux the minimum supported versions are **GCC 14** and **Clang 18** | ||
| - CMake and Ninja | ||
| - Python 3.11 or newer and the `uv` package manager (for the bindings) | ||
| - hwloc 2.9 or newer | ||
| - an MPI implementation such as Open MPI (only for MPI builds) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## ci-split-qa-jobs #180 +/- ##
=================================================
Coverage 95.31% 95.31%
=================================================
Files 14 14
Lines 704 704
Branches 88 88
=================================================
Hits 671 671
Misses 20 20
Partials 13 13
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
Docs preview: https://pr-180.monoprop-docs.pages.dev |
This used to be necessary to make oneTBB visible, but it's likely vestigial now.
Should help catch cases where dependencies are not correctly relocated in the wheel Co-authored-by: Roberto Di Remigio Eikås <robertodr@users.noreply.github.com> Signed-off-by: Roberto Di Remigio Eikås <robertodr@users.noreply.github.com>
|
f6ae926 to
ed9e728
Compare



Summary
The CPU topology discovery and thread pinning used a custom implementation that was not really portable.
hwlocis the standard tool for this job.Changes
hwlocas dependency for cross-platform CPU topology discovery.Checklist
docs/,CONTRIBUTING.md) if neededCHANGELOG/ release notes updated if applicableAI/LLM disclosure
Important
By opening this PR I confirm that I have read CONTRIBUTING.md and I agree to the terms of the Contributor License Agreement.
Warning
If you're contributing on behalf of your employer, contact cla@algorithmiq.fi to arrange a Corporate CLA.
This is part 3 of 3 in a stack made with GitButler: