Re: [eigen] ThreadSafety of Eigen?

[Manuel Yguel <manuel.yguel@xxxxxxxxx>]

Thanks Michael and Benoit, I have a clear picture of the problem now.

Manuel

On Wed, Mar 31, 2010 at 5:00 PM, Benoit Jacob <jacob.benoit.1@xxxxxxxxx> wrote:
> 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
>>>
>>>
>>>
>>
>>
>>
>
>
>