[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. :-)

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?


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