InsightSoftwareConsortium / ITK

Insight Toolkit (ITK) -- Official Repository. ITK builds on a proven, spatially-oriented architecture for processing, segmentation, and registration of scientific images in two, three, or more dimensions.
https://itk.org
Apache License 2.0
1.41k stars 663 forks source link

Replace pthreads and Win32 threads with C++11 threads? #3136

Open seanm opened 2 years ago

seanm commented 2 years ago

I see ITK master uses both pthreads and Win32 threads.

Does this just pre-date C++11 threads? Or is there some need for platform-specific code here? i.e. could we switch to C++11 threads?

thewtex commented 2 years ago

We do use C++11 threads by default in master.

hjmjohnson commented 2 years ago

@thewtex I am assuming that the answer to @seanm 's question is "YES" we should purge the system of pthread and Win32 threads in favor of C++11 therads.

Only supporting and exposing one threading mechanism.

dzenanz commented 2 years ago

Yes, the use of platform-specific threads predates C++11 by more than a decade.

purge the system of pthread and Win32 threads in favor of C++11 threads

Anyone is welcome to take that upon themselves.

dzenanz commented 2 years ago

We do use C++11 threads by default in master.

Yes, ThreadPool uses C++11 threads.

hjmjohnson commented 2 years ago

@dzenanz I'm asking for a little more clarification here so that I can determine the scope of the work I am considering.

A) Are Win32 & pthreads already deprecated and replaced by C++11 threads in the default builds on all platforms from on the master branch? That would imply some tedious but simple housecleaning. I'm willing to have a @kian-weimer take a stab at cleaning that up.

B) If, however, there are critical path dependencies or backward-compatible special builds that need to be maintained, then that task is probably too large to take on without a longer-term strategy in place.

thewtex commented 2 years ago

The implementations are not the default, but they are around for backwards compatibility and testing. Yes, we should purge them, but that should occur in ITK 6.

C++11 threads are the current default for C++ builds.

We also support TBB threading, which is the default for Python package builds in ITK 5.3.

In the future, the best default could be #2831

dzenanz commented 2 years ago

@hjmjohnson Purging native threads is a large and finnicky undertaking.

blowekamp commented 2 years ago

With the prior v5.2 release there were still some issues and instabilities with the default ITK thread pools which cause SimpleITK to set the older platform multithreads as default. There have been various improvements and fixes to the default pool threader that are coming in v5.3 that needs some to to see if they address the previous issues.

IMHO it is premature to remove the older threading model as the threading pools are still maturing. Introducing a new C++11 option would be another option that would need testing, and time to detect issues before it was mature.