Review Request 113591: Reduce UDSEntry memory usage by sharing the contained QStrings if possible

[David Faure]

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

Ship it!


Ship It!

- David Faure


On Nov. 3, 2013, 6:57 p.m., Frank Reininghaus wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/113591/
> -----------------------------------------------------------
> 
> (Updated Nov. 3, 2013, 6:57 p.m.)
> 
> 
> Review request for kdelibs and David Faure.
> 
> 
> Repository: kdelibs
> 
> 
> Description
> -------
> 
> This patch is a subset of https://git.reviewboard.kde.org/r/113355/ (I'll continue working on the other part of that request, which can be dealt with separately, at some later point).
> 
> It adds a unit test to makes sure that saving UDSEntries to a QDataStream and re-loading them works as expected, and makes use of implicit sharing of QStrings in UDSEntryPrivate::load(QDataStream &s, UDSEntry &a) to reduce the memory usage of this class (which is the major consumer of memory in Dolphin and other applications that list the contents of large directory contents with KIO).
> 
> It caches the most recently loaded QString for each UDS field in a simple QVector<QString>. This works because sharable strings like, e.g., the user and the group, usually appear at the same position in the QDataStream when retrieving a large number of UDSEntries that have been stored by a kioslave.
> 
> Note that I had made an earlier attempt to achieve the same thing using a QHash<uint, QString> to look up the cached strings ( http://pastebin.kde.org/p52a24b49 ), but the QVector<QString>-based solution turns out to be faster.
> 
> 
> Diffs
> -----
> 
>   kio/tests/udsentrytest.h PRE-CREATION 
>   kio/tests/udsentrytest.cpp PRE-CREATION 
>   kio/tests/CMakeLists.txt 5a1f9b5 
>   kio/kio/udsentry.cpp 1e1f503 
> 
> Diff: http://git.reviewboard.kde.org/r/113591/diff/
> 
> 
> Testing
> -------
> 
> kdelibs unit tests still pass. The memory usage of both Dolphin and a simple test program that uses KIO::listDir to list the contents of a large directory (see r4 of https://git.reviewboard.kde.org/r/113355/) is reduced by ~128 bytes per item according to my tests.
> 
> A simple benchmark that simulates how 100,000 UDSEntries stored by kio_file are loaded (see r3 of https://git.reviewboard.kde.org/r/113355/) runs in 234 ms instead of 266 ms on my machine - it seems that growing the heap to provide space for the non-shared QStrings is more expensive than comparing all loaded QStrings with the cached values.
> 
> 
> Thanks,
> 
> Frank Reininghaus
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20131114/d1fbc4ab/attachment.htm>