<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/113591/">http://git.reviewboard.kde.org/r/113591/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This review has been submitted with commit 6d8fac9e342cfbcafb20846e647f25696d67da57 by Frank Reininghaus to branch master.</pre>
<br />
<p>- Commit Hook</p>
<br />
<p>On November 3rd, 2013, 6:57 p.m. UTC, Frank Reininghaus wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for kdelibs and David Faure.</div>
<div>By Frank Reininghaus.</div>
<p style="color: grey;"><i>Updated Nov. 3, 2013, 6:57 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kdelibs
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>kio/tests/udsentrytest.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>kio/tests/udsentrytest.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>kio/tests/CMakeLists.txt <span style="color: grey">(5a1f9b5)</span></li>
<li>kio/kio/udsentry.cpp <span style="color: grey">(1e1f503)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/113591/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>