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

Need of a "semi" blocking barrier for bulk #833

Closed
albestro opened this issue Apr 13, 2023 · 0 comments · Fixed by #864
Closed

Need of a "semi" blocking barrier for bulk #833

albestro opened this issue Apr 13, 2023 · 0 comments · Fixed by #864

Comments

@albestro
Copy link
Collaborator

albestro commented Apr 13, 2023

Currently, in pika we have a "yielding" barrier. This is not optimal since lightweight tasks of the bulk might get rescheduled and this add overheads.

It would be mitigated with a "blocking" barrier: in pika is not yet implemented, but a feasibility test has been done with a pthread_barrier (really dangerous, since it can lead to deadlocks, since it blocks the os-thread).

Originally posted by @albestro in #797 (comment)

@albestro albestro added this to the Optimizations milestone Apr 13, 2023
@msimberg msimberg self-assigned this Apr 20, 2023
bors bot added a commit to pika-org/pika that referenced this issue May 10, 2023
672: Unify `pika::spinlock` and `pika::concurrency::detail::spinlock` into one implementation r=msimberg a=msimberg

Fixes #517, i.e. uses a non-yielding everywhere where a spinlock was used previously. The implementations were almost identical, but used a slightly different yielding strategy. The yielding one (`pika::spinlock`) used `yield_while` allowing yielding of the user-level thread. The non-yielding one (`pika::concurrency::detail::spinlock`) was sleeping the OS-thread after one iteration. This now uses the `pika::spinlock` strategy everywhere, except that yielding is disallowed when spinning. The name `pika::spinlock` is removed and only the `detail` one remains.

Note that we used to have _three_ spinlock implementations. Now we still have two. The remaining one is a very basic one with near zero dependencies (no lock registration, no ITT support) that is still used in things like the configuration maps and resource partitioner.

This doesn't change performance in either direction in DLA-Future, but it's a prerequisite for having a semi-blocking barrier (eth-cscs/DLA-Future#833).

We will need to make corresponding changes in pika-algorithms since it uses `pika::spinlock` in a few places.

Co-authored-by: Mikael Simberg <[email protected]>
bors bot added a commit to pika-org/pika that referenced this issue May 10, 2023
672: Unify `pika::spinlock` and `pika::concurrency::detail::spinlock` into one implementation r=msimberg a=msimberg

Fixes #517, i.e. uses a non-yielding everywhere where a spinlock was used previously. The implementations were almost identical, but used a slightly different yielding strategy. The yielding one (`pika::spinlock`) used `yield_while` allowing yielding of the user-level thread. The non-yielding one (`pika::concurrency::detail::spinlock`) was sleeping the OS-thread after one iteration. This now uses the `pika::spinlock` strategy everywhere, except that yielding is disallowed when spinning. The name `pika::spinlock` is removed and only the `detail` one remains.

Note that we used to have _three_ spinlock implementations. Now we still have two. The remaining one is a very basic one with near zero dependencies (no lock registration, no ITT support) that is still used in things like the configuration maps and resource partitioner.

This doesn't change performance in either direction in DLA-Future, but it's a prerequisite for having a semi-blocking barrier (eth-cscs/DLA-Future#833).

We will need to make corresponding changes in pika-algorithms since it uses `pika::spinlock` in a few places.

Co-authored-by: Mikael Simberg <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants