From 974a7e541cc28613cbeed9e0195d945d614d97f4 Mon Sep 17 00:00:00 2001 From: Po Lu Date: Thu, 26 May 2022 10:22:24 +0800 Subject: [PATCH] Make X error checking more asynchronous This makes some operations (such as x-get-atom-name and x-change-window-property) up to 2600x faster by avoiding calls to XSync when setting up an error handler, and when checking for or uncatching errors if all requests were already processed. * src/xterm.c (X_COMPARE_SERIALS): New macro. (struct x_error_message_stack): Add new field `first_request'. (x_find_error_handler): New function. (x_error_catcher): New parameter `stack'. Use it instead. (x_catch_errors_with_handler): Keep a record of the next protocol request serial inside the stack entry. (x_uncatch_errors, x_check_errors, x_had_errors_p, x_clear_errors) (x_error_handler): Avoid XSync if all requests were processed by the X server and look for the error handler matching the display and request serial when processing errors. --- src/xterm.c | 125 ++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 91 insertions(+), 34 deletions(-) diff --git a/src/xterm.c b/src/xterm.c index 1d91055a4af..d518ce331ae 100644 --- a/src/xterm.c +++ b/src/xterm.c @@ -784,6 +784,11 @@ static int current_finish; static struct input_event *current_hold_quit; #endif +/* Compare two request serials A and B with OP, handling + wraparound. */ +#define X_COMPARE_SERIALS(a, op ,b) \ + (((long) (a) - (long) (b)) op 0) + struct x_atom_ref { /* Atom name. */ @@ -21011,52 +21016,78 @@ x_text_icon (struct frame *f, const char *icon_name) #define X_ERROR_MESSAGE_SIZE 200 -/* If non-nil, this should be a string. - It means catch X errors and store the error message in this string. +/* If non-nil, this should be a string. It means catch X errors and + store the error message in this string. The reason we use a stack is that x_catch_error/x_uncatch_error can - be called from a signal handler. -*/ + be called from a signal handler. */ -struct x_error_message_stack { +struct x_error_message_stack +{ char string[X_ERROR_MESSAGE_SIZE]; Display *dpy; x_special_error_handler handler; void *handler_data; struct x_error_message_stack *prev; + + /* The first request that this error handler applies to. Keeping + track of this allows us to avoid an XSync yet still have errors + for previously made requests be handled correctly. */ + unsigned long first_request; }; + static struct x_error_message_stack *x_error_message; +static struct x_error_message_stack * +x_find_error_handler (Display *dpy, XErrorEvent *event) +{ + struct x_error_message_stack *stack; + + stack = x_error_message; + + while (stack) + { + if (X_COMPARE_SERIALS (event->serial, >=, + stack->first_request) + && dpy == stack->dpy) + return stack; + + stack = stack->prev; + } + + return NULL; +} + /* An X error handler which stores the error message in *x_error_message. This is called from x_error_handler if x_catch_errors is in effect. */ static void -x_error_catcher (Display *display, XErrorEvent *event) +x_error_catcher (Display *display, XErrorEvent *event, + struct x_error_message_stack *stack) { XGetErrorText (display, event->error_code, - x_error_message->string, - X_ERROR_MESSAGE_SIZE); - if (x_error_message->handler) - x_error_message->handler (display, event, x_error_message->string, - x_error_message->handler_data); + stack->string, X_ERROR_MESSAGE_SIZE); + + if (stack->handler) + stack->handler (display, event, stack->string, + stack->handler_data); } -/* Begin trapping X errors for display DPY. Actually we trap X errors - for all displays, but DPY should be the display you are actually - operating on. +/* Begin trapping X errors for display DPY. - After calling this function, X protocol errors no longer cause - Emacs to exit; instead, they are recorded in the string - stored in *x_error_message. + After calling this function, X protocol errors generated on DPY no + longer cause Emacs to exit; instead, they are recorded in the + string stored in *x_error_message. Calling x_check_errors signals an Emacs error if an X error has occurred since the last call to x_catch_errors or x_check_errors. - Calling x_uncatch_errors resumes the normal error handling. - Calling x_uncatch_errors_after_check is similar, but skips an XSync - to the server, and should be used only immediately after - x_had_errors_p or x_check_errors. */ + Calling x_uncatch_errors resumes the normal error handling, + skipping an XSync if the last request made is known to have been + processed. Calling x_uncatch_errors_after_check is similar, but + skips an XSync to the server, and should be used only immediately + after x_had_errors_p or x_check_errors. */ void x_catch_errors_with_handler (Display *dpy, x_special_error_handler handler, @@ -21064,14 +21095,12 @@ x_catch_errors_with_handler (Display *dpy, x_special_error_handler handler, { struct x_error_message_stack *data = xmalloc (sizeof *data); - /* Make sure any errors from previous requests have been dealt with. */ - XSync (dpy, False); - data->dpy = dpy; data->string[0] = 0; data->handler = handler; data->handler_data = handler_data; data->prev = x_error_message; + data->first_request = NextRequest (dpy); x_error_message = data; } @@ -21100,8 +21129,7 @@ x_uncatch_errors_after_check (void) unblock_input (); } -/* Undo the last x_catch_errors call. - DPY should be the display that was passed to x_catch_errors. */ +/* Undo the last x_catch_errors call. */ void x_uncatch_errors (void) @@ -21119,7 +21147,11 @@ x_uncatch_errors (void) /* The display may have been closed before this function is called. Check if it is still open before calling XSync. */ - if (x_display_info_for_display (x_error_message->dpy) != 0) + if (x_display_info_for_display (x_error_message->dpy) != 0 + /* There is no point in making this extra sync if all requests + are known to have been fully processed. */ + && (LastKnownRequestProcessed (x_error_message->dpy) + != NextRequest (x_error_message->dpy) - 1)) XSync (x_error_message->dpy, False); tmp = x_error_message; @@ -21135,13 +21167,23 @@ x_uncatch_errors (void) void x_check_errors (Display *dpy, const char *format) { - /* Make sure to catch any errors incurred so far. */ - XSync (dpy, False); + char string[X_ERROR_MESSAGE_SIZE]; + + /* This shouldn't happen, since x_check_errors should be called + immediately inside an x_catch_errors block. */ + if (dpy != x_error_message->dpy) + emacs_abort (); + + /* There is no point in making this extra sync if all requests + are known to have been fully processed. */ + if (LastKnownRequestProcessed (dpy) + != NextRequest (dpy) - 1) + XSync (dpy, False); if (x_error_message->string[0]) { - char string[X_ERROR_MESSAGE_SIZE]; - memcpy (string, x_error_message->string, X_ERROR_MESSAGE_SIZE); + memcpy (string, x_error_message->string, + X_ERROR_MESSAGE_SIZE); x_uncatch_errors (); error (format, string); } @@ -21153,8 +21195,15 @@ x_check_errors (Display *dpy, const char *format) bool x_had_errors_p (Display *dpy) { + /* This shouldn't happen, since x_check_errors should be called + immediately inside an x_catch_errors block. */ + if (dpy != x_error_message->dpy) + emacs_abort (); + /* Make sure to catch any errors incurred so far. */ - XSync (dpy, False); + if (LastKnownRequestProcessed (dpy) + != NextRequest (dpy) - 1) + XSync (dpy, False); return x_error_message->string[0] != 0; } @@ -21164,6 +21213,11 @@ x_had_errors_p (Display *dpy) void x_clear_errors (Display *dpy) { + /* This shouldn't happen, since x_check_errors should be called + immediately inside an x_catch_errors block. */ + if (dpy != x_error_message->dpy) + emacs_abort (); + x_error_message->string[0] = 0; } @@ -21386,6 +21440,7 @@ static void x_error_quitter (Display *, XErrorEvent *); static int x_error_handler (Display *display, XErrorEvent *event) { + struct x_error_message_stack *stack; #ifdef HAVE_XINPUT2 struct x_display_info *dpyinfo; #endif @@ -21416,8 +21471,10 @@ x_error_handler (Display *display, XErrorEvent *event) return 0; #endif - if (x_error_message) - x_error_catcher (display, event); + stack = x_find_error_handler (display, event); + + if (stack) + x_error_catcher (display, event, stack); else x_error_quitter (display, event); return 0; -- 2.39.2