Skip to content

Commit 6de436c

Browse files
authored
respect users OMP limits with setDTthreads (#7389)
1 parent a8ad00f commit 6de436c

File tree

2 files changed

+15
-10
lines changed

2 files changed

+15
-10
lines changed

NEWS.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,8 @@ See [#2611](https://github.com/Rdatatable/data.table/issues/2611) for details. T
342342
343343
21. `[` now preserves existing key(s) when new columns are added before them, instead of incorrectly setting a new column as key, [#7364](https://github.com/Rdatatable/data.table/issues/7364). Thanks @czeildi for the bug report and the fix.
344344
345+
22. `setDTthreads(percent=)` and `setDTthreads(threads=)` now respect `OMP_NUM_THREADS` and `omp_get_max_threads()`, ensuring consistency with `setDTthreads()` (no arguments) when OpenMP environment variables are set, [#7165](https://github.com/Rdatatable/data.table/issues/7165). Previously, explicitly setting a thread count or percentage would ignore these OpenMP limits, potentially exceeding the user's intended thread cap. Thanks to @bastistician for the report and @ben-schwen for the fix.
346+
345347
### NOTES
346348

347349
1. The following in-progress deprecations have proceeded:

src/openmp-utils.c

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,17 @@ static int getIntEnv(const char *name, int def)
2929
static inline int imin(int a, int b) { return a < b ? a : b; }
3030
static inline int imax(int a, int b) { return a > b ? a : b; }
3131

32+
static inline int cap_threads(int n)
33+
{
34+
n = imin(n, omp_get_thread_limit()); // honors OMP_THREAD_LIMIT when OpenMP started; e.g. CRAN sets this to 2. Often INT_MAX meaning unlimited/unset
35+
n = imin(n, omp_get_max_threads()); // honors OMP_NUM_THREADS when OpenMP started, plus reflects any omp_set_* calls made since
36+
// max_threads() -vs- num_procs(): https://software.intel.com/en-us/forums/intel-visual-fortran-compiler-for-windows/topic/302866
37+
n = imin(n, getIntEnv("OMP_THREAD_LIMIT", INT_MAX)); // user might expect `Sys.setenv(OMP_THREAD_LIMIT=2);setDTthreads()` to work. Satisfy this
38+
n = imin(n, getIntEnv("OMP_NUM_THREADS", INT_MAX)); // expectation by reading them again now. OpenMP just reads them on startup (quite reasonably)
39+
n = imax(n, 1); // just in case omp_get_* returned <=0 for any reason, or the env variables above are set <=0
40+
return n;
41+
}
42+
3243
void initDTthreads(void)
3344
{
3445
// called at package startup from init.c
@@ -48,13 +59,7 @@ void initDTthreads(void)
4859
}
4960
ans = imax(omp_get_num_procs() * perc / 100, 1); // imax for when formula would result in 0.
5061
}
51-
ans = imin(ans, omp_get_thread_limit()); // honors OMP_THREAD_LIMIT when OpenMP started; e.g. CRAN sets this to 2. Often INT_MAX meaning unlimited/unset
52-
ans = imin(ans, omp_get_max_threads()); // honors OMP_NUM_THREADS when OpenMP started, plus reflects any omp_set_* calls made since
53-
// max_threads() -vs- num_procs(): https://software.intel.com/en-us/forums/intel-visual-fortran-compiler-for-windows/topic/302866
54-
ans = imin(ans, getIntEnv("OMP_THREAD_LIMIT", INT_MAX)); // user might expect `Sys.setenv(OMP_THREAD_LIMIT=2);setDTthreads()` to work. Satisfy this
55-
ans = imin(ans, getIntEnv("OMP_NUM_THREADS", INT_MAX)); // expectation by reading them again now. OpenMP just reads them on startup (quite reasonably)
56-
ans = imax(ans, 1); // just in case omp_get_* returned <=0 for any reason, or the env variables above are set <=0
57-
DTthreads = ans;
62+
DTthreads = cap_threads(ans);
5863
DTthrottle = imax(1, getIntEnv("R_DATATABLE_THROTTLE", 1024)); // 2nd thread is used only when n>1024, 3rd thread when n>2048, etc
5964
}
6065

@@ -151,9 +156,7 @@ SEXP setDTthreads(SEXP threads, SEXP restore_after_fork, SEXP percent, SEXP thro
151156
} else {
152157
if (n == 0 || n > num_procs) n = num_procs; // setDTthreads(0) == setDTthread(percent=100); i.e. use all logical CPUs (the default in 1.12.0 and before, from 1.12.2 it's 50%)
153158
}
154-
n = imin(n, omp_get_thread_limit()); // can't think why this might be different from its value on startup, but call it just in case
155-
n = imin(n, getIntEnv("OMP_THREAD_LIMIT", INT_MAX)); // user might have called Sys.setenv(OMP_THREAD_LIMIT=) since startup and expect setDTthreads to respect it
156-
DTthreads = imax(n, 1); // imax just in case
159+
DTthreads = cap_threads(n);
157160
// Do not call omp_set_num_threads() here. Any calls to omp_set_num_threads() affect other
158161
// packages and R itself too which has some OpenMP usage. Instead we set our own DTthreads
159162
// static variable and read that from getDTthreads(n, throttle).

0 commit comments

Comments
 (0)