Review Request 123930: WIP: Make the speller dict cache static.

[Kåre Särs]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123930/#review80953
-----------------------------------------------------------


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.

Meanwhile I wonder why ktexteditor calls m_backgroundChecker->setSpeller(m_speller) on every cal to performSpellCheck().

- Kåre Särs


On May 28, 2015, 11:45 p.m., Milian Wolff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123930/
> -----------------------------------------------------------
> 
> (Updated May 28, 2015, 11:45 p.m.)
> 
> 
> Review request for KDE Frameworks, David Faure, Laurent Montel, Martin Tobias Holmedahl Sandsmark, and Kåre Särs.
> 
> 
> Repository: sonnet
> 
> 
> Description
> -------
> 
> This removes the performance bottlenecks related to creating
> temporary Speller objects. And I wonder why one would want a
> per-object cache anyways...
> 
> 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...
> 
> See https://paste.kde.org/pwx76ew6d for more information.
> 
> **BUT**: 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.
> 
> 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.
> 
> 
> Diffs
> -----
> 
>   src/core/speller.cpp dcf98eccb2d82642dc2efe0145ad7ba9a814505f 
> 
> Diff: https://git.reviewboard.kde.org/r/123930/diff/
> 
> 
> Testing
> -------
> 
> ran katepart again - much quicker now, even with auto-language-detection enabled! unit test still work as well.
> 
> 
> Thanks,
> 
> Milian Wolff
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20150529/b40125ea/attachment.html>