Libraries like Xlib/XCB, some XCB language bindings, and potentially others have a common problem: they want to share the X connection with XCB. This requires coordination of request sequence numbers. XCB currently has an Xlib-specific lock, and allows Xlib to block XCB from making requests. The attached patches to XCB replace that lock with a handoff mechanism, xcb_take_socket, allowing external code to ask XCB for permission to take over the write side of the socket and send raw data with xcb_writev. The caller of xcb_take_socket must supply a callback which XCB can call when it wants the write side of the socket back to make a request. This callback synchronizes with the external socket owner, flushes any output queues if appropriate, and then returns the sequence number of the last request sent over the socket. The attached patch to Xlib/XCB converts it to use the new handoff mechanism. Previously, Xlib/XCB used XCB's Xlib lock to prevent XCB from sending requests between calls to Xlib's LockDisplay and UnlockDisplay macros. Xlib/XCB then sent all of its requests using XCB's xcb_send_request, and had to flush its requests when unlocking the display. Using the new handoff mechanism, we replaced much of the existing Xlib/XCB implementation with the use of xcb_take_socket to take ownership of the write side of the X connection socket, and a return_socket callback which writes any outstanding requests with xcb_writev. This approach allows Xlib/XCB to use the same buffering as traditional Xlib did. In particular, programs which use Xlib/XCB and never make XCB calls will never need to hand the socket back to XCB, and vice versa. This allows us to discard large quantities of synchronization code from Xlib/XCB, together with the synchronization bugs present in that code. Several test cases which previously failed now work perfectly, including multi-threaded ico. In addition, the infamous locking correctness assertions, triggered when double-locking or when unlocking without a previous lock, no longer exist, because Xlib/XCB no longer has any reason to care more about application locking than traditional Xlib does. Let's repeat that for emphasis: the locking assertions *no longer exist*. Existing Xlib applications with buggy locking will now work just as well, or as badly, as they did with traditional Xlib. Furthermore, the handoff approach provides great improvements to performance. Results from x11perf's XNoOp test, which represented the worst case for the lock-based Xlib/XCB: Traditional Xlib: average 19100000/sec Lock-based Xlib/XCB: average 3350000/sec Handoff-based Xlib/XCB: average 17400000/sec Thus, for no-ops, the handoff mechanism provides more than a 4x speedup to Xlib/XCB, bringing Xlib/XCB within 9% of traditional Xlib no-op performance. Of course, real-world workloads do not use no-op, so your mileage may vary. In particular, since no-ops represent the worst case, we expect real workloads to more closely match the performance of traditional Xlib. Finally, we anticipate that this new handoff mechanism will prove useful for more than just Xlib/XCB. In particular, several authors or potential authors of language bindings based on the XML-XCB protocol descriptions have expressed an interest in implementing the protocol code in their native language, rather than simply using a foreign function interface (FFI) to call XCB's protocol layer. In the past, we have recommended against this, because then other X client code in the same process (such as other libraries for which the language uses an FFI) could not share the same X connection. Those language bindings could now use XCB's new handoff mechanism when sending requests from their own protocol layer, and rely only on XCB's transport layer. We blame Keith Packard for the fundamental idea of the handoff mechanism, and for most of its advantages. Please review and provide comments. We hope to commit these patches and release new versions of XCB and Xlib/XCB soon. - Josh Triplett and Jamey Sharp
From f7a9ef8199cf39e155e972b06b5e94bc700475ce Mon Sep 17 00:00:00 2001
From: Jamey Sharp <jamey@minilop.net>
Date: Fri, 14 Mar 2008 12:08:32 -0700
Subject: [PATCH 1/5] Remove libxcb-xlib and xcbxlib.h.
---
src/Makefile.am | 9 +----
src/xcb_conn.c | 49 +------------------------------
src/xcb_xlib.c | 87 -------------------------------------------------------
src/xcbint.h | 19 +-----------
src/xcbxlib.h | 44 ----------------------------
5 files changed, 5 insertions(+), 203 deletions(-)
delete mode 100644 src/xcb_xlib.c
delete mode 100644 src/xcbxlib.h
diff --git a/src/Makefile.am b/src/Makefile.am
index fc613b4..fb70c92 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1,5 +1,4 @@
-lib_LTLIBRARIES = libxcb.la \
- libxcb-xlib.la
+lib_LTLIBRARIES = libxcb.la
EXTHEADERS = xproto.h \
bigreq.h \
@@ -29,10 +28,6 @@ libxcb_la_LDFLAGS = -version-info 1:0:0
XCB_LIBS = libxcb.la
-libxcb_xlib_la_LDFLAGS = -version-info 0:0:0
-libxcb_xlib_la_LIBADD = $(XCB_LIBS)
-libxcb_xlib_la_SOURCES = xcb_xlib.c
-
# FIXME: find a way to autogenerate this from the XML files.
if BUILD_COMPOSITE
@@ -237,7 +232,7 @@ endif
-xcbinclude_HEADERS = xcb.h xcbext.h xcbxlib.h $(EXTHEADERS)
+xcbinclude_HEADERS = xcb.h xcbext.h $(EXTHEADERS)
noinst_HEADERS = xcbint.h
BUILT_SOURCES = $(EXTSOURCES) $(EXTHEADERS)
diff --git a/src/xcb_conn.c b/src/xcb_conn.c
index e7856c3..a2b0ab7 100644
--- a/src/xcb_conn.c
+++ b/src/xcb_conn.c
@@ -59,21 +59,6 @@ static int set_fd_flags(const int fd)
return 1;
}
-static int _xcb_xlib_init(_xcb_xlib *xlib)
-{
- xlib->lock = 0;
-#ifndef NDEBUG
- xlib->sloppy_lock = (getenv("LIBXCB_ALLOW_SLOPPY_LOCK") != 0);
-#endif
- pthread_cond_init(&xlib->cond, 0);
- return 1;
-}
-
-static void _xcb_xlib_destroy(_xcb_xlib *xlib)
-{
- pthread_cond_destroy(&xlib->cond);
-}
-
static int write_setup(xcb_connection_t *c, xcb_auth_info_t *auth_info)
{
static const char pad[3];
@@ -230,7 +215,6 @@ xcb_connection_t *xcb_connect_to_fd(int fd, xcb_auth_info_t *auth_info)
if(!(
set_fd_flags(fd) &&
pthread_mutex_init(&c->iolock, 0) == 0 &&
- _xcb_xlib_init(&c->xlib) &&
_xcb_in_init(&c->in) &&
_xcb_out_init(&c->out) &&
write_setup(c, auth_info) &&
@@ -255,7 +239,6 @@ void xcb_disconnect(xcb_connection_t *c)
close(c->fd);
pthread_mutex_destroy(&c->iolock);
- _xcb_xlib_destroy(&c->xlib);
_xcb_in_destroy(&c->in);
_xcb_out_destroy(&c->out);
@@ -275,12 +258,6 @@ void _xcb_conn_shutdown(xcb_connection_t *c)
void _xcb_lock_io(xcb_connection_t *c)
{
pthread_mutex_lock(&c->iolock);
- while(c->xlib.lock)
- {
- if(pthread_equal(c->xlib.thread, pthread_self()))
- break;
- pthread_cond_wait(&c->xlib.cond, &c->iolock);
- }
}
void _xcb_unlock_io(xcb_connection_t *c)
@@ -290,25 +267,12 @@ void _xcb_unlock_io(xcb_connection_t *c)
void _xcb_wait_io(xcb_connection_t *c, pthread_cond_t *cond)
{
- int xlib_locked = c->xlib.lock;
- if(xlib_locked)
- {
- c->xlib.lock = 0;
- pthread_cond_broadcast(&c->xlib.cond);
- }
pthread_cond_wait(cond, &c->iolock);
- if(xlib_locked)
- {
- while(c->xlib.lock)
- pthread_cond_wait(&c->xlib.cond, &c->iolock);
- c->xlib.lock = 1;
- c->xlib.thread = pthread_self();
- }
}
int _xcb_conn_wait(xcb_connection_t *c, pthread_cond_t *cond, struct iovec **vector, int *count)
{
- int ret, xlib_locked;
+ int ret;
fd_set rfds, wfds;
/* If the thing I should be doing is already being done, wait for it. */
@@ -329,12 +293,6 @@ int _xcb_conn_wait(xcb_connection_t *c, pthread_cond_t *cond, struct iovec **vec
++c->out.writing;
}
- xlib_locked = c->xlib.lock;
- if(xlib_locked)
- {
- c->xlib.lock = 0;
- pthread_cond_broadcast(&c->xlib.cond);
- }
_xcb_unlock_io(c);
do {
ret = select(c->fd + 1, &rfds, &wfds, 0, 0);
@@ -345,11 +303,6 @@ int _xcb_conn_wait(xcb_connection_t *c, pthread_cond_t *cond, struct iovec **vec
ret = 0;
}
_xcb_lock_io(c);
- if(xlib_locked)
- {
- c->xlib.lock = 1;
- c->xlib.thread = pthread_self();
- }
if(ret)
{
diff --git a/src/xcb_xlib.c b/src/xcb_xlib.c
deleted file mode 100644
index 1b573e8..0000000
--- a/src/xcb_xlib.c
+++ /dev/null
@@ -1,87 +0,0 @@
-/* Copyright (C) 2005 Bart Massey and Jamey Sharp.
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the "Software"),
- * to deal in the Software without restriction, including without limitation
- * the rights to use, copy, modify, merge, publish, distribute, sublicense,
- * and/or sell copies of the Software, and to permit persons to whom the
- * Software is furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
- * AUTHORS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
- * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
- * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
- *
- * Except as contained in this notice, the names of the authors or their
- * institutions shall not be used in advertising or otherwise to promote the
- * sale, use or other dealings in this Software without prior written
- * authorization from the authors.
- */
-
-#include "xcbxlib.h"
-#include "xcbint.h"
-
-#include <assert.h>
-
-#ifdef HAVE_BACKTRACE
-#include <execinfo.h>
-#include <stdio.h>
-#include <stdlib.h>
-#endif
-
-static void xcb_xlib_printbt(void)
-{
-#ifdef HAVE_BACKTRACE
- void *array[20];
- int size;
- char **strings;
- int i;
-
- size = backtrace(array, 20);
- strings = backtrace_symbols(array, size);
-
- fprintf(stderr, "Locking assertion failure. Backtrace:\n");
-
- for (i = 0; i < size; ++i)
- fprintf(stderr, "#%i %s\n", i, strings[i]);
-
- free(strings);
-#endif
-}
-
-#ifndef NDEBUG
-#define xcb_assert(c,x) do { if (!(x)) { xcb_xlib_printbt(); if (!(c)->xlib.sloppy_lock) assert(x); } } while(0)
-#else
-#define xcb_assert(c,x)
-#endif
-
-unsigned int xcb_get_request_sent(xcb_connection_t *c)
-{
- if(c->has_error)
- return 0;
- return c->out.request;
-}
-
-void xcb_xlib_lock(xcb_connection_t *c)
-{
- _xcb_lock_io(c);
- xcb_assert(c, !c->xlib.lock);
- c->xlib.lock = 1;
- c->xlib.thread = pthread_self();
- _xcb_unlock_io(c);
-}
-
-void xcb_xlib_unlock(xcb_connection_t *c)
-{
- _xcb_lock_io(c);
- xcb_assert(c, c->xlib.lock);
- xcb_assert(c, pthread_equal(c->xlib.thread, pthread_self()));
- c->xlib.lock = 0;
- pthread_cond_broadcast(&c->xlib.cond);
- _xcb_unlock_io(c);
-}
diff --git a/src/xcbint.h b/src/xcbint.h
index ab0264f..86e00a4 100644
--- a/src/xcbint.h
+++ b/src/xcbint.h
@@ -126,16 +126,6 @@ int _xcb_in_read(xcb_connection_t *c);
int _xcb_in_read_block(xcb_connection_t *c, void *buf, int nread);
-/* xcb_xlib.c */
-
-typedef struct _xcb_xlib {
- int lock;
- int sloppy_lock;
- pthread_t thread;
- pthread_cond_t cond;
-} _xcb_xlib;
-
-
/* xcb_xid.c */
typedef struct _xcb_xid {
@@ -173,7 +163,6 @@ struct xcb_connection_t {
/* I/O data */
pthread_mutex_t iolock;
- _xcb_xlib xlib;
_xcb_in in;
_xcb_out out;
@@ -183,6 +172,8 @@ struct xcb_connection_t {
};
void _xcb_conn_shutdown(xcb_connection_t *c);
+void _xcb_lock_io(xcb_connection_t *c);
+void _xcb_unlock_io(xcb_connection_t *c);
void _xcb_wait_io(xcb_connection_t *c, pthread_cond_t *cond);
int _xcb_conn_wait(xcb_connection_t *c, pthread_cond_t *cond, struct iovec **vector, int *count);
@@ -195,10 +186,4 @@ int _xcb_get_auth_info(int fd, xcb_auth_info_t *info, int display);
#pragma GCC visibility pop
#endif
-
-/* xcb_conn.c symbols visible to xcb-xlib */
-
-void _xcb_lock_io(xcb_connection_t *c);
-void _xcb_unlock_io(xcb_connection_t *c);
-
#endif
diff --git a/src/xcbxlib.h b/src/xcbxlib.h
deleted file mode 100644
index 4cb5cd4..0000000
--- a/src/xcbxlib.h
+++ /dev/null
@@ -1,44 +0,0 @@
-/*
- * Copyright (C) 2005 Bart Massey and Jamey Sharp.
- * All Rights Reserved.
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the "Software"),
- * to deal in the Software without restriction, including without limitation
- * the rights to use, copy, modify, merge, publish, distribute, sublicense,
- * and/or sell copies of the Software, and to permit persons to whom the
- * Software is furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
- * AUTHORS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
- * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
- * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
- *
- * Except as contained in this notice, the names of the authors or their
- * institutions shall not be used in advertising or otherwise to promote the
- * sale, use or other dealings in this Software without prior written
- * authorization from the authors.
- */
-
-/* This include file declares functions used by Xlib/XCB, but nothing else
- * should ever use these functions or link to libxcb-xlib. */
-
-#ifndef __XCBXLIB_H
-#define __XCBXLIB_H
-
-#include <pthread.h>
-#include "xcb.h"
-
-/* The caller of this function must hold the xlib lock, using the lock
- * functions below. */
-unsigned int xcb_get_request_sent(xcb_connection_t *c);
-
-void xcb_xlib_lock(xcb_connection_t *c);
-void xcb_xlib_unlock(xcb_connection_t *c);
-
-#endif
--
1.5.4.1
From f75a1ec0d605374ae21014dbf0d15b4a7ac5239c Mon Sep 17 00:00:00 2001
From: Jamey Sharp <jamey@minilop.net>
Date: Fri, 14 Mar 2008 12:08:58 -0700
Subject: [PATCH 2/5] Inline _xcb_lock_io, _xcb_unlock_io, and _xcb_wait_io.
These functions are once again a single pthread call, so just make that
call directly.
---
src/xcb_conn.c | 25 +++++--------------------
src/xcb_in.c | 16 ++++++++--------
src/xcb_out.c | 12 ++++++------
src/xcbint.h | 3 ---
4 files changed, 19 insertions(+), 37 deletions(-)
diff --git a/src/xcb_conn.c b/src/xcb_conn.c
index a2b0ab7..75ab519 100644
--- a/src/xcb_conn.c
+++ b/src/xcb_conn.c
@@ -97,12 +97,12 @@ static int write_setup(xcb_connection_t *c, xcb_auth_info_t *auth_info)
}
assert(count <= sizeof(parts) / sizeof(*parts));
- _xcb_lock_io(c);
+ pthread_mutex_lock(&c->iolock);
{
struct iovec *parts_ptr = parts;
ret = _xcb_out_send(c, &parts_ptr, &count);
}
- _xcb_unlock_io(c);
+ pthread_mutex_unlock(&c->iolock);
return ret;
}
@@ -255,21 +255,6 @@ void _xcb_conn_shutdown(xcb_connection_t *c)
c->has_error = 1;
}
-void _xcb_lock_io(xcb_connection_t *c)
-{
- pthread_mutex_lock(&c->iolock);
-}
-
-void _xcb_unlock_io(xcb_connection_t *c)
-{
- pthread_mutex_unlock(&c->iolock);
-}
-
-void _xcb_wait_io(xcb_connection_t *c, pthread_cond_t *cond)
-{
- pthread_cond_wait(cond, &c->iolock);
-}
-
int _xcb_conn_wait(xcb_connection_t *c, pthread_cond_t *cond, struct iovec **vector, int *count)
{
int ret;
@@ -278,7 +263,7 @@ int _xcb_conn_wait(xcb_connection_t *c, pthread_cond_t *cond, struct iovec **vec
/* If the thing I should be doing is already being done, wait for it. */
if(count ? c->out.writing : c->in.reading)
{
- _xcb_wait_io(c, cond);
+ pthread_cond_wait(cond, &c->iolock);
return 1;
}
@@ -293,7 +278,7 @@ int _xcb_conn_wait(xcb_connection_t *c, pthread_cond_t *cond, struct iovec **vec
++c->out.writing;
}
- _xcb_unlock_io(c);
+ pthread_mutex_unlock(&c->iolock);
do {
ret = select(c->fd + 1, &rfds, &wfds, 0, 0);
} while (ret == -1 && errno == EINTR);
@@ -302,7 +287,7 @@ int _xcb_conn_wait(xcb_connection_t *c, pthread_cond_t *cond, struct iovec **vec
_xcb_conn_shutdown(c);
ret = 0;
}
- _xcb_lock_io(c);
+ pthread_mutex_lock(&c->iolock);
if(ret)
{
diff --git a/src/xcb_in.c b/src/xcb_in.c
index 2997de4..14de474 100644
--- a/src/xcb_in.c
+++ b/src/xcb_in.c
@@ -319,7 +319,7 @@ void *xcb_wait_for_reply(xcb_connection_t *c, unsigned int request, xcb_generic_
if(c->has_error)
return 0;
- _xcb_lock_io(c);
+ pthread_mutex_lock(&c->iolock);
/* If this request has not been written yet, write it. */
if(_xcb_out_flush_to(c, request))
@@ -359,7 +359,7 @@ void *xcb_wait_for_reply(xcb_connection_t *c, unsigned int request, xcb_generic_
}
wake_up_next_reader(c);
- _xcb_unlock_io(c);
+ pthread_mutex_unlock(&c->iolock);
return ret;
}
@@ -374,9 +374,9 @@ int xcb_poll_for_reply(xcb_connection_t *c, unsigned int request, void **reply,
return 1; /* would not block */
}
assert(reply != 0);
- _xcb_lock_io(c);
+ pthread_mutex_lock(&c->iolock);
ret = poll_for_reply(c, request, reply, error);
- _xcb_unlock_io(c);
+ pthread_mutex_unlock(&c->iolock);
return ret;
}
@@ -385,14 +385,14 @@ xcb_generic_event_t *xcb_wait_for_event(xcb_connection_t *c)
xcb_generic_event_t *ret;
if(c->has_error)
return 0;
- _xcb_lock_io(c);
+ pthread_mutex_lock(&c->iolock);
/* get_event returns 0 on empty list. */
while(!(ret = get_event(c)))
if(!_xcb_conn_wait(c, &c->in.event_cond, 0, 0))
break;
wake_up_next_reader(c);
- _xcb_unlock_io(c);
+ pthread_mutex_unlock(&c->iolock);
return ret;
}
@@ -401,12 +401,12 @@ xcb_generic_event_t *xcb_poll_for_event(xcb_connection_t *c)
xcb_generic_event_t *ret = 0;
if(!c->has_error)
{
- _xcb_lock_io(c);
+ pthread_mutex_lock(&c->iolock);
/* FIXME: follow X meets Z architecture changes. */
ret = get_event(c);
if(!ret && _xcb_in_read(c)) /* _xcb_in_read shuts down the connection on error */
ret = get_event(c);
- _xcb_unlock_io(c);
+ pthread_mutex_unlock(&c->iolock);
}
return ret;
}
diff --git a/src/xcb_out.c b/src/xcb_out.c
index 60226e5..bd6d981 100644
--- a/src/xcb_out.c
+++ b/src/xcb_out.c
@@ -187,10 +187,10 @@ unsigned int xcb_send_request(xcb_connection_t *c, int flags, struct iovec *vect
workaround = WORKAROUND_GLX_GET_FB_CONFIGS_BUG;
/* get a sequence number and arrange for delivery. */
- _xcb_lock_io(c);
+ pthread_mutex_lock(&c->iolock);
/* wait for other writing threads to get out of my way. */
while(c->out.writing)
- _xcb_wait_io(c, &c->out.cond);
+ pthread_cond_wait(&c->out.cond, &c->iolock);
request = ++c->out.request;
/* send GetInputFocus (sync) when 64k-2 requests have been sent without
@@ -231,7 +231,7 @@ unsigned int xcb_send_request(xcb_connection_t *c, int flags, struct iovec *vect
_xcb_conn_shutdown(c);
request = 0;
}
- _xcb_unlock_io(c);
+ pthread_mutex_unlock(&c->iolock);
return request;
}
@@ -240,9 +240,9 @@ int xcb_flush(xcb_connection_t *c)
int ret;
if(c->has_error)
return 0;
- _xcb_lock_io(c);
+ pthread_mutex_lock(&c->iolock);
ret = _xcb_out_flush_to(c, c->out.request);
- _xcb_unlock_io(c);
+ pthread_mutex_unlock(&c->iolock);
return ret;
}
@@ -297,7 +297,7 @@ int _xcb_out_flush_to(xcb_connection_t *c, unsigned int request)
return _xcb_out_send(c, &vec_ptr, &count);
}
while(c->out.writing)
- _xcb_wait_io(c, &c->out.cond);
+ pthread_cond_wait(&c->out.cond, &c->iolock);
assert(XCB_SEQUENCE_COMPARE(c->out.request_written, >=, request));
return 1;
}
diff --git a/src/xcbint.h b/src/xcbint.h
index 86e00a4..d610a10 100644
--- a/src/xcbint.h
+++ b/src/xcbint.h
@@ -172,9 +172,6 @@ struct xcb_connection_t {
};
void _xcb_conn_shutdown(xcb_connection_t *c);
-void _xcb_lock_io(xcb_connection_t *c);
-void _xcb_unlock_io(xcb_connection_t *c);
-void _xcb_wait_io(xcb_connection_t *c, pthread_cond_t *cond);
int _xcb_conn_wait(xcb_connection_t *c, pthread_cond_t *cond, struct iovec **vector, int *count);
--
1.5.4.1
From 8e851fc6ac73ff6d11e64be1f0c3a850a5533704 Mon Sep 17 00:00:00 2001
From: Jamey Sharp <jamey@minilop.net>
Date: Fri, 14 Mar 2008 20:18:52 -0700
Subject: [PATCH 3/5] Use sequence number ranges in pending replies
This allows optimizing adjacent pending replies with the same flags, and
will help support default flags for a range of future requests.
Commit by Josh Triplett and Jamey Sharp.
---
src/xcb_in.c | 11 +++++++----
1 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/src/xcb_in.c b/src/xcb_in.c
index 14de474..aa462f0 100644
--- a/src/xcb_in.c
+++ b/src/xcb_in.c
@@ -51,7 +51,8 @@ struct reply_list {
};
typedef struct pending_reply {
- unsigned int request;
+ unsigned int first_request;
+ unsigned int last_request;
enum workarounds workaround;
int flags;
struct pending_reply *next;
@@ -110,7 +111,7 @@ static int read_packet(xcb_connection_t *c)
}
while(c->in.pending_replies &&
- XCB_SEQUENCE_COMPARE (c->in.pending_replies->request, <=, c->in.request_completed))
+ XCB_SEQUENCE_COMPARE (c->in.pending_replies->last_request, <=, c->in.request_completed))
{
pending_reply *oldpend = c->in.pending_replies;
c->in.pending_replies = oldpend->next;
@@ -126,7 +127,9 @@ static int read_packet(xcb_connection_t *c)
if(genrep.response_type == XCB_ERROR || genrep.response_type == XCB_REPLY)
{
pend = c->in.pending_replies;
- if(pend && pend->request != c->in.request_read)
+ if(pend &&
+ (XCB_SEQUENCE_COMPARE(c->in.request_read, <, pend->first_request) ||
+ XCB_SEQUENCE_COMPARE(c->in.request_read, >, pend->last_request)))
pend = 0;
}
@@ -484,7 +487,7 @@ int _xcb_in_expect_reply(xcb_connection_t *c, unsigned int request, enum workaro
_xcb_conn_shutdown(c);
return 0;
}
- pend->request = request;
+ pend->first_request = pend->last_request = request;
pend->workaround = workaround;
pend->flags = flags;
pend->next = 0;
--
1.5.4.1
From 5a71e0fb2554be9bc867a41bd61b9336a5c538a0 Mon Sep 17 00:00:00 2001
From: Jamey Sharp <jamey@minilop.net>
Date: Fri, 14 Mar 2008 21:03:21 -0700
Subject: [PATCH 4/5] Support handing off socket write permission to external code.
Libraries like Xlib, some XCB language bindings, and potentially others
have a common problem: they want to share the X connection with XCB. This
requires coordination of request sequence numbers. Previously, XCB had an
Xlib-specific lock, and allowed Xlib to block XCB from making requests.
Now we've replaced that lock with a handoff mechanism, xcb_take_socket,
allowing external code to ask XCB for permission to take over the write
side of the socket and send raw data with xcb_writev. The caller of
xcb_take_socket must supply a callback which XCB can call when it wants
the write side of the socket back to make a request. This callback
synchronizes with the external socket owner, flushes any output queues if
appropriate, and then returns the sequence number of the last request sent
over the socket.
Commit by Josh Triplett and Jamey Sharp.
Handoff mechanism inspired by Keith Packard.
---
src/Makefile.am | 2 +-
src/xcb_in.c | 36 +++++++++++++++++++++++++++++--
src/xcb_out.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
src/xcbext.h | 16 ++++++++++++++
src/xcbint.h | 14 +++++++++++-
5 files changed, 124 insertions(+), 5 deletions(-)
diff --git a/src/Makefile.am b/src/Makefile.am
index fb70c92..0cd16f5 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -24,7 +24,7 @@ libxcb_la_SOURCES = \
# * If you add an interface, increment current and age and set revision to 0.
# * If you change or remove an interface, increment current and set revision
# and age to 0.
-libxcb_la_LDFLAGS = -version-info 1:0:0
+libxcb_la_LDFLAGS = -version-info 2:0:1
XCB_LIBS = libxcb.la
diff --git a/src/xcb_in.c b/src/xcb_in.c
index aa462f0..0e3beca 100644
--- a/src/xcb_in.c
+++ b/src/xcb_in.c
@@ -111,6 +111,7 @@ static int read_packet(xcb_connection_t *c)
}
while(c->in.pending_replies &&
+ c->in.pending_replies->workaround != WORKAROUND_EXTERNAL_SOCKET_OWNER &&
XCB_SEQUENCE_COMPARE (c->in.pending_replies->last_request, <=, c->in.request_completed))
{
pending_reply *oldpend = c->in.pending_replies;
@@ -128,8 +129,9 @@ static int read_packet(xcb_connection_t *c)
{
pend = c->in.pending_replies;
if(pend &&
- (XCB_SEQUENCE_COMPARE(c->in.request_read, <, pend->first_request) ||
- XCB_SEQUENCE_COMPARE(c->in.request_read, >, pend->last_request)))
+ !(XCB_SEQUENCE_COMPARE(pend->first_request, <=, c->in.request_read) &&
+ (pend->workaround == WORKAROUND_EXTERNAL_SOCKET_OWNER ||
+ XCB_SEQUENCE_COMPARE(c->in.request_read, <=, pend->last_request))))
pend = 0;
}
@@ -325,7 +327,7 @@ void *xcb_wait_for_reply(xcb_connection_t *c, unsigned int request, xcb_generic_
pthread_mutex_lock(&c->iolock);
/* If this request has not been written yet, write it. */
- if(_xcb_out_flush_to(c, request))
+ if(c->out.return_socket || _xcb_out_flush_to(c, request))
{
pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
reader_list reader;
@@ -496,6 +498,34 @@ int _xcb_in_expect_reply(xcb_connection_t *c, unsigned int request, enum workaro
return 1;
}
+int _xcb_in_expect_replies(xcb_connection_t *c, unsigned int request, int flags)
+{
+ pending_reply *pend = malloc(sizeof(pending_reply));
+ assert(flags != 0);
+ if(!pend)
+ {
+ _xcb_conn_shutdown(c);
+ return 0;
+ }
+ pend->first_request = pend->last_request = request;
+ pend->workaround = WORKAROUND_EXTERNAL_SOCKET_OWNER;
+ pend->flags = flags;
+ pend->next = 0;
+ *c->in.pending_replies_tail = pend;
+ c->in.pending_replies_tail = &pend->next;
+ return 1;
+}
+
+void _xcb_in_replies_done(xcb_connection_t *c)
+{
+ struct pending_reply *pend;
+ assert(c->in.pending_replies_tail != &c->in.pending_replies);
+ pend = container_of(c->in.pending_replies_tail, struct pending_reply, next);
+ assert(pend->workaround == WORKAROUND_EXTERNAL_SOCKET_OWNER);
+ pend->last_request = c->out.request;
+ pend->workaround = WORKAROUND_NONE;
+}
+
int _xcb_in_read(xcb_connection_t *c)
{
int n = read(c->fd, c->in.queue + c->in.queue_len, sizeof(c->in.queue) - c->in.queue_len);
diff --git a/src/xcb_out.c b/src/xcb_out.c
index bd6d981..d9d7385 100644
--- a/src/xcb_out.c
+++ b/src/xcb_out.c
@@ -55,6 +55,27 @@ static int write_block(xcb_connection_t *c, struct iovec *vector, int count)
return _xcb_out_send(c, &vector, &count);
}
+static void get_socket_back(xcb_connection_t *c)
+{
+ unsigned int new_request;
+ while(c->out.return_socket && c->out.socket_moving)
+ pthread_cond_wait(&c->out.socket_cond, &c->iolock);
+ if(!c->out.return_socket)
+ return;
+
+ c->out.socket_moving = 1;
+ pthread_mutex_unlock(&c->iolock);
+ new_request = c->out.return_socket(c->out.socket_closure);
+ pthread_mutex_lock(&c->iolock);
+ c->out.socket_moving = 0;
+
+ pthread_cond_signal(&c->out.socket_cond);
+ c->out.return_socket = 0;
+ c->out.socket_closure = 0;
+ c->out.request_written = c->out.request = new_request;
+ _xcb_in_replies_done(c);
+}
+
/* Public interface */
void xcb_prefetch_maximum_request_length(xcb_connection_t *c)
@@ -191,6 +212,7 @@ unsigned int xcb_send_request(xcb_connection_t *c, int flags, struct iovec *vect
/* wait for other writing threads to get out of my way. */
while(c->out.writing)
pthread_cond_wait(&c->out.cond, &c->iolock);
+ get_socket_back(c);
request = ++c->out.request;
/* send GetInputFocus (sync) when 64k-2 requests have been sent without
@@ -235,6 +257,39 @@ unsigned int xcb_send_request(xcb_connection_t *c, int flags, struct iovec *vect
return request;
}
+int xcb_take_socket(xcb_connection_t *c, unsigned int (*return_socket)(void *closure), void *closure, int flags, unsigned int *sent, unsigned int *read)
+{
+ int ret;
+ if(c->has_error)
+ return 0;
+ pthread_mutex_lock(&c->iolock);
+ get_socket_back(c);
+ ret = _xcb_out_flush_to(c, c->out.request);
+ if(ret)
+ {
+ c->out.return_socket = return_socket;
+ c->out.socket_closure = closure;
+ if(flags)
+ _xcb_in_expect_replies(c, c->out.request, flags);
+ assert(c->out.request == c->out.request_written);
+ *sent = c->out.request;
+ *read = c->in.request_read;
+ }
+ pthread_mutex_unlock(&c->iolock);
+ return ret;
+}
+
+int xcb_writev(xcb_connection_t *c, struct iovec *vector, int count)
+{
+ int ret;
+ if(c->has_error)
+ return 0;
+ pthread_mutex_lock(&c->iolock);
+ ret = _xcb_out_send(c, &vector, &count);
+ pthread_mutex_unlock(&c->iolock);
+ return ret;
+}
+
int xcb_flush(xcb_connection_t *c)
{
int ret;
@@ -250,6 +305,12 @@ int xcb_flush(xcb_connection_t *c)
int _xcb_out_init(_xcb_out *out)
{
+ if(pthread_cond_init(&out->socket_cond, 0))
+ return 0;
+ out->return_socket = 0;
+ out->socket_closure = 0;
+ out->socket_moving = 0;
+
if(pthread_cond_init(&out->cond, 0))
return 0;
out->writing = 0;
diff --git a/src/xcbext.h b/src/xcbext.h
index 01dd590..67d4470 100644
--- a/src/xcbext.h
+++ b/src/xcbext.h
@@ -59,6 +59,22 @@ enum xcb_send_request_flags_t {
unsigned int xcb_send_request(xcb_connection_t *c, int flags, struct iovec *vector, const xcb_protocol_request_t *request);
+/* xcb_take_socket allows external code to ask XCB for permission to
+ * take over the write side of the socket and send raw data with
+ * xcb_writev. xcb_take_socket provides the sequence number of the last
+ * request XCB sent and the sequence number of the last response XCB
+ * received. The caller of xcb_take_socket must supply a callback which
+ * XCB can call when it wants the write side of the socket back to make
+ * a request. This callback synchronizes with the external socket
+ * owner, flushes any output queues if appropriate, and then returns the
+ * sequence number of the last request sent over the socket. */
+int xcb_take_socket(xcb_connection_t *c, unsigned int (*return_socket)(void *closure), void *closure, int flags, unsigned int *sent, unsigned int *read);
+
+/* You must own the write-side of the socket (you've called xcb_take_socket,
+ * and haven't returned from return_socket yet) to call xcb_writev. Also, the
+ * iovec must have at least 1 byte of data in it. */
+int xcb_writev(xcb_connection_t *c, struct iovec *vector, int count);
+
/* xcb_in.c */
diff --git a/src/xcbint.h b/src/xcbint.h
index d610a10..ddd7875 100644
--- a/src/xcbint.h
+++ b/src/xcbint.h
@@ -40,7 +40,8 @@
enum workarounds {
WORKAROUND_NONE,
- WORKAROUND_GLX_GET_FB_CONFIGS_BUG
+ WORKAROUND_GLX_GET_FB_CONFIGS_BUG,
+ WORKAROUND_EXTERNAL_SOCKET_OWNER
};
enum lazy_reply_tag
@@ -54,6 +55,10 @@ enum lazy_reply_tag
#define XCB_SEQUENCE_COMPARE(a,op,b) ((int) ((a) - (b)) op 0)
+#define offsetof(type,member) ((size_t) &((type *)0)->member)
+
+#define container_of(pointer,type,member) ((type *)(((char *)(pointer)) - offsetof(type, member)))
+
/* xcb_list.c */
typedef void (*xcb_list_free_func_t)(void *);
@@ -72,6 +77,11 @@ typedef struct _xcb_out {
pthread_cond_t cond;
int writing;
+ pthread_cond_t socket_cond;
+ unsigned int (*return_socket)(void *closure);
+ void *socket_closure;
+ int socket_moving;
+
char queue[4096];
int queue_len;
@@ -121,6 +131,8 @@ int _xcb_in_init(_xcb_in *in);
void _xcb_in_destroy(_xcb_in *in);
int _xcb_in_expect_reply(xcb_connection_t *c, unsigned int request, enum workarounds workaround, int flags);
+int _xcb_in_expect_replies(xcb_connection_t *c, unsigned int request, int flags);
+void _xcb_in_replies_done(xcb_connection_t *c);
int _xcb_in_read(xcb_connection_t *c);
int _xcb_in_read_block(xcb_connection_t *c, void *buf, int nread);
--
1.5.4.1
From 3759ed02f399169c2c8b7f9416bbbcba52fc4ff2 Mon Sep 17 00:00:00 2001
From: Josh Triplett <josh@freedesktop.org>
Date: Sat, 15 Mar 2008 18:17:41 -0700
Subject: [PATCH 5/5] Bump version number to 1.2
---
configure.ac | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/configure.ac b/configure.ac
index 3edaa69..2a85bac 100644
--- a/configure.ac
+++ b/configure.ac
@@ -3,7 +3,7 @@
AC_PREREQ(2.57)
AC_INIT([libxcb],
- 1.1,
+ 1.2,
[xcb@lists.freedesktop.org])
AC_CONFIG_SRCDIR([xcb.pc.in])
AM_INIT_AUTOMAKE([foreign dist-bzip2])
--
1.5.4.1
From 1bd47897b8fc944c4033f527c49d55d87a0280d4 Mon Sep 17 00:00:00 2001
From: Josh Triplett <josh@freedesktop.org>
Date: Sat, 15 Mar 2008 17:22:23 -0700
Subject: [PATCH 1/1] Use XCB's new socket handoff mechanism rather than the old XCB Xlib lock.
Previously, Xlib/XCB used XCB's Xlib lock to prevent XCB from sending
requests between calls to Xlib's LockDisplay and UnlockDisplay macros.
Xlib/XCB then sent all of its requests using XCB's xcb_send_request, and
had to flush its requests when unlocking the display.
XCB 1.2 adds a new socket handoff mechanism, xcb_take_socket. Replace
much of the existing Xlib/XCB implementation with the use of
xcb_take_socket to take ownership of the write side of the X connection
socket, and a return_socket callback which writes any outstanding requests
with xcb_writev. This approach allows Xlib/XCB to use the same buffering
as traditional Xlib did. In particular, programs which use Xlib/XCB and
never make XCB calls will never need to hand the socket back to XCB, and
vice versa.
This allows us to discard large quantities of synchronization code from
Xlib/XCB, together with the synchronization bugs present in that code.
Several test cases which previously failed now work perfectly, including
multi-threaded ico. In addition, the infamous locking correctness
assertions, triggered when double-locking or when unlocking without a
previous lock, no longer exist, because Xlib/XCB no longer has any reason
to care more about application locking than traditional Xlib does.
Furthermore, the handoff approach provides great improvements to
performance. Results from x11perf's XNoOp test, which represented the
worst case for the lock-based Xlib/XCB:
Traditional Xlib: average 19100000/sec
Lock-based Xlib/XCB: average 3350000/sec
Handoff-based Xlib/XCB: average 17400000/sec
Thus, for no-ops, the handoff mechanism provides more than a 4x speedup to
Xlib/XCB, bringing Xlib/XCB within 9% of traditional Xlib no-op
performance. Of course, real-world workloads do not use no-op, so your
mileage may vary. In particular, since no-ops represent the worst case,
we expect real workloads to more closely match the performance of
traditional Xlib.
While removing synchronization code, we changed _XReply to not drop
any locks when calling xcb_wait_for_reply; previously, we had to carefully
avoid a deadlock between the Display lock and the XCB Xlib lock. Holding
the locks reduces implementation complexity and should not impact applications.
Commit by Jamey Sharp and Josh Triplett.
XCB's handoff mechanism inspired by Keith Packard.
---
configure.ac | 4 +-
src/Makefile.am | 1 -
src/OpenDis.c | 27 +++----
src/Xxcbint.h | 21 +-----
src/xcb_io.c | 150 ++++++++++++++++++++---------------
src/xcb_lock.c | 235 -------------------------------------------------------
6 files changed, 100 insertions(+), 338 deletions(-)
delete mode 100644 src/xcb_lock.c
diff --git a/configure.ac b/configure.ac
index 3719f02..8ade3e7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -43,8 +43,8 @@ no)
AC_DEFINE(USE_XCB, 0, [Use XCB for low-level protocol implementation])
;;
*)
- X11_REQUIRES="xcb-xlib >= 0.9.92"
- X11_EXTRA_DEPS="xcb-xlib"
+ X11_REQUIRES="xcb >= 1.2"
+ X11_EXTRA_DEPS="xcb >= 1.2"
xdmauth="no" # XCB handles all auth
AC_DEFINE(USE_XCB, 1, [Use XCB for low-level protocol implementation])
;;
diff --git a/src/Makefile.am b/src/Makefile.am
index 564e03f..3380f81 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -342,7 +342,6 @@ EXTRA_DIST = \
if XCB
libX11_la_SOURCES += \
- xcb_lock.c \
xcb_disp.c \
xcb_io.c \
Xxcbint.h
diff --git a/src/OpenDis.c b/src/OpenDis.c
index 9c327d1..9d61087 100644
--- a/src/OpenDis.c
+++ b/src/OpenDis.c
@@ -260,13 +260,6 @@ XOpenDisplay (
return(NULL);
}
-#if USE_XCB
- if (!_XCBInitDisplayLock(dpy)) {
- OutOfMemory (dpy, setup);
- return(NULL);
- }
-#endif
-
if (!_XPollfdCacheInit(dpy)) {
OutOfMemory (dpy, setup);
return(NULL);
@@ -291,6 +284,10 @@ XOpenDisplay (
return(NULL);
}
dpy->bufmax = dpy->buffer + conn_buf_size;
+#if USE_XCB
+ dpy->xcb->real_bufmax = dpy->bufmax;
+ dpy->bufmax = dpy->buffer;
+#endif
/* Set up the input event queue and input event queue parameters. */
dpy->head = dpy->tail = NULL;
@@ -651,6 +648,12 @@ XOpenDisplay (
UnlockDisplay(dpy);
#endif /* !USE_XCB */
+#if USE_XCB
+ dpy->bigreq_size = xcb_get_maximum_request_length(dpy->xcb->connection);
+ if(dpy->bigreq_size <= dpy->max_request_size)
+ dpy->bigreq_size = 0;
+#endif /* USE_XCB */
+
/*
* Set up other stuff clients are always going to use.
*/
@@ -672,12 +675,6 @@ XOpenDisplay (
*/
(void) XSynchronize(dpy, _Xdebug);
-#if USE_XCB
- dpy->bigreq_size = xcb_get_maximum_request_length(dpy->xcb->connection);
- if(dpy->bigreq_size <= dpy->max_request_size)
- dpy->bigreq_size = 0;
-#endif /* USE_XCB */
-
/*
* get availability of large requests, and
* get the resource manager database off the root window.
@@ -882,10 +879,6 @@ void _XFreeDisplayStructure(dpy)
Xfree (dpy->scratch_buffer);
FreeDisplayLock(dpy);
-#if USE_XCB
- _XCBShutdownDisplayLock(dpy);
-#endif /* USE_XCB */
-
if (dpy->qfree) {
register _XQEvent *qelt = dpy->qfree;
diff --git a/src/Xxcbint.h b/src/Xxcbint.h
index cc73749..e6674f9 100644
--- a/src/Xxcbint.h
+++ b/src/Xxcbint.h
@@ -7,7 +7,6 @@
#include <assert.h>
#include <X11/Xlibint.h>
#include <X11/Xlib-xcb.h>
-#include "locking.h" /* really just want X11/Xthreads.h but can't have it. */
#define XCB_SEQUENCE_COMPARE(a,op,b) ((int) ((a) - (b)) op 0)
#define assert_sequence_less(a,b) assert(XCB_SEQUENCE_COMPARE((a), <=, (b)))
@@ -15,24 +14,19 @@
typedef struct PendingRequest PendingRequest;
struct PendingRequest {
PendingRequest *next;
- xcondition_rec condition;
- int waiters; /* Number of threads waiting; -1 if no wait needed */
unsigned int sequence;
};
typedef struct _X11XCBPrivate {
- struct _XLockPtrs lock_fns;
xcb_connection_t *connection;
PendingRequest *pending_requests;
PendingRequest **pending_requests_tail;
xcb_generic_event_t *next_event;
- const char *request_extra;
- int request_extra_size;
- char *partial_request;
- int partial_request_offset;
+ char *real_bufmax;
char *reply_data;
int reply_length;
int reply_consumed;
+ unsigned int last_flushed;
enum XEventQueueOwner event_owner;
XID next_xid;
} _X11XCBPrivate;
@@ -42,15 +36,4 @@ typedef struct _X11XCBPrivate {
int _XConnectXCB(Display *dpy, _Xconst char *display, char **fullnamep, int *screenp);
void _XFreeX11XCBStructure(Display *dpy);
-/* xcb_lock.c */
-
-int _XCBInitDisplayLock(Display *dpy);
-void _XCBShutdownDisplayLock(Display *dpy);
-
-/* _XGetXCBBuffer and _XPutXCBBuffer calls must be paired and must not
- * be nested. */
-
-void _XGetXCBBuffer(Display *dpy);
-void _XPutXCBBuffer(Display *dpy);
-
#endif /* XXCBINT_H */
diff --git a/src/xcb_io.c b/src/xcb_io.c
index 5f07184..030b69b 100644
--- a/src/xcb_io.c
+++ b/src/xcb_io.c
@@ -5,7 +5,6 @@
#include "locking.h"
#include "Xxcbint.h"
#include <xcb/xcbext.h>
-#include <xcb/xcbxlib.h>
#include <assert.h>
#include <stdlib.h>
@@ -68,15 +67,6 @@ static void check_internal_connections(Display *dpy)
}
}
-static void condition_wait(Display *dpy, xcondition_t cv)
-{
- _XPutXCBBuffer(dpy);
- xcb_xlib_unlock(dpy->xcb->connection);
- ConditionWait(dpy, cv);
- xcb_xlib_lock(dpy->xcb->connection);
- _XGetXCBBuffer(dpy);
-}
-
static void call_handlers(Display *dpy, xcb_generic_reply_t *buf)
{
_XAsyncHandler *async, *next;
@@ -139,18 +129,9 @@ static void process_responses(Display *dpy, int wait_for_first_event, xcb_generi
free(event);
event = wait_or_poll_for_event(dpy, wait_for_first_event);
}
- else if(req && req->waiters != -1)
+ else if(req && req->sequence == current_request)
{
- if(req->sequence == current_request)
- break;
- if(!current_request && !wait_for_first_event)
- break;
- dpy->xcb->next_event = event;
- req->waiters++;
- assert(req->waiters > 0);
- condition_wait(dpy, &req->condition);
- --req->waiters;
- event = dpy->xcb->next_event;
+ break;
}
else if(req && xcb_poll_for_reply(dpy->xcb->connection, req->sequence, &reply, &error))
{
@@ -221,32 +202,71 @@ void _XReadEvents(Display *dpy)
*/
void _XSend(Display *dpy, const char *data, long size)
{
+ static const xReq dummy_request;
+ static char const pad[3];
+ struct iovec vec[3];
+ _XExtension *ext;
xcb_connection_t *c = dpy->xcb->connection;
if(dpy->flags & XlibDisplayIOError)
return;
- assert(!dpy->xcb->request_extra);
- dpy->xcb->request_extra = data;
- dpy->xcb->request_extra_size = size;
+ if(dpy->bufptr == dpy->buffer && !size)
+ return;
- /* give dpy->buffer to XCB */
- _XPutXCBBuffer(dpy);
+ /* iff we asked XCB to set aside errors, we must pick those up
+ * eventually. iff there are async handlers, we may have just
+ * issued requests that will generate replies. in either case,
+ * we need to remember to check later. */
+ if(dpy->xcb->event_owner != XlibOwnsEventQueue || dpy->async_handlers)
+ {
+ unsigned int sequence;
+ for(sequence = dpy->xcb->last_flushed; sequence < dpy->request; ++sequence)
+ {
+ PendingRequest *req = malloc(sizeof(PendingRequest));
+ assert(req);
+ req->next = 0;
+ req->sequence = sequence;
+ *dpy->xcb->pending_requests_tail = req;
+ dpy->xcb->pending_requests_tail = &req->next;
+ }
+ }
+ dpy->xcb->last_flushed = dpy->request;
- if(xcb_flush(c) <= 0)
- _XIOError(dpy);
+ vec[0].iov_base = dpy->buffer;
+ vec[0].iov_len = dpy->bufptr - dpy->buffer;
+ vec[1].iov_base = (caddr_t) data;
+ vec[1].iov_len = size;
+ vec[2].iov_base = (caddr_t) pad;
+ vec[2].iov_len = -size & 3;
+
+ for(ext = dpy->flushes; ext; ext = ext->next_flush)
+ {
+ int i;
+ for(i = 0; i < 3; ++i)
+ if(vec[i].iov_len)
+ ext->before_flush(dpy, &ext->codes, vec[i].iov_base, vec[i].iov_len);
+ }
- /* get a new dpy->buffer */
- _XGetXCBBuffer(dpy);
+ if(xcb_writev(c, vec, 3) < 0)
+ _XIOError(dpy);
+ dpy->bufptr = dpy->buffer;
+ dpy->last_req = (char *) &dummy_request;
check_internal_connections(dpy);
- /* A straight port of XlibInt.c would call _XSetSeqSyncFunction
- * here. However that does no good: unlike traditional Xlib,
- * Xlib/XCB almost never calls _XFlush because _XPutXCBBuffer
- * automatically pushes requests down into XCB, so Xlib's buffer
- * is empty most of the time. Since setting a synchandler has no
- * effect until after UnlockDisplay returns, we may as well do
- * the check in _XUnlockDisplay. */
+ _XSetSeqSyncFunction(dpy);
+}
+
+static unsigned int return_socket(void *closure)
+{
+ unsigned int request;
+ Display *dpy = closure;
+ LockDisplay(dpy);
+ _XSend(dpy, 0, 0);
+ dpy->bufmax = dpy->buffer;
+ request = dpy->request;
+ UnlockDisplay(dpy);
+ return request;
}
/*
@@ -255,6 +275,21 @@ void _XSend(Display *dpy, const char *data, long size)
*/
void _XFlush(Display *dpy)
{
+ if(dpy->bufmax == dpy->buffer)
+ {
+ unsigned int sent, read;
+ int flags = 0;
+ /* if we don't own the event queue, we have to ask XCB
+ * to set our errors aside for us. */
+ if(dpy->xcb->event_owner != XlibOwnsEventQueue)
+ flags = XCB_REQUEST_CHECKED;
+ if(!xcb_take_socket(dpy->xcb->connection, return_socket, dpy,
+ flags, &sent, &read))
+ _XIOError(dpy);
+ dpy->xcb->last_flushed = dpy->request = sent;
+ dpy->last_request_read = read;
+ dpy->bufmax = dpy->xcb->real_bufmax;
+ }
_XSend(dpy, 0, 0);
_XEventsQueued(dpy, QueuedAfterReading);
@@ -295,10 +330,18 @@ XID _XAllocID(Display *dpy)
void _XAllocIDs(Display *dpy, XID *ids, int count)
{
int i;
- _XPutXCBBuffer(dpy);
+#ifdef XTHREADS
+ if (dpy->lock)
+ (*dpy->lock->user_lock_display)(dpy);
+ UnlockDisplay(dpy);
+#endif
for (i = 0; i < count; i++)
ids[i] = xcb_generate_id(dpy->xcb->connection);
- _XGetXCBBuffer(dpy);
+#ifdef XTHREADS
+ LockDisplay(dpy);
+ if (dpy->lock)
+ (*dpy->lock->user_unlock_display)(dpy);
+#endif
}
static void _XFreeReplyData(Display *dpy, Bool force)
@@ -314,14 +357,7 @@ static PendingRequest * insert_pending_request(Display *dpy)
PendingRequest **cur = &dpy->xcb->pending_requests;
while(*cur && XCB_SEQUENCE_COMPARE((*cur)->sequence, <, dpy->request))
cur = &((*cur)->next);
- if(*cur && (*cur)->sequence == dpy->request)
- {
- /* Replacing an existing PendingRequest should only happen once,
- when calling _XReply, and the replaced PendingRequest must
- not have a condition set. */
- assert((*cur)->waiters == -1);
- }
- else
+ if(!*cur || (*cur)->sequence != dpy->request)
{
PendingRequest *node = malloc(sizeof(PendingRequest));
assert(node);
@@ -331,8 +367,6 @@ static PendingRequest * insert_pending_request(Display *dpy)
dpy->xcb->pending_requests_tail = &(node->next);
*cur = node;
}
- (*cur)->waiters = 0;
- xcondition_init(&((*cur)->condition));
return *cur;
}
@@ -354,27 +388,15 @@ Status _XReply(Display *dpy, xReply *rep, int extra, Bool discard)
if(dpy->flags & XlibDisplayIOError)
return 0;
- /* Internals of UnlockDisplay done by hand here, so that we can
- insert_pending_request *after* we _XPutXCBBuffer, but before we
- unlock the display. */
- _XPutXCBBuffer(dpy);
+ _XSend(dpy, 0, 0);
current = insert_pending_request(dpy);
- if(!dpy->lock || dpy->lock->locking_level == 0)
- xcb_xlib_unlock(dpy->xcb->connection);
- if(dpy->xcb->lock_fns.unlock_display)
- dpy->xcb->lock_fns.unlock_display(dpy);
+ /* FIXME: drop the Display lock while waiting?
+ * Complicates process_responses. */
reply = xcb_wait_for_reply(c, current->sequence, &error);
- LockDisplay(dpy);
check_internal_connections(dpy);
process_responses(dpy, 0, &error, current->sequence);
- if(current->waiters)
- { /* The ConditionBroadcast macro contains an if; braces needed here. */
- ConditionBroadcast(dpy, ¤t->condition);
- }
- --current->waiters;
-
if(error)
{
_XExtension *ext;
diff --git a/src/xcb_lock.c b/src/xcb_lock.c
deleted file mode 100644
index 71b2383..0000000
--- a/src/xcb_lock.c
+++ /dev/null
@@ -1,235 +0,0 @@
-/* Copyright (C) 2003-2006 Jamey Sharp, Josh Triplett
- * This file is licensed under the MIT license. See the file COPYING. */
-
-#ifdef HAVE_CONFIG_H
-#include <config.h>
-#endif
-
-#include "Xlibint.h"
-#include "locking.h"
-#include "Xxcbint.h"
-#include <xcb/xcbext.h>
-#include <xcb/xcbxlib.h>
-
-#include <pthread.h>
-
-static void _XCBLockDisplay(Display *dpy)
-{
- if(dpy->xcb->lock_fns.lock_display)
- dpy->xcb->lock_fns.lock_display(dpy);
- if(!dpy->lock || dpy->lock->locking_level == 0)
- xcb_xlib_lock(dpy->xcb->connection);
- if(!(dpy->flags & XlibDisplayIOError))
- _XGetXCBBuffer(dpy);
-}
-
-/* XXX: If you change this function, update _XReply's copy of its guts! */
-static void _XCBUnlockDisplay(Display *dpy)
-{
- if(!(dpy->flags & XlibDisplayIOError))
- {
- _XPutXCBBuffer(dpy);
- assert(dpy->xcb->partial_request == 0);
- assert(xcb_get_request_sent(dpy->xcb->connection) == dpy->request);
-
- /* Traditional Xlib does this in _XSend; see the Xlib/XCB version
- * of that function for why we do it here instead. */
- _XSetSeqSyncFunction(dpy);
- }
-
- if(!dpy->lock || dpy->lock->locking_level == 0)
- xcb_xlib_unlock(dpy->xcb->connection);
- if(dpy->xcb->lock_fns.unlock_display)
- dpy->xcb->lock_fns.unlock_display(dpy);
-}
-
-int _XCBInitDisplayLock(Display *dpy)
-{
- if(!dpy->lock_fns && !(dpy->lock_fns = Xcalloc(1, sizeof(*dpy->lock_fns))))
- return 0;
- dpy->xcb->lock_fns.lock_display = dpy->lock_fns->lock_display;
- dpy->lock_fns->lock_display = _XCBLockDisplay;
- dpy->xcb->lock_fns.unlock_display = dpy->lock_fns->unlock_display;
- dpy->lock_fns->unlock_display = _XCBUnlockDisplay;
- return 1;
-}
-
-void _XCBShutdownDisplayLock(Display *dpy)
-{
- if(dpy->lock_fns) {
- Xfree((char *)dpy->lock_fns);
- dpy->lock_fns = NULL;
- }
-}
-
-void _XGetXCBBuffer(Display *dpy)
-{
- static const xReq dummy_request;
- unsigned int xcb_req = xcb_get_request_sent(dpy->xcb->connection);
- if(xcb_connection_has_error(dpy->xcb->connection))
- _XIOError(dpy);
-
- /* if Xlib has a partial request pending then XCB doesn't know about
- * the current request yet */
- if(dpy->xcb->partial_request)
- ++xcb_req;
-
- assert(XCB_SEQUENCE_COMPARE(xcb_req, >=, dpy->request));
- dpy->request = xcb_req;
-
- dpy->last_req = (char *) &dummy_request;
-}
-
-static size_t request_length(struct iovec *vec)
-{
- /* we have at least part of a request. dig out the length field.
- * note that length fields are always in vec[0]: Xlib doesn't split
- * fixed-length request parts. */
- size_t len;
- assert(vec[0].iov_len >= 4);
- len = ((uint16_t *) vec[0].iov_base)[1];
- if(len == 0)
- {
- /* it's a bigrequest. dig out the *real* length field. */
- assert(vec[0].iov_len >= 8);
- len = ((uint32_t *) vec[0].iov_base)[1];
- }
- return len << 2;
-}
-
-static inline int issue_complete_request(Display *dpy, int veclen, struct iovec *vec)
-{
- xcb_protocol_request_t xcb_req = { 0 };
- unsigned int sequence;
- int flags = XCB_REQUEST_RAW;
- int i;
- size_t len;
-
- /* skip empty iovecs. if no iovecs remain, we're done. */
- assert(veclen >= 0);
- while(veclen > 0 && vec[0].iov_len == 0)
- --veclen, ++vec;
- if(!veclen)
- return 0;
-
- len = request_length(vec);
-
- /* do we have enough data for a complete request? how many iovec
- * elements does it span? */
- for(i = 0; i < veclen; ++i)
- {
- size_t oldlen = len;
- len -= vec[i].iov_len;
- /* if len is now 0 or has wrapped, we have enough data. */
- if((len - 1) > oldlen)
- break;
- }
- if(i == veclen)
- return 0;
-
- /* we have enough data to issue one complete request. the remaining
- * code can't fail. */
-
- /* len says how far we overshot our data needs. (it's "negative" if
- * we actually overshot, or 0 if we're right on.) */
- vec[i].iov_len += len;
- xcb_req.count = i + 1;
- xcb_req.opcode = ((uint8_t *) vec[0].iov_base)[0];
-
- /* if we don't own the event queue, we have to ask XCB to set our
- * errors aside for us. */
- if(dpy->xcb->event_owner != XlibOwnsEventQueue)
- flags |= XCB_REQUEST_CHECKED;
-
- /* XCB will always skip request 0; account for that in the Xlib count */
- if (xcb_get_request_sent(dpy->xcb->connection) == 0xffffffff)
- dpy->request++;
- /* send the accumulated request. */
- sequence = xcb_send_request(dpy->xcb->connection, flags, vec, &xcb_req);
- if(!sequence)
- _XIOError(dpy);
-
- /* update the iovecs to refer only to data not yet sent. */
- vec[i].iov_len = -len;
-
- /* iff we asked XCB to set aside errors, we must pick those up
- * eventually. iff there are async handlers, we may have just
- * issued requests that will generate replies. in either case,
- * we need to remember to check later. */
- if(flags & XCB_REQUEST_CHECKED || dpy->async_handlers)
- {
- PendingRequest *req = malloc(sizeof(PendingRequest));
- assert(req);
- req->next = 0;
- req->waiters = -1;
- req->sequence = sequence;
- *dpy->xcb->pending_requests_tail = req;
- dpy->xcb->pending_requests_tail = &req->next;
- }
- return 1;
-}
-
-void _XPutXCBBuffer(Display *dpy)
-{
- static char const pad[3];
- const int padsize = -dpy->xcb->request_extra_size & 3;
- xcb_connection_t *c = dpy->xcb->connection;
- _XExtension *ext;
- struct iovec iov[6];
-
- assert_sequence_less(dpy->last_request_read, dpy->request);
- assert_sequence_less(xcb_get_request_sent(c), dpy->request);
-
- for(ext = dpy->flushes; ext; ext = ext->next_flush)
- {
- ext->before_flush(dpy, &ext->codes, dpy->buffer, dpy->bufptr - dpy->buffer);
- if(dpy->xcb->request_extra)
- {
- ext->before_flush(dpy, &ext->codes, dpy->xcb->request_extra, dpy->xcb->request_extra_size);
- if(padsize)
- ext->before_flush(dpy, &ext->codes, pad, padsize);
- }
- }
-
- iov[2].iov_base = dpy->xcb->partial_request;
- iov[2].iov_len = dpy->xcb->partial_request_offset;
- iov[3].iov_base = dpy->buffer;
- iov[3].iov_len = dpy->bufptr - dpy->buffer;
- iov[4].iov_base = (caddr_t) dpy->xcb->request_extra;
- iov[4].iov_len = dpy->xcb->request_extra_size;
- iov[5].iov_base = (caddr_t) pad;
- iov[5].iov_len = padsize;
-
- while(issue_complete_request(dpy, 4, iov + 2))
- /* empty */;
-
- /* first discard any completed partial_request. */
- if(iov[2].iov_len == 0 && dpy->xcb->partial_request)
- {
- free(dpy->xcb->partial_request);
- dpy->xcb->partial_request = 0;
- dpy->xcb->partial_request_offset = 0;
- }
-
- /* is there anything to copy into partial_request? */
- if(iov[3].iov_len != 0 || iov[4].iov_len != 0 || iov[5].iov_len != 0)
- {
- int i;
- if(!dpy->xcb->partial_request)
- {
- size_t len = request_length(iov + 3);
- assert(!dpy->xcb->partial_request_offset);
- dpy->xcb->partial_request = malloc(len);
- assert(dpy->xcb->partial_request);
- }
- for(i = 3; i < sizeof(iov) / sizeof(*iov); ++i)
- {
- memcpy(dpy->xcb->partial_request + dpy->xcb->partial_request_offset, iov[i].iov_base, iov[i].iov_len);
- dpy->xcb->partial_request_offset += iov[i].iov_len;
- }
- }
-
- dpy->xcb->request_extra = 0;
- dpy->xcb->request_extra_size = 0;
- dpy->bufptr = dpy->buffer;
-}
--
1.5.4.1
Attachment:
signature.asc
Description: Digital signature