|
View:
New views
7 Messages
—
Rating Filter:
Alert me
|
|
|
[PATCH] XCBify xlsatomsLike 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 pharris@... Toll Free: 1 877 359 4866 From 88a5cc490fba1f8841079e6055add369918e85a3 Mon Sep 17 00:00:00 2001 From: Peter Harris <pharris@...> 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 <pharris@...> --- 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 _______________________________________________ Xcb mailing list Xcb@... http://lists.freedesktop.org/mailman/listinfo/xcb |
|
|
Re: [PATCH] XCBify xlsatomsI had no idea there even *was* an xlsatoms. :-)
Nice, thanks much! Bart In message <4AF9E141.9040000@...> you 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 _______________________________________________ Xcb mailing list Xcb@... http://lists.freedesktop.org/mailman/listinfo/xcb |
|
|
Re: [PATCH] XCBify xlsatomsHi Peter!
It looks to me like you don't have commit rights to xorg git, but I think you should just push this, assuming you're satisfied with it. So I suggest following the instructions at http://www.freedesktop.org/wiki/AccountRequests in the "Requesting Modifications" section to get your account added to the xorg group. On Tue, Nov 10, 2009 at 1:55 PM, Peter Harris <pharris@...> wrote: > Over my DSL connection, this version completes in 0.58s, compared to > 26.78s for the original xlsatoms. That's so cool. Thanks Peter! Jamey _______________________________________________ Xcb mailing list Xcb@... http://lists.freedesktop.org/mailman/listinfo/xcb |
|
|
Re: [PATCH] XCBify xlsatomsI'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 > pharris@... Toll Free: 1 877 359 4866 > From 88a5cc490fba1f8841079e6055add369918e85a3 Mon Sep 17 00:00:00 2001 > From: Peter Harris <pharris@...> > 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 <pharris@...> > --- > 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@... > http://lists.x.org/mailman/listinfo/xorg-devel _______________________________________________ Xcb mailing list Xcb@... http://lists.freedesktop.org/mailman/listinfo/xcb |
|
|
Re: [PATCH] XCBify xlsatomsOn Wed, Nov 11, 2009 at 9:04 PM, Jeremy Huddleston wrote:
> 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. It seems a lot of work for minimal gain. Personally, I would find it easier to check out the 1.x branch (and/or download the 1.x tarball) than to pass a magic option to configure. Especially given: $ wc -l xlsclients.c 0001-xlsclients.patch 320 xlsclients.c 690 0001-xlsclients.patch $ wc -l xlsatoms.c 0001-Convert-xlsatoms-to-XCB.patch 234 xlsatoms.c 254 0001-Convert-xlsatoms-to-XCB.patch These are effectively rewrites. To leave the code in a readable state, you'd wind up with the Xlib version inside one giant #ifdef, and the XCB version inside the #else (or vice versa). If these were larger programs, I'd agree with you. But these are very small wrappers around basic X protocol operations. There's really very little (mostly just argument parsing and the copyright notice) that wasn't Xlib specific inside either of them. The last time this came up: http://lists.freedesktop.org/archives/xcb/2009-September/005014.html http://lists.freedesktop.org/archives/xcb/2009-September/005019.html Peter Harris _______________________________________________ Xcb mailing list Xcb@... http://lists.freedesktop.org/mailman/listinfo/xcb |
|
|
Re: [PATCH] XCBify xlsatomsOn Nov 11, 2009, at 19:33, Peter Harris wrote: > On Wed, Nov 11, 2009 at 9:04 PM, Jeremy Huddleston wrote: >> 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. > > It seems a lot of work for minimal gain. Personally, I would find it > easier to check out the 1.x branch (and/or download the 1.x tarball) > than to pass a magic option to configure. Especially given: Yeah, I'll bite. That's true. I guess we have to throw out the old at some point. Reviewed-By: Jeremy Huddleston <jeremyhu@...> _______________________________________________ Xcb mailing list Xcb@... http://lists.freedesktop.org/mailman/listinfo/xcb |
|
|
Re: [PATCH] XCBify xlsatomsI'd rather simplify the code. These are apps that few
people know exist, and even fewer care about. IMHO let's make them easy to maintain, and not gum them up with old Xlib code and ifdefs. Bart In message <05520906-7A08-46B3-9159-8C01373EF6E2@...> you wrote: > 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. Xcb mailing list Xcb@... http://lists.freedesktop.org/mailman/listinfo/xcb |
| Free embeddable forum powered by Nabble | Forum Help |