unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#57727: 29.0.50; Optimize tty display updates
@ 2022-09-11 10:03 Gerd Möllmann
  2022-09-11 11:18 ` Lars Ingebrigtsen
                   ` (3 more replies)
  0 siblings, 4 replies; 19+ messages in thread
From: Gerd Möllmann @ 2022-09-11 10:03 UTC (permalink / raw)
  To: 57727

[-- Attachment #1: Type: text/plain, Size: 819 bytes --]

This is a wishlist item for the implementation of what was discussed
recently on emacs-devel, namely using a larger output buffer, and
avoiding fflush as a means of improving tty display update speed.

The patch below is my first take on that.  It adds a low-level interface
consisting of two functions tty--set-output-buffer-size and
tty-output-buffer-size to change and retrieve the output buffer size of
a device.  A buffer size of 0 is taken to mean "do what we did since
1991, and a non-zero value is used as the buffer size set with setvbuf.

Default is currently to do everything as it always was.  One has to do
something like

  (tty--set-output-buffer-size (* 64 1024))

to change to the new behavior.

Feedback welcome, especially for the default, i.e. leaving everything
as-is, and for the Lisp interface.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch for master --]
[-- Type: text/x-patch, Size: 4577 bytes --]

From 597d17f6ecc866328e3c5c3438b1d7fec3b2bbea Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Gerd=20M=C3=B6llmann?= <gerd@gnu.org>
Date: Sun, 11 Sep 2022 11:42:18 +0200
Subject: [PATCH] Optimize tty display updates

* src/dispnew.c (update_frame_1): Don'f flush if tty's
output_buffer_size is non-zero.
* src/sysdep.c (init_sys_modes): Setvbuf depending on the tty's
output_buffer_size.
* src/term.c (Ftty__set_output_buffer_size, Ftty__output_buffer_size):
Low-level interface for setting and retrieving a tty's output buffer
size.
(syms_of_term): Defsubr the new functions.
* src/termchar.h (struct tty_display_info): New member
output_buffer_size.
---
 src/dispnew.c  |  4 +++-
 src/sysdep.c   |  5 ++++-
 src/term.c     | 39 +++++++++++++++++++++++++++++++++++++++
 src/termchar.h |  5 +++++
 4 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/src/dispnew.c b/src/dispnew.c
