Review Request: startup faster; don't wait for kwin or kcm's in ksmserver

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

Review request for kdelibs.


Summary
-------

* The fallback logic in wmProcessChange() does not work for kwin anyway cause kwin is the fallback. So, just don't wait and increase startup by 1/2 second for me.

* Waiting for the initialization of kcm's is not needed. Saves another 1/2 second.


Diffs
-----

  /trunk/KDE/kdebase/workspace/ksmserver/startup.cpp 1042693

Diff: http://reviewboard.kde.org/r/2034/diff


Testing
-------


Thanks,

Sebastian

[Michael Pyne-3]

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



/trunk/KDE/kdebase/workspace/ksmserver/startup.cpp
<http://reviewboard.kde.org/r/2034/#comment2296>

    Weren't these 2 connections just established on line 152?



/trunk/KDE/kdebase/workspace/ksmserver/startup.cpp
<http://reviewboard.kde.org/r/2034/#comment2295>

    Why leave this commented out if we're not doing this anymore?  We have SVN, I'd recommend removing it unless it's needed for understanding for someone coming across it later. (likewise with the line 2 below this).



/trunk/KDE/kdebase/workspace/ksmserver/startup.cpp
<http://reviewboard.kde.org/r/2034/#comment2297>

    Was this connected anywhere else?  You removed the connection on line 211 (now 219) so it seems that you could remove this disconnect as well.


- Michael


On 2009-11-01 15:44:54, Sebastian Sauer wrote:

>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2034/
> -----------------------------------------------------------
>
> (Updated 2009-11-01 15:44:54)
>
>
> Review request for kdelibs.
>
>
> Summary
> -------
>
> * The fallback logic in wmProcessChange() does not work for kwin anyway cause kwin is the fallback. So, just don't wait and increase startup by 1/2 second for me.
>
> * Waiting for the initialization of kcm's is not needed. Saves another 1/2 second.
>
>
> Diffs
> -----
>
>   /trunk/KDE/kdebase/workspace/ksmserver/startup.cpp 1042693
>
> Diff: http://reviewboard.kde.org/r/2034/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sebastian
>
>

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

(Updated 2009-11-02 22:05:47.121904)


Review request for kdelibs.


Changes
-------

updated patch.


Summary
-------

* The fallback logic in wmProcessChange() does not work for kwin anyway cause kwin is the fallback. So, just don't wait and increase startup by 1/2 second for me.

* Waiting for the initialization of kcm's is not needed. Saves another 1/2 second.


Diffs (updated)
-----

  /trunk/KDE/kdebase/workspace/ksmserver/server.h 1042693
  /trunk/KDE/kdebase/workspace/ksmserver/startup.cpp 1042693

Diff: http://reviewboard.kde.org/r/2034/diff


Testing
-------


Thanks,

Sebastian

> On 2009-11-01 19:32:04, Michael Pyne wrote:
> > /trunk/KDE/kdebase/workspace/ksmserver/startup.cpp, line 162
> > <http://reviewboard.kde.org/r/2034/diff/2/?file=13608#file13608line162>
> >
> >     Weren't these 2 connections just established on line 152?

true, fixed.


> On 2009-11-01 19:32:04, Michael Pyne wrote:
> > /trunk/KDE/kdebase/workspace/ksmserver/startup.cpp, line 219
> > <http://reviewboard.kde.org/r/2034/diff/2/?file=13608#file13608line219>
> >
> >     Why leave this commented out if we're not doing this anymore?  We have SVN, I'd recommend removing it unless it's needed for understanding for someone coming across it later. (likewise with the line 2 below this).

fixed.


> On 2009-11-01 19:32:04, Michael Pyne wrote:
> > /trunk/KDE/kdebase/workspace/ksmserver/startup.cpp, line 234
> > <http://reviewboard.kde.org/r/2034/diff/2/?file=13608#file13608line234>
> >
> >     Was this connected anywhere else?  You removed the connection on line 211 (now 219) so it seems that you could remove this disconnect as well.

fixed.


- Sebastian


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


On 2009-11-02 22:05:47, Sebastian Sauer wrote:

