Re: [Emacs-diffs] master 5c9304e: Disable some display optimizations when frames need redisplay

[Stefan Monnier <monnier@iro.umontreal.ca>]

>> So, the value 2 (aka REDISPLAY_SOME) already gives you the information
>> that "only frames F1 and F2 need to be redisplayed".
> But you need to examine the frame's 'redisplay' flag anyway.

Which frame(s)?  If windows_or_buffers_changed is 0, you shouldn't need
to examine any frame's `redisplay' flag, AFAIK.

> So that special value of windows_or_buffers_changed just adds
> management (you need to reset it, and it can acquire higher values
> depending on what redisplay_internal discovers), without adding any
> value.

I'm not sure how much there is to gain from removing it, since it's also
used for wset_redisplay and bset_redisplay.  But it sounds like you have
a plan.

> The loop over all frames checking whether any have the 'redisplay'
> flag set is fast, so there's no need for a global flag to convey that.

Indeed, such a loop is cheap.  But as long as windows_or_buffers_changed
is used for the wset_redisplay and bset_redisplay, I don't see much
benefit in removing it from fset_redisplay.

But of course, looping over all windows is also fairly cheap, so I guess
you're planning to remove it also from wset_redisplay and bset_redisplay?

>> 100% agreement, and that's why I introduced those `redisplay' bits and
>> the corresponding REDISPLAY_SOME value for windows_or_buffers_changed.
>> 
>> And AFAIK it already has the effect that when we call fset_redisplay only
>> those frames get redisplayed.

> Maybe so, but the effect of this on frames is completely undocumented,

I tried to document it in those var's comment:

   /* Nonzero if we should redraw the mode lines on the next redisplay.
      If it has value REDISPLAY_SOME, then only redisplay the mode lines where
      the `redisplay' bit has been set.  Otherwise, redisplay all mode lines
      (the number used is then only used to track down the cause for this
      full-redisplay).  */

   int update_mode_lines;

   /* Nonzero if window sizes or contents other than selected-window have
      changed since last redisplay that finished.
      If it has value REDISPLAY_SOME, then only redisplay the windows where
      the `redisplay' bit has been set.  Otherwise, redisplay all windows
      (the number used is then only used to track down the cause for this
      full-redisplay).  */

   int windows_or_buffers_changed;

I see it's not very specific about frame/window/buffer's `redisplay'
flag, tho.

>> Also rather than "don't set windows_or_buffers_changed in
>> fset_redisplay", it seems like it would be safer to try and eliminate
>> (one by one) the places where we check the value of
>> windows_or_buffers_changed instead of checking f->redisplay or something
>> like that.
> You cannot eliminate windows_or_buffers_changed without replacing them
> with something.

I lost you here.  I thought you're suggesting to get rid of this
variable.  I'm only suggesting to remove (step by step) some of those
places where we check its value and I'm not suggesting to remove them
without replacing them with something, but rather I suggest to replace
those checks by checks of things like f->redisplay.


        Stefan