Re: [eigen] Help on solving a race condition

[Gael Guennebaud <gael.guennebaud@xxxxxxxxx>]

Hi,


After some more thoughts, I'm finally unsure about the thread local
storage solution. Indeed, this solution seems to be quite fragile
because it requires compiler/platform specific code.

So I propose to simply rewrite the code like this:

 static std::ptrdiff_t m_l1CacheSize = 0;
 static std::ptrdiff_t m_l2CacheSize = 0;

 static bool initialized = false;
 if(!initialized)
 {
  m_l1CacheSize = manage_caching_sizes_helper(queryL1CacheSize(),8 * 1024);
  m_l2CacheSize =
manage_caching_sizes_helper(queryTopLevelCacheSize(),1*1024*1024);
  initialized = true;
 }

With this solution it might happen that m_l1CacheSize is overwritten
at the same time it is read by another thread, but since it gets
overwritten by the same value this cannot be an issue, right?

gael.

On Fri, Jun 8, 2012 at 8:54 PM, Gael Guennebaud
<gael.guennebaud@xxxxxxxxx> wrote:
> On Fri, Jun 8, 2012 at 6:13 PM, Rhys Ulerich <rhys.ulerich@xxxxxxxxx> wrote:
>>> Then I guess the only safe and clean solution is to request users to
>>> call a Eigen::init_parallel() or something.
>>
>> That's safe but I'd not call it clean.  It's error prone.  And verbose.
>>
>>> The problem with this approach, is that the cache-sizes are recomputed
>>>for every thread.
>>
>> That doesn't seem terrible to me if I don't have to explicitly call
>> Eigen::init_parallel() as a tradeoff.  Is this a slow process?
>
> as I said in a previous email, after some benchmarking, this approach
> is not that slow: compared to the thread creation cost it's a no-op.
>
>>>> This solution is openmp specific; what if someone wants to use pthreads or
>>>> some other threading system ?
>>
>> Why not #ifdef on compiler versions to dig out the vendor-specific
>> thread-local storage extension when OpenMP isn't active:
>>
>> #if defined(_OPENMP)
>> # define TLS
>> #elseif defined(__GNUC__) && __GNUC__ > 3
>> # define TLS __thread
>> #elseif....
>> # define TLS ...
>> #else
>> # define TLS
>> #endif
>>
>> static TLS std::ptrdiff_t m_l1CacheSize = 0;
>> static TLS std::ptrdiff_t m_l2CacheSize = 0;
>> #pragma omp threadprivate(m_l1CacheSize, m_l2CacheSize)
>>
>> #ifdef TLS
>> #undef TLS
>> #endif
>>
>> Notice that no vendor-specific threadlocal keyword is used when OMP is
>> active because OMP is the better way to go.
>
> ah great, I did not know there existed such compiler extensions.
> That's a lead to pursue.
>
> gael
>
>> It's a bit ugly from the implementer's side, but it the number of
>> vendor-specific thread-local keywords should be small.  From the end
>> user's perspective, however, it is seamless.
>>
>> - Rhys
>>
>>