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 optional scheduler mode parameter to create_thread_pool function #3230

Merged
merged 1 commit into from Apr 5, 2018

Conversation

msimberg
Copy link
Contributor

@msimberg msimberg commented Mar 12, 2018

Proposed Changes

  • Add a default_mode value to the scheduler mode enum
  • Add a an optional scheduler mode parameter to create_thread_pool with a default value of default_mode

Any background context you want to provide?

Addresses #3082, but I'm not sure what more we might want. This at least simplifies the use cases in the suspension tests (one does not need to use the callback version of create_thread_pool anymore to change scheduler modes).

@@ -53,7 +54,8 @@ namespace hpx { namespace resource { namespace detail

private:
init_pool_data(const std::string &name,
scheduling_policy = scheduling_policy::unspecified);
scheduling_policy = scheduling_policy::unspecified,
hpx::util::optional<hpx::threads::policies::scheduler_mode> = hpx::util::nullopt);
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't it be simpler to define an enumerator value representing a 'default_mode'?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @hkaiser once again for keeping me in check. That's a really good idea and further simplifies usage (because one can say "I want defaults plus this flag" or "I want defaults and definitely not this flag").

@hkaiser hkaiser added this to the 1.1.0 milestone Mar 12, 2018
@msimberg msimberg removed this from the 1.1.0 milestone Mar 22, 2018
@msimberg
Copy link
Contributor Author

msimberg commented Apr 4, 2018

@hkaiser I've rebased and updated this to add only a new enum value (default_mode) instead of using optional.

The timeout on throttle_timed is worrying but most likely an old, latent issue, unlikely to be caused by this PR. I will look into that separately.

@hkaiser hkaiser added this to the 1.2.0 milestone Apr 4, 2018
Copy link
Member

@hkaiser hkaiser left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM now.

@msimberg msimberg merged commit daf2a8d into STEllAR-GROUP:master Apr 5, 2018
@msimberg msimberg deleted the create-thread-pool branch April 5, 2018 06:44
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.

None yet

2 participants