Review Request 108992: Simple optimizations in SignalPlotter

[Raul Fernandes]

> On Feb. 18, 2013, 1:39 a.m., Aleix Pol Gonzalez wrote:
> > I don't see loosening the variables' scope as a codebase improvement. Mostly otherwise.
> > 
> > Also I'd like to know how you measured this 5% of improvement, which either way I'm unsure if it's worth it considering that this patch makes everything global, now.
> 
> Raul Fernandes wrote:
>     I think it is the worst response that I can have.
>     Never in my entire life I saw anyone that complains about creating classes outside loops looses the scope because it is one of the most basic forms of optimizing the code. Creating classes inside loops is a great waste of resources.
>     That why KDE4 is still bloat and slow.
>     Is is so true that KDE3 is still alive.
>     I think some developers should learn how to write better and fast code and avoid some commentaries like this or "This is only one more full update. Who cares??" (I saw this insanity in some place in KDE's code).
>     This patch is one of those that I should not put in review because it is so basic, but I do because I don't want to commit anything without approval.
> 
> Aaron J. Seigo wrote:
>     Raul, thanks for the patch. We appreciate contributions such as these, particularly ones that work on improving what exists .. and this bit of code definitely could use it.
>     
>     However, we do not interact with each other in the way you did in your comment here. We try to show each other respect and understanding. Not only do we find more enjoyment and team spirit this way, but it prevents people from treating us the same way. There are issues with your patch which I cover in my review, but I don't suggest you ought to learn how to write better code or otherwise insult you. What would that achieve, besides getting on your nerves? Nothing.
>     
>     You did not answer Aleix's question about how you measured the improvement, which is a very valid question. The least one can do is respect an honest question with an honest answer, yes?
>     
>     Our code of conduct describes our commitments to each other in these things: http://www.kde.org/code-of-conduct/
>     
>     I hope you can receive this comment in the constructive manner which I intend it, and look forward to more of your patches in the future. :)
> 
> Raul Fernandes wrote:
>     Sorry about that but it was a reaction to one thing that always happens in open software.
>     It is sometimes hard to a new contributor send patches to projects because the maintainers tend to reject new views from people outside. Sometimes in a rough way.
>     I have several patches optimizing the code in KDE and cannot send upstream because this way of thinking.
>     There are lot of developers in KDE that doesn't care about speed, maybe because they use superfast computers.
>     But I use a core i5 third generation in work place and the KDE is laggy to me. Even the 4.10.
>     This is why the KDE3 is still alive and the razor-qt exists. The reason they use is the speed.
>     Considering the speed of computers today, the KDE should executes smooothly in any hardware with all effects active.
>     Answering how I discovered the 5% of improvement I have profiled the code.
>     I didn't attach the output of profiler because it takes a lot of space and the patch seems too trivial to me.
>     This is the point. If such trivial patch have this reception, imagine other that makes huge changes in algoritms choosing one more complicated for the sake of speed.
>     In this case I have one. Grouping all drawPath calls to avoid the context switching in QPainter.

One thing good to say is the SignalPlotter continues to draw the contents to QPainter even it is not exposed, wasting CPU cycles that should be used by another thing.


- Raul


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


On Feb. 17, 2013, 12:57 p.m., Raul Fernandes wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108992/
> -----------------------------------------------------------
> 
> (Updated Feb. 17, 2013, 12:57 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/20130222/61a9f864/attachment-0001.html>