[Marble-devel] Review Request: Minor speed-up: do not carry out multiplications inside the inner loop.

[Torsten Rahn]

> On 2010-01-22 18:47:56, Bernhard Beschow wrote:
> > FYI: This optimization is called an "induction variable elimination" (see http://en.wikipedia.org/wiki/Induction_variable_recognition_and_elimination) and is considered by a modern compiler automatically. However, a compiler may even decide that this optimization would result in additional memory access, in wich case the multiplication is cheaper. In the end, the compiler will rewrite the code anyway, namely in a way it considers optimal. Lesson learned: Write readable code; the compiler will optimize for you. :-)
> 
> Torsten Rahn wrote:
>     Interesting read. I also like
>     
>     http://en.wikipedia.org/wiki/Loop_optimization
>     
>     I usually prefer to not be overly optimistic regarding the built-in AI inside the compiler. ;-) 
>     But in this case it sounds like the compiler can indeed easily make a better judgement on what will yield better results. Maybe this also explains why I don't see a noticable differece between the two? 
>     
>     Sounds like we better should revert this patch?
>     
>     Or is the compiler also capable of doing the inverse optimization if mutiplication is faster? -- Then it wouldn't matter much.
>     
>     Ariya: What do you think?
>     
>
> 
> Ariya Hidayat wrote:
>     The reason you don't see any impact is because the bottleneck is somewhere else (as you can guess, I am working on that as well :).
>     
>     I am pretty sure the compiler can do reverse optimization, especially if it can do a vectorized operation for both variables. The addition version is simpler and still readable vs the multiplication version that I guess it is still worth to keep it (for all other dumb compilers out there).
>     
>     However I agree with the general idea of not optimizing too much.
>     
>     Torsten, feel free to revert the patch. I'll submit another series of patches for this once I got the loop moved into the texture mapper and completely switch to Bresenham-like approach.

> The reason you don't see any impact is because the bottleneck is somewhere else

Thought so :)

> (as you can guess, I am working on that as well :).
> I'll submit another series of patches for this once I got the loop moved into 
> the texture mapper and completely switch to Bresenham-like approach.

Cool, I'm looking forward to see the results :) 
Hopefully the solution will be less complicated and still faster. *crossing fingers*


> I am pretty sure the compiler can do reverse optimization, 

Ok, then let's leave it as is unless somebody is coming up with so reason to revert.


- Torsten


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/2650/#review3792
-----------------------------------------------------------


On 2010-01-20 07:24:04, Ariya Hidayat wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2650/
> -----------------------------------------------------------
> 
> (Updated 2010-01-20 07:24:04)
> 
> 
> Review request for marble.
> 
> 
> Summary
> -------
> 
> This is for pixelValueApprox function. We skip the multiplications by doing variable increments ourselves.
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdeedu/marble/src/lib/AbstractScanlineTextureMapper.cpp 1077405 
> 
> Diff: http://reviewboard.kde.org/r/2650/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ariya
> 
>