[Xcb] Doubled API size (was: xcb-util: ewmh library)

[Jamey Sharp]

On Wed, Dec 2, 2009 at 4:34 PM, Peter Harris <pharris at opentext.com> wrote:
> Jamey Sharp wrote:
>> But it seems that those cases are rare, so doubling the size of the
>> API and the implementation was probably a poor choice. What could we
>> have done instead?
>
> I assume that this is a rhetorical question, but it has been tickling my
> brain.
> ...
> This is academic, of course, as we don't want to break the API now.

Semi-rhetorical. I'm definitely interested in answers. :-) We might do
an XCB 2.0 someday, for example; or find a place to split XCB into
low-level and high-level APIs, like with socket-handoff, and provide
multiple competing high-level APIs that can share the socket.

Observation: I think it's pretty rare that a caller cares about
exactly which error occurred in response to a specific request. So it
seems to me that the normal use of the checked API we've provided
would actually mean only "keep any error that occurs here out of the
event queue"; I suspect the contents of the error are usually ignored,
or at best printed as debugging messages.

> We could have avoided doubling the size of the generated API by adding
> two new core functions:
>  - xcb_check_cookie(connection, cookie) - only valid if called
> immediately after a default-unchecked function, before calling any other
> functions. With this restriction, we'd know that there is no possible
> way for the error to have already been read.
>  - xcb_uncheck_cookie(connection, cookie) - moves any associated error
> (or future error) back to the error queue. Works on both default-checked
> cookies and xcb_check()'d cookies.

I'm not yet convinced this is feasible as described, because I don't
see how these can be thread-safe. Ideas?

The thread-safety issue with xcb_check_cookie should be obvious. For
xcb_uncheck_cookie, my main complaint is that errors might appear in
the event queue out-of-order, which is obnoxious at the least and a
correctness problem if sharing the connection with Xlib. As a
secondary matter, callers need to be careful to call either
xcb_uncheck_cookie or xcb_wait_for_reply but not both, or risk
non-deterministic behavior.

However, I've been toying with the idea of constructing a
non-thread-safe request queue (call it a request context?) from an
xcb_connection_t. Sequence numbers wouldn't be assigned until you
flush the local request queue to the socket, and replies would be
fetched from the request context. That would make your proposed
functions safe, as long as they aren't called after the requests are
flushed, but I'm not fond of multi-part construction APIs when I can
avoid them.

So how about this: each of these request contexts could have an
optional error queue attached to it. In that case, all errors for
requests issued in that context would go to the local error queue
instead of the global event queue. "Checked" becomes a property of the
request context, not the individual request. This is inspired by
having seen at XDC the way that Chris Wilson wanted to use XCB in
cairo, which just needs to invalidate the surface if any X error
occurred on any request it issued. Further simplifying things, I
suppose you could ask the request context to discard errors and just
record a flag: "Has an X error occurred in this context?"

On those occasions where an application needs to issue a request and
then find out exactly which error occurred in response, I guess you
could construct a request context to use for just that request, then
discard it after getting the reply or error. (Just remember to flush
your contexts in the right order, if that matters to you.)

All of the above could be implemented in a layer on top of XCB today,
using socket-handoff. If anybody else thinks this is a good idea I'd
love to see a prototype library that does it using only libxcb's
current API, and see some code using it successfully. Then we could
consider changes in libxcb to make this style more efficient.

(The original reason I'd been thinking along these lines was to reduce
the overhead of locking in common cases. Synchronously assigning a
sequence number and queueing a request is a bit heavy-weight in
high-throughput applications, although I think those aren't common in
X outside of benchmarks.)

> Given those two functions, this becomes an attractive function:
>  - xcb_discard_cookie(connection, cookie) - performs an
> xcb_uncheck_cookie(), and discards any reply (or future reply).
>
> In particular, xcb_discard_cookie would be useful for garbage collected
> languages that want to bind to C libxcb instead of implementing their
> own XCB core (for interoperability reasons). When toying with a Lua
> wrapper, I kept having to *_reply(cookie) in the cookie garbage
> collector in order to avoid leaks. It would be more efficient to not
> wait for the network.

You can already avoid waiting for the network, if you do some
book-keeping: Whenever the application waits for a reply or receives
an event, force xcb_wait_for_reply on all earlier cookies. You know
what order you submitted them in, so that's not too hard. If you keep
your list of unforced cookies where the garbage collector can see it,
then you don't have to worry about anything being freed too soon.

Carsten, I bet you can make that work for your auto_ptr too.

> I'm not sure how many applications issue speculative requests in order
> to avoid future latency, but I could have used discard_cookie when
> converting xlsclients (see the "drain stale replies" loop).

Presumably there aren't any doing that right now, since Xlib makes it
hard. :-) I hope more apps and libraries will start issuing
speculative requests though.

Jamey