search

FreeRDP / FreeRDP-old

DEPRECATED VERSION - Check https://github.com/FreeRDP/FreeRDP : FreeRDP is a free remote desktop protocol client
http://www.freerdp.com
Apache License 2.0
80 stars 882 forks source link

DirectFB: Improve performance of event handling #44

Open g-reno opened 13 years ago

g-reno commented 13 years ago

I noticed that the event handling is setup as a running loop in freerdp.

That works fine for workstations and PC's that have unlimited power sources.

But for laptops and embedded devices running loops can be battery killers.

Maybe we could explore some type of wait_event mechanism for event handling.

I stuck in a DEBUG statement to see what was going on and I could see that the loop was going round n' round all the time even though my hands were off the keyboard and mouse and there was nothing happening on the server.

    0000 1a 00 17 00 ec 03 ea 03 01 00 00 01 0c 00 27 00 ..............'.
    0010 00 00 00 00 00 00 03 00 32 00                   ........2.
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG process_new_pointer_pdu (1062):
    DBG process_color_pointer_common (1001): cursor = -1409016256

    DBG cache_put_cursor (353):
    DBG run_dfbfreerdp (568): start of loop
    DBG process_cached_pointer_pdu (1024):
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG process_new_pointer_pdu (1062):
    DBG process_color_pointer_common (1001): cursor = -1409026032

    DBG cache_put_cursor (353):
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
    DBG run_dfbfreerdp (568): start of loop
otavio commented 13 years ago

Take a look on wait_obj. It might be what you're looking for.

g-reno commented 13 years ago

Just looking around for running loops in the tree:

$ grep -RHn 'while (1)' *
channels/rdpdr/smartcard/scard_operations.c:461:    while (1)
channels/rdpdr/smartcard/scard_operations.c:660:    while (1)
channels/rdpdr/rdpdr_main.c:827:    while (1)
channels/rdpdr/rdpdr_main.c:896:    while (1)
channels/rdpdr/rdpdr_scard.c:78:    while (1)
channels/rdpdbg/rdpdbg_main.c:112:  while (1)
channels/rdpdbg/rdpdbg_main.c:157:  while (1)
channels/rdpsnd/rdpsnd_main.c:203:  while (1)
channels/rdpsnd/rdpsnd_main.c:581:  while (1)
channels/rdpsnd/rdpsnd_main.c:632:  while (1)
channels/drdynvc/drdynvc_main.c:462:    while (1)
channels/drdynvc/drdynvc_main.c:508:    while (1)
channels/drdynvc/audin/alsa/audin_alsa.c:199:       while (1)
channels/cliprdr/cliprdr_x11.c:892: while (1)
channels/cliprdr/cliprdr_main.c:189:    while (1)
channels/cliprdr/cliprdr_main.c:234:    while (1)
dfb/dfbfreerdp.c:566:   while (1)
dfb/dfbfreerdp.c:743:   while (1)
libfreerdp-core/mppc.c:334:         while (1);
libfreerdp-core/mppc.c:368: while (1);
libfreerdp-gdi/gdi_32bpp.c:1086:    while (1)
libfreerdp-gdi/gdi_8bpp.c:844:  while (1)
libfreerdp-gdi/gdi_16bpp.c:1013:    while (1)
win/wfreerdp/wfreerdp.c:541:    while (1)
win/wfreerdp/wfreerdp.c:712:    while (1)
X11/xfreerdp.c:694: while (1)
X11/xfreerdp.c:872: while (1)

If these are all running continuously in the different threads they will definitely be a battery killer on any portable device.

.

otavio commented 13 years ago

We surelly welcome patches to address those :-D

g-reno commented 13 years ago

I'm sure. :-)

I think this one is going to take a team effort to look at this.

The scope is pretty big and you would have to do all of it in order to accomplish the goal.

.

g-reno commented 13 years ago

Yep, just what I was afraid of:

  PID USER      PR  NI  VIRT  RES  SHR S %CPU %MEM    TIME+  COMMAND                                                            
 9979 root      20   0  446m  20m 2920 S 21.6  0.3   0:06.60 lt-dfbfreerdp

This is just at idle with no plugins, no user activity, no server activity.

That level of idle CPU usage will kill a battery on a portable device in short order.

otavio commented 13 years ago

Yes ... I fully agree.

Surely this is a team-work and we ought to commit to avoid this to happen again in the future however it is natural to have those issues as the project start to mature itself.

Currently I am quite busy on other stuff so I won't be at help for now. If you can start looking at it and propose patches it would be awesome.

I am sure that others can help with this issue and also help to review proposed patches to address these issues.

g-reno commented 13 years ago

Right now it looks like these loops are all "polling" loops.

I think what we need are "wait event" loops that wait for an event interrupt to occur before they run the next cycle.

These are the copyrights from the files uniquely sorted, so probably they should make up the team I would think.

   Copyright 2009-2011 Jay Sorg
   Copyright 2010-2011 Vic Lee
   Copyright 2010 Marc-Andre Moreau 
   Copyright 2011 Eduardo Fiss Beloni 
   Copyright 2011 O.S. Systems Software Ltda.
   Copyright (c) 2009-2011 Jay Sorg
   Copyright (c) 2010-2011 Vic Lee
   Copyright (C) Alexi Volkov  2006
   Copyright (C) Jay Sorg 2009-2011
   Copyright (C) Jay Sorg 2010-2011
   Copyright (C) Matthew Chapman 1999-2008
otavio commented 13 years ago

Some of those do that. You can even see how it is done to try to accommodate it your the one you wish.

g-reno commented 13 years ago

Ok, I just reconfigured to compile X11 version.

It is showing less cpu usage than dfbfreerdp version.

So what is needed to make dfbfreerdp to behave like xfreerdp?

Maybe it is that improvements for event handling did not get migrated into the dfbfreerdp UI.

I'll try to look at this some more maybe tomorrow.

Maybe Jay or Vic could chime in here.