[Insight-developers] Re: patch and some questions [Re: use environment variable to limit number of parallel threads]

[Luis Ibanez]

Hi Torsten,

Since we are so close to cutting ITK 3.4, and this is not a bug fix
but rather a feature request, would you mind reporting this in the
MANTIS bug tracker as a feature request ?

In this way we can take care of it after we reopen the repository
next week.


    Thanks


       Luis


------------------------
Torsten Rohlfing wrote:
> 
> Okay, how's this for a patch:
> 
> Index: Code/Common/itkMultiThreader.cxx
> ===================================================================
> RCS file: /cvsroot/Insight/Insight/Code/Common/itkMultiThreader.cxx,v
> retrieving revision 1.40
> diff -r1.40 itkMultiThreader.cxx
> 45a46,47
>  > #include <itksys/SystemTools.hxx>
>  >
> 81c83,90
> <   if (m_GlobalDefaultNumberOfThreads == 0)
> ---
>  >   // Check for global setting of thread number from environment variable
>  >   itksys_stl::string itkGlobalDefaultNumberOfThreadsEnv = "0";
>  >   if ( itksys::SystemTools::GetEnv( 
> "ITK_GLOBAL_DEFAULT_NUMBER_OF_THREADS", 
> itkGlobalDefaultNumberOfThreadsEnv ) )
>  >     {
>  >     m_GlobalDefaultNumberOfThreads = atoi( 
> itkGlobalDefaultNumberOfThreadsEnv.c_str() );
>  >     }
>  >
>  >   if (m_GlobalDefaultNumberOfThreads <= 0)
> 
> By changing the range check following the GetEnv from "==0" to "<=0", 
> this should safeguard against negative numbers that may be resulting 
> from "atoi" on the environment variable.
> 
> By the way: I noticed that the documentation for 
> GetGlobalDefaultNumberOfThreads() states
> 
>  /** Set/Get the value which is used to initialize the NumberOfThreads
>   * in the constructor.  Initially this default is set to the number of
>   * processors or 8 (which ever is less). */
> 
> but it doesn't seem the default limit of 8 is actually implemented. Is 
> the documentation out of date maybe?
> 
> Also, the value returned by GetGlobalMaximumNumberOfThreads() is 
> supposed to override any other setting, according to the documentation, 
> but I can't find that implemented anywhere either.
> 
> Cheers!
>  TR
> 
> Bill Lorensen wrote:
> 
>> If we do add this functionality, we should use the kwsys GetEnv() even 
>> though it just calls getenv().
>>  
>> Bill
>>
>>  
>> On 9/20/07, *Torsten Rohlfing* <torsten at synapse.sri.com 
>> <mailto:torsten at synapse.sri.com>> wrote:
>>
>>     Luis:
>>
>>     Sounds pretty much like what I was envisioning.
>>
>>     Item 2) in my opinion only serves to allow people who distribute
>>     pre-compiled ITK-based software to conscientiously ignore
>>     user-specified
>>     CPU limitations. I am not sure that's a good thing, 'cos I believe I
>>     know better how many CPUs a tool on my system should be using than
>>     does
>>     the guy who compiled it elsewhere. Also, it's probably not necessary,
>>     because if, as a developer, you are THAT determined to force the
>>     maximum
>>     number of parallel threads, you probably don't mind hacking the
>>     code a
>>     bit to disable the getenv().
>>
>>     The bottom line is -- if you add option 1) alone, no one will ever
>>     notice any changes unless they define the environment variable. In
>>     that
>>     case, they probably know what they are doing. And you're not further
>>     cluttering the Cmake configuration options.
>>
>>     But anyway, a combination of 1/2 would certainly work for me; I
>>     personally don't use any pre-compiled ITK apps, so having 1) gives me
>>     what I need, and 2) remains under my control.
>>
>>     By the way -- does anyone know whether any of ITK's supported
>>     platforms
>>     would have trouble with the getenv() call?
>>
>>     Thanks everyone!
>>     Torsten
>>
>>     Luis Ibanez wrote:
>>     >
>>     > Hi Torsten,
>>     >
>>     > Here is one option that come to mind:
>>     >
>>     > 1) In itkMultiThreader.cxx lines 80-145 method
>>     >    GetGlobalDefaultNumberOfThreads()
>>     >
>>     >    insert one option where the number of Threads
>>     >    is taken from the environment variable, like:
>>     >
>>     >    std::string numberOfThreadsString =
>>     >     getenv("ITK_DEFAULT_NUMBER_OF_THREADS");
>>     >    m_GlobalDefaultNumberOfThreads =
>>     >      atoi(  numberOfThreadsString.c_str() )
>>     >
>>     > 2) Surrond that option with a CMake enabled define
>>     >    such as ITK_USE_NUMBER_OF_THREADS_FROM_ENVIRONMENT,
>>     >    this define will be set in the itkConfigure.h.in
>>     <http://itkConfigure.h.in>
>>     >    and will be controlled by an advanced option in
>>     >    CMake.
>>     >
>>     >
>>     > Note that the same can be done for the "Maximum"
>>     > number of Threads.
>>     >
>>     >
>>     > In this way, you can get ITK to choose the number of
>>     > threads from the environment variable, but *only* if
>>     > ITK was configured with that CMake variable enabled.
>>     >
>>     >
>>     >
>>     >   Regards,
>>     >
>>     >
>>     >      Luis
>>     >
>>     >
>>
>>     _______________________________________________
>>     Insight-developers mailing list
>>     Insight-developers at itk.org <mailto:Insight-developers at itk.org>
>>     http://www.itk.org/mailman/listinfo/insight-developers
>>
>>
> 
>