<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="https://git.reviewboard.kde.org/r/118587/">https://git.reviewboard.kde.org/r/118587/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 7th, 2014, 10:01 a.m. UTC, <b>Oswald Buddenhagen</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;">note that you could make it even more efficient with a more drastic approach: have the entry map hold buffer fragments to start with. the entries would also have variants (or at least byte arrays) as a cache for the already-converted (and overwritten) values. that would mean keeping the raw config file in memory, but that shouldn't be significant compared to everything else.
i have no idea whether keeping a string table of the converted values on top of that would still add value. it could if the many identical values are actually all used (identical keys should not matter, as they are never converted - unless you enumerate them).</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;">Yes, interesting ideas. But it would mean one has to keep the full config file QByteArray in memory. I'm not sure that is beneficial from a memory usage pov. I think I'll eventually try it out in kf5 land. This was easy to get done for kdelibs 4.
I'll fix the issue you pointed out and try out the improved qHash from Qt5 and resubmit next week.
Thanks!</pre>
<br />
<p>- Milian</p>
<br />
<p>On June 6th, 2014, 11:35 a.m. UTC, Milian Wolff wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://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, David Faure, Matthew Dawson, and Oswald Buddenhagen.</div>
<div>By Milian Wolff.</div>
<p style="color: grey;"><i>Updated June 6, 2014, 11:35 a.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;">Optimize KConfigIniBackend::parseConfig by reducing allocations.
Yet another awesome application of the Qt implicit sharing trick.
Since config files often contain only few different keys and even
value strings, we can share them. This reduces memory consumption
and also speeds up parsing, as we do not have to allocate the
duplicated strings, but can simply reuse the previous values.
The most extreme case for this of my knowledge, is KatePart:
katesyntaxhighlightingrc has more then 20k lines which triggered
about 30k allocations on startup. With this patch applied, this
value goes down dramatically. I added a simple static counter for
the cache hit/miss ratio, which resulted in 5442 cache misses compared
to 43624 cache hits across all KConfig files parsed by kwrite on startup.
</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;">Unit tests all pass. My malloc tracer shows that the allocations are all gone.
My malloc tracer showed before:
17421 allocations at:
0x7fee73692b97 QByteArray::QByteArray(char const*, int) /usr/lib/libQtCore.so.4
0x7fee73bb7cee ? /usr/lib/libkdecore.so.5
0x7fee73bb7fc4 ? /usr/lib/libkdecore.so.5
0x7fee73ba1320 ? /usr/lib/libkdecore.so.5
0x7fee73ba1731 KConfig::KConfig(QString const&, QFlags<KConfig::OpenFlag>, char const*) /usr/lib/libkdecore.so.5
0x7fee64830c06 KateHlManager::KateHlManager() in /ssd/milian/projects/kde4/kate/part/syntax/katesyntaxmanager.cpp:76 /ssd/milian/projects/compiled/kde4/lib/libkatepartinterfaces.so.4
12699 allocations at:
0x7fee73692b97 QByteArray::QByteArray(char const*, int) /usr/lib/libQtCore.so.4
0x7fee73bb7cd7 ? /usr/lib/libkdecore.so.5
0x7fee73bb7fc4 ? /usr/lib/libkdecore.so.5
0x7fee73ba1320 ? /usr/lib/libkdecore.so.5
0x7fee73ba1731 KConfig::KConfig(QString const&, QFlags<KConfig::OpenFlag>, char const*) /usr/lib/libkdecore.so.5
0x7fee64830c06 KateHlManager::KateHlManager() in /ssd/milian/projects/kde4/kate/part/syntax/katesyntaxmanager.cpp:76 /ssd/milian/projects/compiled/kde4/lib/libkatepartinterfaces.so.4
These where the allocation hotspots number #1 and #3 respectively. With the patch applied, the two locations are not under the top 10 anymore.</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/config/bufferfragment_p.h <span style="color: grey">(5a753ad)</span></li>
<li>kdecore/config/kconfigini.cpp <span style="color: grey">(8ec43c5)</span></li>
<li>kdecore/config/kconfigini_p.h <span style="color: grey">(d7aa43e)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/118587/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>