Review Request 123852: Optimize: Do not wipe dict cache when copying speller objects.

[Kåre Särs]

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


+1 

I don't see the reason to clear the cache on the copy assignment, but I don't dare to give a "ship it" as I'm not that familiar with the code.

- Kåre Särs


On maj 19, 2015, 9:49 f.m., Milian Wolff wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123852/
> -----------------------------------------------------------
> 
> (Updated maj 19, 2015, 9:49 f.m.)
> 
> 
> Review request for KDE Frameworks, Laurent Montel, Martin Tobias Holmedahl Sandsmark, and Kåre Särs.
> 
> 
> Repository: sonnet
> 
> 
> Description
> -------
> 
> This removes a serious performance penalty from enabling on-the-fly spell
> checking in KTextEditor. For some reason, the copy assignment of a Speller
> object invalidated the internal cache which happened very often from the Kate
> code base. Now, the cache is kept valid and reused, and the performance is good
> again. I'm not sure whether this has any unintentional side-effects, but the
> tests work fine and spell checking in KatePart still looks good as well,
> and is now fast again.
> 
> E.g. previously I easily ended up with heaptrack reports such as this one:
> 
> 2284529 calls to allocation functions with 16.23MB peak consumption from
> HashMgr::add_word(char const*, int, int, unsigned short*, int, char const*, bool)
>   in /usr/lib/libhunspell-1.3.so.0
> 1978045 calls with 2.30MB peak consumption from:
>     HashMgr::load_tables(char const*, char const*)
>       in /usr/lib/libhunspell-1.3.so.0
>     HashMgr::HashMgr(char const*, char const*, char const*)
>       in /usr/lib/libhunspell-1.3.so.0
>     Hunspell::Hunspell(char const*, char const*, char const*)
>       in /usr/lib/libhunspell-1.3.so.0
>     HunspellDict
>       at .../sonnet/src/plugins/hunspell/hunspelldict.cpp:36
>       in /home/milian/projects/compiled/kf5/lib64/plugins/kf5/sonnet/hunspell.so
>     HunspellClient::createSpeller(QString const&)
>       at .../sonnet/src/plugins/hunspell/hunspellclient.cpp:43
>       in /home/milian/projects/compiled/kf5/lib64/plugins/kf5/sonnet/hunspell.so
>     Sonnet::Loader::createSpeller(QString const&, QString const&) const
>       at .../sonnet/src/core/loader.cpp:103
>       in /home/milian/projects/compiled/kf5/lib64/libKF5SonnetCore.so.5
>     Sonnet::Speller::Private::updateDict()
>       at .../sonnet/src/core/speller.cpp:64
>       in /home/milian/projects/compiled/kf5/lib64/libKF5SonnetCore.so.5
>     Sonnet::Speller::Private::recreateDict()
>       at .../sonnet/src/core/speller.cpp:79
>       in /home/milian/projects/compiled/kf5/lib64/libKF5SonnetCore.so.5
>     Sonnet::Speller::operator=(Sonnet::Speller const&)
>       at .../sonnet/src/core/speller.cpp:111
>       in /home/milian/projects/compiled/kf5/lib64/libKF5SonnetCore.so.5
>     Sonnet::BackgroundChecker::setSpeller(Sonnet::Speller const&)
>       at .../sonnet/src/core/backgroundchecker.cpp:131
>       in /home/milian/projects/compiled/kf5/lib64/libKF5SonnetCore.so.5
>     KateOnTheFlyChecker::performSpellCheck()
>       at .../ktexteditor/src/spellcheck/ontheflycheck.cpp:405
>       in /home/milian/projects/compiled/kf5/lib64/libKF5TextEditor.so.5
> 
> 
> Diffs
> -----
> 
>   src/core/speller.cpp 3903b42ebb4f7cb98a049fcf7a291c74dd9457e0 
> 
> Diff: https://git.reviewboard.kde.org/r/123852/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Milian Wolff
> 
>

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