<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/101554/">http://git.reviewboard.kde.org/r/101554/</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 9th, 2011, 9:49 a.m., <b>Aaron J. Seigo</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;">my biggest question with this change would be performance. QFileInfo is not fast, and to make matters "worse" this code in KStandardDirs influences start up speed. have you measured the performance after this change relative to before?
with Qt5 this may become a moot issue with the changes coming in the QFile backends ... but i don't think it makes sense to introduce possible performance regressions on code that currently works.
is there a platform for which the current code fails?</pre>
</blockquote>
<p>On June 9th, 2011, 3:05 p.m., <b>Bernhard Beschow</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;">I did the patch solely for simplicity and portability reasons, and perhaps (or hopefully) also in the Platform 11 spirit. :) After all, "outsourcing" platform-specific details to Qt wherever we can seems like a good idea to me, such that we can take advantage of new platforms much quicker.
That said, I couldn't measure any performance difference when logging into a plasma session. In both cases, it takes ~13 secs for the wallpaper to appear, and ~30 secs until the KWallet password dialog appears.</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;">I understand the reasoning but the fact that you did not properly measure the differences shows such pieces of code would be better left untouched untill there is convincing data. KStandardDirs is critical place for KDE apps. IMHO simplifing such lower-level internal (tested!) code may not pay off.
Thanks for your effort.</pre>
<br />
<p>- Jarosław</p>
<br />
<p>On June 9th, 2011, 7:06 a.m., Bernhard Beschow 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, kdewin and David Faure.</div>
<div>By Bernhard Beschow.</div>
<p style="color: grey;"><i>Updated June 9, 2011, 7:06 a.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;">Increase portability of kdecore a tiny bit by making KStandardDirs use QDirIterator rater than platform-specific code.</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'm running my platform 4.6 with this patch and haven't noticed any regressions yet.</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/kernel/kstandarddirs.cpp <span style="color: grey">(ba90270)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/101554/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>