[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-core-devel
Subject: Re: Review Request 118587: Optimize KConfigIniBackend::parseConfig by reducing allocations.
From: "Oswald Buddenhagen" <ossi () kde ! org>
Date: 2014-06-19 10:00:19
Message-ID: 20140619100019.5749.72362 () probe ! kde ! org
[Download RAW message or body]
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118587/#review60482
-----------------------------------------------------------
Ship it!
Ship It!
- Oswald Buddenhagen
On June 18, 2014, 11:18 a.m., Milian Wolff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118587/
> -----------------------------------------------------------
>
> (Updated June 18, 2014, 11:18 a.m.)
>
>
> Review request for kdelibs, David Faure, Matthew Dawson, and Oswald Buddenhagen.
>
>
> Repository: kdelibs
>
>
> Description
> -------
>
> 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.
>
>
> Diffs
> -----
>
> kdecore/config/bufferfragment_p.h 5a753ad
> kdecore/config/kconfigini.cpp 8ec43c5
> kdecore/config/kconfigini_p.h d7aa43e
>
> Diff: https://git.reviewboard.kde.org/r/118587/diff/
>
>
> Testing
> -------
>
> 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.
>
> Thanks,
>
> Milian Wolff
>
>
[Attachment #3 (text/html)]
<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 />
<p>Ship it!</p>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Ship It!</pre> <br />
<p>- Oswald Buddenhagen</p>
<br />
<p>On June 18th, 2014, 11:18 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 18, 2014, 11:18 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>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic