Subject: bug#61779: 30.0.50; excessive redisplay when the mini window is active

From: Po Lu <[email-masked]>
Date: Sat, 25 Feb 2023 14:16:37 +0800

For some reason redisplay_internal will always update the mini window
even if its buffer contents have not changed.  I guess there is probably
a missing optimization wrt to updating the echo area contents, and
window_outdated does not take the echo area message into account:

	  && (CHARPOS (tlbufpos) == ZV
	      || FETCH_BYTE (BYTEPOS (tlbufpos)) == '\n'))
	/* Former continuation line has disappeared by becoming empty.  */
	goto cancel;
      else if (window_outdated (w) || MINI_WINDOW_P (w)) <==============
	{
	  /* We have to handle the case of continuation around a
	     wide-column character (see the comment in indent.c around
	     line 1340).

I am not sure that is why, however, so I can't just change that.
Anyway, update_window_line does not draw anything since the contents of
the glyph row do not change after redisplay, so that part is fine.  The
problem is that the redisplay will lead to update_window being called,
and gui_update_window_end will always call display_and_set_cursor
afterwards to display the cursor, which is not okay, as it generates a
lot of X protocol traffic when redisplay is called and redisplays the
mini-window.  Now add to that a process generating a lot of output, and
by doing so, calls to redisplay_preserve_echo_area, and you have an
absurd number of X requests, enough to saturate any reasonable network
connection.

Here is a patch to resolve the problem by not redrawing the cursor when
Emacs knows it is already there.  I've not seen any ill effects so far,
but the comment originally there seems to know better, so what are the
cases where ``phys_cursor_on_p is 1 but the cursor has been erased''?

Thanks.

diff --git a/src/xdisp.c b/src/xdisp.c
index c5c4321af77..39568bb6b1f 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -33333,12 +33333,21 @@ display_and_set_cursor (struct window *w, bool on,
 	      && new_cursor_width != w->phys_cursor_width)))
     erase_phys_cursor (w);
 
-  /* Don't check phys_cursor_on_p here because that flag is only set
-     to false in some cases where we know that the cursor has been
-     completely erased, to avoid the extra work of erasing the cursor
-     twice.  In other words, phys_cursor_on_p can be true and the cursor
-     still not be visible, or it has only been partly erased.  */
-  if (on)
+  /* XXX: this previously read:
+
+       Don't check phys_cursor_on_p here because that flag is only set
+       to false in some cases where we know that the cursor has been
+       completely erased, to avoid the extra work of erasing the
+       cursor twice.  In other words, phys_cursor_on_p can be true and
+       the cursor still not be visible, or it has only been partly
+       erased.
+
+     However, not checking this flag results in too much cursor redraw
+     if the mini-window is being redisplayed, since process output
+     results in calls to redisplay_internal, which always redisplays
+     the mini window regardless of whether or not its contents have
+     changed.  */
+  if (on && !w->phys_cursor_on_p)
     {
       w->phys_cursor_ascent = glyph_row->ascent;
       w->phys_cursor_height = glyph_row->height;