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

Default pool rename #2926

Merged
merged 4 commits into from Oct 4, 2017
Merged

Default pool rename #2926

merged 4 commits into from Oct 4, 2017

Conversation

biddisco
Copy link
Contributor

@biddisco biddisco commented Oct 2, 2017

this fixes #2925

@@ -361,7 +361,7 @@ namespace hpx { namespace resource { namespace detail
// exclusively if dynamic pools are enabled.
// Also, by default, the first PU is always exclusive
// (to avoid deadlocks).
add_resource(p, "default",
add_resource(p, initial_thread_pools_[0].pool_name_,
Copy link
Member

Choose a reason for hiding this comment

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

May I suggest you use the get_default_pool_name() member function everywhere here instead of the initial_thread_pools_[0].pool_name_ you currently use?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

inside detail::partitioner I used initial_thread_pools_[0].pool_name, but outside it, I used get_default_pool_name. Not a big deal. I will change it.

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.

LGTM, thanks!

Users might want to create numa-pool-0, numa-pool-1 etc
and rename the default pool to one of them to make it easier
to loop over pools in code and not treat pool 0 specially.
@biddisco biddisco merged commit 264d4e5 into master Oct 4, 2017
@biddisco biddisco deleted the default_pool_rename branch October 4, 2017 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Default pool cannot be renamed
2 participants