Review Request 108992: Simple optimizations in SignalPlotter

[Raul Fernandes]

> On Feb. 22, 2013, 1:32 a.m., Aaron J. Seigo wrote:
> > plasma/widgets/signalplotter.cpp, line 230
> > <http://git.reviewboard.kde.org/r/108992/diff/1/?file=114209#file114209line230>
> >
> >     no point in taking PODs out of the loop
> 
> Raul Fernandes wrote:
>     Actually, the benefit exists in PODs too (the compiler has to adjust the stack each iteration) but is minor. I agree.
>     I don't agree that it uglifies the code or make it less readable.
>     To me, it doesn't make any difference to readability and has the (little) gain in performance. My opinion.
>
> 
> Aaron J. Seigo wrote:
>     So we have a difference in opinion when it comes to readability, and that's what it is: an opinion. As the maintainer, my opinion wins in a tie :) My reason for this is that we end up with more variables outside the scope they are used which means to understand the loop one needs to read more outside the loop, and should other variables of similar name appear later on .....
>     
>     PODs have very, very little overhead and we value the readability over such micro-optimizations. to confirm this with actual numbers i wrote a small test program that interates 10 million times performing simple math on 6 ints that are declared in the loop or out of it. result? neither exceeded 1ms. so in this case, it is not worth it.
>     
>     Conclusion: PODs will remain in the loops. Thanks :)

Ok, no problem. It is better to push these changes than push nothing. I will update the patch latter.


- Raul


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


On Feb. 22, 2013, 7:17 p.m., Raul Fernandes wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108992/
> -----------------------------------------------------------
> 
> (Updated Feb. 22, 2013, 7:17 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Description
> -------
> 
> - create variables and classes outside the loops
> - reserve space in QList if we know already how many items will be added (avoid unnecessary reallocations)
> - use const_iterator when possible
> - remove a useless call (p->setPen(Qt::NoPen) - it will be set latter before be used)
> - avoid multiplications (x3, x2, x1 and x0)
> 
> 
> Diffs
> -----
> 
>   plasma/widgets/signalplotter.cpp 8e9e294 
> 
> Diff: http://git.reviewboard.kde.org/r/108992/diff/
> 
> 
> Testing
> -------
> 
> I have tested with KDE 4.10 with no problems.
> I have seen a improvement of about 5% in drawPlots() function, the most expensive function in painting.
> 
> 
> Thanks,
> 
> Raul Fernandes
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20130307/b3a98647/attachment-0001.html>