Review Request 118985: KSharedConfig: only write to mainConfig and wasTestModeEnabled in the main thread

[Matthew Dawson]

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


I'm not sure about moving the warning about immutable files to only happen on the main thread, as it is possible that an application may use it KSharedConfig on an alternate thread only.

As this is mainly an optimization, I think the best thing is to just move to using the load() function on the atomic integer, as I believe that doesn't require processors to synchronize, and won't limit optimizations.  Otherwise, I'm fine with the immutable files as that is simpler code wise.  Thoughts?


src/core/ksharedconfig.cpp
<https://git.reviewboard.kde.org/r/118985/#comment42592>

    Thinking more on this, both ways are incorrect.  This should instead be based on a per thread variable, as the list needs to be cleared in all threads if this is the case.  I think the easy way to handle this is to move wasTestModeEnabled into GlobalSharedConfigList.



src/core/ksharedconfig.cpp
<https://git.reviewboard.kde.org/r/118985/#comment42593>

    As this seems to be mainly an optimization to ensure the config is not unnecessarily deleted, I think it makes sense to still run this on alternate threads.


- Matthew Dawson


On June 27, 2014, 6:31 p.m., David Faure wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118985/
> -----------------------------------------------------------
> 
> (Updated June 27, 2014, 6:31 p.m.)
> 
> 
> Review request for KDE Frameworks and Matthew Dawson.
> 
> 
> Repository: kconfig
> 
> 
> Description
> -------
> 
> KSharedConfig: only write to mainConfig and wasTestModeEnabled in the main thread
> 
> As a side effect, this removes the need for the atomic ints, since
> that code now only runs in the main thread.
> 
> 
> Diffs
> -----
> 
>   src/core/ksharedconfig.cpp b7d155d5893502921d35d7dd971188b6a93a0620 
> 
> Diff: https://git.reviewboard.kde.org/r/118985/diff/
> 
> 
> Testing
> -------
> 
> no regression in "make test" in kconfig; still debugging races in helgrind threadtest (in kio).
> 
> 
> Thanks,
> 
> David Faure
> 
>

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