Re: [eigen] Mapping array of scalars into quaternions

[ Thread Index | Date Index | More Archives ]

Everytime you add a EIGEN_STRONG_INLINE on a function that has a
nontrivial body of its own (rather than just forwarding the call to
another function), I need to see some justification (as far as I can
see, that would have to be justified by performance measurements). I'm
by no means an expert, i'm really honestly asking. My fear is about
uselessly increasing code size (and killing instruction caches).

OK, I perform some benchmark on simple operations with gcc4.4, icc and vc++

1/ q = q1*q2;
2/ q = q1*q2*q3.inverse()*q4;
3/ v = q._transformVector(v1);

(v : vector, q : quaternion)

using EIGEN_STRONG_INLINE didn't improve the performance with vc++. The asm generated is never completly inlined both with inline and EIGEN_STRONG_INLINE. It's also have no influence with gcc, since the asm generated is always fully inlined.

with icc (disabling any SSE extension) I have the following results for 10 trials of 100,000,000 operations on a Intel xeon X5550:

mean for 100,000,000 operations

1/   2.22s |	1.70s
2/   8.21s |	6.24s
3/   2.63s |	2.02s

the ratio is roughly the same (0.76).

using EIGEN_STRONG_INLINE instead of inline, inline properly all function call.

But, we don't use this kind of function very often in our project. So, I have not a strong requirement for using EIGEN_STRONG_INLINE. If you think it's too harmfull to use this instruction, its not a problem for me.

 > I also have a few superficial comments:

First, "inline" keywords need only be put on function definitions, no
need to put them on function definitions [1]. I realize that
Quaternion.h already had this problem before your patch. I just am
noticing it now, because EIGEN_STRONG_INLINE takes more space on my

:) When using icc, EIGEN_STRONG_INLINE is only effective if put in class body. It must be specific to the "__force_inline" keywords. "inline" keyword may be remove from the class body and only put in front of method definition.

Second, in the unit test (thanks for writing it), you could just
reserve static arrays with EIGEN_ALIGN16, see what we do in

Third, when you significantly extend/rewrite a file, add yourself a
copyright line. I already added you one on Quaternion.h, also do it on

I'll change these to points and I'm waiting a reply about inlining to send a new patch.


Mail converted by MHonArc 2.6.19+