Package: emacs;
Reported by: Po Lu <luangruo <at> yahoo.com>
Date: Sat, 25 Feb 2023 06:20:01 UTC
Severity: normal
Found in version 30.0.50
To reply to this bug, email your comments to 61779 AT debbugs.gnu.org.
Toggle the display of automated, internal messages from the tracker.
View this report as an mbox folder, status mbox, maintainer mbox
bug-gnu-emacs <at> gnu.org
:bug#61779
; Package emacs
.
(Sat, 25 Feb 2023 06:20:01 GMT) Full text and rfc822 format available.Po Lu <luangruo <at> yahoo.com>
:bug-gnu-emacs <at> gnu.org
.
(Sat, 25 Feb 2023 06:20:02 GMT) Full text and rfc822 format available.Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
From: Po Lu <luangruo <at> yahoo.com> To: bug-gnu-emacs <at> gnu.org Subject: 30.0.50; excessive redisplay when the mini window is active 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;
bug-gnu-emacs <at> gnu.org
:bug#61779
; Package emacs
.
(Sat, 25 Feb 2023 08:30:02 GMT) Full text and rfc822 format available.Message #8 received at 61779 <at> debbugs.gnu.org (full text, mbox):
From: Eli Zaretskii <eliz <at> gnu.org> To: Po Lu <luangruo <at> yahoo.com> Cc: 61779 <at> debbugs.gnu.org Subject: Re: bug#61779: 30.0.50; excessive redisplay when the mini window is active Date: Sat, 25 Feb 2023 10:29:07 +0200
> 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 <at> 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. 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).
bug-gnu-emacs <at> gnu.org
:bug#61779
; Package emacs
.
(Sat, 25 Feb 2023 09:40:02 GMT) Full text and rfc822 format available.Message #11 received at 61779 <at> debbugs.gnu.org (full text, mbox):
From: Po Lu <luangruo <at> yahoo.com> To: Eli Zaretskii <eliz <at> gnu.org> Cc: 61779 <at> debbugs.gnu.org Subject: Re: bug#61779: 30.0.50; excessive redisplay when the mini window is active Date: Sat, 25 Feb 2023 17:37:14 +0800
Eli Zaretskii <eliz <at> 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 <at> 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.
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.