* Re: emacs-27 d7a4cea: ; Add a new item to TODO [not found] ` <20201217142237.1F01120AE6@vcs0.savannah.gnu.org> @ 2020-12-17 16:22 ` Stefan Monnier 2020-12-17 17:02 ` Stefan Monnier 2020-12-17 17:03 ` Eli Zaretskii 0 siblings, 2 replies; 5+ messages in thread From: Stefan Monnier @ 2020-12-17 16:22 UTC (permalink / raw) To: Eli Zaretskii; +Cc: emacs-devel > +** Make redisplay smarter about which parts to redraw > +Currently, redisplay has only 2 levels of redrawing: either it > +redisplays only the selected window on the selected frame, or it > +redisplays all the windows on all the frames. This doesn't scale well > +when the number of visible frames is large. I don't think that's true: the change I installed some years back (along with the `pre-redisplay-function` and its use for the region highlight) does specifically try and select a particular subset of frames/windows. That's what the `wset_redisplay`, `fset_redisplay` and `bset_redisplay` are about. And indeed `pre-redisplay-function` is passed the specific set of windows that have been found to need a redisplay and this set can be much more varied than what you describe above. > +Currently, two variables are used to make the decision what to > +redisplay: update_mode_lines and windows_or_buffers_changed. There's also a `redisplay` "bit" on frames, windows, and buffer text objects. > +One way of making this change is to go through all the places that set > +update_mode_lines and windows_or_buffers_changed, figure out which > +portions of the Emacs display could be affected by each change, and > +then implement the bitmap which will record each of these affected > +display portions. The logic in redisplay_internal will then need to > +be restructured so as to support this fine-grained redisplay. Indeed, that's very useful and I did use exactly this approach back then (and as the rest of your text describes, there are more opportunities). In order to make this easier I changed the code such that every assignment to those two variable sets it to a different value, and then in at every redisplay cycle are record which value is found by increasing the counter in the corresponding slot of `redisplay--all-windows-cause` and `redisplay--mode-lines-cause`. Stefan ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: emacs-27 d7a4cea: ; Add a new item to TODO 2020-12-17 16:22 ` emacs-27 d7a4cea: ; Add a new item to TODO Stefan Monnier @ 2020-12-17 17:02 ` Stefan Monnier 2020-12-17 17:03 ` Eli Zaretskii 1 sibling, 0 replies; 5+ messages in thread From: Stefan Monnier @ 2020-12-17 17:02 UTC (permalink / raw) To: Eli Zaretskii; +Cc: emacs-devel > in at every redisplay cycle are record which value is found by > increasing the counter in the corresponding slot of > `redisplay--all-windows-cause` and `redisplay--mode-lines-cause`. For example, in my currently running Emacs session, I see: redisplay--all-windows-cause: #s(hash-table size 65 test eql rehash-size 1.5 rehash-threshold 0.8125 data (31 631 0 205211 2 35619 52 388 19 178)) redisplay--mode-lines-cause: #s(hash-table size 65 test eql rehash-size 1.5 rehash-threshold 0.8125 data (12 789 15 29 0 204962 23 644 2 34019 17 18 18 13 14 302 42 563 11 738 32 410 10 26)) So, in about 200k cases, the "cause" for redisplay was 0 meaning that Emacs only redisplayed the selected window. In about 35k cases, the "cause" was 2, meaning that Emacs considered more than just the selected frame, but did not consider all windows&frames (instead it only considered those windows&frames with the `redisplay` bit set). Of the remaining cases (where it did consider all windows&frames), 631 were due to "windows_or_buffers_changed == 31" which grep windows_or_buffers_changed.\*31 src/*.c shows is due to a call to `set_window_scroll_bars`. And 789 were due to update_mode_lines == 12 which comes from kill-all-local-variables. Both of those cases seem easy to fix with the patch below which I just pushed to master. Of course, as more and more code uses `[fbw]set_redisplay` instead of just setting `windows_or_buffers_changed`, those two above `redisplay--*-cause` variables lose some of their usefulness, because when the cause is 2 we may still be redisplaying too much. Stefan diff --git a/src/buffer.c b/src/buffer.c index 4215acbf1d..dfc34faf6e 100644 --- a/src/buffer.c +++ b/src/buffer.c @@ -2814,7 +2814,7 @@ DEFUN ("kill-all-local-variables", Fkill_all_local_variables, /* Force mode-line redisplay. Useful here because all major mode commands call this function. */ - update_mode_lines = 12; + bset_update_mode_line (current_buffer); return Qnil; } diff --git a/src/window.c b/src/window.c index 4eab786958..bcc989b5a7 100644 --- a/src/window.c +++ b/src/window.c @@ -7822,7 +7822,7 @@ set_window_scroll_bars (struct window *w, Lisp_Object width, if more than a single window needs to be considered, see redisplay_internal. */ if (changed) - windows_or_buffers_changed = 31; + wset_redisplay (w); return changed ? w : NULL; } ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: emacs-27 d7a4cea: ; Add a new item to TODO 2020-12-17 16:22 ` emacs-27 d7a4cea: ; Add a new item to TODO Stefan Monnier 2020-12-17 17:02 ` Stefan Monnier @ 2020-12-17 17:03 ` Eli Zaretskii 2020-12-17 17:51 ` Stefan Monnier 1 sibling, 1 reply; 5+ messages in thread From: Eli Zaretskii @ 2020-12-17 17:03 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel > From: Stefan Monnier <monnier@iro.umontreal.ca> > Cc: emacs-devel@gnu.org > Date: Thu, 17 Dec 2020 11:22:49 -0500 > > > +** Make redisplay smarter about which parts to redraw > > +Currently, redisplay has only 2 levels of redrawing: either it > > +redisplays only the selected window on the selected frame, or it > > +redisplays all the windows on all the frames. This doesn't scale well > > +when the number of visible frames is large. > > I don't think that's true: the change I installed some years back (along > with the `pre-redisplay-function` and its use for the region highlight) > does specifically try and select a particular subset of frames/windows. > That's what the `wset_redisplay`, `fset_redisplay` and `bset_redisplay` > are about. See bug#42406. My personal experience is that fset_redisplay and wset_redisplay are not "strong" enough to cause redisplay in many situations, so we need to use those 2 variables to enforce more thorough redisplay. In particular, there are the cases where elements of display may need to be redrawn that don't belong to any window. > > +Currently, two variables are used to make the decision what to > > +redisplay: update_mode_lines and windows_or_buffers_changed. > > There's also a `redisplay` "bit" on frames, windows, and buffer > text objects. They are not very useful, and the problematic cases don't involve them anyway. > In order to make this easier I changed the code such that every > assignment to those two variable sets it to a different value, and then > in at every redisplay cycle are record which value is found by > increasing the counter in the corresponding slot of > `redisplay--all-windows-cause` and `redisplay--mode-lines-cause`. That's useful when debugging excess redisplays, and can also help in analyzing the use cases to decide on the bits. But we need to go further than that, IMO, because the current logic is still quite simplistic. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: emacs-27 d7a4cea: ; Add a new item to TODO 2020-12-17 17:03 ` Eli Zaretskii @ 2020-12-17 17:51 ` Stefan Monnier 2020-12-17 18:20 ` Eli Zaretskii 0 siblings, 1 reply; 5+ messages in thread From: Stefan Monnier @ 2020-12-17 17:51 UTC (permalink / raw) To: Eli Zaretskii; +Cc: emacs-devel >> > +** Make redisplay smarter about which parts to redraw >> > +Currently, redisplay has only 2 levels of redrawing: either it >> > +redisplays only the selected window on the selected frame, or it >> > +redisplays all the windows on all the frames. This doesn't scale well >> > +when the number of visible frames is large. >> >> I don't think that's true: the change I installed some years back (along >> with the `pre-redisplay-function` and its use for the region highlight) >> does specifically try and select a particular subset of frames/windows. >> That's what the `wset_redisplay`, `fset_redisplay` and `bset_redisplay` >> are about. > > See bug#42406. Thanks. > My personal experience is that fset_redisplay and wset_redisplay are > not "strong" enough to cause redisplay in many situations, so we need > to use those 2 variables to enforce more thorough redisplay. In > particular, there are the cases where elements of display may need to > be redrawn that don't belong to any window. I fully agree. But the above paragraph you installed is factually incorrect, so anyone who reads this TODO and wants to act on it will start with an incorrect understanding of the problem at hand and may even give up right away seeing code like `bset_redisplay` and thinking that the problem has already been solved. > That's useful when debugging excess redisplays, and can also help in > analyzing the use cases to decide on the bits. But we need to go > further than that, IMO, because the current logic is still quite > simplistic. No argument from me on that ;-) Stefan ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: emacs-27 d7a4cea: ; Add a new item to TODO 2020-12-17 17:51 ` Stefan Monnier @ 2020-12-17 18:20 ` Eli Zaretskii 0 siblings, 0 replies; 5+ messages in thread From: Eli Zaretskii @ 2020-12-17 18:20 UTC (permalink / raw) To: Stefan Monnier; +Cc: emacs-devel > From: Stefan Monnier <monnier@iro.umontreal.ca> > Cc: emacs-devel@gnu.org > Date: Thu, 17 Dec 2020 12:51:09 -0500 > > I fully agree. But the above paragraph you installed is factually > incorrect, so anyone who reads this TODO and wants to act on it will > start with an incorrect understanding of the problem at hand and may > even give up right away seeing code like `bset_redisplay` and thinking > that the problem has already been solved. Feel free to make that text more accurate, sure. As long as we agree that the problem exists, I see no need to argue. The relevant code is complex enough for me to welcome any insights from anyone, certainly from you. I don't claim to have a 100% clear vision of everything that's going on with these flags. I write that text so that we don't forget the problem, and so that maybe someone will want to work on this, and make the maintenance easier. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-12-17 18:20 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20201217142235.14287.97153@vcs0.savannah.gnu.org> [not found] ` <20201217142237.1F01120AE6@vcs0.savannah.gnu.org> 2020-12-17 16:22 ` emacs-27 d7a4cea: ; Add a new item to TODO Stefan Monnier 2020-12-17 17:02 ` Stefan Monnier 2020-12-17 17:03 ` Eli Zaretskii 2020-12-17 17:51 ` Stefan Monnier 2020-12-17 18:20 ` Eli Zaretskii
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).