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

Extend launch policy to carry stack size and scheduling hint in addition to priority #5543

Merged
merged 1 commit into from
Oct 4, 2021

Conversation

hkaiser
Copy link
Member

@hkaiser hkaiser commented Sep 10, 2021

This PR extends hpx::launch and related types (i.e. hpx::launch::async, etc.) such that they contain the scheduling hint and the stack size in addition to the thread priority.

This will be needed for the direct scoped execution to be able to control whether threads should be executed directly or not. This also simplifies a couple of internal interfaces.

As a flyby, this PR changes all thread enums to be of size std::int8_t, which allows for the launch policy to fit into a 64 bit register.

@hkaiser hkaiser force-pushed the launch_policy branch 5 times, most recently from 6a9ac23 to 5298ddc Compare September 13, 2021 21:24
Copy link
Contributor

@msimberg msimberg left a comment

Choose a reason for hiding this comment

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

Nice! This seems to have the nice side-effect of simplifying all the functions that take priorities, stacksizes, and hints.

@hkaiser hkaiser force-pushed the launch_policy branch 2 times, most recently from 89296df to b85ca0a Compare September 14, 2021 17:00
msimberg
msimberg previously approved these changes Sep 15, 2021
Copy link
Contributor

@msimberg msimberg left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks @hkaiser!

Except that most CI configurations don't like the changes... This can go in once they are happy.

@hkaiser
Copy link
Member Author

hkaiser commented Sep 17, 2021

This looks good now, I believe.

@msimberg
Copy link
Contributor

I suppose these would be fitting to change in this PR: https://app.codacy.com/gh/STEllAR-GROUP/hpx/pullRequest?prid=8056581. Are you able to reproduce this: https://cdash.cscs.ch/testDetails.php?test=51703242&build=180874?

@hkaiser
Copy link
Member Author

hkaiser commented Sep 20, 2021

I suppose these would be fitting to change in this PR: https://app.codacy.com/gh/STEllAR-GROUP/hpx/pullRequest?prid=8056581.

These are deliberate, I'll ignore the tests for codacy.

Are you able to reproduce this: https://cdash.cscs.ch/testDetails.php?test=51703242&build=180874?

Not yet, looks like something compiler specific, though.

@msimberg
Copy link
Contributor

@hkaiser looks like this is ready to go. Did you find out what was causing https://cdash.cscs.ch/testDetails.php?test=51703242&build=180874?

@hkaiser
Copy link
Member Author

hkaiser commented Sep 30, 2021

@hkaiser looks like this is ready to go. Did you find out what was causing https://cdash.cscs.ch/testDetails.php?test=51703242&build=180874?

Not yet, I wasn't even able to reproduce it locally...

@hkaiser
Copy link
Member Author

hkaiser commented Sep 30, 2021

@hkaiser looks like this is ready to go. Did you find out what was causing https://cdash.cscs.ch/testDetails.php?test=51703242&build=180874?

Not yet, I wasn't even able to reproduce it locally...

@msimberg do you recall whether this particular platform has any special configurations?

@msimberg
Copy link
Contributor

@hkaiser I think C++20 is the only possibly relevant configuration option:

export CXX_STD="20"
.

@msimberg msimberg merged commit 20293c7 into master Oct 4, 2021
@msimberg msimberg deleted the launch_policy branch October 4, 2021 07:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants