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

[Torsten Rohlfing]

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
>
>


-- 
Torsten Rohlfing, PhD          SRI International, Neuroscience Program
 Research Scientist             333 Ravenswood Ave, Menlo Park, CA 94025
  Phone: ++1 (650) 859-3379      Fax: ++1 (650) 859-2743
   torsten at synapse.sri.com        http://www.stanford.edu/~rohlfing/

     "Though this be madness, yet there is a method in't"