>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2034/
> -----------------------------------------------------------
>
> (Updated 2009-11-02 22:05:47)
>
>
> Review request for kdelibs.
>
>
> Summary
> -------
>
> * The fallback logic in wmProcessChange() does not work for kwin anyway cause kwin is the fallback. So, just don't wait and increase startup by 1/2 second for me.
>
> * Waiting for the initialization of kcm's is not needed. Saves another 1/2 second.
>
>
> Diffs
> -----
>
>   /trunk/KDE/kdebase/workspace/ksmserver/server.h 1042693
>   /trunk/KDE/kdebase/workspace/ksmserver/startup.cpp 1042693
>
> Diff: http://reviewboard.kde.org/r/2034/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sebastian
>
>

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

(Updated 2009-11-03 10:31:32.615226)


Review request for kdelibs.


Changes
-------

updated; just always skip to autostart0 from launchWM now. Fallback can still happen at a later point. This way also !=kwin users profit.


Summary
-------

* The fallback logic in wmProcessChange() does not work for kwin anyway cause kwin is the fallback. So, just don't wait and increase startup by 1/2 second for me.

* Waiting for the initialization of kcm's is not needed. Saves another 1/2 second.


Diffs (updated)
-----

  /trunk/KDE/kdebase/workspace/ksmserver/server.h 1044052
  /trunk/KDE/kdebase/workspace/ksmserver/startup.cpp 1044052

Diff: http://reviewboard.kde.org/r/2034/diff


Testing
-------


Thanks,

Sebastian

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


I disagree with the first change and find the second one questionable. What are the specs of your machine and 1/2 second out of how many?

The reason for waiting for the WM to finish setup is not the fallback, but making sure the WM is ready. There are some aspects related to window management where applications just read the status during their startup and don't update later. So waiting for the WM ensures this is set up properly before other applications may read that (although, thinking of that, this is currently a bit broken anyway, as KWin lets ksmserver continue a bit too soon).

The second case is something similar. The kcminit modules do various setup and it is a question if it is ok to let normal apps start while this is still taking place.


- Lubos


On 2009-11-03 10:31:32, Sebastian Sauer wrote:

>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2034/
> -----------------------------------------------------------
>
> (Updated 2009-11-03 10:31:32)
>
>
> Review request for kdelibs.
>
>
> Summary
> -------
>
> * The fallback logic in wmProcessChange() does not work for kwin anyway cause kwin is the fallback. So, just don't wait and increase startup by 1/2 second for me.
>
> * Waiting for the initialization of kcm's is not needed. Saves another 1/2 second.
>
>
> Diffs
> -----
>
>   /trunk/KDE/kdebase/workspace/ksmserver/server.h 1044052
>   /trunk/KDE/kdebase/workspace/ksmserver/startup.cpp 1044052
>
> Diff: http://reviewboard.kde.org/r/2034/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sebastian
>
>

> On 2009-11-04 10:10:06, Lubos Lunak wrote:
> > I disagree with the first change and find the second one questionable. What are the specs of your machine and 1/2 second out of how many?
> >
> > The reason for waiting for the WM to finish setup is not the fallback, but making sure the WM is ready. There are some aspects related to window management where applications just read the status during their startup and don't update later. So waiting for the WM ensures this is set up properly before other applications may read that (although, thinking of that, this is currently a bit broken anyway, as KWin lets ksmserver continue a bit too soon).
> >
> > The second case is something similar. The kcminit modules do various setup and it is a question if it is ok to let normal apps start while this is still taking place.
> >

Specs are relative. Cold start (not initial) takes up to 9 seconds till the plasma desktop is visible / ksplashx closes.

The thing is that the wm starts very early. Even before plasma which does not need the wm at this point. The first "real" applications are rather late started/restored. Maybe we should just wait in KSMServer::autoStart1Done() till the wm is ready? Would that be enough?

The kde startup process has lot of potential for optimizations. Time expensive things like e.g. kcminit are atm blocking the whole startup. Long term we need to have expensive things run in parallel at startup. Anyway, if you think that this is not a good idea then it should be maybe done another way.


- Sebastian


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


On 2009-11-03 10:31:32, Sebastian Sauer wrote:

>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2034/
> -----------------------------------------------------------
>
> (Updated 2009-11-03 10:31:32)
>
>
> Review request for kdelibs.
>
>
> Summary
> -------
>
> * The fallback logic in wmProcessChange() does not work for kwin anyway cause kwin is the fallback. So, just don't wait and increase startup by 1/2 second for me.
>
> * Waiting for the initialization of kcm's is not needed. Saves another 1/2 second.
>
>
> Diffs
> -----
>
>   /trunk/KDE/kdebase/workspace/ksmserver/server.h 1044052
>   /trunk/KDE/kdebase/workspace/ksmserver/startup.cpp 1044052
>
> Diff: http://reviewboard.kde.org/r/2034/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Sebastian
>
>

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

