GNU bug report logs - #66020
[PATCH] Reduce GC churn in read_process_output

Previous Next

Full log

View this message in rfc822 format

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: Paul Eggert <eggert <at> cs.ucla.edu>, stefankangas <at> gmail.com, 66020 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca
Subject: bug#66020: (bug#64735 spin-off): regarding the default for read-process-output-max
Date: Sun, 24 Sep 2023 08:29:37 +0300

> Date: Sun, 24 Sep 2023 00:51:28 +0300
> From: Dmitry Gutov <dmitry <at> gutov.dev>
> Cc: 66020 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca, stefankangas <at> gmail.com
> 
> But it seems I've found an answer to one previous question: "can we find 
> out how much is already buffered in advance?"
> 
> The patch below asks that from the OS (how portable is this? not sure) 
> and allocates a larger buffer when more output has been buffered. If we 
> keep OS's default value of pipe buffer size (64K on Linux and 16K-ish on 
> macOS, according to 
> https://unix.stackexchange.com/questions/11946/how-big-is-the-pipe-buffer), 
> that means auto-scaling the buffer on Emacs's side depending on how much 
> the process outputs. The effect on performance is similar to the 
> previous (looping) patch (0.70 -> 0.65), and is comparable to bumping 
> read-process-output-max to 65536.
> 
> So if we do decide to bump the default, I suppose the below should not 
> be necessary. And I don't know whether we should be concerned about 
> fragmentation: this way buffers do get allocates in different sizes 
> (almost always multiples of 4096, but with rare exceptions among larger 
> values).
> 
> diff --git a/src/process.c b/src/process.c
> index 2376d0f288d..13cf6d6c50d 100644
> --- a/src/process.c
> +++ b/src/process.c
> @@ -6137,7 +6145,18 @@
>     specpdl_ref count = SPECPDL_INDEX ();
>     Lisp_Object odeactivate;
>     char *chars;
> 
> +#ifdef USABLE_FIONREAD
> +#ifdef DATAGRAM_SOCKETS
> +  if (!DATAGRAM_CHAN_P (channel))
> +#endif
> +    {
> +      int available_read;
> +      ioctl (p->infd, FIONREAD, &available_read);
> +      readmax = MAX (readmax, available_read);
> +    }
> +#endif
> +
>     USE_SAFE_ALLOCA;
>     chars = SAFE_ALLOCA (sizeof coding->carryover + readmax);
> 
> What do people think?

I think we should increase the default size, and the rest (querying
the system about the pipe size) looks like an unnecessary complication
to me.

I've added Paul Eggert to this discussion, as I'd like to hear his
opinions about this stuff.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.