Skip to content

common: restore OpenMP thread limit after GraphicsMagick init #22438

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.

Sign up for GitHub

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

Draft
kofa73 wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

common: restore OpenMP thread limit after GraphicsMagick init #22438

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2deca37
common: restore OpenMP thread limit after GraphicsMagick init
kofa73 Sep 29, 2026
1b4ab6f
common: cap the OpenMP thread count at OMP_THREAD_LIMIT
kofa73 Oct 3, 2026
6778d31
RELEASE_NOTES.md: darktable-cli crash with --threads below CPU count
kofa73 Oct 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions RELEASE_NOTES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,9 @@ changes (where available).
results depending on the number of CPU threads, and occasionally
wrong colors near the left or right image edge.

- Fixed darktable-cli crashing when `--threads` was set below the
number of CPUs.

## Lua

### API Version
Expand Down
9 changes: 9 additions & 0 deletions src/common/darktable.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1660,6 +1660,8 @@ int dt_init(int argc,
}

#ifdef _OPENMP
// avoid allocating per-thread buffers for workers excluded by the hard limit
darktable.num_openmp_threads = MIN(darktable.num_openmp_threads, omp_get_thread_limit());
omp_set_num_threads(darktable.num_openmp_threads);
omp_set_dynamic(FALSE);
#endif
Expand Down Expand Up @@ -2052,6 +2054,13 @@ int dt_init(int argc,
MagickWandGenesis();
#endif

#ifdef _OPENMP
// GraphicsMagick's init resets the thread count to the number of CPUs,
// overriding --threads. dt_alloc_perthread() pools are sized by
// dt_get_num_threads(), so a larger team would write past their end.
omp_set_num_threads(darktable.num_openmp_threads);
#endif

@TurboGit TurboGit Sep 30, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

And what about the opposite, if GraphicsMagick do set a threads count below darktable.num_openmp_threads?

@kofa73 kofa73 Sep 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Sorry, had to run (literally).

Deslopping (manually, so I learn something, probably nothing new to anyone else): The original issue was that -t 4 resulted in allocating buffers for 4 threads, then GM reset OpenMP to use 12 threads -> crash.
What was tested now:
OMP_NUM_THREADS sets the default number of threads
-t n overrides that to n (darktable.num_openmp_threads) -- and this number is also used for the allocation
OMP_THREAD_LIMIT m sets a hard maximum.

With OMP_NUM_THREADS = 1 and -t 4, initial number of threads is 1, overridden to 4, buffers are allocated according to that: 4 threads, buffers for 4 threads, OK.
With OMP_THREAD_LIMIT = 1 and -t 4, the hard limit is 1; buffers are still allocated according to -t 4, buffers are allocated for 4 threads, 1 thread is used, wasteful, but no crash.

--- original LLM answer below ---

(Codex)

The same call covers that case. It restores darktable's configured count whether GraphicsMagick raised or lowered it.

By reading the code, Codex checked GraphicsMagick 1.3.46's initialization and darktable's per-thread allocator. Fewer workers cannot overrun buffers sized for the configured count. The reset is unconditional.

A runtime probe with 4 requested threads confirmed that GraphicsMagick selected 1 with OMP_NUM_THREADS=1, then the reset restored 4. With OMP_THREAD_LIMIT=2, the actual team stayed at 2. The build passed, and CPU exports using 0068-rawdenoise-xtrans matched the control pixel for pixel in both cases.

@TurboGit TurboGit Sep 30, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't understand this, if GM asked for 2 threads and we set back to 16 we should have the same issue and then potentially crash in GM this time, no?

Or do you mean that Codex has analyzed all the GM sources to ensure that this is not an issue for GM? That seems very fragile anyway as we depends on GM source code changes.

@kofa73 kofa73 Sep 30, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It ran the integration tests with those environment variables and thread counts (and it also read the sources). I'll continue tomorrow, it's late.


#ifdef HAVE_LIBHEIF
dt_splash_screen_set_progress(_("initializing libheif"));
heif_init(NULL);
Expand Down
Loading