<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/123930/">https://git.reviewboard.kde.org/r/123930/</a>
</td>
</tr>
</table>
<br />
<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;">Having a static cache sounds reasonable to me and I think it might be a good change, but I'll let people with more experience with sonnet give the thumbs up or down.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Meanwhile I wonder why ktexteditor calls m_backgroundChecker->setSpeller(m_speller) on every cal to performSpellCheck().</p></pre>
<br />
<p>- Kåre Särs</p>
<br />
<p>On May 28th, 2015, 11:45 p.m. UTC, Milian Wolff 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, David Faure, Laurent Montel, Martin Tobias Holmedahl Sandsmark, and Kåre Särs.</div>
<div>By Milian Wolff.</div>
<p style="color: grey;"><i>Updated May 28, 2015, 11:45 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
sonnet
</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;">This removes the performance bottlenecks related to creating
temporary Speller objects. And I wonder why one would want a
per-object cache anyways...</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">To see the bad performance, simply enable language detection in
Katepart. heaptrack showed then hundreds of thousands of allocations
in hunspell due to the repeated creation of speller plugins for
temporary Speller() objects, or even Speller objects in QList...</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">See https://paste.kde.org/pwx76ew6d for more information.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><strong style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">BUT</strong>: I wonder whether this is safe. When we create spellers for more than MAXLANGUAGES = 5 different languages, the cache will start to delete old items and then the speller objects have dangling pointers. I think using QCache is not going to work here anymore. I'll rewrite this patch accordingly, if I get an OK that this is how it should be.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Note: Sonnet itself is not advertised as threadsafe, and indeed https://git.reviewboard.kde.org/r/106242/ shows that this is not intended. Also, the Settings object in the Loader is also accessed from all Speller objects, and thus one must not use Speller objects from multiple threads anyways.</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;">ran katepart again - much quicker now, even with auto-language-detection enabled! unit test still work as well.</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/core/speller.cpp <span style="color: grey">(dcf98eccb2d82642dc2efe0145ad7ba9a814505f)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/123930/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>