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


While I'm fine with the idea behind this optimization, I worry that this 
implementation could create situations were a configuration change is not 
picked up by the system.  For instance, what happens if the user doesn't 
immediately call readConfig?  This could create some subtle bugs in downstream 
code.

I had two ideas for how this optimization could be implemented:
1) Lazy load the KConfig object the first time it is used.  Then, in 
readConfig, the call to be reparseConfiguration could be avoided if the KConfig 
is created due to its call.  This would retain the current behaviour, while 
ensuring readConfig reads in the most configuration.  Other uses of the KConfig 
will have to ensure the KConfig object has already been created, and if the 
user calls one of those functions before readConfig, they will still double 
read the configuration.  But since this is just status quo, I'm not too worried.
2) Alternately, create a set of construction functions, like make_shared, that 
imitate the creation of a KConfigSkeleton subclass, and then reading the 
configuration through readConfig.  Internally, it can use a private readConfig 
function to ensure the configuration is no re-read.  This would require changes 
to applications to avoid the extra configuration call, unfortunately.

I saw RR #115964, and I assume that some of the reductions to the readings of 
oxygenrc are caused by the sharing the KConfig between some KConfigSkeleton's?  
If so, I'd suggest implementing solution 1 for the general case, and then 
making a special construction function to handle shared KConfig's.  I don't 
want to avoid calling reparseConfiguration without some warning around this, as 
it may again cause some surprises.  A new appropriately named function 
shouldn't be too bad though, as opposed to changing the behaviour of the 
constructor.

- Matthew John Dawson


On Feb. 27, 2014, 4:18 p.m., David Faure wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116461/
> -----------------------------------------------------------
> 
> (Updated Feb. 27, 2014, 4:18 p.m.)
> 
> 
> Review request for KDE Frameworks and Matthew John Dawson.
> 
> 
> Repository: kconfig
> 
> 
> Description
> -------
> 
> KConfigSkeleton: avoid calling reparseConfiguration() immediately after 
> creation.
> 
> KConfig already parses the config files from disk in the constructor,
> which is necessary for non-KConfigXT users. However when using KConfigXT
> the first thing one has to do after creation is to call readConfig(),
> which should therefore not call reparseConfiguration the first time.
> 
> strace -e open kate |& grep -v NOENT | grep oxygenrc | wc -l
>   went from 4 to 1 --> bingo, goal reached!
>   (and when looking for kdeglobals, from 10 to 7)
> 
> 
> Diffs
> -----
> 
>   src/core/kcoreconfigskeleton.cpp 9c5fb4a80d500e81b483b749a137ad5f2c99a55f 
>   src/core/kcoreconfigskeleton_p.h 0b020ed3493186e699d872ddc7a9f9294d797a95 
> 
> Diff: https://git.reviewboard.kde.org/r/116461/diff/
> 
> 
> Testing
> -------
> 
> (see commit log) + unittests in kconfig still pass.
> 
> 
> Thanks,
> 
> David Faure
> 
>

_______________________________________________
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Reply via email to