BUG: Do not build / use Pool multi-threader without winthreads or pth…

[hjmjohnson]

No description provided.

[hjmjohnson]

http://review.source.kitware.com/#/c/23525/

Matt McCormick
Uploaded patch set 1.Jun 13 11:40 AM

Francois Budin
Patch Set 1: Code-Review+1Jun 13 12:02 PM

Kitware Build Robot
Patch Set 1: Verified-1 Build Failed: CDash filtered results: https://open.cdash.org/index.php?&project=Insight&filtercount=3&field1=buildname/string&compare1=63&value1=23525-1&field2=buildstarttime/date&compare2=83&value2=2015-03-01&field3=buildstarttime/date&compare3=84&value3=2029-01-01Jun 13 1:56 PM

Dzenan Zukic
Jun 14 6:50 AM
↩
Patch Set 1: Code-Review+1

(1 comment)

Interface-wise, this is looking good!

Modules/Core/Common/src/itkMultiThreaderBase.cxx
Line 29:
You should probably move this definition to CMake code and have Pool-related tests only if this is enabled.

[hjmjohnson]

@dzenanz This has gotten out of date, and I think you are the most appropriate person to address these issues. Can you take over this PR?

[dzenanz]

I think that this patch was made for compatibility with emscripten. The part of the patch where GetGlobalDefaultNumberOfThreads and GetGlobalDefaultNumberOfThreadsByPlatform are moved to MultiThreaderBase can be separated and merged already. Emscripten-specific changes can be done at a later time. Unless @thewtex wants to finish the job now 😄

[dzenanz]

I rebased this on current master, but I guess #ifdefs need a proper update.

[dzenanz]

Example runtime error running /Users/vsts/agent/2.144.0/work/1/s-build/bin/ITKCommon2TestDriver "itkMultiThreaderTypeFromEnvironmentTest" "PooL":

ITK test driver caught an ITK exception:

itk::ExceptionObject (0x7fec3b801fd0)
Location: "unknown" 
File: /Users/vsts/agent/2.144.0/work/1/s/Modules/Core/Common/src/itkMultiThreaderBase.cxx
Line: 521
Description: itk::ERROR: ITK has been built without PoolMultiThreader support!

[dzenanz]

The problem was that check #if defined( ITK_USE_PTHREADS ) || defined( ITK_USE_WIN32_THREADS ) in itkMultiThreaderBase.cxx was before any header was included, so it was always false.

[thewtex]

Thanks for the updates @hjmjohnson and @dzenanz ! LGTM! :-)