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

Add pika::wait #704

Merged
merged 1 commit into from
Jul 12, 2023
Merged

Add pika::wait #704

merged 1 commit into from
Jul 12, 2023

Conversation

msimberg
Copy link
Contributor

Fixes #696.

Calls thread_manager::wait. pika::wait can also be called on a pika thread, in which case it'll ignore the calling thread and wait for all other work to finished. The behaviour is undefined/will likely deadlock if pika::wait is called concurrently from multiple pika threads.

Note, I made wait return an int to be consistent with the other related functions, but I opened #703 to possibly change it to void.

@msimberg msimberg added this to the 0.17.0 milestone Jun 21, 2023
@msimberg msimberg self-assigned this Jun 21, 2023
@msimberg
Copy link
Contributor Author

bors try

bors bot added a commit that referenced this pull request Jun 21, 2023
@bors
Copy link
Contributor

bors bot commented Jun 21, 2023

try

Build failed:

@msimberg
Copy link
Contributor Author

bors try

bors bot added a commit that referenced this pull request Jun 21, 2023
@bors
Copy link
Contributor

bors bot commented Jun 21, 2023

try

Build failed:

@msimberg msimberg marked this pull request as draft June 21, 2023 11:55
@msimberg
Copy link
Contributor Author

This seems to have exposed some problems in thread_manager::wait. I will try to get back to this in a few weeks.

@msimberg msimberg removed this from the 0.17.0 milestone Jun 21, 2023
@msimberg
Copy link
Contributor Author

This is on hold because I had forgotten about the probabilistic nature of checking if the runtime is idle. We don't have a global indicator to check that all of the thread pools and MPI/CUDA polling is idle, which means that it's inherently racy to individually check those for idleness and infer idleness of the runtime from those.

We may need to try to introduce a global counter but we have to be careful not to kill performance with a global atomic counter.

@msimberg msimberg mentioned this pull request Jul 11, 2023
@msimberg msimberg marked this pull request as ready for review July 11, 2023 09:08
@msimberg
Copy link
Contributor Author

I've removed the test from this PR and left only the pika::wait wrapper for thread_manager::wait so that we can start replacing uses of thread_manager::wait in DLA-Future with a public API function. The test is in #704 but still needs a deterministic check for the runtime being empty.

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!

@msimberg
Copy link
Contributor Author

bors merge

bors bot added a commit that referenced this pull request Jul 12, 2023
704: Add `pika::wait` r=msimberg a=msimberg

Fixes #696.

Calls `thread_manager::wait`. `pika::wait` can also be called on a pika thread, in which case it'll ignore the calling thread and wait for all other work to finished. The behaviour is undefined/will likely deadlock if `pika::wait` is called concurrently from multiple pika threads.

Note, I made `wait` return an `int` to be consistent with the other related functions, but I opened #703 to possibly change it to `void`.

Co-authored-by: Mikael Simberg <mikael.simberg@iki.fi>
@bors
Copy link
Contributor

bors bot commented Jul 12, 2023

Build failed:

@msimberg
Copy link
Contributor Author

Merging despite failures, which seem to mostly be timeouts on header tests.

@msimberg msimberg merged commit 419e357 into pika-org:main Jul 12, 2023
@msimberg msimberg deleted the pika-wait branch July 12, 2023 12:30
@aurianer aurianer added this to the 0.17.0 milestone Jul 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Add pika::wait function to wait for runtime to be idle
2 participants