x11: cache X11 atoms

On every keymap notify event, the keymap should be refreshed, which
fetches the required X11 atoms. A big keymap might have a few hundred of
atoms.

A profile by a user has shown this *might* be slow when some intensive
amount of keymap activity is occurring. It might also be slow on a
remote X server.

While I'm not really sure this is the actual bottleneck, caching the
atoms is easy enough and only needs a couple kb of memory, so do that.

On the added bench-x11:

Before: retrieved 2500 keymaps from X in 11.233237s
After : retrieved 2500 keymaps from X in 1.592339s

Signed-off-by: Ran Benita <ran@unusedvar.com>
master
Ran Benita 2020-11-19 00:28:37 +02:00
parent f41e609bbe
commit 1bd3b3c7cb
5 changed files with 171 additions and 5 deletions

108
bench/x11.c Normal file
View File

@ -0,0 +1,108 @@
/*
* Copyright © 2020 Ran Benita <ran@unusedvar.com>
*
* 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 (including the next
* paragraph) 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 OR COPYRIGHT HOLDERS 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.
*/
#include "config.h"
#include <assert.h>
#include <stdlib.h>
#include <xcb/xkb.h>
#include "xkbcommon/xkbcommon.h"
#include "xkbcommon/xkbcommon-x11.h"
#include "bench.h"
#define BENCHMARK_ITERATIONS 2500
int
main(void)
{
int ret;
xcb_connection_t *conn;
int32_t device_id;
struct xkb_context *ctx;
struct bench bench;
char *elapsed;
conn = xcb_connect(NULL, NULL);
if (!conn || xcb_connection_has_error(conn)) {
fprintf(stderr, "Couldn't connect to X server: error code %d\n",
conn ? xcb_connection_has_error(conn) : -1);
ret = -1;
goto err_out;
}
ret = xkb_x11_setup_xkb_extension(conn,
XKB_X11_MIN_MAJOR_XKB_VERSION,
XKB_X11_MIN_MINOR_XKB_VERSION,
XKB_X11_SETUP_XKB_EXTENSION_NO_FLAGS,
NULL, NULL, NULL, NULL);
if (!ret) {
fprintf(stderr, "Couldn't setup XKB extension\n");
goto err_conn;
}
device_id = xkb_x11_get_core_keyboard_device_id(conn);
if (device_id == -1) {
ret = -1;
fprintf(stderr, "Couldn't find core keyboard device\n");
goto err_conn;
}
ctx = xkb_context_new(XKB_CONTEXT_NO_FLAGS);
if (!ctx) {
ret = -1;
fprintf(stderr, "Couldn't create xkb context\n");
goto err_conn;
}
bench_start(&bench);
for (int i = 0; i < BENCHMARK_ITERATIONS; i++) {
struct xkb_keymap *keymap;
struct xkb_state *state;
keymap = xkb_x11_keymap_new_from_device(ctx, conn, device_id,
XKB_KEYMAP_COMPILE_NO_FLAGS);
assert(keymap);
state = xkb_x11_state_new_from_device(keymap, conn, device_id);
assert(state);
xkb_state_unref(state);
xkb_keymap_unref(keymap);
}
bench_stop(&bench);
ret = 0;
elapsed = bench_elapsed_str(&bench);
fprintf(stderr, "retrieved %d keymaps from X in %ss\n",
BENCHMARK_ITERATIONS, elapsed);
free(elapsed);
xkb_context_unref(ctx);
err_conn:
xcb_disconnect(conn);
err_out:
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
}

View File

@ -701,6 +701,13 @@ benchmark(
executable('bench-compose', 'bench/compose.c', dependencies: test_dep),
env: bench_env,
)
if get_option('enable-x11')
benchmark(
'x11',
executable('bench-x11', 'bench/x11.c', dependencies: x11_test_dep),
env: bench_env,
)
endif
# Documentation.

View File

@ -210,6 +210,7 @@ xkb_context_unref(struct xkb_context *ctx)
if (!ctx || --ctx->refcnt > 0)
return;
free(ctx->x11_atom_cache);
xkb_context_include_path_clear(ctx);
atom_table_free(ctx->atom_table);
free(ctx);
@ -323,6 +324,8 @@ xkb_context_new(enum xkb_context_flags flags)
return NULL;
}
ctx->x11_atom_cache = NULL;
return ctx;
}

View File

@ -45,6 +45,9 @@ struct xkb_context {
struct atom_table *atom_table;
/* Used and allocated by xkbcommon-x11, free()d with the context. */
void *x11_atom_cache;
/* Buffer for the *Text() functions. */
char text_buffer[2048];
size_t text_next;

View File

@ -155,6 +155,20 @@ get_atom_name(xcb_connection_t *conn, xcb_atom_t atom, char **out)
return true;
}
struct x11_atom_cache {
/*
* Invalidate the cache based on the XCB connection.
* X11 atoms are actually not per connection or client, but per X server
* session. But better be safe just in case we survive an X server restart.
*/
xcb_connection_t *conn;
struct {
xcb_atom_t from;
xkb_atom_t to;
} cache[256];
size_t len;
};
bool
adopt_atoms(struct xkb_context *ctx, xcb_connection_t *conn,
const xcb_atom_t *from, xkb_atom_t *to, const size_t count)
@ -163,24 +177,49 @@ adopt_atoms(struct xkb_context *ctx, xcb_connection_t *conn,
xcb_get_atom_name_cookie_t cookies[SIZE];
const size_t num_batches = ROUNDUP(count, SIZE) / SIZE;
if (!ctx->x11_atom_cache) {
ctx->x11_atom_cache = calloc(1, sizeof(struct x11_atom_cache));
}
/* Can be NULL in case the malloc failed. */
struct x11_atom_cache *cache = ctx->x11_atom_cache;
if (cache && cache->conn != conn) {
cache->conn = conn;
cache->len = 0;
}
memset(to, 0, count * sizeof(*to));
/* Send and collect the atoms in batches of reasonable SIZE. */
for (size_t batch = 0; batch < num_batches; batch++) {
const size_t start = batch * SIZE;
const size_t stop = MIN((batch + 1) * SIZE, count);
/* Send. */
for (size_t i = start; i < stop; i++)
if (from[i] != XCB_ATOM_NONE)
for (size_t i = start; i < stop; i++) {
bool cache_hit = false;
if (cache) {
for (size_t c = 0; c < cache->len; c++) {
if (cache->cache[c].from == from[i]) {
to[i] = cache->cache[c].to;
cache_hit = true;
break;
}
}
}
if (!cache_hit && from[i] != XCB_ATOM_NONE)
cookies[i % SIZE] = xcb_get_atom_name(conn, from[i]);
}
/* Collect. */
for (size_t i = start; i < stop; i++) {
xcb_get_atom_name_reply_t *reply;
if (from[i] == XCB_ATOM_NONE) {
to[i] = XKB_ATOM_NONE;
if (from[i] == XCB_ATOM_NONE)
continue;
/* Was filled from cache. */
if (to[i] != 0)
continue;
}
reply = xcb_get_atom_name_reply(conn, cookies[i % SIZE], NULL);
if (!reply)
@ -194,6 +233,12 @@ adopt_atoms(struct xkb_context *ctx, xcb_connection_t *conn,
if (to[i] == XKB_ATOM_NONE)
goto err_discard;
if (cache && cache->len < ARRAY_SIZE(cache->cache)) {
size_t idx = cache->len++;
cache->cache[idx].from = from[i];
cache->cache[idx].to = to[i];
}
continue;
/*