[Xcb] [Bug 4232] xlsclients much too slow over network

[Josh Triplett]

On Wed, Sep 02, 2009 at 11:49:08AM -0400, Peter Harris wrote:
> Barton C Massey wrote:
> > In message <20090902004224.547D813005A at annarchy.freedesktop.org> you wrote:
> >> Could you please try the attached XCB conversion of
> >> xlsclients.c? I'm curious to know how much it helps your
> >> case.
> > 
> > You beat me to it---*very* quick and cool.
> 
> Thanks. It looked like a fun project, and something that wouldn't take
> more than a day. I'm glad to see I was right (although it was a near thing).
> 
> >  I'll be curious
> > to see how it works out also.  Thanks much for this work,
> > Peter!
> 
> Some notes on this implementation:
> 
>  - I forgot to mention that if trying to build inside the xlsclients
> tree, you have to manually modify the Makefile to include at least
> -lxcb-atom (from xcb/util).

xlsclients uses autoconf, so hopefully you just need to modify the
PKG_CHECK_MODULES line in configure.ac.

>  - If I calculate it correctly, this version should operate in roughly
> (deepest_window_tree + 1) round trips. The previous version used roughly
> ((average_tree_depth * average_tree_width * 2 + number_of_atoms) *
> number_of_toplevel_windows) round trips. Where number_of_atoms is 2 by
> default (5 when the -l "verbose" switch is given), and "tree depth"
> excludes windows below any window with WM_STATE set.

A nice improvement indeed!  This seems like the minimum possible number
of round-trips.

>  - Most of those round-trips were spent in XmuClientWindow, and most of
> the rewrite was open-coding XmuClientWindow so it could be run in parallel.
> 
>  - This version uses more RAM -- when you have multiple requests in
> flight, you have to keep track of them. I haven't measured to see how
> much more RAM is used; it should be a trivial amount on modern machines,
> but it might matter for people doing embedded work (Nokia N800 or similar?).

The tablets have plenty of RAM, relatively speaking.  So does almost any
embedded device capable of running X.  For that matter, they almost
certainly won't run xlsclients, and in any case they won't have that
many simultaneous clients. :)

>  - This version uses more bandwidth. This is an intentional trade-off in
> order to reduce the number of round-trips. For example, this version
> issues query_tree in parallel with get_property(WM_STATE), and then
> throws the subtree away if the window does turn out to have a WM_STATE
> property.

Without knowing a lot about how xlsclients works: why throw that
information away rather than using it?

>  - Even on my local box (a dual P3-Xeon), this version is much faster
> (~30ms, vs ~200ms for the original code). I see similar times across a
> 100Mbit wired network.

Nice!

>  - Code review welcome. Certainly someone else should have a critical
> look at it (and maybe even fix the TODO bits) before this goes upstream.
> 
> 
> Left as an exercise for the reader:
>  - Issue WM_COMMAND (and friends) property requests in parallel with
> walking the tree. Should save one (1) round-trip, at the cost of even
> more wasted bandwidth.

*shrug*

>  - Shrink the string property request limits from 1MB (default lifted
> from Xlib) to something more sensible (1kB, maybe? 100 bytes?).

On my system, the manpage mentions a -m option to set the maximum
command length, which defaults to 10000.  Does that help?

>  - Re-implement strnlen for people who have only ISO C.

Ow, sigh.

>  - Replace XDisplayName (with xcb_parse_display?) to remove the need to
> link with Xlib.

Yay!

>  - Optionally operate in near-synchronous mode, which would reduce peak
> memory usage at the cost of extra round trips.

If you wanted to go this route, the right approach seems like putting an
upper bound on the number of requests in flight.  That doesn't seem
worth doing to me, though.

>  - autofoo

Should just require changing the one line in configure.ac.

- Josh Triplett