search

SuperElastix / elastix

Official elastix repository
http://elastix.dev
Apache License 2.0
449 stars 110 forks source link

PERF: Replace PlatformMultiThreader with MultiThreaderBase #1150

Closed N-Dekker closed 2 weeks ago

N-Dekker commented 2 weeks ago

MultiThreaderBase typically uses a thread pool, by default. As set by itk::MultiThreaderBase::GetGlobalDefaultThreaderPrivate(): https://github.com/InsightSoftwareConsortium/ITK/blob/v5.4.0/Modules/Core/Common/src/itkMultiThreaderBase.cxx#L164

See also: "Platform multi-threader should be avoided, as it introduces the biggest overhead". https://github.com/InsightSoftwareConsortium/ITK/blob/v5.4.0/Modules/Core/Common/include/itkMultiThreaderBase.h#L199-L200

Might improve the performance of:

(as noted below, in comments "in-line" with the code changes.)

@mstaring @stefanklein FYI, I already noticed before that elastix was already using the ITK thread pool, even before I started adding more multi-threading. With this commit, I exhaustively replaced every remaining "PlatformMultiThreader" with "MultiThreaderBase". MultiThreaderBase is typically using the ITK thread pool. (When looking in the code, note that the old ThreaderType was defined as PlatformMultiThreader.)

N-Dekker commented 2 weeks ago

I think this pull request is OK, just wondering if any performance effect will be noticed in practice. As I said before, this PR affects: