From c033970163bada868361a3cd58cc24ae5d4785a8 Mon Sep 17 00:00:00 2001 From: Uli Schlachter Date: Sun, 7 Mar 2021 09:24:40 +0100 Subject: [PATCH] Save another GetAtomName round trip Both get_atom_name() and the new atom interner required a round trip. Move get_atom_name() into the atom interner to save one more round trip. This brings xkb_x11_keymap_new_from_device() down to two round trips, which is the minimum possible number. (Also, I think the new code in keymap.c is more readable than the mess I previously created) With this last commit in the series, this definitely: Fixes: https://github.com/xkbcommon/libxkbcommon/pull/217 Signed-off-by: Uli Schlachter --- src/x11/keymap.c | 33 +++++------------- src/x11/util.c | 85 +++++++++++++++++++++++----------------------- src/x11/x11-priv.h | 24 +++++++------ 3 files changed, 66 insertions(+), 76 deletions(-) diff --git a/src/x11/keymap.c b/src/x11/keymap.c index 6164d83..1e7fc3b 100644 --- a/src/x11/keymap.c +++ b/src/x11/keymap.c @@ -1079,25 +1079,15 @@ get_names(struct xkb_keymap *keymap, struct x11_atom_interner *interner, reply->which, &list); - xcb_get_atom_name_cookie_t cookies[4]; - get_atom_name(conn, list.keycodesName, &cookies[0]); - get_atom_name(conn, list.symbolsName, &cookies[1]); - get_atom_name(conn, list.typesName, &cookies[2]); - get_atom_name(conn, list.compatName, &cookies[3]); - - /* We need to ensure all replies are collected and thus no short-circuit */ - bool atom_success = true; - atom_success &= get_atom_name_reply(conn, list.keycodesName, cookies[0], - &keymap->keycodes_section_name); - atom_success &= get_atom_name_reply(conn, list.symbolsName, cookies[1], - &keymap->symbols_section_name); - atom_success &= get_atom_name_reply(conn, list.typesName, cookies[2], - &keymap->types_section_name); - atom_success &= get_atom_name_reply(conn, list.compatName, cookies[3], - &keymap->compat_section_name); - - if (!atom_success || - !get_type_names(keymap, interner, reply, &list) || + x11_atom_interner_get_escaped_atom_name(interner, list.keycodesName, + &keymap->keycodes_section_name); + x11_atom_interner_get_escaped_atom_name(interner, list.symbolsName, + &keymap->symbols_section_name); + x11_atom_interner_get_escaped_atom_name(interner, list.typesName, + &keymap->types_section_name); + x11_atom_interner_get_escaped_atom_name(interner, list.compatName, + &keymap->compat_section_name); + if (!get_type_names(keymap, interner, reply, &list) || !get_indicator_names(keymap, interner, reply, &list) || !get_vmod_names(keymap, interner, reply, &list) || !get_group_names(keymap, interner, reply, &list) || @@ -1105,11 +1095,6 @@ get_names(struct xkb_keymap *keymap, struct x11_atom_interner *interner, !get_aliases(keymap, conn, reply, &list)) goto fail; - XkbEscapeMapName(keymap->keycodes_section_name); - XkbEscapeMapName(keymap->symbols_section_name); - XkbEscapeMapName(keymap->types_section_name); - XkbEscapeMapName(keymap->compat_section_name); - free(reply); return true; diff --git a/src/x11/util.c b/src/x11/util.c index 6618dfe..ac8f061 100644 --- a/src/x11/util.c +++ b/src/x11/util.c @@ -124,48 +124,6 @@ xkb_x11_get_core_keyboard_device_id(xcb_connection_t *conn) return device_id; } -void -get_atom_name(xcb_connection_t *conn, xcb_atom_t atom, - xcb_get_atom_name_cookie_t *cookie) -{ - if (atom == 0) { - cookie->sequence = 0; - } else { - *cookie = xcb_get_atom_name(conn, atom); - } -} - -bool -get_atom_name_reply(xcb_connection_t *conn, xcb_atom_t atom, - xcb_get_atom_name_cookie_t cookie, char **out) -{ - xcb_get_atom_name_reply_t *reply; - int length; - char *name; - - if (atom == 0) { - *out = NULL; - assert(cookie.sequence == 0); - return true; - } - - reply = xcb_get_atom_name_reply(conn, cookie, NULL); - if (!reply) - return false; - - length = xcb_get_atom_name_name_length(reply); - name = xcb_get_atom_name_name(reply); - - *out = strndup(name, length); - if (!*out) { - free(reply); - return false; - } - - free(reply); - return true; -} - struct x11_atom_cache { /* * Invalidate the cache based on the XCB connection. @@ -204,6 +162,7 @@ x11_atom_interner_init(struct x11_atom_interner *interner, interner->conn = conn; interner->num_pending = 0; interner->num_copies = 0; + interner->num_escaped = 0; } void @@ -298,6 +257,48 @@ void x11_atom_interner_round_trip(struct x11_atom_interner *interner) { } } + for (size_t i = 0; i < interner->num_escaped; i++) { + xcb_get_atom_name_reply_t *reply; + int length; + char *name; + char **out = interner->escaped[i].out; + + reply = xcb_get_atom_name_reply(conn, interner->escaped[i].cookie, NULL); + *interner->escaped[i].out = NULL; + if (!reply) { + interner->had_error = true; + } else { + length = xcb_get_atom_name_name_length(reply); + name = xcb_get_atom_name_name(reply); + + *out = strndup(name, length); + free(reply); + if (*out == NULL) { + interner->had_error = true; + } else { + XkbEscapeMapName(*out); + } + } + } + interner->num_pending = 0; interner->num_copies = 0; + interner->num_escaped = 0; +} + +void +x11_atom_interner_get_escaped_atom_name(struct x11_atom_interner *interner, + xcb_atom_t atom, char **out) +{ + if (atom == 0) { + *out = NULL; + return; + } + size_t idx = interner->num_escaped++; + /* There can only be a fixed number of calls to this function "in-flight", + * thus we assert this number. Increase the array size if this assert fails. + */ + assert(idx < ARRAY_SIZE(interner->escaped)); + interner->escaped[idx].out = out; + interner->escaped[idx].cookie = xcb_get_atom_name(interner->conn, atom); } diff --git a/src/x11/x11-priv.h b/src/x11/x11-priv.h index 9a6e8e0..480590d 100644 --- a/src/x11/x11-priv.h +++ b/src/x11/x11-priv.h @@ -29,16 +29,6 @@ #include "keymap.h" #include "xkbcommon/xkbcommon-x11.h" -/* Preparation for get_atom_name_reply() */ -void -get_atom_name(xcb_connection_t *conn, xcb_atom_t atom, - xcb_get_atom_name_cookie_t *cookie); - -/* Get a strdup'd name of an X atom. */ -bool -get_atom_name_reply(xcb_connection_t *conn, xcb_atom_t atom, - xcb_get_atom_name_cookie_t cookie, char **out); - struct x11_atom_interner { struct xkb_context *ctx; xcb_connection_t *conn; @@ -56,6 +46,12 @@ struct x11_atom_interner { xkb_atom_t *out; } copies[128]; size_t num_copies; + /* These are not interned, but saved directly (after XkbEscapeMapName) */ + struct { + xcb_get_atom_name_cookie_t cookie; + char **out; + } escaped[4]; + size_t num_escaped; }; void @@ -79,4 +75,12 @@ x11_atom_interner_adopt_atoms(struct x11_atom_interner *interner, const xcb_atom_t *from, xkb_atom_t *to, size_t count); +/* + * Get a strdup'd and XkbEscapeMapName'd name of an X atom. The actual write is + * delayed until the next call to x11_atom_interner_round_trip(). + */ +void +x11_atom_interner_get_escaped_atom_name(struct x11_atom_interner *interner, + xcb_atom_t atom, char **out); + #endif