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

[Po Lu]

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Sat, 25 Feb 2023 14:16:37 +0800
>> From:  Po Lu via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
>> 
>> 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''?
>
> This is okay for master, thanks.

The comment that was there still makes me uncomfortable, so:

> Btw, the code which tests MINI_WINDOW_P here:
>
>>        && (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).
>
> is very old, dating back to the initial revision of xdisp.c before the
> Emacs 21 display engine changes, so maybe we should try removing that
> instead (assuming that it is related to the more general change
> regarding the cursor you want to install).

I guess that's the better idea, since it's the only reasonable way I
found to make update_window be called extremely often without any
changes actually happening.