AW: [eigen] ThreadSafety of Eigen?

["Schmidt, Michael" <Michael.Schmidt@xxxxxxxxxxxxxxxxxxx>]

I'll try to shed some more light on the problem, although I am not exactly an expert on this matter myself:

First of all, as Benoit mentioned in his first reply, we should distinguish thread-safety and reentrancy in this discussion. In my first post, I used the term "thread-safe" but what I was referring to was reentrancy (as Benoit and Björn have probably correctly assumed). Therefore, the following discussion was related to reentrancy of the library.

Now, what's the difference between the two? Threadsafety is concerned with the concurrent (modifying) access to a common data object (e.g. instance of some class) from two or more threads. This requires synchronization of such accesses e.g. using locks or similar (unless it's strictly read-only accesses, including any internal state of the object). Probably due to the fact that such concurrent accesses are not often required and synchronization usually induces quite some overhead, the lib does not provide thread-safety and leaves the synchronization problem to the users of the lib. In contrast, reentrancy is concerned with the question whether it is safe to call one method on different data objects concurrently from different threads (or in other words: use the library independently in different threads). A short definition of reentrancy is: If you supply the same data to a function, it will always return the same result, independent of what is going on elsewhere (referring to both, what happened before the call in the calling thread and what is happening in other threads). The major point to take care of in this respect is, that the functions should not contain non-const static members (which would represent a modifiable state of the function). Furthermore, the initialization of const static members is difficult since it has to be done in a threadsafe way. Therefore, it is advisable to move const static members of a function into some external location, e.g. a static class instance holding the variables as public members, and take care that this static class instance is initialized at the begin of program execution.

That said, and given the (almost complete) reentrancy of the Eigen library, the presence of non-const members in algorithm classes does not mean the algorithms are not reentrant: If you create multiple algorithm instances in different threads they will work correct. It is just not a good idea to access one instance concurrently from different threads.

I hope this roughly describes the problem, otherwise please correct me :)

Michael

-----Ursprüngliche Nachricht-----
Von: Listengine [mailto:listengine@xxxxxxxxxxxxxxxxx] Im Auftrag von Manuel Yguel
Gesendet: Mittwoch, 31. März 2010 16:29
An: eigen@xxxxxxxxxxxxxxxxxxx
Betreff: Re: [eigen] ThreadSafety of Eigen?

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