bug#21473: 24.5; very slow tooltip display to sort-of-slow remote display

[Ken Raeburn <raeburn@permabit.com>]

> On Oct 4, 2015, at 05:45, Eli Zaretskii <eliz@gnu.org> wrote:
> 
>> From: Ken Raeburn <raeburn@permabit.com>
>> Cc: 21473@debbugs.gnu.org
>> Date: Thu, 01 Oct 2015 06:01:35 -0400
>> 
>> We do all of this in "x_set_mouse_color" because the cursor color is a
>> property of the cursor object, so we create new cursors, set their
>> colors, and then start using them in place of the old ones.
> 
> Sorry, I don't understand: by "cursor" here do you mean the mouse
> pointer?  If not, i.e. if this is the text cursor, then how is it
> related to x_set_mouse_color?

Yes, the mouse pointer. “Cursor” is the term used in the code (and in X docs) for the shape and color of the mouse pointer.

> 
>> Below are the _XReply call stacks (with counts) I found once I worked up
>> my color handling changes. There were 61 XSync calls in all, this time.
>> This was for moving the mouse into the frame to the mode line (with
>> focus elsewhere), and waiting for the tooltip to pop up.
>> 
>>      5 _XReply « XSync « x_check_errors « x_set_mouse_color « x_set_frame_parameters « x_default_parameter « x_create_tip_frame « Fx_show_tip « Ffuncall « exec_byte_code « funcall_lambda « Ffuncall « exec_byte_code « funcall_lambda « Ffuncall « run_hook_with_args « Ffuncall « exec_byte_code « funcall_lambda « Ffuncall « Fapply « Ffuncall « exec_byte_code « funcall_lambda « Ffuncall « call1 « timer_check_2 « timer_check « readable_events « get_input_pending
> 
> Any idea why we need to call
> 
>  x_default_parameter (f, parms, Qmouse_color, build_string ("black"),
> 		       "pointerColor", "Foreground", RES_TYPE_STRING);
> 
> when creating a tip frame?  Do we want the tip frames to be able to
> support mouse highlight or something?  If so, we could make this
> conditional on some option, because the absolute majority of tooltips
> don't use that.
> 
>>      4 _XReply « XSync « x_had_errors_p « x_real_pos_and_offsets « x_real_positions « handle_one_xevent « XTread_socket « gobble_input « handle_async_input « process_pending_signals « xg_select « wait_reading_process_output « sit_for « read_char « read_key_sequence « command_loop_1 « internal_condition_case « command_loop_2 « internal_catch « command_loop « recursive_edit_1 « Frecursive_edit « main
>>      2 _XReply « XTranslateCoordinates « x_real_pos_and_offsets « x_real_positions « handle_one_xevent « XTread_socket « gobble_input « handle_async_input « process_pending_signals « xg_select « wait_reading_process_output « sit_for « read_char « read_key_sequence « command_loop_1 « internal_condition_case « command_loop_2 « internal_catch « command_loop « recursive_edit_1 « Frecursive_edit « main
>>      2 _XReply « XSync « x_uncatch_errors « x_real_pos_and_offsets « x_real_positions « handle_one_xevent « XTread_socket « gobble_input « handle_async_input « process_pending_signals « xg_select « wait_reading_process_output « sit_for « read_char « read_key_sequence « command_loop_1 « internal_condition_case « command_loop_2 « internal_catch « command_loop « recursive_edit_1 « Frecursive_edit « main
> 
> These are X events that come through the socket, so I don't see how we
> can avoid them.  I don't know enough about X to answer your questions
> about the possible way of avoiding these XSync calls.

Coordinate translation we presumably can’t avoid.  The XSync calls all seem to be about making sure we’re receiving any error events at the points in the code where we want to check for them… and in some cases, I think that’s only because the Xlib interfaces are kind of fuzzy about how to detect if a query for information worked or not.

I’ve been experimenting with using XCB interfaces to work around this.  With the XCB interfaces I can indeed send a GetGeometry request and a TranslateCoordinates request and a couple more, and then flush the output buffer, and then wait for all the results to come in, and find out whether I got a reply back or an error.  So I think in some of these cases the XSync calls can eventually go away, if XCB is available.

> 
>>      2 _XReply « XSync « x_uncatch_errors « xfont_list_pattern « xfont_list « font_list_entities « font_find_for_lface « font_load_for_lface « font_open_by_spec « font_open_by_name « x_default_font_parameter « x_create_tip_frame « Fx_show_tip « Ffuncall « exec_byte_code « funcall_lambda « Ffuncall « exec_byte_code « funcall_lambda « Ffuncall « run_hook_with_args « Ffuncall « exec_byte_code « funcall_lambda « Ffuncall « Fapply « Ffuncall « exec_byte_code « funcall_lambda « Ffuncall
> 
> This loads fonts for the tip frame, so I don't think we can avoid
> them.  The font parameter in the tip-frame parameter alist can be
> changed by the caller.

The XListFonts call we need (though I think once again there are cases where we ask for the same info more than once), but the XSync call here is again about synchronizing error event handling.



>>      1 _XReply « XParseColor « x_parse_color « x_defined_color « x_decode_color « x_set_background_color « x_set_frame_parameters « x_default_parameter « x_create_tip_frame « Fx_show_tip « Ffuncall « exec_byte_code « funcall_lambda « Ffuncall « exec_byte_code « funcall_lambda « Ffuncall « run_hook_with_args « Ffuncall « exec_byte_code « funcall_lambda « Ffuncall « Fapply « Ffuncall « exec_byte_code « funcall_lambda « Ffuncall « call1 « timer_check_2 « timer_check
> 
> Colors for the tip frame; looks unavoidable (modulo some color
> caching).

I’ve been working on a set of patches relating to speeding this up:

 - Cache XParseColor results.

 - Avoid most direct XAllocColor calls for TrueColor displays.  Turns out the TrueColor visual type uses pixel values that encode the RGB values directly, so we don’t actually have to send a message to the server to allocate a color cell.  This already was happening in image.c, I mostly just expanded on it.

(I think those two made the biggest difference.)

 - Make x_set_mouse_color record serial numbers and use a new error handling routine to check them, reducing the number of XSync calls but not getting rid of them entirely.

 - Use XCB calls in x_real_pos_and_offsets and get_current_wm_state to manage the error handling more directly, so we don’t need to call XSync at all.  This one still needs a lot of work.  The font-listing code is next on my list.

 - Try to defer garbage collection while running commands like x-create-frame.

With these changes I’ve been able to shave the 5+ second delay for creating the tooltip on my remote connection down to about 2 seconds (including the tooltip delay).  Normal frame creation is also faster.  But there’s still room for improvement.

Ken