Skip to content

Conversation

@xvallspl
Copy link
Contributor

Introduce a new helper returning the number of logical CPUs available to the current process. In case of having an affinity mask, it will return in accordance to it (IMT & tbb required for this).

Function naming can be improved. I wanted to keep tbb away from ImplicitMT and that's why the core function is defined in TPoolManager.hxx. Should it be a member of the class?

Also delete useless return.

@phsft-bot
Copy link

Starting build on gcc49/centos7, native/mac1012, gcc49/slc6, gcc62/slc6, native/ubuntu14 with flags -Dvc=OFF -Dimt=ON -Dccache=ON
How to customize builds

Introduce a new helper returning the number of logical CPUs available
to the current process. In case of having an affinity mask, it will
return in accordance to it (IMT & tbb required for this).

Also delete useless return.
@phsft-bot
Copy link

Starting build on gcc49/centos7, native/mac1012, gcc49/slc6, gcc62/slc6, native/ubuntu14 with flags -Dvc=OFF -Dimt=ON -Dccache=ON
How to customize builds

}

////////////////////////////////////////////////////////////////////////////////
/// Returns the maximum number of threads the user can instantiate.
Copy link
Member

Choose a reason for hiding this comment

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

This comment is incorrect (at least for the !IMT case). Could you copy the comment from GetMaxNThreadsAvailable()?

Also this doesn't return the available threads but their number. I'd suggest to use the same name for both the TROOT member and the ROOT::Internal member.


/// Get the maximum number of logical CPUs available.
/// In case of having an affinity mask (TBB), return the logical CPU available to the current process in accordance with it.
Int_t GetMaxNThreadsAvailable();
Copy link
Member

Choose a reason for hiding this comment

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

Can you adjust the function name to the documentation? It does not return the maximum number of threads.

@phsft-bot
Copy link

Starting build on gcc49/centos7, native/mac1012, gcc49/slc6, gcc62/slc6, native/ubuntu14 with flags -Dvc=OFF -Dimt=ON -Dccache=ON
How to customize builds

@xvallspl
Copy link
Contributor Author

Still not happy with the function naming. Any suggestions?

@phsft-bot
Copy link

Starting build on gcc49/centos7, native/mac1012, gcc49/slc6, gcc62/slc6, native/ubuntu14 with flags -Dvc=OFF -Dimt=ON -Dccache=ON
How to customize builds

@Axel-Naumann
Copy link
Member

Axel-Naumann commented Jul 13, 2017

I think the name is perfect given the current implementation!

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.

3 participants