|Re: [eigen] Mapping array of scalars into quaternions|
[ Thread Index |
| More lists.tuxfamily.org/eigen 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
inline | EIGEN_STRONG_INLINE
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
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 . 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
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.