(Updated 2009-11-07 14:46:51.492620)


Review request for kdelibs.


Changes
-------

Updated patch to take Lubos feedback into account. Changes;
* Wait for the wm in autoStart1Done() to be sure the wm is there if apps are autostarted.
* Reverted the 'do no wait for initialization of kcm's' part. I'll look how to delay the init and kbuildsycoca later / with the next patch.


Summary
-------

* The fallback logic in wmProcessChange() does not work for kwin anyway cause kwin is the fallback. So, just don't wait and increase startup by 1/2 second for me.

* Waiting for the initialization of kcm's is not needed. Saves another 1/2 second.


Diffs (updated)
-----

  /trunk/KDE/kdebase/workspace/ksmserver/server.h 1045961
  /trunk/KDE/kdebase/workspace/ksmserver/startup.cpp 1045961

Diff: http://reviewboard.kde.org/r/2034/diff


Testing
-------


Thanks,

Sebastian

[Lubos Lunak]

On Saturday 07 of November 2009, Sebastian Sauer wrote:

> > On 2009-11-04 10:10:06, Lubos Lunak wrote:
> > > The reason for waiting for the WM to finish setup is not the fallback,
> > > but making sure the WM is ready. There are some aspects related to
> > > window management where applications just read the status during their
> > > startup and don't update later. So waiting for the WM ensures this is
> > > set up properly before other applications may read that (although,
> > > thinking of that, this is currently a bit broken anyway, as KWin lets
> > > ksmserver continue a bit too soon).
> > >
> > > The second case is something similar. The kcminit modules do various
> > > setup and it is a question if it is ok to let normal apps start while
> > > this is still taking place.
>
> Specs are relative. Cold start (not initial) takes up to 9 seconds till the
> plasma desktop is visible / ksplashx closes.

 Hmm. Twice 1/2s is not that insignificant then :-/.

> The thing is that the wm starts very early. Even before plasma which does
> not need the wm at this point. The first "real" applications are rather
> late started/restored. Maybe we should just wait in
> KSMServer::autoStart1Done() till the wm is ready? Would that be enough?

 Plasma is not different from other applications in this regard, in fact, it
may depend more on it - compositing status, number of virtual desktops, etc.
I have no idea what would happen if e.g. Plasma thought there was just 1
virtual desktop because it saw that too soon.

> The kde startup process has lot of potential for optimizations.

 It is intentionally designed that way. The original application launching
code (around KSMServer::tryRestoreNext()) is written to launch apps one by
one. IIRC the reason for that was that computers[*] of those days were bad at
handling the load and putting sleeps here and there actually improved the
overall situation. I doubt anyone has benchmarked this in a reasonably recent
time.

[*] Or maybe we could blame the kernel.

> Time
> expensive things like e.g. kcminit are atm blocking the whole startup. Long
> term we need to have expensive things run in parallel at startup. Anyway,
> if you think that this is not a good idea then it should be maybe done
> another way.

 I don't know if there is another way. The code can be changed to launch
normal apps in parallel if that helps, there would be no problem with that.
But the rest currently has certain ordering (see ksmserver/README) with some
implications. For example startup phase 0 (i.e. Plasma) is done quite early
and waited for completion in order to have the desktop ready, hide the splash
and give the impression that the desktop or more or less ready while normal
apps are still being loaded. Moving things one way can mean the splash is
shown needlessly long when the desktop could be already usable, opposite way
would be showing desktop that is still not usable (as is the case with some
Windows versions and AFAIK it's rather annoying).

 If you would have some time to spend on this, it would be interesting if you
could analyze the KDE startup as a whole and see where the time is spent
(note: cold vs warm disk caches did tripple the number when I tested that the
last time, so pay attention to that). With that thinking about doing some
rearranging should be simpler.

--
Lubos Lunak
KDE developer
--------------------------------------------------------------
SUSE LINUX, s.r.o.   e-mail: l.lunak@... , l.lunak@...
Lihovarska 1060/12   tel: [phone]
190 00 Prague 9      fax: [phone]
Czech Republic       http://www.suse.cz