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.

Sign up for GitHub

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

Jump to bottom

Add more verbose message when default thread pool doesn't have any threads assigned to it #619

Merged
merged 1 commit into from
Mar 8, 2023
Merged

Add more verbose message when default thread pool doesn't have any threads assigned to it #619

merged 1 commit into from
Mar 8, 2023

Conversation

msimberg
Copy link
Contributor

@msimberg msimberg commented Mar 7, 2023

This makes the error message more verbose when the default thread pool doesn't have any threads assigned to it.

In addition to saying "check your bindings/options" this will now also print the number of threads that pika is trying to use and the number of thread pools that it's configured to use. It will also print the process mask that pika has detected (if it's configured to use one). It will also print the value of OMP_PROC_BIND (if set). Finally, it prints two possible reasons for this happening. I hope this is an improvement, but I think there is still lots of room for improvement.

@RMeli, @albestro, @rasolca wishes and comments would be very much appreciated, even ones that may be a bit more difficult to implement. We may get to them some other day and it's good to know what actually helps you.

Two things that I'm not doing here are:

  • print the assignment of threads to pools, because this error happens while assigning threads to pools. I think we could do something reasonable here because at least in theory all threads should already be assigned at this point, but this needs more work.
  • Explain what exactly is setting various options (i.e. if the number of threads is set by command line or config file or something else). This would need bigger changes to the configuration module.

Also adds a member function to affinity_data to query if it is using a process mask.

@msimberg msimberg added this to the 0.14.0 milestone Mar 7, 2023
@msimberg msimberg requested review from rasolca and albestro March 7, 2023 10:43
@msimberg msimberg requested a review from aurianer as a code owner March 7, 2023 10:43
@msimberg msimberg self-assigned this Mar 7, 2023
@msimberg msimberg requested a review from biddisco as a code owner March 7, 2023 10:43
@pika-bot
Copy link
Collaborator

pika-bot commented Mar 7, 2023

Performance test report

pika Performance

Comparison

BENCHMARKNO-EXECUTOR
Future Overhead - Create Thread Hierarchical - Latch---

Info

PropertyBeforeAfter
pika Commit190f18968fc5e7
pika Datetime2022-09-16T08:18:06+00:002023-03-07T10:43:56+00:00
Clusternamedaintdaint
Hostnamenid00074nid00446
Datetime2022-09-16T10:25:01.976661+02:002023-03-07T13:38:32.686984+01:00
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
Envfile

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 force-pushed the more-verbose-no-threads-in-default-pool branch from 3c9ab85 to d57b9ac Compare March 7, 2023 15:16
@pika-bot
Copy link
Collaborator

pika-bot commented Mar 7, 2023

Performance test report

pika Performance

Comparison

BENCHMARKNO-EXECUTOR
Future Overhead - Create Thread Hierarchical - Latch---

Info

PropertyBeforeAfter
pika Commit190f1893d13aca
pika Datetime2022-09-16T08:18:06+00:002023-03-07T15:16:33+00:00
Datetime2022-09-16T10:25:01.976661+02:002023-03-07T16:25:11.893150+01:00
Hostnamenid00074nid00408
Clusternamedaintdaint
Envfile
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

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…

aurianer
aurianer approved these changes Mar 8, 2023
View reviewed changes
Copy link
Contributor

@aurianer aurianer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks!

@aurianer
Copy link
Contributor

aurianer commented Mar 8, 2023

bors merge

@bors
Copy link
Contributor

bors bot commented Mar 8, 2023

Build succeeded:

@bors bors bot merged commit 8df8e59 into pika-org:main Mar 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aurianer aurianer aurianer approved these changes

@rasolca rasolca Awaiting requested review from rasolca

@albestro albestro Awaiting requested review from albestro

@biddisco biddisco Awaiting requested review from biddisco biddisco is a code owner

Assignees

@msimberg msimberg

Labels
None yet
Projects
pika planning board
Archived in project
Milestone
0.14.0
Development

Successfully merging this pull request may close these issues.
3 participants
@msimberg @pika-bot @aurianer