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 loopOn 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
Testing
Diffs
|