Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix use --pika:print-bind, --pika:bind=none, and a non-full process mask on the process together #1082

Merged
merged 2 commits into from
Mar 26, 2024

Conversation

msimberg
Copy link
Contributor

When using --pika:print-bind the runtime checks that the mask reported by hwloc and the mask supposedly used by pika are the same. When --pika:bind=none is used we store an empty mask for worker threads. In this case we also don't set bindings for the worker threads, because we assume that the process has no binding set. However, in the case that the full process already has a mask set (e.g. with hwloc-bind or slurm) the worker threads will inherit that mask even with --pika:bind=none. The worker threads then end up with a non-full process mask even if the user asked for a full process mask (no binding).

This fixes the issue by always setting a mask, even if the mask is empty/full. This also adds a simple hello_world test to the CircleCI configuration. I'm not adding this a CTest test since 1. I'm not quite sure how to do that cleanly and 2. I'd like to avoid making the hwloc-bind executable a dependency for tests. Ideas for better integration into the tests is welcome though.

When --pika:bind=none is requested the thread mask is empty, but this is equivalent to using all PUs
on the system. In the case that the process itself has a thread mask already set that is not the
full mask, not setting the full mask explicitly would lead to the wrong thread bindings.
@msimberg msimberg self-assigned this Mar 21, 2024
@msimberg msimberg added this to the 0.24.0 milestone Mar 21, 2024
@msimberg msimberg changed the title Fix use --pika:print-bind, --pika:bind=none and a non-full process mask on the process together Fix use --pika:print-bind, --pika:bind=none, and a non-full process mask on the process together Mar 21, 2024
@pika-bot
Copy link
Collaborator

Performance test report

pika Performance

Comparison

BENCHMARKRESULT
Task Overhead - Create Thread Hierarchical - Latch-

Info

PropertyBeforeAfter
pika Commit0abc084965b6d4
pika Datetime2024-02-19T15:15:15+00:002024-03-21T13:27:33+00:00
Hostnamenid00025nid00025
Compiler/apps/daint/SSL/pika/spack/lib/spack/env/clang/clang++ 11.0.1/apps/daint/SSL/pika/spack/lib/spack/env/clang/clang++ 11.0.1
Datetime2024-02-19T16:26:16.072067+01:002024-03-21T14:35:54.068392+01:00
Envfile
Clusternamedaintdaint

Explanation of Symbols

SymbolMEANING
=No performance change (confidence interval within ±1%)
(=)Probably no performance change (confidence interval within ±2%)
(+)/(-)Very small performance improvement/degradation (≤1%)
+/-Small performance improvement/degradation (>10%)
++/--Large performance improvement/degradation (>10%)
+++/---Very large performance improvement/degradation (>10%)
?Probably no change, but quite large uncertainty (confidence interval with ±5%)
??Unclear result, very large uncertainty (±10%)
???Something unexpected…

@msimberg msimberg added this pull request to the merge queue Mar 25, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 25, 2024
@msimberg msimberg merged commit 672868e into pika-org:main Mar 26, 2024
36 of 38 checks passed
@msimberg msimberg deleted the bind-none-print-bind branch March 26, 2024 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Archive
Development

Successfully merging this pull request may close these issues.

2 participants