> 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 > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
