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

On February 22nd, 2013, 1:32 a.m. UTC, Aaron J. Seigo wrote:

plasma/widgets/signalplotter.cpp (Diff revision 1)
void SignalPlotter::reorderPlots(const QList<uint>& newOrder)
230
    int newIndex;
no point in taking PODs out of the loop

On February 22nd, 2013, 7:15 p.m. UTC, 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.
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 :)

- Aaron J.


On February 22nd, 2013, 7:17 p.m. UTC, Raul Fernandes wrote:

Review request for Plasma.
By Raul Fernandes.

Updated Feb. 22, 2013, 7:17 p.m.

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)

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.

Diffs

  • plasma/widgets/signalplotter.cpp (8e9e294)

View Diff