Re: [eigen] ThreadSafety of Eigen?

[Benoit Jacob <jacob.benoit.1@xxxxxxxxx>]

2010/3/31 Manuel Yguel <manuel.yguel@xxxxxxxxx>:
> I am not familiar with such kind of problems, so could you be more precise.
> Can you explain at which level it is to be considered.
> For instance a lot of factorization algorithms: QR, LU, SVD etc, store
> internal variables inside a class, so the usage of an object of one of
> those classes is not thread-safe.

By thread-safe we mean what i described in my first email: if
you have two matrices m1 and m2, then you can have thread1 working on
m1 and thread2 working on m2 without any interference.

By re-entrant we mean that the same function can be called
concurrently by 2 threads.

Eigen satisfies both of these conditions (except for the bugs that Bjorn found).

Having member data in class HouseholderQR is not at all a problem in
this respect. As long as you don't try to work on the same
HouseholderQR object concurrently in 2 threads. That would fail, but
it's not what we mean by thread-safe.

Benoit


> As this is obvious, I think the discussion does not apply there but
> however I would really like to know more, where people expect to have
> thread safety.
>
> If I miss some point of the doc describing that, disregard this post
> and please just send me the link.
>
>
> - cheers,
>
> Manuel
>
>
> On Wed, Mar 31, 2010 at 4:11 PM, Benoit Jacob <jacob.benoit.1@xxxxxxxxx> wrote:
>> 2010/3/31 Björn Piltz <bjornpiltz@xxxxxxxxxxxxxx>:
>>> Some thoughts on the subject:
>>> Benoit is saying that Eigen is reentrant - as it should be. That is what
>>> most people mean by thread-safe, even if it isn't thread safe in the sense
>>> that different threads can safely manipulate the same matrix.
>>> However, this isn't entirely true.
>>> <begin nitpicking>
>>> There are a couple of problematic uses of static data in Eigen.
>>> There are numerous uses of const static global or member variables. e.g
>>> static _EIGEN_DECLARE_CONST_FAST_Packet4f(ZERO, 0);
>>
>> OK, i didn't think of that. As you say it's not problematic as it
>> doesn't change.
>>
>>> or
>>> class MatrixFunction{
>>>   static const int Rows = Traits::RowsAtCompileTime;
>>
>> That one is a mistake: it should be an enum. Let's fix that. Making it
>> a "static const int" is just giving the compiler the additional work
>> of having to understand that it can optimize it away at compile time.
>>
>>> These are unproblematic, since these variables are initialized at program
>>> start and never change. One only has to consider the "static initialization
>>> order fiasco" - but that is hardly a risk here - and the runtime overhead at
>>> startup, which should be minimal for these examples. There are a couple of
>>> globals in src/Core/arch/AltiVec/PacketMath.h which haven't been marked as
>>> const even though they should be, but that's an easy fix.
>>> The finer subtleties occur in examples where static variables are declared
>>> in a function. These variables will be initialized lazily. i.e. the first
>>> time the function is called.
>>> The use of static variables in blueNorm() in src/core/stablenorm.h is just
>>> plain wrong. The good news is that this is the only example I found of
>>> blatant non-reentrancy.
>>
>> (pinging Gael as I believe he copied that code from elsewhere)
>>
>>> MatrixBase<Derived>::blueNorm() const
>>> {
>>>   static int nmax = -1;
>>>   static RealScalar b1, b2, s1m, s2m, overfl, rbig, relerr;
>>>   if(nmax <= 0)
>>>   {
>>>     // calculate b1, b2, etc
>>> ...
>>>     nmax = nbig;
>>>   }
>>
>> OK indeed this is not reentrant! Thanks for spotting it. For Gael.
>>
>>> An even subtler example would be:
>>> QuaternionBase<Derived>::slerp(Scalar t, const QuaternionBase<OtherDerived>&
>>> other) const
>>> {
>>>   static const Scalar one = Scalar(1) - NumTraits<Scalar>::epsilon();
>>> ...
>>> This example is almost certainly safe, at least as long Scalar is a POD.. The
>>> problem is that either the expression is so simple the compiler optimizes it
>>> away - in which case there is no reason declaring it static - or it isn't.
>>
>> This one for Gael too :)
>>
>> Benoit
>>
>>> Then it might actually be inefficient, because  there is a runtime check,
>>> which involves locking(GCC and MSVC treat these cases differently), to see
>>> if the variable has been initialized or not.
>>> The fact is that *there is no way to use lazily initialized non-POD static
>>> variables that does not include locks or some atomic API*.
>>> For reference,
>>> see http://www.aristeia.com/Papers/DDJ_Jul_Aug_2004_revised.pdf and http://stackoverflow.com/questions/6915/thread-safe-lazy-contruction-of-a-singleton-in-c
>>> So, to return to blueNorm(), I see two ways of fixing this
>>> A: Mark the function as non-reentrant. This would be Eigen's only such
>>> function then, or
>>> B: Put b1, b2 etc in a class, a global instance of which will be initialized
>>> at startup. I think this initialization might be optimized away in programs
>>> that never call the function(that needs to be checked).
>>> Don't be tempted to stick to lazily initialization(through double-checked
>>> locking or something equally clever), since it just isn't thread-safe.
>>> To end this mail on a positive note: reentrancy is actually one of the
>>> reasons I decided to switch to Eigen in the first place, after having used
>>> numerous other libraries lacking in this respect.
>>> best
>>> Björn
>>
>>
>>
>
>
>