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

set explicit number of threads in every OpenMP parallel region #6135

Merged
merged 4 commits into from
Oct 10, 2023

Conversation

jameslamb
Copy link
Collaborator

@jameslamb jameslamb commented Oct 9, 2023

Contributes to #4705
Contributes to #5124
Contributes to #5987

Short Description

Proposes adding an explicit num_threads clause to every OpenMP parallel region in this project that doesn't have one, and a CI check to enforce that.

As of this PR, such regions will still fall back to the global value from omp_get_num_threads()... I'll resolve that and propose switching them to a LightGBM-controlled value in a follow-up PR.

Longer Description

LightGBM uses OpenMP for to parallelize many operations via multi-threading.

It allows users to customize the degree of parallelism (i.e. number of threads) by passing through a parameter num_threads in several parts of the the public API, which is then used to set the global1 default number of threads for OpenMP via omp_set_num_threads().

OMP_SET_NUM_THREADS(config.num_threads);

inline void OMP_SET_NUM_THREADS(int num_threads) {
static const int default_omp_num_threads = OMP_NUM_THREADS();
if (num_threads > 0) {
omp_set_num_threads(num_threads);
} else {
omp_set_num_threads(default_omp_num_threads);
}
}

Many parallel regions in the project then implicitly rely on that global configuration to determine the number of threads used, in blocks like this without a num_threads() clause:

LightGBM/src/c_api.cpp

Lines 440 to 441 in 992f505

#pragma omp parallel for schedule(static)
for (int i = 0; i < nrow; ++i) {

The MSVC docs for OpenMP2 have a good description of how OpenMP implementations determine the number of threads for a parallel region without an explicit num_threads clause:

To determine the number of threads that are requested, the following rules will be considered in order. The first rule whose condition is met will be applied:

1. If the num_threads clause is present, then the value of the integer expression is the number of threads requested.
2. If the omp_set_num_threads library function has been called, then the value of the argument in the most recently executed call is the number of threads requested.
3. If the environment variable OMP_NUM_THREADS is defined, then the value of this environment variable is the number of threads requested.
4. If none of the methods above is used, then the number of threads requested is implementation-defined.

As described in #4705, that reliance on global state can lead to undesirable side effects. LightGBM's use of omp_set_num_threads() affects all OpenMP-using routines in the same process, and conversely LightGBM can be affected by any other programs in the same process which call omp_set_num_threads().

Footnotes

  1. By "global" here, I mean "affecting and affected by all OpenMP-using routine in the same process".

  2. https://learn.microsoft.com/en-us/cpp/parallel/openmp/2-directives?view=msvc-170#23-parallel-construct. See also the OpenMP spec: https://www.openmp.org/spec-html/5.0/openmpse14.html#x54-800002.6.

@jameslamb jameslamb added the fix label Oct 9, 2023
@jameslamb jameslamb changed the title WIP: set explicit number of threads in every OpenMP parallel region set explicit number of threads in every OpenMP parallel region Oct 9, 2023
@jameslamb jameslamb marked this pull request as ready for review October 9, 2023 16:18
@jameslamb
Copy link
Collaborator Author

thanks @guolinke !

Copy link

This pull request has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants