----------------------------------------------------------- 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 > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
