Review Request 123341: Optimize reading Sonnet settings by minimizing the cals to save()
Laurent Montel
montel at kde.org
Mon Apr 13 05:28:37 UTC 2015
> On avr. 12, 2015, 8:03 matin, Laurent Montel wrote:
> > Perhaps it's time to create an unittest :)
> > We need to be sure that it will save when we change settings no ?
>
> Kåre Särs wrote:
> Any hints on how to test that QSettings::setValue() is called properly (or save()) without writing/modifying the Sonnet settings?
You can perhaps change value save, and rechange value and see if it was saved no ?
- Laurent
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123341/#review78846
-----------------------------------------------------------
On avr. 12, 2015, 5:57 après-midi, Kåre Särs wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123341/
> -----------------------------------------------------------
>
> (Updated avr. 12, 2015, 5:57 après-midi)
>
>
> Review request for KDE Frameworks and Martin Tobias Holmedahl Sandsmark.
>
>
> Repository: sonnet
>
>
> Description
> -------
>
> The curent implementation can causes ~25 cals to Settings::save() for every created Speller instance.
>
> The Settings::restore() function ends with setQuietIgnoreList(...) which would call save() for evey ignore-word.
>
> This patch removes the save() cals in the setFoo() functions of Settings and replace it by a boolean that indicates if the settign is changed or not. Then in the Speller class save() is called when needed.
>
> The reason for this patch is that it took Kate ~2 minutes to open a kate session with ~340 files. The implementation in Kate called reload() for every file. There is a RR for Kate to remove that unneeded reload.
>
> This patch also prepares Sonnet::Settings for being used in the public API to set aplication specific Sonnet settings without saving them in the global settings.
>
> The Settings class is exported but the header is private. This change is BIC, but since the class is private, my interpretation is that it should not mater.
>
> If accepted I will also add a Review Request to make the Settings class public to enable application specific settings.
>
>
> Diffs
> -----
>
> src/core/settings.cpp b5c4198
> src/core/settings_p.h 0d48889
> src/core/speller.cpp 3903b42
> src/ui/configwidget.cpp 02f2a26
>
> Diff: https://git.reviewboard.kde.org/r/123341/diff/
>
>
> Testing
> -------
>
> Tested with Kate and with the patch the startup time was back to "normal". No test cases :(
>
>
> Thanks,
>
> Kåre Särs
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20150413/40a20040/attachment.html>
More information about the Kde-frameworks-devel
mailing list