index 8932f103f1..b786f0f1ba 100644
--- a/src/dispnew.c
+++ b/src/dispnew.c
@@ -4929,7 +4929,9 @@ update_frame_1 (struct frame *f, bool force_p, bool inhibit_id_p,
     {
       if (MATRIX_ROW_ENABLED_P (desired_matrix, i))
 	{
-	  if (FRAME_TERMCAP_P (f))
+	  /* Note that output_buffer_size being 0 means that we want the
+	     old default behavior of flushing output every now and then.  */
+	  if (FRAME_TERMCAP_P (f) && FRAME_TTY (f)->output_buffer_size == 0)
 	    {
 	      /* Flush out every so many lines.
 		 Also flush out if likely to have more than 1k buffered
diff --git a/src/sysdep.c b/src/sysdep.c
index efd9638b07..abb385d138 100644
--- a/src/sysdep.c
+++ b/src/sysdep.c
@@ -1304,7 +1304,10 @@ init_sys_modes (struct tty_display_info *tty_out)
     }
 #endif /* F_GETOWN */
 
-  setvbuf (tty_out->output, NULL, _IOFBF, BUFSIZ);
+  const size_t buffer_size = (tty_out->output_buffer_size
+			      ? tty_out->output_buffer_size
+			      : BUFSIZ);
+  setvbuf (tty_out->output, NULL, _IOFBF, buffer_size);
 
   if (tty_out->terminal->set_terminal_modes_hook)
     tty_out->terminal->set_terminal_modes_hook (tty_out->terminal);
diff --git a/src/term.c b/src/term.c
index 2e43d89232..8e19c96672 100644
--- a/src/term.c
+++ b/src/term.c
@@ -2400,6 +2400,43 @@ DEFUN ("resume-tty", Fresume_tty, Sresume_tty, 0, 1, 0,
   return Qnil;
 }
 
+DEFUN ("tty--set-output-buffer-size", Ftty__set_output_buffer_size,
+       Stty__set_output_buffer_size, 1, 2, 0, doc:
+       /* Set the output buffer size for a TTY.
+
+SIZE zero means use the system's default value.  If SIZE is
+non-zero,this also avoids flushing the output stream.
+
+TTY may be a terminal object, a frame, or nil (meaning the selected
+frame's terminal).
+
+This function temporarily suspends and resumes the terminal
+device.  */)
+  (Lisp_Object size, Lisp_Object tty)
+{
+  if (!TYPE_RANGED_FIXNUMP (size_t, size))
+    error ("Invalid output buffer size");
+  Fsuspend_tty(tty);
+  struct terminal *terminal = decode_tty_terminal (tty);
+  terminal->display_info.tty->output_buffer_size
+    = XFIXNUM (size) <= 0 ? 0 : XFIXNUM (size);
+  return Fresume_tty(tty);
+}
+
+DEFUN ("tty--output-buffer-size", Ftty__output_buffer_size,
+       Stty__output_buffer_size, 0, 1, 0, doc:
+       /* Return the output buffer size of TTY.
+
+TTY may be a terminal object, a frame, or nil (meaning the selected
+frame's terminal).
+
+A value of zero means TTY uses the system's default value.  */)
+  (Lisp_Object tty)
+{
+  struct terminal *terminal = decode_tty_terminal (tty);
+  return make_fixnum (terminal->display_info.tty->output_buffer_size);
+}
+
 \f
 /***********************************************************************
 			       Mouse
@@ -4556,6 +4593,8 @@ syms_of_term (void)
   defsubr (&Stty_top_frame);
   defsubr (&Ssuspend_tty);
   defsubr (&Sresume_tty);
+  defsubr (&Stty__set_output_buffer_size);
+  defsubr (&Stty__output_buffer_size);
 #ifdef HAVE_GPM
   defsubr (&Sgpm_mouse_start);
   defsubr (&Sgpm_mouse_stop);
diff --git a/src/termchar.h b/src/termchar.h
index 49560dbc2a..0f17246411 100644
--- a/src/termchar.h
+++ b/src/termchar.h
@@ -53,6 +53,11 @@ #define EMACS_TERMCHAR_H
   FILE *output;                 /* The stream to be used for terminal output.
                                    NULL if the terminal is suspended. */
 
+  /* Size of output buffer.  A value of zero means use the default of
+     BUFIZE.  If non-zero, also minimize writes to the tty by avoiding
+     calls to flush.  */
+  size_t output_buffer_size;
+
   FILE *termscript;             /* If nonzero, send all terminal output
                                    characters to this stream also.  */
 
-- 
2.37.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* bug#57727: 29.0.50; Optimize tty display updates
  2022-09-11 10:03 bug#57727: 29.0.50; Optimize tty display updates Gerd Möllmann
@ 2022-09-11 11:18 ` Lars Ingebrigtsen
  2022-09-12  6:02   ` Gerd Möllmann
  2022-09-11 13:49 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 19+ messages in thread
From: Lars Ingebrigtsen @ 2022-09-11 11:18 UTC (permalink / raw)
  To: Gerd Möllmann; +Cc: 57727

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> The patch below is my first take on that.  It adds a low-level interface
> consisting of two functions tty--set-output-buffer-size and
> tty-output-buffer-size to change and retrieve the output buffer size of
> a device.  A buffer size of 0 is taken to mean "do what we did since
> 1991, and a non-zero value is used as the buffer size set with setvbuf.

Makes sense to me.

> Default is currently to do everything as it always was.  One has to do
> something like
>
>   (tty--set-output-buffer-size (* 64 1024))
>
> to change to the new behavior.
>
> Feedback welcome, especially for the default, i.e. leaving everything
> as-is, and for the Lisp interface.

Perhaps adding a defcustom for this where the :set calls
`tty--set-output-buffer-size' might be a nice interface?





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#57727: 29.0.50; Optimize tty display updates
  2022-09-11 10:03 bug#57727: 29.0.50; Optimize tty display updates Gerd Möllmann
  2022-09-11 11:18 ` Lars Ingebrigtsen
@ 2022-09-11 13:49 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2022-09-12  6:33   ` Gerd Möllmann
  2022-09-17 13:34 ` Gerd Möllmann
  2022-09-17 21:09 ` Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors
  3 siblings, 1 reply; 19+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2022-09-11 13:49 UTC (permalink / raw)
  To: Gerd Möllmann; +Cc: 57727

> +DEFUN ("tty--set-output-buffer-size", Ftty__set_output_buffer_size,
> +       Stty__set_output_buffer_size, 1, 2, 0, doc:
> +       /* Set the output buffer size for a TTY.
> +
> +SIZE zero means use the system's default value.  If SIZE is
> +non-zero,this also avoids flushing the output stream.
           ^^
           SPC

> +TTY may be a terminal object, a frame, or nil (meaning the selected
> +frame's terminal).
> +
> +This function temporarily suspends and resumes the terminal
> +device.  */)
> +  (Lisp_Object size, Lisp_Object tty)
> +{
> +  if (!TYPE_RANGED_FIXNUMP (size_t, size))
> +    error ("Invalid output buffer size");
> +  Fsuspend_tty(tty);
                ^^
                SPC

> +  struct terminal *terminal = decode_tty_terminal (tty);
> +  terminal->display_info.tty->output_buffer_size
> +    = XFIXNUM (size) <= 0 ? 0 : XFIXNUM (size);
> +  return Fresume_tty(tty);
                      ^^
                      SPC

That seems a bit over-engineered to me.
Why not just a DEVAR_BOOL to control whether we flush or not?
If someone wants to try out different buffer sizes, I suspect that
recompiling is a good enough solution (or provide a DEFVAR_INT for that
and let the tester(s) call `suspend/resume-tty` by hand).


        Stefan






^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#57727: 29.0.50; Optimize tty display updates
  2022-09-11 11:18 ` Lars Ingebrigtsen
@ 2022-09-12  6:02   ` Gerd Möllmann
  2022-09-12 11:21     ` Eli Zaretskii
  0 siblings, 1 reply; 19+ messages in thread
From: Gerd Möllmann @ 2022-09-12  6:02 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 57727

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Perhaps adding a defcustom for this where the :set calls
> `tty--set-output-buffer-size' might be a nice interface?

ATM, this is terminal-specific, so we'd need some way of specifying for
which frames, terminals or what this option applies.

(I'm assuming from the code that Emacs allows opening frames on multiple
ttys, which was news to me, but that doesn't mean anything of course.)





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#57727: 29.0.50; Optimize tty display updates
  2022-09-11 13:49 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2022-09-12  6:33   ` Gerd Möllmann
  2022-09-12 11:29     ` Eli Zaretskii
  2022-09-12 14:18     ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 2 replies; 19+ messages in thread
From: Gerd Möllmann @ 2022-09-12  6:33 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 57727

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> +  return Fresume_tty(tty);
>                       ^^
>                       SPC

Thanks, I'll fix these.

> That seems a bit over-engineered to me.
> Why not just a DEVAR_BOOL to control whether we flush or not?

Ok.  My thoughts went like so:

1. Multi-tty make me feel it's natural to make the behavior terminal
dependent.  At least I don't consider unreasonable for a user to expect
being able to tailor the behavior depending on the terminal.

2. I don't believe that just avoiding fflush will be enough.  THe code
currently uses a buffer of size BUFSIZ, which is OS-specific.  On my
system, for instance, BUFSIZ = 1024.  Don't know about MS-Windows today,
but I remember it being 512 there at some time.  At work, we had one
Linux distribution that used 32K (maybe Arch, I don't remember).  And so
on.

3. The coupling of setting the buffer size with not flushing is a bit
strange, but my reasoning would be that setting a larger buffer and
still flushing is kind of nonsensical.

4. From the recent discussion of supporting images on ttys I take away
that using a large buffer might help with that because of more data
being sent to the terminal.

(Not a reason, but I wonder if different buffer sizes might also help
reduce flickering with some terminal emulators, like Alacritty.
Whatever, just wanted to mention this.)

> If someone wants to try out different buffer sizes, I suspect that
> recompiling is a good enough solution (or provide a DEFVAR_INT for that
> and let the tester(s) call `suspend/resume-tty` by hand).

I didn't do that because of multi-tty.  But letting users suspend/resume
manually is of course an option.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#57727: 29.0.50; Optimize tty display updates
  2022-09-12  6:02   ` Gerd Möllmann
@ 2022-09-12 11:21     ` Eli Zaretskii
  0 siblings, 0 replies; 19+ messages in thread
From: Eli Zaretskii @ 2022-09-12 11:21 UTC (permalink / raw)
  To: Gerd Möllmann; +Cc: larsi, 57727

> Cc: 57727@debbugs.gnu.org
> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Date: Mon, 12 Sep 2022 08:02:01 +0200
> 
> I'm assuming from the code that Emacs allows opening frames on multiple
> ttys, which was news to me

It was added in Emacs 23.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#57727: 29.0.50; Optimize tty display updates
  2022-09-12  6:33   ` Gerd Möllmann
@ 2022-09-12 11:29     ` Eli Zaretskii
  2022-09-12 11:52       ` Gerd Möllmann
  2022-09-12 14:18     ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2022-09-12 11:29 UTC (permalink / raw)
  To: Gerd Möllmann; +Cc: monnier, 57727

> Cc: 57727@debbugs.gnu.org
> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Date: Mon, 12 Sep 2022 08:33:12 +0200
> 
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
> 
> > That seems a bit over-engineered to me.
> > Why not just a DEVAR_BOOL to control whether we flush or not?
> 
> Ok.  My thoughts went like so:

FWIW, I don't think this is over-engineered.  The suspend/resume dance
caused me to raise a brow, but I guess you cannot otherwise reset the
terminal settings?

> 2. I don't believe that just avoiding fflush will be enough.  THe code
> currently uses a buffer of size BUFSIZ, which is OS-specific.  On my
> system, for instance, BUFSIZ = 1024.  Don't know about MS-Windows today,
> but I remember it being 512 there at some time.

It's still 512.  But buffered stdio functions in the MS-Windows
runtime use 4K buffers, AFAIK, not 512B buffers.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#57727: 29.0.50; Optimize tty display updates
  2022-09-12 11:29     ` Eli Zaretskii
@ 2022-09-12 11:52       ` Gerd Möllmann
  2022-09-12 13:15         ` Gerd Möllmann
  0 siblings, 1 reply; 19+ messages in thread
From: Gerd Möllmann @ 2022-09-12 11:52 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: monnier, 57727

Eli Zaretskii <eliz@gnu.org> writes:

> FWIW, I don't think this is over-engineered.  The suspend/resume dance
> caused me to raise a brow, but I guess you cannot otherwise reset the
> terminal settings?

I'll check if can find a way to not suspend/resume.  In principle, it
should be possible to fflush/fclose/fopen/setvbuf, but I have to check
if that is sufficient.  Suspend/resume looked a cheap hack to get what I
wanted.






^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#57727: 29.0.50; Optimize tty display updates
  2022-09-12 11:52       ` Gerd Möllmann
@ 2022-09-12 13:15         ` Gerd Möllmann
  0 siblings, 0 replies; 19+ messages in thread
From: Gerd Möllmann @ 2022-09-12 13:15 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: monnier, 57727

Gerd Möllmann <gerd.moellmann@gmail.com> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>> FWIW, I don't think this is over-engineered.  The suspend/resume dance
>> caused me to raise a brow, but I guess you cannot otherwise reset the
>> terminal settings?
>
> I'll check if can find a way to not suspend/resume.  In principle, it
> should be possible to fflush/fclose/fopen/setvbuf, but I have to check
> if that is sufficient.  Suspend/resume looked a cheap hack to get what I
> wanted.

I'm afraid that doesn't work well.

The problem is that a setvbuf with a new buffer size requires an fclose,
by the C standard.  The fclose closes the file handle used with fdopen,
so we have to go through all what suspend/resume already do.

Too bad.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#57727: 29.0.50; Optimize tty display updates
  2022-09-12  6:33   ` Gerd Möllmann
  2022-09-12 11:29     ` Eli Zaretskii
@ 2022-09-12 14:18     ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2022-09-13  5:53       ` Gerd Möllmann
  1 sibling, 1 reply; 19+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2022-09-12 14:18 UTC (permalink / raw)
  To: Gerd Möllmann; +Cc: 57727

> 1. Multi-tty make me feel it's natural to make the behavior terminal
> dependent.  At least I don't consider unreasonable for a user to expect
> being able to tailor the behavior depending on the terminal.

I guess the reason why I think it's over-engineered is that I feel it's
not something which end-users will want to play with or configure
per-terminal: we should have a setting that works well everywhere.

The config vars are only needed to help find that universal setting.

My guess is that the exact setting won't matter very much anyway as long
as it's big enough to cover most redisplays (since we `fflush` anyway at
the end of `update_frame`).

For that same reason, I expect that using the OS's default will be
good enough and it will be difficult to come up with good ways for users
to test other values and report meaningful information about the impact.

> 3. The coupling of setting the buffer size with not flushing is a bit
> strange, but my reasoning would be that setting a larger buffer and
> still flushing is kind of nonsensical.

Agreed.

> 4. From the recent discussion of supporting images on ttys I take away
> that using a large buffer might help with that because of more data
> being sent to the terminal.

Could be.  Tho I suspect we'd usually want to send a file name rather
a file's data, but in any case, this is still hypothetical, so I see no
need to cross this bridge yet.

>> If someone wants to try out different buffer sizes, I suspect that
>> recompiling is a good enough solution (or provide a DEFVAR_INT for that
>> and let the tester(s) call `suspend/resume-tty` by hand).
> I didn't do that because of multi-tty.  But letting users suspend/resume
> manually is of course an option.

To the extent that I see it as a "debugging" functionality, it seems
sufficient (another option is to tell people to use an Emacs daemon so
they can set the var before opening the tty).


        Stefan






^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#57727: 29.0.50; Optimize tty display updates
  2022-09-12 14:18     ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2022-09-13  5:53       ` Gerd Möllmann
  2022-09-13 11:33         ` Eli Zaretskii
  2022-09-13 13:48         ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  0 siblings, 2 replies; 19+ messages in thread
From: Gerd Möllmann @ 2022-09-13  5:53 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 57727

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> 1. Multi-tty make me feel it's natural to make the behavior terminal
>> dependent.  At least I don't consider unreasonable for a user to expect
>> being able to tailor the behavior depending on the terminal.
>
> I guess the reason why I think it's over-engineered is that I feel it's
> not something which end-users will want to play with or configure
> per-terminal: we should have a setting that works well everywhere.

I really good default would indeed be a Good Thing.  But, on the other
hand, I think it's not likely that we find something that works for
everyone all the time.

> The config vars are only needed to help find that universal setting.
>
> My guess is that the exact setting won't matter very much anyway as long
> as it's big enough to cover most redisplays (since we `fflush` anyway at
> the end of `update_frame`).
>
> For that same reason, I expect that using the OS's default will be
> good enough and it will be difficult to come up with good ways for users
> to test other values and report meaningful information about the
> impact.

As far as the OS default goes (1024 on my system), I don't think I agree
completely.  A frame on a full-size terminal window has a width of
ca. 380 columns, which is a bit much for a buffer of 1024.

I fully agree that finding a good default value is hard in every
respect, though.  But I actually count that as an argument in favor of
making it an option.

>> 4. From the recent discussion of supporting images on ttys I take away
>> that using a large buffer might help with that because of more data
>> being sent to the terminal.
>
> Could be.  Tho I suspect we'd usually want to send a file name rather
> a file's data, but in any case, this is still hypothetical, so I see no
> need to cross this bridge yet.

Agreed.

>>> If someone wants to try out different buffer sizes, I suspect that
>>> recompiling is a good enough solution (or provide a DEFVAR_INT for that
>>> and let the tester(s) call `suspend/resume-tty` by hand).
>> I didn't do that because of multi-tty.  But letting users suspend/resume
>> manually is of course an option.
>
> To the extent that I see it as a "debugging" functionality, it seems
> sufficient (another option is to tell people to use an Emacs daemon so
> they can set the var before opening the tty).

Ok, we disagree here.

How can we proceed?  What do the maintainers think?  I could also just
put in on a branch, for later.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#57727: 29.0.50; Optimize tty display updates
  2022-09-13  5:53       ` Gerd Möllmann
@ 2022-09-13 11:33         ` Eli Zaretskii
  2022-09-13 11:36           ` Lars Ingebrigtsen
  2022-09-13 13:48         ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2022-09-13 11:33 UTC (permalink / raw)
  To: Gerd Möllmann, Lars Ingebrigtsen; +Cc: monnier, 57727

> Cc: 57727@debbugs.gnu.org
> From: Gerd Möllmann <gerd.moellmann@gmail.com>
> Date: Tue, 13 Sep 2022 07:53:47 +0200
> 
> How can we proceed?  What do the maintainers think?  I could also just
> put in on a branch, for later.

I'm okay with having this on master.  We can always fine-tune it
later, given that we get some feedback.

Lars?





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#57727: 29.0.50; Optimize tty display updates
  2022-09-13 11:33         ` Eli Zaretskii
@ 2022-09-13 11:36           ` Lars Ingebrigtsen
  2022-09-13 12:44             ` Gerd Möllmann
  0 siblings, 1 reply; 19+ messages in thread
From: Lars Ingebrigtsen @ 2022-09-13 11:36 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Gerd Möllmann, monnier, 57727

Eli Zaretskii <eliz@gnu.org> writes:

>> How can we proceed?  What do the maintainers think?  I could also just
>> put in on a branch, for later.
>
> I'm okay with having this on master.  We can always fine-tune it
> later, given that we get some feedback.
>
> Lars?

Yup, fine with me, too.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#57727: 29.0.50; Optimize tty display updates
  2022-09-13 11:36           ` Lars Ingebrigtsen
@ 2022-09-13 12:44             ` Gerd Möllmann
  0 siblings, 0 replies; 19+ messages in thread
From: Gerd Möllmann @ 2022-09-13 12:44 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: Eli Zaretskii, monnier, 57727

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> How can we proceed?  What do the maintainers think?  I could also just
>>> put in on a branch, for later.
>>
>> I'm okay with having this on master.  We can always fine-tune it
>> later, given that we get some feedback.
>>
>> Lars?
>
> Yup, fine with me, too.

Ok, thanks!

I'll make up something for NEWS then and commit in the next days.






^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#57727: 29.0.50; Optimize tty display updates
  2022-09-13  5:53       ` Gerd Möllmann
  2022-09-13 11:33         ` Eli Zaretskii
@ 2022-09-13 13:48         ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2022-09-13 13:53           ` Gerd Möllmann
  1 sibling, 1 reply; 19+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2022-09-13 13:48 UTC (permalink / raw)
  To: Gerd Möllmann; +Cc: 57727

Gerd Möllmann [2022-09-13 07:53:47] wrote:
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>>> 1. Multi-tty make me feel it's natural to make the behavior terminal
>>> dependent.  At least I don't consider unreasonable for a user to expect
>>> being able to tailor the behavior depending on the terminal.
>>
>> I guess the reason why I think it's over-engineered is that I feel it's
>> not something which end-users will want to play with or configure
>> per-terminal: we should have a setting that works well everywhere.
>
> I really good default would indeed be a Good Thing.  But, on the other
> hand, I think it's not likely that we find something that works for
> everyone all the time.

Really?  AFAICT noone has complained about the existing behavior (the
change under discussion was prompted by looking at the code, no?), so
what we currently have seems to work quite well "for everyone all the time".

> As far as the OS default goes (1024 on my system), I don't think I agree
> completely.  A frame on a full-size terminal window has a width of
> ca. 380 columns, which is a bit much for a buffer of 1024.

I tend to agree that 1kB sounds a bit small (from a network perspective,
I'd expect >1.5kB, to fill a typical Ethernet frame) but whether it
matters or not really depends how the "per packet" vs "per byte"
overheads compare.


        Stefan






^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#57727: 29.0.50; Optimize tty display updates
  2022-09-13 13:48         ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2022-09-13 13:53           ` Gerd Möllmann
  0 siblings, 0 replies; 19+ messages in thread
From: Gerd Möllmann @ 2022-09-13 13:53 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 57727

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Gerd Möllmann [2022-09-13 07:53:47] wrote:
>> I really good default would indeed be a Good Thing.  But, on the other
>> hand, I think it's not likely that we find something that works for
>> everyone all the time.
>
> Really?  AFAICT noone has complained about the existing behavior (the
> change under discussion was prompted by looking at the code, no?), so
> what we currently have seems to work quite well "for everyone all the time".

True.  I was thinking about "technically optimal" as good.  There are
other ways good can be considered good, of course :-).





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#57727: 29.0.50; Optimize tty display updates
  2022-09-11 10:03 bug#57727: 29.0.50; Optimize tty display updates Gerd Möllmann
  2022-09-11 11:18 ` Lars Ingebrigtsen
  2022-09-11 13:49 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2022-09-17 13:34 ` Gerd Möllmann
  2022-09-17 21:09 ` Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors
  3 siblings, 0 replies; 19+ messages in thread
From: Gerd Möllmann @ 2022-09-17 13:34 UTC (permalink / raw)
  To: 57727

Pushed to master.  Closing.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#57727: 29.0.50; Optimize tty display updates
  2022-09-11 10:03 bug#57727: 29.0.50; Optimize tty display updates Gerd Möllmann
                   ` (2 preceding siblings ...)
  2022-09-17 13:34 ` Gerd Möllmann
@ 2022-09-17 21:09 ` Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2022-09-18  5:34   ` Gerd Möllmann
  3 siblings, 1 reply; 19+ messages in thread
From: Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2022-09-17 21:09 UTC (permalink / raw)
  To: Gerd Möllmann; +Cc: 57727

Sorry for crashing the party late:

Gerd Möllmann [2022-09-11 12:03 +0200] wrote:

> +DEFUN ("tty--output-buffer-size", Ftty__output_buffer_size,
> +       Stty__output_buffer_size, 0, 1, 0, doc:
> +       /* Return the output buffer size of TTY.
> +
> +TTY may be a terminal object, a frame, or nil (meaning the selected
> +frame's terminal).
> +
> +A value of zero means TTY uses the system's default value.  */)
> +  (Lisp_Object tty)
> +{
> +  struct terminal *terminal = decode_tty_terminal (tty);
> +  return make_fixnum (terminal->display_info.tty->output_buffer_size);

What should tty--output-buffer-size do when called from a graphical
terminal (i.e. when decode_tty_terminal returns NULL)?

It currently crashes, but that's what every other C program is doing,
very banal.  Maybe Emacs should do something more eccentric, like return
0 or -1, signal an error, or do my taxes as consolation.

> +DEFUN ("tty--set-output-buffer-size", Ftty__set_output_buffer_size,
> +       Stty__set_output_buffer_size, 1, 2, 0, doc:
> +       /* Set the output buffer size for a TTY.
> +
> +SIZE zero means use the system's default value.  If SIZE is
> +non-zero,this also avoids flushing the output stream.
> +
> +TTY may be a terminal object, a frame, or nil (meaning the selected
> +frame's terminal).
> +
> +This function temporarily suspends and resumes the terminal
> +device.  */)
> +  (Lisp_Object size, Lisp_Object tty)
> +{
> +  if (!TYPE_RANGED_FIXNUMP (size_t, size))
> +    error ("Invalid output buffer size");

Just curious: given this is an internal "--" function, do we want to
tell the user what a valid size would be?  E.g. along the lines of:

  size_t sz = check_uinteger_max (size, min (MOST_POSITIVE_FIXNUM, SIZE_MAX));

Or would that be too much?

BTW, is there a known upper limit for the buffer size in the OS?

> +  Fsuspend_tty(tty);
> +  struct terminal *terminal = decode_tty_terminal (tty);

Here, terminal should theoretically be non-NULL, as Fsuspend_tty would
have otherwise signalled.  Maybe we should eassert this assumption, or
explicitly check for NULL?

Thanks,

-- 
Basil





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#57727: 29.0.50; Optimize tty display updates
  2022-09-17 21:09 ` Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2022-09-18  5:34   ` Gerd Möllmann
  0 siblings, 0 replies; 19+ messages in thread
From: Gerd Möllmann @ 2022-09-18  5:34 UTC (permalink / raw)
  To: Basil L. Contovounesios; +Cc: 57727

"Basil L. Contovounesios" <contovob@tcd.ie> writes:

> What should tty--output-buffer-size do when called from a graphical
> terminal (i.e. when decode_tty_terminal returns NULL)?
>
> It currently crashes, but that's what every other C program is doing,
> very banal.  Maybe Emacs should do something more eccentric, like return
> 0 or -1, signal an error, or do my taxes as consolation.

Oops, it should signal.

> Just curious: given this is an internal "--" function, do we want to
> tell the user what a valid size would be?  E.g. along the lines of:
>
>   size_t sz = check_uinteger_max (size, min (MOST_POSITIVE_FIXNUM, SIZE_MAX));
>
> Or would that be too much?
>
> BTW, is there a known upper limit for the buffer size in the OS?

I didn't want to do that because I don't know how to determine SIZE_MAX,
so I let users do what the like, as long as it doesn't make Emacs crash
:-).

>
>> +  Fsuspend_tty(tty);
>> +  struct terminal *terminal = decode_tty_terminal (tty);
>
> Here, terminal should theoretically be non-NULL, as Fsuspend_tty would
> have otherwise signalled.  Maybe we should eassert this assumption, or
> explicitly check for NULL?

I don't think that's necessary, or in other words, I think eassert
wouldn't give as valuable addtional information here.





^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2022-09-18  5:34 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-11 10:03 bug#57727: 29.0.50; Optimize tty display updates Gerd Möllmann
2022-09-11 11:18 ` Lars Ingebrigtsen
2022-09-12  6:02   ` Gerd Möllmann
2022-09-12 11:21     ` Eli Zaretskii
2022-09-11 13:49 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-09-12  6:33   ` Gerd Möllmann
2022-09-12 11:29     ` Eli Zaretskii
2022-09-12 11:52       ` Gerd Möllmann
2022-09-12 13:15         ` Gerd Möllmann
2022-09-12 14:18     ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-09-13  5:53       ` Gerd Möllmann
2022-09-13 11:33         ` Eli Zaretskii
2022-09-13 11:36           ` Lars Ingebrigtsen
2022-09-13 12:44             ` Gerd Möllmann
2022-09-13 13:48         ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-09-13 13:53           ` Gerd Möllmann
2022-09-17 13:34 ` Gerd Möllmann
2022-09-17 21:09 ` Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-09-18  5:34   ` Gerd Möllmann

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).