fix: 🧹 use hwloc instead of custom topology discovery - #180
Conversation
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 |
|
f6ae926 to
ed9e728
Compare
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>
ed9e728 to
d582e87
Compare
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>
When needed, can be run as uvx gcovr, using an ephemeral environment
There is a single job running on ubuntu-26.04 with python 3.11 collecting coverage information from: * Python tests * C++ unit tests * line hits from calling the C++ compiled code from the Python API
Thus avoiding a rebuild when invoking pytest
Fallout from removing most of the cmake presets
Needs lcov format, plus some workarounds, to merge the Python and C++ results.
So nanobind_stubgen can find hwloc correctly.
490a7f9 to
eecc303
Compare
|
also here I managed to make an utter mess 😮💨 Will reopen after salvaging the meaningful changeset in the sea of commits |



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: