Re: Review Request: Delay initialization of PowerDevil and DeviceAutomounter

["Lubos Lunak"]

> On 2010-03-22 09:28:26, Lubos Lunak wrote:
> > Why don't you change X-KDE-Kded-phase=1 to 2 instead of this?
> >
> 
> Sebastian Sauer wrote:
>     because;
>     1. they are not the same
>     2. the one does not exclude the other
>     
>     So, you suggest to additionally change them both from X-KDE-Kded-phase=1 
> to X-KDE-Kded-phase=2? Sounds good for me :)
>

I'm not familiar with the modules enough to suggest, I'm asking. If you change 
the phase to 2, your change is needless, as such modules are loaded late enough 
to for delayed class initialization to not matter, and if the modules would be 
needed sooner, they'd be loaded and initialized on-demand. So my question was 
if it really is necessary for the modules to be loaded in phase 1 (and, also, 
are you sure it doesn't just make it seemingly faster by making phase 1 loading 
faster but only moving the work done to a different time?).


- Lubos


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/3331/#review4605
-----------------------------------------------------------


On 2010-03-21 17:49:17, Sebastian Sauer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3331/
> -----------------------------------------------------------
> 
> (Updated 2010-03-21 17:49:17)
> 
> 
> Review request for kdelibs and Dario Freddi.
> 
> 
> Summary
> -------
> 
> The patch delays the initialization of PowerDevil and DeviceAutomounter and 
> improves that way KDED startup phase 1 by close to a second (out of 15, 
> coldstart).
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdebase/runtime/solid-device-automounter/kded/DeviceAutomounter.h 
> 1105732 
>   
> trunk/KDE/kdebase/runtime/solid-device-automounter/kded/DeviceAutomounter.cpp 
> 1105732 
>   trunk/KDE/kdebase/workspace/powerdevil/daemon/PowerDevilDaemon.h 1105732 
>   trunk/KDE/kdebase/workspace/powerdevil/daemon/PowerDevilDaemon.cpp 1105732 
> 
> Diff: http://reviewboard.kde.org/r/3331/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sebastian
> 
>