I'd rather see these xcb versions have a configure option (--enable-xcb for 
example)... I'm fine with it being on by default, but it's nice to have a 
fallback and easy regression testing.

On Nov 10, 2009, at 13:55, Peter Harris wrote:

> Like xlsclients, there are a whole lot of round trips in xlsatoms that
> can be avoided by using XCB. Unlike xlsclients, I am not aware of any
> end-user that has reported a bug against xlsatoms.
> 
> Over my DSL connection, this version completes in 0.58s, compared to
> 26.78s for the original xlsatoms.
> 
> Peter Harris
> -- 
>               Open Text Connectivity Solutions Group
> Peter Harris                    http://connectivity.opentext.com/
> Research and Development        Phone: +1 905 762 6001
> phar...@opentext.com            Toll Free: 1 877 359 4866
> From 88a5cc490fba1f8841079e6055add369918e85a3 Mon Sep 17 00:00:00 2001
> From: Peter Harris <phar...@opentext.com>
> Date: Mon, 19 Oct 2009 18:13:03 -0400
> Subject: [PATCH] Convert xlsatoms to XCB
> 
> This dramatically improves latency, at the cost of a small amount of
> bandwidth.
> 
> Signed-off-by: Peter Harris <phar...@opentext.com>
> ---
> configure.ac |    2 +-
> xlsatoms.c   |  131 ++++++++++++++++++++++++++++++++++++---------------------
> 2 files changed, 84 insertions(+), 49 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index af7e086..34725f1 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -39,7 +39,7 @@ AC_PROG_INSTALL
> XORG_DEFAULT_OPTIONS
> 
> # Checks for pkg-config packages
> -PKG_CHECK_MODULES(XLSATOMS, x11 xmuu)
> +PKG_CHECK_MODULES(XLSATOMS, xcb)
> AC_SUBST(XLSATOMS_CFLAGS)
> AC_SUBST(XLSATOMS_LIBS)
> 
> diff --git a/xlsatoms.c b/xlsatoms.c
> index 6e09b21..604a0ca 100644
> --- a/xlsatoms.c
> +++ b/xlsatoms.c
> @@ -29,18 +29,21 @@ in this Software without prior written authorization from 
> The Open Group.
> 
> #include <stdio.h>
> #include <stdlib.h>
> -#include <X11/Xos.h>
> -#include <X11/Xlib.h>
> -#include <X11/Xproto.h>
> -#include <X11/Xmu/Error.h>
> +#include <string.h>
> +#include <xcb/xcb.h>
> +#include <xcb/xproto.h>
> +
> +#define ATOMS_PER_BATCH 100 /* This number can be tuned 
> +                             higher for fewer round-trips
> +                             lower for less bandwidth wasted */
> 
> static char *ProgramName;
> +static char *DisplayString;
> 
> -static void do_name ( Display *dpy, char *format, char *name );
> +static void do_name ( xcb_connection_t *c, char *format, char *name );
> static int parse_range ( char *range, long *lowp, long *highp );
> -static void do_range ( Display *dpy, char *format, char *range );
> -static int catcher ( Display *dpy, XErrorEvent *err );
> -static void list_atoms ( Display *dpy, char *format, int mask, 
> +static void do_range ( xcb_connection_t *c, char *format, char *range );
> +static void list_atoms ( xcb_connection_t *c, char *format, int mask, 
>                        long low, long high );
> 
> static void 
> @@ -67,7 +70,7 @@ main(int argc, char *argv[])
>     char *format = "%lu\t%s";
>     int i, doit;
>     int didit = 0;
> -    Display *dpy = NULL;
> +    xcb_connection_t *c = NULL;
> 
>     ProgramName = argv[0];
> 
> @@ -88,14 +91,14 @@ main(int argc, char *argv[])
>                 case 'r':                     /* -range num-[num] */
>                   if (++i >= argc) usage ();
>                   if (doit) {
> -                     do_range (dpy, format, argv[i]);
> +                     do_range (c, format, argv[i]);
>                       didit = 1;
>                   }
>                   continue;
>                 case 'n':                     /* -name string */
>                   if (++i >= argc) usage ();
>                   if (doit) {
> -                     do_name (dpy, format, argv[i]);
> +                     do_name (c, format, argv[i]);
>                       didit = 1;
>                   }
>                   continue;
> @@ -104,33 +107,42 @@ main(int argc, char *argv[])
>           usage ();
>       }
>       if (!doit) {
> -         dpy = XOpenDisplay (displayname);
> -         if (!dpy) {
> +         DisplayString = displayname;
> +         if (!DisplayString)
> +             DisplayString = getenv("DISPLAY");
> +         if (!DisplayString)
> +             DisplayString = "";
> +         c = xcb_connect(displayname, NULL);
> +         if (!c || xcb_connection_has_error(c)) {
>               fprintf (stderr, "%s:  unable to open display \"%s\"\n",
> -                      ProgramName, XDisplayName (displayname));
> +                      ProgramName, DisplayString);
>               exit (1);
>           }
>       } else
>           if (!didit)         /* no options, default is list all */
> -             list_atoms(dpy, format, 0, 0, 0);
> +             list_atoms(c, format, 0, 0, 0);
>     }
> 
> -    XCloseDisplay (dpy);
> +    xcb_disconnect(c);
>     exit (0);
> }
> 
> static void
> -do_name(Display *dpy, char *format, char *name)
> +do_name(xcb_connection_t *c, char *format, char *name)
> {
> -    Atom a = XInternAtom (dpy, name, True);
> +    xcb_intern_atom_reply_t *a = xcb_intern_atom_reply(c, 
> +     xcb_intern_atom_unchecked(c, 1, strlen(name), name), NULL);
> 
> -    if (a != None) {
> -     printf (format, (unsigned long) a, name);
> +    if (a && a->atom != XCB_NONE) {
> +     printf (format, (unsigned long) a->atom, name);
>       putchar ('\n');
>     } else {
>       fprintf (stderr, "%s:  no atom named \"%s\" on server \"%s\"\n",
> -              ProgramName, name, DisplayString(dpy));
> +              ProgramName, name, DisplayString);
>     }
> +
> +    if (a)
> +     free(a);
> }
> 
> 
> @@ -173,62 +185,85 @@ parse_range(char *range, long *lowp, long *highp)
> }
> 
> static void
> -do_range(Display *dpy, char *format, char *range)
> +do_range(xcb_connection_t *c, char *format, char *range)
> {
>     int mask;
>     long low, high;
> 
>     mask = parse_range (range, &low, &high);
> -    list_atoms (dpy, format, mask, low, high);
> +    list_atoms (c, format, mask, low, high);
> }
> 
> -
> -static int 
> -catcher(Display *dpy, XErrorEvent *err)
> +static int
> +say_batch(xcb_connection_t *c, char *format, xcb_get_atom_name_cookie_t 
> *cookie, long low, long count)
> {
> -    if (err->request_code != X_GetAtomName) {
> -     XmuPrintDefaultErrorMessage (dpy, err, stderr);
> +    xcb_generic_error_t *e;
> +    char atom_name[1024];
> +    long i;
> +    int done = 0;
> +
> +    for (i = 0; i < count; i++)
> +     cookie[i] = xcb_get_atom_name(c, i + low);
> +
> +    for (i = 0; i < count; i++) {
> +     xcb_get_atom_name_reply_t *r;
> +     r = xcb_get_atom_name_reply(c, cookie[i], &e);
> +     if (r) {
> +         /* We could just use %*.s in 'format', but we want to be compatible
> +            with legacy command line usage */
> +         snprintf(atom_name, sizeof(atom_name), "%.*s",
> +             r->name_len, xcb_get_atom_name_name(r));
> +
> +         printf (format, i + low, atom_name);
> +         putchar ('\n');
> +         free(r);
> +     }
> +     if (e) {
> +         done = 1;
> +         free(e);
> +     }
>     }
> -    return 0;
> +
> +    return done;
> }
> 
> static void
> -list_atoms(Display *dpy, char *format, int mask, long low, long high)
> +list_atoms(xcb_connection_t *c, char *format, int mask, long low, long high)
> {
> -    XErrorHandler oldhandler = XSetErrorHandler (catcher);
> +    xcb_get_atom_name_cookie_t *cookie_jar;
> +    int done = 0;
> 
>     switch (mask) {
>       case RangeHigh:
>       low = 1;
>       /* fall through */
>       case (RangeLow | RangeHigh):
> -     for (; low <= high; low++) {
> -         char *s = XGetAtomName (dpy, (Atom)low);
> -         if (s) {
> -             printf (format, low, s);
> -             putchar ('\n');
> -             XFree (s);
> -         }
> +     cookie_jar = malloc((high - low + 1) * 
> sizeof(xcb_get_atom_name_cookie_t));
> +        if (!cookie_jar) {
> +         fprintf(stderr, "Out of memory allocating space for %ld atom 
> requests\n", high - low);
> +         return;
>       }
> +
> +     say_batch(c, format, cookie_jar, low, high - low + 1);
> +     free(cookie_jar);
>       break;
> 
>       default:
>       low = 1;
>       /* fall through */
>       case RangeLow:
> -     for (; ; low++) {
> -         char *s = XGetAtomName (dpy, (Atom)low);
> -         if (s) {
> -             printf (format, low, s);
> -             putchar ('\n');
> -             XFree (s);
> -         } else {
> -             break;
> -         }
> +     cookie_jar = malloc(ATOMS_PER_BATCH * 
> sizeof(xcb_get_atom_name_cookie_t));
> +        if (!cookie_jar) {
> +         fprintf(stderr, "Out of memory allocating space for %ld atom 
> requests\n", (long) ATOMS_PER_BATCH);
> +         return;
> +     }
> +     while (!done) {
> +         done = say_batch(c, format, cookie_jar, low, ATOMS_PER_BATCH);
> +         low += ATOMS_PER_BATCH;
>       }
> +     free(cookie_jar);
>       break;
>     }
> 
> -    XSetErrorHandler (oldhandler);
>     return;
> }
> -- 
> 1.6.5.1.1367.gcd48
> 
> _______________________________________________
> xorg-devel mailing list
> xorg-devel@lists.x.org
> http://lists.x.org/mailman/listinfo/xorg-devel

Attachment: smime.p7s
Description: S/MIME cryptographic signature

_______________________________________________
xorg-devel mailing list
xorg-devel@lists.x.org
http://lists.x.org/mailman/listinfo/xorg-devel

Reply via email to