----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113591/#review43914 -----------------------------------------------------------
This review has been submitted with commit 6d8fac9e342cfbcafb20846e647f25696d67da57 by Frank Reininghaus to branch master. - Commit Hook 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 > >
