Review Request: enhance performance at updating KWallet Editor Tree

[David Faure]

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


Long-standing open reviews are bad, they lead to duplicate work :(


allyourbase.cpp
<http://git.reviewboard.kde.org/r/105633/#comment13522>

    Well, it doesn't really refresh anymore, if you use a cache :-)
    
    The whole point of this method is to get the new count from the daemon.
    
    The problem was that it was called far too often -- I just fixed that
    [and you had too, below... I hadn't seen this pending review before investigating the bug... sigh]



kwalleteditor.cpp
<http://git.reviewboard.kde.org/r/105633/#comment13523>

    All this was useful when updateEntries was called too often. Now that it's only called on actual changes, does it matter as much?



kwalleteditor.cpp
<http://git.reviewboard.kde.org/r/105633/#comment13524>

    This is the actual bugfix -- this method was wrongly assuming the item actually got renamed.
    But then, if it wasn't renamed, there's no reason to call setText() back either.
    
    I committed my fix for this (to 4.9 and master, see bug 279161 for ref). I think this change request can be discarded, apart maybe from the QSet which could help lower CPU usage when adding something to the wallet while kwalletmanager is running, or when renaming an entry in kwalletmanager.


- David Faure


On July 21, 2012, 1:20 p.m., Martin Koller wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105633/
> -----------------------------------------------------------
> 
> (Updated July 21, 2012, 1:20 p.m.)
> 
> 
> Review request for KDE Utils, Michael Leupold and Raphael Kubo da Costa.
> 
> 
> Description
> -------
> 
> I have > 340 entries in "Form Data" in the wallet. When opening the wallet editor, it takes about 9 Seconds until the tree is displayed!
> I investigated the problem and found out that this is due to 2 reasons:
> 1.) The list of entries is checked against existing entries in the tree, which is done by linear search in the given entries list and in the tree
>     which results in a quadratic complexity.
> 2.) unneeded duplicate dbus calls
> 
> I solved the first by using QSet for fast lookup and I reduced the second problem by avoiding a duplicate, unneeded query over dbus
> 
> 
> This addresses bugs 279161 and 284671.
>     http://bugs.kde.org/show_bug.cgi?id=279161
>     http://bugs.kde.org/show_bug.cgi?id=284671
> 
> 
> Diffs
> -----
> 
>   allyourbase.h d3452cd 
>   allyourbase.cpp f57eb68 
>   kwalleteditor.cpp 777ce8d 
> 
> Diff: http://git.reviewboard.kde.org/r/105633/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Martin Koller
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-utils-devel/attachments/20120813/51ae5f22/attachment.html>