[Kstars-devel] XYZ Rotation matrix in 3.5.x

[James Bowlin]

On Thursday 15 June 2006 02:10, Luciano Montanaro wrote:
> Well, a 20% improvement is not something to sneeze at. Maybe it's not 
> sufficient to visibly improve things, but it may be one of the needed 
> optimizations.

There is some added expense/baggage using the rotation matrix.  First,
each SkyPoint needs storage space for 3 more doubles, the x-y-z.  Also,
in the simple way I implemented it, it does not work for AltAz coordinates.
It is possible that this could be fixed by recalculating the x-y-z
coordinates for all SkyPoints.  But these would then need to get updated
(I think).  Also, I don't know how to deal with refraction correction.

If it didn't come with this extra baggage, then I would have suggested that
we adopt it.  If it had given us a factor of 2 or 3 speed improvement then
I would have argued that the extra expense would be worth it.  But as it
stands now, I think our efforts would be better spent elsewhere.  But I
don't have strong feelings about it.  If other people want this code in
the 4.0 trunk, I would be happy to help port it.  

> > So it appears that modern processors can do simple sin() and cos()
> > calculations almost as fast as multiplication.  This implies that there
> > is probably not much speed boost from caching sin() and cos() values,
> > at least for modern processors.
> >
> 
> Are you sure the speedup is not masked by the overhead of function calls?
> Maybe the toScreen function should be inlined.

This is possible.  It could also be masked by the time it takes to do the
actual drawing on the screen.  The times roughly doubled when I switched
from outline mode to filled mode for drawing the Milky Way.   I'll try
inlining and see if that makes any difference.  

> Also in the toScreen function, you evaluate     
> 
>     double Width = ( width() * screen_scale );
>     double Height = ( height() * screen_scale );
> 
> each time, while it is a value which will not change between invocations.
> (You could actually cache Width * 0.5 and Height * 0.5, from what I see.

Good idea.  But I don't expect this change to give us a startling speed boost.

> Obviously, these optimizations will not do much good if the problem is the 
> function call overhead anyway.
> 
> > For completeness, I've included the new code below.  Only the first two
> > routines are new.  98% of slowToScreenXYZ() came from the original
> > toXY(). Maybe there is a more efficient way of doing the matrix
> > calculation, making use of the SSE registers or something.  I tried
> > removing the sqrt() from both branches but this didn't significantly
> > alter the timing results.
> >
> 
> Maybe. But vector instructions works best if you feed them with, well, 
> vectors, so probably it should be better to ave a function to iterate over 
> a vector of coordinates.

I can give this a try by putting the celestial x-y-z values in an array and
also put the nine rotation coefficients in an array (or an array of arrays).
But I doubt that the compiler is being given the flags that tell it to use
the SSE registers.  Usually this sort of thing is hand-coded but maybe
compilers have advanced so far that they are now good at this sort of thing.
But you did remind me, I remember working on some code like this about 15
years ago and there was a mild speed boost when the data was put into arrays.

Thanks for the comments Luciano.  You have made me realize that I should
probably run a few more tests: inlining toScreen(); caching the Width and
Height; using arrays for the matrix evaluation; and also running a test that
just calls toScreen() and toXY() repeatedly to eliminate masking due to the
drawing functions.

I had been hoping that this might be a quick "end around" the speed problems
in the 4.0 code that would make it more responsive and thus a more user
friendly development platform.   Unless I've made a mistake (or a series of
mistakes) it doesn't look like that is going to happen.

 
-- 
Peace, James