Re: [Emacs-diffs] master 29c360e: Ensure redisplay after "C-x C-e"

Re: [Emacs-diffs] master 29c360e: Ensure redisplay after "C-x C-e"

[Stefan Monnier]

> +      ;; If we are going to display the result in the echo area, force
> +      ;; a more thorough redisplay, in case the sexp we evaluated
> +      ;; changes something that should affect the display of the
> +      ;; current window.  Otherwise, Emacs might decide that only the
> +      ;; echo area needs to be redisplayed.
> +      (if (eq standard-output t)
> +          (force-mode-line-update 'all)))))
 
What does this have to do with elisp--eval-last-sexp?

Some changes aren't immediately reflected on screen (e.g. toggling
a minor-mode variable) unless one explicitly requests a redisplay or
call force-mode-line-update.

That's always been the case for setting line-spacing.  Sometimes you get
lucky, and after setting line-spacing the change is immediately
reflected on screen because you happen to make some other change at the
same time, but it's just wrong to rely on that.

So I think the above change is a bad idea.  If we want to "fix it
right", then we should look for ways to automatically react to changes
to line-spacing, rather than doing (force-mode-line-update 'all) in
elisp--eval-last-sexp, which only catches this problem when it goes
through elisp--eval-last-sexp, and which causes unnecessary redisplay
work in the 99.9% of the cases where elisp--eval-last-sexp does
something else.


        Stefan

Re: [Emacs-diffs] master 29c360e: Ensure redisplay after "C-x C-e"

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Eli Zaretskii <eliz@gnu.org>
> Date: Fri, 06 Nov 2015 09:18:41 -0500
> 
> > +      ;; If we are going to display the result in the echo area, force
> > +      ;; a more thorough redisplay, in case the sexp we evaluated
> > +      ;; changes something that should affect the display of the
> > +      ;; current window.  Otherwise, Emacs might decide that only the
> > +      ;; echo area needs to be redisplayed.
> > +      (if (eq standard-output t)
> > +          (force-mode-line-update 'all)))))
>  
> What does this have to do with elisp--eval-last-sexp?

"C-x C-e"

> Some changes aren't immediately reflected on screen (e.g. toggling
> a minor-mode variable) unless one explicitly requests a redisplay or
> call force-mode-line-update.

It's IMO confusing and unexpected to have this when you evaluate that.

> That's always been the case for setting line-spacing.

No, it worked correctly before Emacs 24.4, according to my testing.
Probably because we used to trigger thorough redisplay "for other
reasons".

> So I think the above change is a bad idea.  If we want to "fix it
> right", then we should look for ways to automatically react to changes
> to line-spacing, rather than doing (force-mode-line-update 'all) in
> elisp--eval-last-sexp, which only catches this problem when it goes
> through elisp--eval-last-sexp, and which causes unnecessary redisplay
> work in the 99.9% of the cases where elisp--eval-last-sexp does
> something else.

Why should we care about performance of "C-x C-e"?

Re: [Emacs-diffs] master 29c360e: Ensure redisplay after "C-x C-e"

[Artur Malabarba]

> "C-x C-e"

C-x C-e is not the only way to evaluate arbitrary lisp code, there are
plenty of others (eval-buffer, eval-region, eval-defun,
eval-expression, etc).
Sounds odd to single out eval-last-sexp here.

Wouldn't it be better to redisplay more aggressively? Maybe ensure
that the calling the `eval` function always causes a redisplay after
the command-loop?

Re: [Emacs-diffs] master 29c360e: Ensure redisplay after "C-x C-e"

[Eli Zaretskii]

> Date: Fri, 6 Nov 2015 14:53:01 +0000
> From: Artur Malabarba <bruce.connor.am@gmail.com>
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, emacs-devel <emacs-devel@gnu.org>
> 
> > "C-x C-e"
> 
> C-x C-e is not the only way to evaluate arbitrary lisp code, there are
> plenty of others (eval-buffer, eval-region, eval-defun,
> eval-expression, etc).

Does any of them cause the problem I tried to fix?  IOW, if any of
these include the form (setq line-spacing 1.0), does evaluating it
fail to redraw the selected window with the new line-spacing?

> Sounds odd to single out eval-last-sexp here.

I singled it out because it was the only situation I saw where the
evaluation had no effect.

> Wouldn't it be better to redisplay more aggressively? Maybe ensure
> that the calling the `eval` function always causes a redisplay after
> the command-loop?

The problem here was not to call redisplay -- it is always entered
after Emacs finishes running the last command, in this case "C-x C-e".
The problem was to tell redisplay it should consider redrawing the
selected window even though no change happened in the buffer displayed
in that window.

Re: [Emacs-diffs] master 29c360e: Ensure redisplay after "C-x C-e"

[Artur Malabarba]

2015-11-06 15:30 GMT+00:00 Eli Zaretskii <eliz@gnu.org>:
>> Date: Fri, 6 Nov 2015 14:53:01 +0000
>> From: Artur Malabarba <bruce.connor.am@gmail.com>
>> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, emacs-devel <emacs-devel@gnu.org>
>>
>> > "C-x C-e"
>>
>> C-x C-e is not the only way to evaluate arbitrary lisp code, there are
>> plenty of others (,
>> eval-expression, etc).
>
> Does any of them cause the problem I tried to fix?  IOW, if any of
> these include the form (setq line-spacing 1.0), does evaluating it
> fail to redraw the selected window with the new line-spacing?

eval-buffer, eval-region, and eval-defun all do (you have to invoke
them with keys, not with M-x).

Re: [Emacs-diffs] master 29c360e: Ensure redisplay after "C-x C-e"

[Eli Zaretskii]

> Date: Fri, 6 Nov 2015 16:11:43 +0000
> From: Artur Malabarba <bruce.connor.am@gmail.com>
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, emacs-devel <emacs-devel@gnu.org>
> 
> 2015-11-06 15:30 GMT+00:00 Eli Zaretskii <eliz@gnu.org>:
> >> Date: Fri, 6 Nov 2015 14:53:01 +0000
> >> From: Artur Malabarba <bruce.connor.am@gmail.com>
> >> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, emacs-devel <emacs-devel@gnu.org>
> >>
> >> > "C-x C-e"
> >>
> >> C-x C-e is not the only way to evaluate arbitrary lisp code, there are
> >> plenty of others (,
> >> eval-expression, etc).
> >
> > Does any of them cause the problem I tried to fix?  IOW, if any of
> > these include the form (setq line-spacing 1.0), does evaluating it
> > fail to redraw the selected window with the new line-spacing?
> 
> eval-buffer, eval-region, and eval-defun all do (you have to invoke
> them with keys, not with M-x).

Thanks, I will think about a solution to at least some of these.

But in any case, "C-x C-e" is frequently used to test the effect of a
variable, so having at least that produce an immediate effect is IMO a
Good Thing.

Re: [Emacs-diffs] master 29c360e: Ensure redisplay after "C-x C-e"

[Eli Zaretskii]

> Date: Fri, 06 Nov 2015 18:30:52 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> > Date: Fri, 6 Nov 2015 16:11:43 +0000
> > From: Artur Malabarba <bruce.connor.am@gmail.com>
> > Cc: Stefan Monnier <monnier@iro.umontreal.ca>, emacs-devel <emacs-devel@gnu.org>
> > 
> > 2015-11-06 15:30 GMT+00:00 Eli Zaretskii <eliz@gnu.org>:
> > >> Date: Fri, 6 Nov 2015 14:53:01 +0000
> > >> From: Artur Malabarba <bruce.connor.am@gmail.com>
> > >> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, emacs-devel <emacs-devel@gnu.org>
> > >>
> > >> > "C-x C-e"
> > >>
> > >> C-x C-e is not the only way to evaluate arbitrary lisp code, there are
> > >> plenty of others (,
> > >> eval-expression, etc).
> > >
> > > Does any of them cause the problem I tried to fix?  IOW, if any of
> > > these include the form (setq line-spacing 1.0), does evaluating it
> > > fail to redraw the selected window with the new line-spacing?
> > 
> > eval-buffer, eval-region, and eval-defun all do (you have to invoke
> > them with keys, not with M-x).
> 
> Thanks, I will think about a solution to at least some of these.
> 
> But in any case, "C-x C-e" is frequently used to test the effect of a
> variable, so having at least that produce an immediate effect is IMO a
> Good Thing.

Do you like the changes in commit 19e09cf better?

Re: [Emacs-diffs] master 29c360e: Ensure redisplay after "C-x C-e"

[Stefan Monnier]

> Do you like the changes in commit 19e09cf better?

The performance impact is a bit of an issue, but yes, I like it better.


        Stefan

Re: [Emacs-diffs] master 29c360e: Ensure redisplay after "C-x C-e"

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: bruce.connor.am@gmail.com,  emacs-devel@gnu.org
> Date: Sat, 07 Nov 2015 18:41:22 -0500
> 
> > Do you like the changes in commit 19e09cf better?
> 
> The performance impact is a bit of an issue

You mean, the 5% slow-down?

Re: [Emacs-diffs] master 29c360e: Ensure redisplay after "C-x C-e"

[David Kastrup]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Stefan Monnier <monnier@iro.umontreal.ca>
>> Cc: bruce.connor.am@gmail.com,  emacs-devel@gnu.org
>> Date: Sat, 07 Nov 2015 18:41:22 -0500
>> 
>> > Do you like the changes in commit 19e09cf better?
>> 
>> The performance impact is a bit of an issue
>
> You mean, the 5% slow-down?

5% slowdown in overall operation for addressing a redisplay quirk seems
like a bad tradeoff.  Redisplay quirks are a dime a dozen, after all.

-- 
David Kastrup

Re: [Emacs-diffs] master 29c360e: Ensure redisplay after "C-x C-e"

[Eli Zaretskii]

> From: David Kastrup <dak@gnu.org>
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>,  bruce.connor.am@gmail.com,  emacs-devel@gnu.org
> Date: Sun, 08 Nov 2015 10:23:27 +0100
> 
> 5% slowdown in overall operation for addressing a redisplay quirk seems
> like a bad tradeoff.

Accurate display is not a "quirk".  A "real-time display editor"
shouldn't produce display that is out of sync with the internal state,
certainly not when Emacs is idle.

RE: [Emacs-diffs] master 29c360e: Ensure redisplay after "C-x C-e"

[Drew Adams]

> 5% slowdown in overall operation for addressing a redisplay quirk seems
> like a bad tradeoff.  Redisplay quirks are a dime a dozen, after all.

  5% performance hit each
               X  1 dozen
  -----------------------
  60% performance hit per dozen

;-)

Re: [Emacs-diffs] master 29c360e: Ensure redisplay after "C-x C-e"

[David Kastrup]

Drew Adams <drew.adams@oracle.com> writes:

>> 5% slowdown in overall operation for addressing a redisplay quirk seems
>> like a bad tradeoff.  Redisplay quirks are a dime a dozen, after all.
>
>   5% performance hit each
>                X  1 dozen
>   -----------------------
>   60% performance hit per dozen

It's more like a dime will buy you 100*(1.05^12 - 1)%, close to 80%.

Just for an automatic mode-line update.  And it's not like the mode line
is not _documented_ to require a manual kick after changes.

-- 
David Kastrup

Re: [Emacs-diffs] master 29c360e: Ensure redisplay after "C-x C-e"

[Stefan Monnier]

>> The performance impact is a bit of an issue
> You mean, the 5% slow-down?

Yes.  I think it's acceptable only because I know it can be optimized
away by moving it into the CONSTANT_P branch.


        Stefan

Re: [Emacs-diffs] master 29c360e: Ensure redisplay after "C-x C-e"

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: bruce.connor.am@gmail.com,  emacs-devel@gnu.org
> Date: Sun, 08 Nov 2015 17:25:24 -0500
> 
> >> The performance impact is a bit of an issue
> > You mean, the 5% slow-down?
> 
> Yes.  I think it's acceptable only because I know it can be optimized
> away by moving it into the CONSTANT_P branch.

I hope someone will implement CONSTANT_P, then.

Re: [Emacs-diffs] master 29c360e: Ensure redisplay after "C-x C-e"

[Stefan Monnier]

> Wouldn't it be better to redisplay more aggressively?

Please no.  CPU power is far from free.

> Maybe ensure that the calling the `eval` function always causes
> a redisplay after the command-loop?

`eval' is called in *many* places, so this would be terrible.
Redisplaying all mode-lines can take a significant amount of time,
especially if you have a hundred windows or more (common use case for
me, at least).


        Stefan

Re: [Emacs-diffs] master 29c360e: Ensure redisplay after "C-x C-e"

[Stefan Monnier]

>> That's always been the case for setting line-spacing.
> No, it worked correctly before Emacs 24.4, according to my testing.

Yes, but that was an accident.  I think what you did is add an arbitrary
hack to paper over this, and I think it'd be better to do it either of:
- live with the apparent regression, telling users that they should
  simply be happy to have enjoyed this accident in the past.
- fix it "right".  E.g. add ad-hoc code to redisplay_internal that tries
  to detect changes to this variable, or add a "write barrier" on that
  variable, or deprecated the `line-spacing' variable in favor of
  a `set-line-spacing' function (thus providing the write-barrier), ...

> Probably because we used to trigger thorough redisplay "for other
> reasons".

Exactly.  People you did this and got the redisplay properly refreshed
were just lucky.

If you fix this case in the way you did, then I see no reason not to
go further down that road and just always redisplay all mode-lines, on
the off-chance that the user set line-spacing, or toggled a minor mode
variable without calling something like force-mode-line-update, set
a variable `toto' which happens to be used by one of the (:eval...)
nodes of the mode-line-format of some of the displayed buffers, ...

> Why should we care about performance of "C-x C-e"?

Why not?  I just think your addition of force-mode-line-update will be
wasted work in 99.9% of the cases, and it will only cover very few of
the cases where a force-mode-line-update is needed.


        Stefan


PS: I even several times found that the "lucky accident" which causes
the mode-line to be recomputed after things like M-: is a mis-feature
because it doesn't let me know when a call to force-mode-line-update
is needed.

Re: [Emacs-diffs] master 29c360e: Ensure redisplay after "C-x C-e"

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org
> Date: Fri, 06 Nov 2015 10:30:51 -0500
> 
> >> That's always been the case for setting line-spacing.
> > No, it worked correctly before Emacs 24.4, according to my testing.
> 
> Yes, but that was an accident.  I think what you did is add an arbitrary
> hack to paper over this, and I think it'd be better to do it either of:
> - live with the apparent regression, telling users that they should
>   simply be happy to have enjoyed this accident in the past.

I don't like this alternative.  Redisplay should be correct before it
is fast.  Users rightfully expect changes to such variables to have
effect immediately, so not doing that looks like a bug.  How do you
explain that, after evaluating (setq line-spacing 1.0) nothing
happens, but as soon as you type "M-x", the new setting takes effect?

> - fix it "right".  E.g. add ad-hoc code to redisplay_internal that tries
>   to detect changes to this variable, or add a "write barrier" on that
>   variable, or deprecated the `line-spacing' variable in favor of
>   a `set-line-spacing' function (thus providing the write-barrier), ...

This is not the only such variable, there are others.  Adding ad-hoc
code for each one sounds _really_ hacky.

I cannot take the other possibilities seriously, and I don't think you
do, either.

> If you fix this case in the way you did, then I see no reason not to
> go further down that road and just always redisplay all mode-lines, on
> the off-chance that the user set line-spacing, or toggled a minor mode
> variable without calling something like force-mode-line-update, set
> a variable `toto' which happens to be used by one of the (:eval...)
> nodes of the mode-line-format of some of the displayed buffers, ...

I see no reason to go down that road.  I don't think there are any
problems to fix there, and simply redisplaying all mode lines all the
time will have no effect except slowing down Emacs.

> > Why should we care about performance of "C-x C-e"?
> 
> Why not?

Because it's not performance-critical, and cannot be, ever.

> I just think your addition of force-mode-line-update will be wasted
> work in 99.9% of the cases, and it will only cover very few of the
> cases where a force-mode-line-update is needed.

Please show at least a couple of other cases.  Then maybe I will
change my mind.

> PS: I even several times found that the "lucky accident" which causes
> the mode-line to be recomputed after things like M-: is a mis-feature
> because it doesn't let me know when a call to force-mode-line-update
> is needed.

I see no way around that, as long as going to minibuffer causes
changes in display, like the menu bar, the tool bar, etc.

Re: [Emacs-diffs] master 29c360e: Ensure redisplay after "C-x C-e"

[Eli Zaretskii]

> Date: Fri, 06 Nov 2015 17:47:49 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: emacs-devel@gnu.org
> 
> > Yes, but that was an accident.  I think what you did is add an arbitrary
> > hack to paper over this, and I think it'd be better to do it either of:
> > - live with the apparent regression, telling users that they should
> >   simply be happy to have enjoyed this accident in the past.
> 
> I don't like this alternative.  Redisplay should be correct before it
> is fast.  Users rightfully expect changes to such variables to have
> effect immediately, so not doing that looks like a bug.  How do you
> explain that, after evaluating (setq line-spacing 1.0) nothing
> happens, but as soon as you type "M-x", the new setting takes effect?

In the last sentence, I meant to ask how you explain this to the users.

Re: [Emacs-diffs] master 29c360e: Ensure redisplay after "C-x C-e"

[Stefan Monnier]

>> - live with the apparent regression, telling users that they should
>> simply be happy to have enjoyed this accident in the past.
> I don't like this alternative.  Redisplay should be correct before it
> is fast.

We agree in general, of course.  But it's always been the case that some
changes require a call to force-mode-line-update.  Notice that 

> Users rightfully expect changes to such variables to have
> effect immediately, so not doing that looks like a bug.  How do you
> explain that, after evaluating (setq line-spacing 1.0) nothing
> happens, but as soon as you type "M-x", the new setting takes effect?

I explain it saying that this variable value is only taken into account
if something is redisplayed.  And the user moves on very happy.

E.g. note how in bug#21835, the lack of redisplay is not a cause for
reporting a bug.  It's just a minor surprise worthy of a "Note" in
passing to the actual problem of how the cursor is displayed.

If you don't like this answer, how 'bout I return the question:

   How do you explain to the user that when he used C-x C-e the display
   was immediately updated, but when I put that same code into his
   hand-made interactive function, it stopped working and started to
   only take effect after something like M-x?

I really can't see why we should hide this weakness of our
redisplay system in the case of C-x C-e.  Either this weakness exists
and the Elisp coder will have to know about it sooner or later, or we
fix it for real.

> This is not the only such variable, there are others.  Adding ad-hoc
> code for each one sounds _really_ hacky.

Agreed.

> I cannot take the other possibilities seriously, and I don't think you
> do, either.

A `set-line-spacing' function, no, but a write-barrier, yes.
We've already had discussions on emacs-devel to add such a generic
feature cheaply, even with patches submitted.

>> > Why should we care about performance of "C-x C-e"?
>> Why not?
> Because it's not performance-critical, and cannot be, ever.

Of course it can be performance critical in keyboard-macros.

>> I just think your addition of force-mode-line-update will be wasted
>> work in 99.9% of the cases, and it will only cover very few of the
>> cases where a force-mode-line-update is needed.
> Please show at least a couple of other cases.

"grep force-mode-line-update" shows it's still needed at tons of
other places.  Adding it in C-x C-e won't help them.


        Stefan

Re: [Emacs-diffs] master 29c360e: Ensure redisplay after "C-x C-e"

[Eli Zaretskii]

> From: Stefan Monnier <monnier@IRO.UMontreal.CA>
> Cc: emacs-devel@gnu.org
> Date: Fri, 06 Nov 2015 16:47:58 -0500
> 
> >> - live with the apparent regression, telling users that they should
> >> simply be happy to have enjoyed this accident in the past.
> > I don't like this alternative.  Redisplay should be correct before it
> > is fast.
> 
> We agree in general, of course.  But it's always been the case that some
> changes require a call to force-mode-line-update.  Notice that 
> 
> > Users rightfully expect changes to such variables to have
> > effect immediately, so not doing that looks like a bug.  How do you
> > explain that, after evaluating (setq line-spacing 1.0) nothing
> > happens, but as soon as you type "M-x", the new setting takes effect?
> 
> I explain it saying that this variable value is only taken into account
> if something is redisplayed.  And the user moves on very happy.

Not this user.  And so I don't want to explain this away like that.

> E.g. note how in bug#21835, the lack of redisplay is not a cause for
> reporting a bug.  It's just a minor surprise worthy of a "Note" in
> passing to the actual problem of how the cursor is displayed.

True for the person who reported that bug.  Not true for me.  Any
change in display when you press M-x is a subtle redisplay bug that
needs to be fixed.  It means some redisplay optimization was used
when it shouldn't have been.

> If you don't like this answer, how 'bout I return the question:
> 
>    How do you explain to the user that when he used C-x C-e the display
>    was immediately updated, but when I put that same code into his
>    hand-made interactive function, it stopped working and started to
>    only take effect after something like M-x?

Well, I was bothered by that as well, so the follow-up commit fixed
that, I hope.  Do you like it better?

> I really can't see why we should hide this weakness of our
> redisplay system in the case of C-x C-e.  Either this weakness exists
> and the Elisp coder will have to know about it sooner or later, or we
> fix it for real.

I hope I've now fixed it for real.

> > This is not the only such variable, there are others.  Adding ad-hoc
> > code for each one sounds _really_ hacky.
> 
> Agreed.

The changes I made in 19e09cf are hopefully less hacky.

> > I cannot take the other possibilities seriously, and I don't think you
> > do, either.
> 
> A `set-line-spacing' function, no, but a write-barrier, yes.
> We've already had discussions on emacs-devel to add such a generic
> feature cheaply, even with patches submitted.

Please point to those patches, I don't think I have a clear idea what
you allude to here.

> >> > Why should we care about performance of "C-x C-e"?
> >> Why not?
> > Because it's not performance-critical, and cannot be, ever.
> 
> Of course it can be performance critical in keyboard-macros.

Redisplay happens only once after the macro finishes, no?  Or did you
have some very specific macro in mind?

> >> I just think your addition of force-mode-line-update will be wasted
> >> work in 99.9% of the cases, and it will only cover very few of the
> >> cases where a force-mode-line-update is needed.
> > Please show at least a couple of other cases.
> 
> "grep force-mode-line-update" shows it's still needed at tons of
> other places.  Adding it in C-x C-e won't help them.

It wasn't supposed to help them.  Or maybe I completely misunderstand
what you wanted to say here.