<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/104052/">http://git.reviewboard.kde.org/r/104052/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 23rd, 2012, 6:33 p.m., <b>Milian Wolff</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">given that you have unit tests at hand, why not write a benchmark to get proper before/after numbers that show the impact of this patch?</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Right.. That's not exactly easy to "just" do since i would have to read a dir with images, insert it in the cache and read it back from the cache. So not going to do that. What i did do is change the size to "SizeEnormous" so the icon is a bit bigger (128x128) and just let QBENCHMARK do it's thing with insert(write) and find(read). The results are as follows:
-- Before patch --
READ : 0.019 msecs per iteration (total: 79, iterations: 4096)
WRITE: 0.010 msecs per iteration (total: 88, iterations: 8192)
-- After patch --
READ : 0.019 msecs per iteration (total: 79, iterations: 4096)
WRITE: 0.0026 msecs per iteration (total: 87, iterations: 32768)
Reading is equal in speed, writing is ~5x faster after the patch.</pre>
<br />
<p>- Mark</p>
<br />
<p>On February 23rd, 2012, 6:21 p.m., Mark Gaiser wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for kdelibs, David Faure and Michael Pyne.</div>
<div>By Mark Gaiser.</div>
<p style="color: grey;"><i>Updated Feb. 23, 2012, 6:21 p.m.</i></p>
<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;">I was running KWin through callgrind to see where possible bottlenecks are. I wasn't expecting much since it improved greatly during the 4.8 dev cycle, however one stood out. The saving of PNG images was taking about 1/5th of the time in KWin that i could see directly. That looked like something i might be able to optimize.
What this patch is doing is storing the actual image bits to prevent saving a PNG image to the mmapped cache. That was a hot code path in time (cycles), not even in calls. I've also reduced the amount of memory copies to a bare minimum by adding a rawFind function to KSharedDataCache which fills a QByteArray::fromRawData thus preventing a expensive memory copy. The rawFind is used for looking up an image and fetching it's data without copying it. That is done because QImage seems to make a copy itself internally. I don't have any performance measurements, however, prior to this patch my kwin test was using up ~5.000.000.000 cycles. After this patch it's using up 1.370.000.000. I don't have raw performance numbers to see if the cache itself is actually faster, it certainly has become a lot cheaper to use the cache. Logic wise i would say creating a QImage from the cached data should be way faster now since there is no step involved anymore in decoding the image. Storing is certainly an order of magnitude faster.
Special thanks go to David Faure for helping me a great deal with this.</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;">I've also written a bunch of test cases (greatly improved by David Faure) to see if i didn't break anything. According to the test (which is also comparing the actual image bits) it's all passing just fine.</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>kdecore/util/kshareddatacache.h <span style="color: grey">(339cecc)</span></li>
<li>kdecore/util/kshareddatacache.cpp <span style="color: grey">(9fe3995)</span></li>
<li>kdeui/tests/CMakeLists.txt <span style="color: grey">(63788f6)</span></li>
<li>kdeui/tests/kimagecachetests.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>kdeui/tests/kimagecachetests.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>kdeui/util/kimagecache.cpp <span style="color: grey">(a5bbbe1)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/104052/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>