[Xcb] [PATCH] allow runtime change of XCB buffer size
Barton C Massey
bart at cs.pdx.edu
Fri Sep 26 13:18:11 PDT 2008
Thanks for taking the initiative in proposing a patch for
this!
However, I'm reluctant to accept the environment variable
and runtime part of the patch until someone can demonstrate
to me that (a) they understand XCB's use of the buffer well
enough to make a plausible argument that we won't break
things in subtle ways (Jamey and Josh automatically qualify
if they take the time to respond :-) and (b) they have a
good validation plan for checking that things still work.
Tests would be good; proofs would be even better, but I
don't expect anyone to go there. Concerns I have are that
apps (specifically Xlib) and XCB disagreeing about the
buffer size might cause subtle bugs, and that changing the
buffer size while XCB is running might cause subtle bugs.
Failing good QA, I think just bumping the default buffer
size to something like 64KB and calling it a day is a better
plan. This should be sufficient to solve our immediate
problem, without creating new things to tweak and manage.
Bart
In message <1222437909-30753-2-git-send-email-julien at danjou.info> you wrote:
> Signed-off-by: Julien Danjou <julien at danjou.info>
> ---
> src/xcb_conn.c | 16 ++++++++++++++--
> src/xcb_in.c | 10 ++++++++--
> src/xcb_out.c | 9 +++++++--
> src/xcbint.h | 10 ++++++----
> 4 files changed, 35 insertions(+), 10 deletions(-)
>
> diff --git a/src/xcb_conn.c b/src/xcb_conn.c
> index 02f60bd..f75f55e 100644
> --- a/src/xcb_conn.c
> +++ b/src/xcb_conn.c
> @@ -220,6 +220,8 @@ int xcb_connection_has_error(xcb_connection_t *c)
> xcb_connection_t *xcb_connect_to_fd(int fd, xcb_auth_info_t *auth_info)
> {
> xcb_connection_t* c;
> + char *xcb_buffer_size;
> + int queue_size;
>
> c = calloc(1, sizeof(xcb_connection_t));
> if(!c)
> @@ -227,12 +229,22 @@ xcb_connection_t *xcb_connect_to_fd(int fd, xcb_auth_info_t *auth_info)
>
> c->fd = fd;
>
> + xcb_buffer_size = getenv("LIBXCB_QUEUE_BUFFER_SIZE");
> + if(xcb_buffer_size)
> + {
> + queue_size = 1024 * strtol(xcb_buffer_size, NULL, 10);
> + if(queue_size < 1024)
> + queue_size = 1024;
> + }
> + else
> + queue_size = XCB_QUEUE_BUFFER_SIZE;
> +
> 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) &&
> + _xcb_in_init(&c->in, queue_size) &&
> + _xcb_out_init(&c->out, queue_size) &&
> write_setup(c, auth_info) &&
> read_setup(c) &&
> _xcb_ext_init(c) &&
> diff --git a/src/xcb_in.c b/src/xcb_in.c
> index f613772..5cbef5b 100644
> --- a/src/xcb_in.c
> +++ b/src/xcb_in.c
> @@ -455,12 +455,18 @@ xcb_generic_error_t *xcb_request_check(xcb_connection_t *c, xcb_void_cookie_t co
>
> /* Private interface */
>
> -int _xcb_in_init(_xcb_in *in)
> +int _xcb_in_init(_xcb_in *in, int queue_size)
> {
> if(pthread_cond_init(&in->event_cond, 0))
> return 0;
> in->reading = 0;
>
> + in->queue = calloc(1, queue_size);
> + if(!in->queue)
> + return 0;
> +
> + in->queue_size = queue_size;
> +
> in->queue_len = 0;
>
> in->request_read = 0;
> @@ -517,7 +523,7 @@ int _xcb_in_expect_reply(xcb_connection_t *c, unsigned int request, enum workaro
>
> 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);
> + int n = read(c->fd, c->in.queue + c->in.queue_len, c->in.queue_size - c->in.queue_len);
> if(n > 0)
> c->in.queue_len += n;
> while(read_packet(c))
> diff --git a/src/xcb_out.c b/src/xcb_out.c
> index 000b121..55af007 100644
> --- a/src/xcb_out.c
> +++ b/src/xcb_out.c
> @@ -37,7 +37,7 @@
>
> static int write_block(xcb_connection_t *c, struct iovec *vector, int count)
> {
> - while(count && c->out.queue_len + vector[0].iov_len <= sizeof(c->out.queue))
> + while(count && c->out.queue_len + vector[0].iov_len <= c->out.queue_size)
> {
> memcpy(c->out.queue + c->out.queue_len, vector[0].iov_base, vector[0].iov_len);
> c->out.queue_len += vector[0].iov_len;
> @@ -248,12 +248,17 @@ int xcb_flush(xcb_connection_t *c)
>
> /* Private interface */
>
> -int _xcb_out_init(_xcb_out *out)
> +int _xcb_out_init(_xcb_out *out, int queue_size)
> {
> if(pthread_cond_init(&out->cond, 0))
> return 0;
> out->writing = 0;
>
> + out->queue = calloc(1, queue_size);
> + if(!out->queue)
> + return 0;
> +
> + out->queue_size = queue_size;
> out->queue_len = 0;
>
> out->request = 0;
> diff --git a/src/xcbint.h b/src/xcbint.h
> index 2bc6d33..2089fa2 100644
> --- a/src/xcbint.h
> +++ b/src/xcbint.h
> @@ -72,7 +72,8 @@ typedef struct _xcb_out {
> pthread_cond_t cond;
> int writing;
>
> - char queue[XCB_QUEUE_BUFFER_SIZE];
> + char *queue;
> + int queue_size;
> int queue_len;
>
> unsigned int request;
> @@ -86,7 +87,7 @@ typedef struct _xcb_out {
> } maximum_request_length;
> } _xcb_out;
>
> -int _xcb_out_init(_xcb_out *out);
> +int _xcb_out_init(_xcb_out *out, int queue_size);
> void _xcb_out_destroy(_xcb_out *out);
>
> int _xcb_out_send(xcb_connection_t *c, struct iovec **vector, int *count);
> @@ -99,7 +100,8 @@ typedef struct _xcb_in {
> pthread_cond_t event_cond;
> int reading;
>
> - char queue[XCB_QUEUE_BUFFER_SIZE];
> + char *queue;
> + int queue_size;
> int queue_len;
>
> unsigned int request_expected;
> @@ -117,7 +119,7 @@ typedef struct _xcb_in {
> struct pending_reply **pending_replies_tail;
> } _xcb_in;
>
> -int _xcb_in_init(_xcb_in *in);
> +int _xcb_in_init(_xcb_in *in, int queue_size);
> void _xcb_in_destroy(_xcb_in *in);
>
> int _xcb_in_expect_reply(xcb_connection_t *c, unsigned int request, enum workarounds workaround, int flags);
> --
> 1.6.0.1
>
> _______________________________________________
> Xcb mailing list
> Xcb at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/xcb
More information about the Xcb
mailing list