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

Implements #15, request for hardware concurrency utility function #447

Merged
merged 2 commits into from
Jul 18, 2019

Conversation

kdt3rd
Copy link
Contributor

@kdt3rd kdt3rd commented Jul 17, 2019

new static member of ThreadPool, call as
ThreadPool::hardwareConcurrency, so no abi breakage or api change

Signed-off-by: Kimball Thurston kdt3rd@gmail.com

…ency utility function

new static member of ThreadPool, call as
ThreadPool::hardwareConcurrency, so no abi breakage or api change

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>
Copy link
Contributor

@lgritz lgritz left a comment

Choose a reason for hiding this comment

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

LGTM, but refresh my memory, are we not C++11 minimum moving forward? Why not use std::thread::hardware_concurrency always? Or expect users to do that?

@meshula
Copy link
Contributor

meshula commented Jul 17, 2019

same question as Larry, see this comment by @cary-ilm - #443 (comment)

@axxel
Copy link
Contributor

axxel commented Jul 17, 2019

+1 for letting the user use std::thread::hardware_concurrency on the client side.

@kdt3rd
Copy link
Contributor Author

kdt3rd commented Jul 17, 2019

This function, when in c++11 mode is just the same (and why I called it what I did), clients can use whatever. We haven't removed the old c++98 code yet, and this ticket was on my assigned list, so I did something about it :)

Copy link
Contributor

@meshula meshula left a comment

Choose a reason for hiding this comment

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

Cool, I'm fine with resolving FORCE_CXX03 across the codebase as a separate task

After discussion with phillman, renamed to give this routine a purpose
beyond some soon to be deleted legacy support, and clarified this in the
comment documenting the function.

Signed-off-by: Kimball Thurston <kdt3rd@gmail.com>
@kdt3rd kdt3rd merged commit e8dc432 into AcademySoftwareFoundation:master Jul 18, 2019
@kdt3rd kdt3rd deleted the fix_15 branch July 18, 2019 10:43
@cary-ilm cary-ilm added this to the v2.4.0 milestone Aug 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants