D14408: SymbolView: Avoid unneeded update of current item
noreply at phabricator.kde.org
Fri Jul 27 15:16:14 BST 2018
loh.tar added a comment.
Thanks, that was much better.
It seams we have different priorities or tastes when is something an acceptable waste or effort.
- Your point is, as long as everything still works fluffy enough there is no need to change something.
- My point is, avoid as much work (at CPU side, not at coder side) as possible, do only things once and only when there is a need for.
> I actually tried adding the timer to the function to measure the time. I tried it on a almost 4000 lines long C++ file (far too big ;) and it has around 200 symbols. In the measurements I did, it never reached a full millisecond to finish, it always printed 0ms.
Sadly ignored you my CPU use observation and still arguments with these less than 1ms. Would you say you could not reproduce my observation that would that be a point.
> I'm just saying that I don't think the complexity added for the micro-optimization is worth it. It just becomes harder to maintain.
That is sadly not a hard fact, its a matter of taste when is something too complex. With that can you almost ever reject some request. When I look at the changes appears they not so complex to me. The existing quirk, how the item is searched, was already before such inscrutable. Sorry, not my code.
In the meantime I have an idea how to solve that something simpler, but not really clean. The main problem is, that you can't sort the tree by there line number column due to string compare -> 1 10 2 20. So my idea is that the line numbers, after the tree is new finished, to pad with "0" to become 0001 0002 0010 0020. But then must be made a decision how many lines to support.
> Making the delay longer (200 ms) does not decrease the number of times the function is called.
Which function? cursorPositionChanged() of cause not, updateCurrTreeItem() absolutely, as long as the user is active.
> It still stays at once when the key is pressed and once when the key is released.
It seams you mean cursorPositionChanged(),that's irrelevant. The timer is restarted and updateCurrTreeItem() only called once.
> The bad thing is that the user actually feels that the plugin is slower as it now takes 200ms for the current-item to change in stead of just 100ms previously.
Oh, you should try me other patch, there IS it notable, but here I have doubts.
> One thing that you could cache without much complexity is the last cursor position and only start the timer if the line changes.
With that I started, then I thought we can do it better.
To: loh.tar, #kate, sars
Cc: ngraham, sars, kwrite-devel, #kate, michaelh, kevinapavew, demsking, cullmann, dhaumann
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the KWrite-Devel