<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/123924/">https://git.reviewboard.kde.org/r/123924/</a>
</td>
</tr>
</table>
<br />
<p>Ship it!</p>
<div>
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/123924/diff/1/?file=378265#file378265line116" style="color: black; font-weight: bold; text-decoration: underline;">src/kiconengine.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">QList<QSize> KIconEngine::availableSizes(QIcon::Mode mode, QIcon::State state) const</pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">112</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="n">QList</span><span class="o"><</span><span class="n">QSize</span><span class="o">></span> <span class="n">KIconEngine</span><span class="o">::</span><span class="n">availableSizes</span><span class="p">(</span><span class="n">QIcon</span><span class="o">::</span><span class="n">Mode</span> <span class="n">mode</span><span class="p">,</span> <span class="n">QIcon</span><span class="o">::</span><span class="n">State</span> <span class="n">state</span><span class="p">)</span> <span class="k">const</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">116</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="n">QList</span><span class="o"><</span><span class="n">QSize</span><span class="o">></span> <span class="n">KIconEngine</span><span class="o">::</span><span class="n">availableSizes</span><span class="p">(</span><span class="n">QIcon</span><span class="o">::</span><span class="n">Mode</span> <span class="n">mode</span><span class="p">,</span> <span class="n">QIcon</span><span class="o">::</span><span class="n">State</span> <span class="n">state</span><span class="p">)</span> <span class="k">const</span></pre></td>
</tr>
</tbody>
</table>
<div style="margin-left: 2em;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">in KF6, this should also not use QList, but QVector instead. on 32bit platforms, a QList<QSize> is pretty damn slow</p></pre>
</div>
</div>
<br />
<p>- Milian Wolff</p>
<br />
<p>On May 28th, 2015, 2:41 p.m. UTC, Aleix Pol Gonzalez wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for KDE Frameworks and Christoph Feck.</div>
<div>By Aleix Pol Gonzalez.</div>
<p style="color: grey;"><i>Updated May 28, 2015, 2:41 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kiconthemes
</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">It's called a ton of times (over 30k times when opening kdevelop and changing area at some point) and it's quite slow nowadays. The reason is that we're retrieving the icon path every time to see if it's available (because it goes through aaaaaaall of the themes' directories to find the best available icon).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This patch adds a dictionary in the KIconLoader so that we can cache for the icons we already found, clears it when the theme changes.
Also we're construct the same sizes list every time, now we're always returning the same list.</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Tests still pass.
It doesn't show a big problem in the same use-case in kdevelop anymore (99.5% of hits).</p></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>src/kiconengine.h <span style="color: grey">(461efff)</span></li>
<li>src/kiconengine.cpp <span style="color: grey">(208c055)</span></li>
<li>src/kiconloader.h <span style="color: grey">(7944726)</span></li>
<li>src/kiconloader.cpp <span style="color: grey">(c47eecb)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/123924/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>