bug#66288: 29.1; Performance regression using pipe for subprocess

bug#66288: 29.1; Performance regression using pipe for subprocess

[Chris Hanson]

When using "xscheme.el" to start and interact with MIT/GNU Scheme in a
subprocess, the performance significantly degraded in Emacs 29.1.  It
worked well in older releases.

Here is a recipe:

     emacs -Q
     M-x load-library RET xscheme RET
     M-x run-scheme RET

     and see how slowly the process output is printed as Scheme starts.
     Compare this to Emacs 28.2 or earlier.

I've played around quite a bit to figure out what's going on, and found
that changing xscheme-start-process so that start-process used a pty
instead of a pipe eliminated the performance regression.  That isn't
really a solution since there's some complicated interrupt stuff going
on that crashes the subprocess when a pty is used.  I'm trying to track
that down and fix it but have not succeeded so far.


In GNU Emacs 29.1 (build 2, x86_64-pc-linux-gnu, X toolkit, cairo
  version 1.16.0, Xaw3d scroll bars) of 2023-08-03 built on kleph
Windowing system distributor 'The X.Org Foundation', version 11.0.12101004
System Description: Ubuntu 22.04.3 LTS

Configured using:
  'configure --with-x-toolkit=athena --with-tree-sitter
  --with-native-compilation'

Configured features:
CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG
LIBSELINUX LIBXML2 MODULES NATIVE_COMP NOTIFY INOTIFY PDUMPER PNG RSVG
SECCOMP SOUND THREADS TIFF TOOLKIT_SCROLL_BARS TREE_SITTER X11 XAW3D
XDBE XIM XINPUT2 XPM LUCID ZLIB

Important settings:
   value of $LANG: en_US.UTF-8
   value of $XMODIFIERS: @im=ibus
   locale-coding-system: utf-8-unix

Major mode: Scheme Interaction

Minor modes in effect:
   diff-auto-refine-mode: t
   windmove-mode: t
   which-function-mode: t
   which-key-mode: t
   savehist-mode: t
   global-git-commit-mode: t
   shell-dirtrack-mode: t
   server-mode: t
   global-edit-server-edit-mode: t
   desktop-save-mode: t
   global-auto-revert-mode: t
   override-global-mode: t
   global-eldoc-mode: t
   show-paren-mode: t
   electric-indent-mode: t
   mouse-wheel-mode: t
   file-name-shadow-mode: t
   global-font-lock-mode: t
   font-lock-mode: t
   line-number-mode: t
   transient-mark-mode: t
   auto-composition-mode: t
   auto-encryption-mode: t
   auto-compression-mode: t

Load-path shadows:
/home/cph/.emacs.d/elpa/transient-20230919.2146/transient hides 
/usr/local/share/emacs/29.1/lisp/transient
/home/cph/.emacs.d/elpa/use-package-20230426.2324/use-package hides 
/usr/local/share/emacs/29.1/lisp/use-package/use-package
/home/cph/.emacs.d/elpa/use-package-20230426.2324/use-package-jump hides 
/usr/local/share/emacs/29.1/lisp/use-package/use-package-jump
/home/cph/.emacs.d/elpa/bind-key-20230203.2004/bind-key hides 
/usr/local/share/emacs/29.1/lisp/use-package/bind-key
/home/cph/.emacs.d/elpa/use-package-20230426.2324/use-package-bind-key 
hides /usr/local/share/emacs/29.1/lisp/use-package/use-package-bind-key
/home/cph/.emacs.d/elpa/use-package-20230426.2324/use-package-delight 
hides /usr/local/share/emacs/29.1/lisp/use-package/use-package-delight
/home/cph/.emacs.d/elpa/use-package-20230426.2324/use-package-lint hides 
/usr/local/share/emacs/29.1/lisp/use-package/use-package-lint
/home/cph/.emacs.d/elpa/use-package-20230426.2324/use-package-core hides 
/usr/local/share/emacs/29.1/lisp/use-package/use-package-core
/home/cph/.emacs.d/elpa/use-package-20230426.2324/use-package-diminish 
hides /usr/local/share/emacs/29.1/lisp/use-package/use-package-diminish
/home/cph/.emacs.d/elpa/use-package-20230426.2324/use-package-ensure 
hides /usr/local/share/emacs/29.1/lisp/use-package/use-package-ensure
/home/cph/elisp/xscheme hides 
/usr/local/share/emacs/29.1/lisp/progmodes/xscheme
/home/cph/.emacs.d/elpa/flim-20230808.1153/sasl hides 
/usr/local/share/emacs/29.1/lisp/net/sasl
/home/cph/.emacs.d/elpa/seq-2.24/seq hides 
/usr/local/share/emacs/29.1/lisp/emacs-lisp/seq

Features:
(shadow mail-extr emacsbug pcmpl-gnu goto-addr perl-mode two-column
delsel rect view ucs-normalize novice ibuf-ext wdired macros fileloop
git-rebase markdown-mode eglot external-completion array jsonrpc ert
ewoc flymake-proc flymake c-ts-mode arc-mode archive-mode sort
pcmpl-unix vc-src vc-sccs vc-svn vc-cvs vc-rcs log-view vc bug-reference
magit-extras face-remap magit-submodule magit-blame magit-stash
magit-reflog magit-bisect magit-push magit-pull magit-fetch magit-clone
magit-remote magit-commit magit-sequence magit-notes magit-worktree
magit-tag magit-merge magit-branch magit-reset magit-files magit-refs
magit-status magit magit-repos magit-apply magit-wip magit-log
magit-diff smerge-mode diff magit-core magit-autorevert magit-margin
magit-transient magit-process magit-mode git-timemachine tramp-cmds
rfc2104 conf-mode compare-w pp ispell ibuffer ibuffer-loaddefs vc-hg
vc-bzr tramp-cache time-stamp tramp-sh tramp tramp-loaddefs trampver
tramp-integration tramp-compat tabify man pulse grep compile
display-line-numbers shortdoc misearch multi-isearch help-fns radix-tree
cl-print debug backtrace sh-script executable xscheme files-x mule-util
org-indent org-element org-persist org-id org-refile avl-tree generator
oc-basic ol-eww ol-rmail ol-mhe ol-irc ol-info ol-gnus nnselect gnus-art
mm-uu mml2015 mm-view mml-smime smime gnutls dig gnus-sum gnus-group
gnus-undo gnus-start gnus-dbus dbus gnus-cloud nnimap nnmail mail-source
utf7 nnoo parse-time gnus-spec gnus-int gnus-range gnus-win ol-docview
doc-view jka-compr image-mode exif ol-bibtex bibtex iso8601 ol-bbdb
ol-w3m ol-doi org-link-doi org ob ob-tangle ob-ref ob-lob ob-table
ob-exp org-macro org-src ob-comint org-pcomplete org-list org-footnote
org-faces org-entities noutline outline ob-emacs-lisp ob-core ob-eval
org-cycle org-table ol org-fold org-fold-core org-keys oc org-loaddefs
find-func cal-menu calendar cal-loaddefs org-version org-compat org-macs
mhtml-mode css-mode smie eww xdg url-queue thingatpt shr pixel-fill
kinsoku url-file svg xml mm-url gnus nnheader range wid-edit color
rng-xsd xsd-regexp rng-cmpct rng-nxml rng-valid rng-loc rng-uri
rng-parse nxml-parse rng-match rng-dt rng-util rng-pttrn nxml-ns
nxml-mode nxml-outln nxml-rap sgml-mode facemenu dom nxml-util nxml-enc
xmltok scheme js c-ts-common treesit cc-mode cc-fonts cc-guess cc-menus
cc-cmds cc-styles cc-align cc-engine cc-vars cc-defs dired-aux vc-git
diff-mode vc-dispatcher whitespace zenburn-theme windmove which-func
imenu which-key savehist git-commit magit-git magit-base magit-section
cursor-sensor crm with-editor comp comp-cstr warnings icons shell
pcomplete comint ansi-osc ansi-color transient format-spec server rx
log-edit message sendmail yank-media puny rfc822 mml mml-sec epa derived
epg rfc6068 epg-config gnus-util text-property-search time-date
mm-decode mm-bodies mm-encode mail-parse rfc2231 rfc2047 rfc2045 mm-util
ietf-drums mail-prsvr mailabbrev mail-utils gmm-utils mailheader
pcvs-util add-log compat finder-inf exec-path-from-shell edit-server
advice dumb-jump popup dash s xref project ring dired-x dired
dired-loaddefs desktop frameset cph-commands autorevert filenotify
cl-extra help-mode edmacro kmacro use-package use-package-ensure
use-package-delight use-package-diminish use-package-bind-key bind-key
easy-mmode use-package-core dumb-jump-autoloads edit-server-autoloads
exec-path-from-shell-autoloads git-timemachine-autoloads magit-autoloads
pcase git-commit-autoloads magit-section-autoloads dash-autoloads
markdown-mode-autoloads nov-autoloads esxml-autoloads kv-autoloads
popup-autoloads s-autoloads transient-autoloads w3m-load w3m-autoloads
wanderlust-autoloads semi-autoloads flim-autoloads oauth2-autoloads
apel-autoloads which-key-autoloads with-editor-autoloads info
compat-autoloads seq-autoloads package browse-url url url-proxy
url-privacy url-expand url-methods url-history url-cookie
generate-lisp-file url-domsuf url-util mailcap url-handlers url-parse
auth-source cl-seq eieio eieio-core cl-macs password-cache json subr-x
map byte-opt gv bytecomp byte-compile url-vars cl-loaddefs cl-lib rmc
iso-transl tooltip cconv eldoc paren electric uniquify ediff-hook
vc-hooks lisp-float-type elisp-mode mwheel term/x-win x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode lisp-mode prog-mode register
page tab-bar menu-bar rfn-eshadow isearch easymenu timer select
scroll-bar mouse jit-lock font-lock syntax font-core term/tty-colors
frame minibuffer nadvice seq simple cl-generic indonesian philippine
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite emoji-zwj charscript
charprop case-table epa-hook jka-cmpr-hook help abbrev obarray oclosure
cl-preloaded button loaddefs theme-loaddefs faces cus-face macroexp
files window text-properties overlay sha1 md5 base64 format env
code-pages mule custom widget keymap hashtable-print-readable backquote
threads dbusbind inotify dynamic-setting system-font-setting
font-render-setting cairo x-toolkit xinput2 x multi-tty
make-network-process native-compile emacs)

Memory information:
((conses 16 1000671 332582)
  (symbols 48 42644 63)
  (strings 32 178355 25408)
  (string-bytes 1 6621053)
  (vectors 16 113786)
  (vector-slots 8 2668289 163190)
  (floats 8 662 413)
  (intervals 56 41165 25081)
  (buffers 984 86))

bug#66288: 29.1; Performance regression using pipe for subprocess

[Eli Zaretskii]

> Date: Sat, 30 Sep 2023 20:57:31 -0400
> From: Chris Hanson <cph@chris-hanson.org>
> 
> When using "xscheme.el" to start and interact with MIT/GNU Scheme in a
> subprocess, the performance significantly degraded in Emacs 29.1.  It
> worked well in older releases.
> 
> Here is a recipe:
> 
>      emacs -Q
>      M-x load-library RET xscheme RET
>      M-x run-scheme RET
> 
>      and see how slowly the process output is printed as Scheme starts.
>      Compare this to Emacs 28.2 or earlier.

Please post the comparison as you see it on your system, preferably in
quantitative terms (e.g., time it takes to read and process some chunk
of text in both versions), and using the same version of MIT/GNU
Scheme.

FWIW, I see no changes in xscheme.el between v28.1 and v29.1, except
some minor aesthetic changes and renames of functions.  So I wonder
how come you see a significant slowdown.

bug#66288: 29.1; Performance regression using pipe for subprocess

[Chris Hanson]

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

On 10/1/23 04:39, Eli Zaretskii wrote:
>> Date: Sat, 30 Sep 2023 20:57:31 -0400
>> From: Chris Hanson <cph@chris-hanson.org>
>>
>> When using "xscheme.el" to start and interact with MIT/GNU Scheme in a
>> subprocess, the performance significantly degraded in Emacs 29.1.  It
>> worked well in older releases.
>>
>> Here is a recipe:
>>
>>       emacs -Q
>>       M-x load-library RET xscheme RET
>>       M-x run-scheme RET
>>
>>       and see how slowly the process output is printed as Scheme starts.
>>       Compare this to Emacs 28.2 or earlier.
> 
> Please post the comparison as you see it on your system, preferably in
> quantitative terms (e.g., time it takes to read and process some chunk
> of text in both versions), and using the same version of MIT/GNU
> Scheme.
> 
> FWIW, I see no changes in xscheme.el between v28.1 and v29.1, except
> some minor aesthetic changes and renames of functions.  So I wonder
> how come you see a significant slowdown.

Attached find two screen grabs showing 28.2 and 29.1; you'll see the 
difference is dramatic.  In both cases the same MIT/GNU Scheme version 
was used.  (FYI: I'm the MIT/GNU Scheme maintainer, as well as the 
original author of "xscheme.el".)

I saw that there were no relevant differences in "xscheme.el" but I 
never thought that was relevant.

I believe this has something to do with how piped subprocesses are being 
managed.  I've not looked deeply into the C code for this, but I could 
find no mention of anything to do with pipes in NEWS.

I don't think there's anything funny going on with how MIT/GNU Scheme 
manages stdout but I'll look into it.

[-- Attachment #2: vokoscreenNG-2023-10-01_13-39-22.mkv --]
[-- Type: video/x-matroska, Size: 228051 bytes --]

[-- Attachment #3: vokoscreenNG-2023-10-01_13-40-38.mkv --]
[-- Type: video/x-matroska, Size: 147283 bytes --]

bug#66288: 29.1; Performance regression using pipe for subprocess

[Eli Zaretskii]

> Date: Sun, 1 Oct 2023 14:02:26 -0400
> Cc: 66288@debbugs.gnu.org
> From: Chris Hanson <cph@chris-hanson.org>
> 
> > Please post the comparison as you see it on your system, preferably in
> > quantitative terms (e.g., time it takes to read and process some chunk
> > of text in both versions), and using the same version of MIT/GNU
> > Scheme.
> > 
> > FWIW, I see no changes in xscheme.el between v28.1 and v29.1, except
> > some minor aesthetic changes and renames of functions.  So I wonder
> > how come you see a significant slowdown.
> 
> Attached find two screen grabs showing 28.2 and 29.1; you'll see the 
> difference is dramatic.  In both cases the same MIT/GNU Scheme version 
> was used.  (FYI: I'm the MIT/GNU Scheme maintainer, as well as the 
> original author of "xscheme.el".)

Thanks, but I cannot view these files here.  And viewing them might
change the timing anyway, which is why I prefer that you time this on
your system and provide the numbers with explanations how each number
was measured and what did Emacs do during that time.

> I saw that there were no relevant differences in "xscheme.el" but I 
> never thought that was relevant.
> 
> I believe this has something to do with how piped subprocesses are being 
> managed.  I've not looked deeply into the C code for this, but I could 
> find no mention of anything to do with pipes in NEWS.

Because AFAIK we didn't change anything in that department.

Showing a profile ("M-x profiler-start RET RET", run the slow recipe,
then "M-x profiler-report RET", then post the fully-expanded profile)
could also provide some useful ideas about the source of the issue.

Thanks.

bug#66288: 29.1; Performance regression using pipe for subprocess

[Eli Zaretskii]

> Cc: 66288@debbugs.gnu.org
> Date: Mon, 02 Oct 2023 08:02:14 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > Attached find two screen grabs showing 28.2 and 29.1; you'll see the 
> > difference is dramatic.  In both cases the same MIT/GNU Scheme version 
> > was used.  (FYI: I'm the MIT/GNU Scheme maintainer, as well as the 
> > original author of "xscheme.el".)
> 
> Thanks, but I cannot view these files here.

Btw, for the benefit of those who will be able to view these files:
which one belongs to what Emacs version?

bug#66288: 29.1; Performance regression using pipe for subprocess

[Eli Zaretskii]

> Cc: 66288@debbugs.gnu.org
> Date: Mon, 02 Oct 2023 08:02:14 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > I saw that there were no relevant differences in "xscheme.el" but I 
> > never thought that was relevant.
> > 
> > I believe this has something to do with how piped subprocesses are being 
> > managed.  I've not looked deeply into the C code for this, but I could 
> > find no mention of anything to do with pipes in NEWS.
> 
> Because AFAIK we didn't change anything in that department.

I've now identified 3 changes in Emacs 29 which could potentially
affect your case.  Not sure if they do, but it might be worth your
while to check them first.

First, Emacs 29 uses posix_spawn by default on systems where it is
available and usable.  You will see this fragment at the beginning of
callproc.c:

  /* In order to be able to use `posix_spawn', it needs to support some
     variant of `chdir' as well as `setsid'.  */
  #if defined HAVE_SPAWN_H && defined HAVE_POSIX_SPAWN        \
    && defined HAVE_POSIX_SPAWNATTR_SETFLAGS                  \
    && (defined HAVE_POSIX_SPAWN_FILE_ACTIONS_ADDCHDIR        \
	|| defined HAVE_POSIX_SPAWN_FILE_ACTIONS_ADDCHDIR_NP) \
    && defined HAVE_DECL_POSIX_SPAWN_SETSID                   \
    && HAVE_DECL_POSIX_SPAWN_SETSID == 1			    \
    /* posix_spawnattr_setflags rejects POSIX_SPAWN_SETSID on \
       Haiku */						    \
    && !defined HAIKU
  # include <spawn.h>
  # define USABLE_POSIX_SPAWN 1
  #else
  # define USABLE_POSIX_SPAWN 0
  #endif

If on your system USABLE_POSIX_SPAWN gets the value 1 here, edit
callproc.c to force it to zero, then rebuild Emacs, and see if this
affects the behavior.

Next, we have the following two code fragments in
wait_reading_process_output, which are new in Emacs 29:

Code fragment#1:

      if ((read_kbd
	   /* The following code doesn't make any sense for just the
	      wait_for_cell case, because detect_input_pending returns
	      whether or not the keyboard buffer isn't empty or there
	      is mouse movement.  Any keyboard input that arrives
	      while waiting for a cell will cause the select call to
	      be skipped, and gobble_input to be called even when
	      there is no input available from the terminal itself.
	      Skipping the call to select also causes the timeout to
	      be ignored.  (bug#46935) */
	   /* || !NILP (wait_for_cell) */)
	  && detect_input_pending ())

Code fragment#2:

  #if !defined USABLE_SIGIO && !defined WINDOWSNT
	    /* If we're polling for input, don't get stuck in select for
	       more than 25 msec. */
	    struct timespec short_timeout = make_timespec (0, 25000000);
	    if ((read_kbd || !NILP (wait_for_cell))
		&& timespec_cmp (short_timeout, timeout) < 0)
	      timeout = short_timeout;
  #endif

(I think the second one should not affect you because your system
should have USABLE_SIGIO defined, but maybe I'm mistaken.)  Compare
these with Emacs 28, and try reverting to 28.2 code to see if that
changes anything in your case.

Finally, if you describe in plain English how xscheme.el reads
subprocess output at the stage where you see the slowdown, it might
give further ideas.  I'm not familiar with xscheme.el, and figuring
out which code gets executed when one runs "run-scheme" is not
trivial, so a detailed enough description might help.  Specifically,
how does xscheme.el decide how much of the subprocess's output to read
and display?

bug#66288: 29.1; Performance regression using pipe for subprocess

[Chris Hanson]

The 13-39-22 video is Emacs 29.1; the 13-40-38 video is 28.2.

On 10/2/23 01:07, Eli Zaretskii wrote:
>> Cc: 66288@debbugs.gnu.org
>> Date: Mon, 02 Oct 2023 08:02:14 +0300
>> From: Eli Zaretskii <eliz@gnu.org>
>>
>>> Attached find two screen grabs showing 28.2 and 29.1; you'll see the
>>> difference is dramatic.  In both cases the same MIT/GNU Scheme version
>>> was used.  (FYI: I'm the MIT/GNU Scheme maintainer, as well as the
>>> original author of "xscheme.el".)
>>
>> Thanks, but I cannot view these files here.
> 
> Btw, for the benefit of those who will be able to view these files:
> which one belongs to what Emacs version?

bug#66288: 29.1; Performance regression using pipe for subprocess

[Chris Hanson]

On 10/2/23 01:36, Eli Zaretskii wrote:
>> Cc: 66288@debbugs.gnu.org
>> Date: Mon, 02 Oct 2023 08:02:14 +0300
>> From: Eli Zaretskii <eliz@gnu.org>
>>
>>> I saw that there were no relevant differences in "xscheme.el" but I
>>> never thought that was relevant.
>>>
>>> I believe this has something to do with how piped subprocesses are being
>>> managed.  I've not looked deeply into the C code for this, but I could
>>> find no mention of anything to do with pipes in NEWS.
>>
>> Because AFAIK we didn't change anything in that department.
> 
> I've now identified 3 changes in Emacs 29 which could potentially
> affect your case.  Not sure if they do, but it might be worth your
> while to check them first.
> 
> First, Emacs 29 uses posix_spawn by default on systems where it is
> available and usable.  You will see this fragment at the beginning of
> callproc.c:
> 
>    /* In order to be able to use `posix_spawn', it needs to support some
>       variant of `chdir' as well as `setsid'.  */
>    #if defined HAVE_SPAWN_H && defined HAVE_POSIX_SPAWN        \
>      && defined HAVE_POSIX_SPAWNATTR_SETFLAGS                  \
>      && (defined HAVE_POSIX_SPAWN_FILE_ACTIONS_ADDCHDIR        \
> 	|| defined HAVE_POSIX_SPAWN_FILE_ACTIONS_ADDCHDIR_NP) \
>      && defined HAVE_DECL_POSIX_SPAWN_SETSID                   \
>      && HAVE_DECL_POSIX_SPAWN_SETSID == 1			    \
>      /* posix_spawnattr_setflags rejects POSIX_SPAWN_SETSID on \
>         Haiku */						    \
>      && !defined HAIKU
>    # include <spawn.h>
>    # define USABLE_POSIX_SPAWN 1
>    #else
>    # define USABLE_POSIX_SPAWN 0
>    #endif
> 
> If on your system USABLE_POSIX_SPAWN gets the value 1 here, edit
> callproc.c to force it to zero, then rebuild Emacs, and see if this
> affects the behavior.
> 
> Next, we have the following two code fragments in
> wait_reading_process_output, which are new in Emacs 29:
> 
> Code fragment#1:
> 
>        if ((read_kbd
> 	   /* The following code doesn't make any sense for just the
> 	      wait_for_cell case, because detect_input_pending returns
> 	      whether or not the keyboard buffer isn't empty or there
> 	      is mouse movement.  Any keyboard input that arrives
> 	      while waiting for a cell will cause the select call to
> 	      be skipped, and gobble_input to be called even when
> 	      there is no input available from the terminal itself.
> 	      Skipping the call to select also causes the timeout to
> 	      be ignored.  (bug#46935) */
> 	   /* || !NILP (wait_for_cell) */)
> 	  && detect_input_pending ())
> 
> Code fragment#2:
> 
>    #if !defined USABLE_SIGIO && !defined WINDOWSNT
> 	    /* If we're polling for input, don't get stuck in select for
> 	       more than 25 msec. */
> 	    struct timespec short_timeout = make_timespec (0, 25000000);
> 	    if ((read_kbd || !NILP (wait_for_cell))
> 		&& timespec_cmp (short_timeout, timeout) < 0)
> 	      timeout = short_timeout;
>    #endif
> 
> (I think the second one should not affect you because your system
> should have USABLE_SIGIO defined, but maybe I'm mistaken.)  Compare
> these with Emacs 28, and try reverting to 28.2 code to see if that
> changes anything in your case.

None of the three fragments made any difference.

> Finally, if you describe in plain English how xscheme.el reads
> subprocess output at the stage where you see the slowdown, it might
> give further ideas.  I'm not familiar with xscheme.el, and figuring
> out which code gets executed when one runs "run-scheme" is not
> trivial, so a detailed enough description might help.  Specifically,
> how does xscheme.el decide how much of the subprocess's output to read
> and display?

The process filter has one complexity: it looks for encoded commands 
from the subprocess, which are of the form "ESC <char>" or "ESC <char> 
<string> ESC", depending on the <char>.  There is a small state machine 
to do that, which searches the output string for ESC using 
`string-search'. In this case there are no commands embedded, so that 
should not be relevant.

The ordinary text is inserted into the process buffer using standard 
filter-output code, except it looks for BEL and translates that to 
(beep) if found.  In this case there are no occurrences of BEL in the 
output, so that's not relevant.  So, basically the output string is 
passed to `insert', making sure that process mark and point are updated 
appropriately.

I contrived a small example test and ran it under both editors (see 
below).  It does some printing and then shows the time taken in the 
subprocess.  This should be valid since Scheme will block while waiting 
on Emacs to process the output.

The reported times are in milliseconds, with 28.2 taking 1ms and 29.1 
taking 880ms (increasing the test loop from 20 to 200, the times are 8ms 
and 9974ms respectively).  As I said before, that's pretty dramatic: 
about 3 orders of magnitude.  It feels like that in normal use too -- 
it's like being 30-40 years in the past, when that kind of performance 
was expected.

28.2:
--------------------------------
(show-time
  (lambda ()
    (for-each write-line (iota 20))))
0
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
;process time: 0 (0 RUN + 0 GC); real time: 1
--------------------------------

29.1:
--------------------------------
(show-time
  (lambda ()
    (for-each write-line (iota 20))))
0
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
;process time: 0 (0 RUN + 0 GC); real time: 880
--------------------------------

bug#66288: 29.1; Performance regression using pipe for subprocess

[Gerd Möllmann]

Chris Hanson <cph@chris-hanson.org> writes:

FWIW, I can't reproduce the slowdown on macOS.  I get the

> ;process time: 0 (0 RUN + 0 GC); real time: 1

on the emacs-29 branch with emacs-29.1 and HEAD (same on master).

bug#66288: 29.1; Performance regression using pipe for subprocess

[Dmitry Gutov]

On 02/10/2023 22:12, Gerd Möllmann wrote:
> Chris Hanson<cph@chris-hanson.org>  writes:
> 
> FWIW, I can't reproduce the slowdown on macOS.  I get the
> 
>> ;process time: 0 (0 RUN + 0 GC); real time: 1
> on the emacs-29 branch with emacs-29.1 and HEAD (same on master).

Curious: I reproduced it once (master, an older session), but then not 
anymore, all subsequent attempts look fine/instant.

That's with MIT Scheme 11.2, though (the output is a little different, 
chiefly the list of bindings at the top).

bug#66288: 29.1; Performance regression using pipe for subprocess

[Gerd Möllmann]

Dmitry Gutov <dmitry@gutov.dev> writes:

> On 02/10/2023 22:12, Gerd Möllmann wrote:
>> Chris Hanson<cph@chris-hanson.org>  writes:
>> FWIW, I can't reproduce the slowdown on macOS.  I get the
>> 
>>> ;process time: 0 (0 RUN + 0 GC); real time: 1
>> on the emacs-29 branch with emacs-29.1 and HEAD (same on master).
>
> Curious: I reproduced it once (master, an older session), but then not
> anymore, all subsequent attempts look fine/instant.
>
> That's with MIT Scheme 11.2, though (the output is a little different,
> chiefly the list of bindings at the top).

I've been using 12.1.  Was that macOS?

bug#66288: 29.1; Performance regression using pipe for subprocess

[Dmitry Gutov]

On 02/10/2023 22:40, Gerd Möllmann wrote:
> Dmitry Gutov<dmitry@gutov.dev>  writes:
> 
>> On 02/10/2023 22:12, Gerd Möllmann wrote:
>>> Chris Hanson<cph@chris-hanson.org>   writes:
>>> FWIW, I can't reproduce the slowdown on macOS.  I get the
>>>
>>>> ;process time: 0 (0 RUN + 0 GC); real time: 1
>>> on the emacs-29 branch with emacs-29.1 and HEAD (same on master).
>> Curious: I reproduced it once (master, an older session), but then not
>> anymore, all subsequent attempts look fine/instant.
>>
>> That's with MIT Scheme 11.2, though (the output is a little different,
>> chiefly the list of bindings at the top).
> I've been using 12.1.  Was that macOS?

Linux, of course (it's visible in my User-Agent header, too).

bug#66288: 29.1; Performance regression using pipe for subprocess

[Chris Hanson]

I've been running the tests with a pre-release version of MIT/GNU 
Scheme. But I re-ran them with 11.2 and it had the same behavior as the 
pre-release.

I'm running Ubuntu 22.04 LTS, and I built 28.2 and 29.1 from source (I 
don't usually use the Emacs that comes with the distro). I've also seen 
the slowdown on my laptop with Debian 12, though I haven't done as much 
testing there.

I have an old laptop running macOS; I can try that too.

One other thing comes to mind: is it possible that this is a problem 
with native compilation? I built both 28.2 and 29.1 to use native 
compilation. Did anything relevant change in 29.1?

On 10/2/23 15:27, Dmitry Gutov wrote:
> On 02/10/2023 22:12, Gerd Möllmann wrote:
>> Chris Hanson<cph@chris-hanson.org>  writes:
>>
>> FWIW, I can't reproduce the slowdown on macOS.  I get the
>>
>>> ;process time: 0 (0 RUN + 0 GC); real time: 1
>> on the emacs-29 branch with emacs-29.1 and HEAD (same on master).
> 
> Curious: I reproduced it once (master, an older session), but then not 
> anymore, all subsequent attempts look fine/instant.
> 
> That's with MIT Scheme 11.2, though (the output is a little different, 
> chiefly the list of bindings at the top).

bug#66288: 29.1; Performance regression using pipe for subprocess

[Gerd Möllmann]

Chris Hanson <cph@chris-hanson.org> writes:

> I've been running the tests with a pre-release version of MIT/GNU
> Scheme. But I re-ran them with 11.2 and it had the same behavior as
> the pre-release.
>
> I'm running Ubuntu 22.04 LTS, and I built 28.2 and 29.1 from source (I
> don't usually use the Emacs that comes with the distro). I've also
> seen the slowdown on my laptop with Debian 12, though I haven't done
> as much testing there.
>
> I have an old laptop running macOS; I can try that too.

If I could reproduce this somehow, I could try a git bisect.

Or, if you are building from source already, maybe you could try that?

> One other thing comes to mind: is it possible that this is a problem
> with native compilation? I built both 28.2 and 29.1 to use native
> compilation. Did anything relevant change in 29.1?

I've built emacs-29.1 with native compilation now, and it behaves the
same as without for me.

My system, BTW, is macOS 13.6 Ventura, x86_64, updated with OCLP because
Apple doesn't support my old Macbook Pro anymore.

BTW, I'm always testing with emacs -Q.  Just to be 100% sure - you do
the same?

bug#66288: 29.1; Performance regression using pipe for subprocess

[Eli Zaretskii]

> Date: Mon, 2 Oct 2023 19:23:25 -0400
> Cc: 66288@debbugs.gnu.org, Eli Zaretskii <eliz@gnu.org>,
>  Gerd Möllmann <gerd.moellmann@gmail.com>
> From: Chris Hanson <cph@chris-hanson.org>
> 
> I've been running the tests with a pre-release version of MIT/GNU 
> Scheme. But I re-ran them with 11.2 and it had the same behavior as the 
> pre-release.
> 
> I'm running Ubuntu 22.04 LTS, and I built 28.2 and 29.1 from source (I 
> don't usually use the Emacs that comes with the distro).

Are you building with exactly the same configure- and build-time
options?  Can you show the values of the following variables in both
versions:

  system-configuration
  system-configuration-options
  system-configuration-features
  locale-coding-system

And what does the following produce in both versions:

  emacs -Q
  M-x load-library RET emacsbug RET
  M-: (emacs-build-description) RET

Also, what are the values of the following variables in src/Makefile:

  CFLAGS
  CPPFLAGS
  LDFFLAGS
  C_SWITCH_SYSTEM

> I've also seen the slowdown on my laptop with Debian 12, though I
> haven't done as much testing there.

I guess both of those machines have something in common: your
preferred configuration of the system and its various features and
libraries?

I think at this point, since none of the initial guesses seems to be
correct, running your recipe under perf and looking at the differences
would be our best bet?  That, and bisecting the Git repository.

> One other thing comes to mind: is it possible that this is a problem 
> with native compilation? I built both 28.2 and 29.1 to use native 
> compilation. Did anything relevant change in 29.1?

It's unlikely to matter, but to eliminate this variable, please try
building without native-compilation and see if the difference
persists.

Thanks.

bug#66288: 29.1; Performance regression using pipe for subprocess

[Eli Zaretskii]

> Cc: gerd.moellmann@gmail.com, dmitry@gutov.dev, 66288@debbugs.gnu.org
> Date: Tue, 03 Oct 2023 09:22:44 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > One other thing comes to mind: is it possible that this is a problem 
> > with native compilation? I built both 28.2 and 29.1 to use native 
> > compilation. Did anything relevant change in 29.1?
> 
> It's unlikely to matter, but to eliminate this variable, please try
> building without native-compilation and see if the difference
> persists.

One other thing to try is to disable encoding and decoding of the text
we exchange with the subprocess.  Like this:

  emacs -Q
  M-x load-library RET xscheme RET
  C-x RET c no-conversion RET M-x run-scheme RET

and see if this performs better than what you see by default.

bug#66288: 29.1; Performance regression using pipe for subprocess

[Eli Zaretskii]

> Date: Mon, 2 Oct 2023 14:22:06 -0400
> Cc: 66288@debbugs.gnu.org
> From: Chris Hanson <cph@chris-hanson.org>
> 
> > Finally, if you describe in plain English how xscheme.el reads
> > subprocess output at the stage where you see the slowdown, it might
> > give further ideas.  I'm not familiar with xscheme.el, and figuring
> > out which code gets executed when one runs "run-scheme" is not
> > trivial, so a detailed enough description might help.  Specifically,
> > how does xscheme.el decide how much of the subprocess's output to read
> > and display?
> 
> The process filter has one complexity: it looks for encoded commands 
> from the subprocess, which are of the form "ESC <char>" or "ESC <char> 
> <string> ESC", depending on the <char>.  There is a small state machine 
> to do that, which searches the output string for ESC using 
> `string-search'. In this case there are no commands embedded, so that 
> should not be relevant.
> 
> The ordinary text is inserted into the process buffer using standard 
> filter-output code, except it looks for BEL and translates that to 
> (beep) if found.  In this case there are no occurrences of BEL in the 
> output, so that's not relevant.  So, basically the output string is 
> passed to `insert', making sure that process mark and point are updated 
> appropriately.

Thanks.  It would be good to see the Lisp profiler results for your
recipes, in both versions of Emacs, to understand whether any of these
Lisp parts have anything to do with the issue.  (You say that these
aspects of the processing are not relevant, but maybe they have some
overhead even when the special characters and commands do not appear
in the output?)

bug#66288: 29.1; Performance regression using pipe for subprocess

[Eli Zaretskii]

> Cc: gerd.moellmann@gmail.com, dmitry@gutov.dev, 66288@debbugs.gnu.org
> Date: Tue, 03 Oct 2023 09:22:44 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> Are you building with exactly the same configure- and build-time
> options?  Can you show the values of the following variables in both
> versions:
> 
>   system-configuration
>   system-configuration-options
>   system-configuration-features
>   locale-coding-system
> 
> And what does the following produce in both versions:
> 
>   emacs -Q
>   M-x load-library RET emacsbug RET
>   M-: (emacs-build-description) RET
> 
> Also, what are the values of the following variables in src/Makefile:
> 
>   CFLAGS
>   CPPFLAGS
>   LDFFLAGS
>   C_SWITCH_SYSTEM

None of the above is needed anymore, as they don't seem to be
relevant.

The Lisp profiler seems to point to redisplay as the culprit:
redisplay_internal seems to take most of the time in these recipes.

> I think at this point, since none of the initial guesses seems to be
> correct, running your recipe under perf and looking at the differences
> would be our best bet?  That, and bisecting the Git repository.

This would still be useful.  I'm trying to see what has changed
between Emacs 28 and Emacs 29 in this regard that causes such a
massive slowdown, but bisecting and/or perf data could provide
valuable inputs to guide the search.

> > One other thing comes to mind: is it possible that this is a problem 
> > with native compilation? I built both 28.2 and 29.1 to use native 
> > compilation. Did anything relevant change in 29.1?
> 
> It's unlikely to matter, but to eliminate this variable, please try
> building without native-compilation and see if the difference
> persists.

I've tried this both with and without native-compilation, and the
result is the same.  So native-compilation is not a significant factor
here.

bug#66288: 29.1; Performance regression using pipe for subprocess

[Dmitry Gutov]

On 03/10/2023 09:22, Eli Zaretskii wrote:
> I think at this point, since none of the initial guesses seems to be
> correct, running your recipe under perf and looking at the differences
> would be our best bet?  That, and bisecting the Git repository.

I would start with our native profiler (M-x profiler-start ... M-x 
profiler-report). It's a bit easier to use and interpret.

But if doesn't show anything conclusive, then perf is indeed the next step.

bug#66288: 29.1; Performance regression using pipe for subprocess

[Eli Zaretskii]

> Date: Tue, 3 Oct 2023 20:42:20 +0300
> Cc: 66288@debbugs.gnu.org, gerd.moellmann@gmail.com
> From: Dmitry Gutov <dmitry@gutov.dev>
> 
> On 03/10/2023 09:22, Eli Zaretskii wrote:
> > I think at this point, since none of the initial guesses seems to be
> > correct, running your recipe under perf and looking at the differences
> > would be our best bet?  That, and bisecting the Git repository.
> 
> I would start with our native profiler (M-x profiler-start ... M-x 
> profiler-report). It's a bit easier to use and interpret.

Already done, see my other message.

> But if doesn't show anything conclusive, then perf is indeed the next step.

The problem seems to be in redisplay, so perf and bisecting are the
most relevant tools.

I will meanwhile try to trace the code and find what has changed.
(The usual suspect -- long-line optimizations -- seems to be off the
hook, according to my testing.)

bug#66288: 29.1; Performance regression using pipe for subprocess

[Chris Hanson]

On 10/3/23 13:24, Eli Zaretskii wrote:
> None of the above is needed anymore, as they don't seem to be
> relevant.
> 
> The Lisp profiler seems to point to redisplay as the culprit:
> redisplay_internal seems to take most of the time in these recipes.

Detailed examination of the Lisp profiler output shows that most of the 
redisplay  time is spent in fontification. I don't know if that helps.

>> I think at this point, since none of the initial guesses seems to be
>> correct, running your recipe under perf and looking at the differences
>> would be our best bet?  That, and bisecting the Git repository.
> 
> This would still be useful.  I'm trying to see what has changed
> between Emacs 28 and Emacs 29 in this regard that causes such a
> massive slowdown, but bisecting and/or perf data could provide
> valuable inputs to guide the search.

I've not used the perf-tools before. There seem to be quite a few of 
them, and it's not obvious to me what you want me to run. Any suggestions?

bug#66288: 29.1; Performance regression using pipe for subprocess

[Dmitry Gutov]

On 03/10/2023 22:12, Chris Hanson wrote:
> On 10/3/23 13:24, Eli Zaretskii wrote:
>> None of the above is needed anymore, as they don't seem to be
>> relevant.
>>
>> The Lisp profiler seems to point to redisplay as the culprit:
>> redisplay_internal seems to take most of the time in these recipes.
> 
> Detailed examination of the Lisp profiler output shows that most of the 
> redisplay  time is spent in fontification. I don't know if that helps.

I'd take a look at the profile specifically (it's not always easy to 
interpret).

Also, when you say fontification, does it include specific Lisp 
functions in the output? Ones that are defined in xscheme.el? (JFYI, you 
can press TAB on entries in the report to drill down the call three).

>>> I think at this point, since none of the initial guesses seems to be
>>> correct, running your recipe under perf and looking at the differences
>>> would be our best bet?  That, and bisecting the Git repository.
>>
>> This would still be useful.  I'm trying to see what has changed
>> between Emacs 28 and Emacs 29 in this regard that causes such a
>> massive slowdown, but bisecting and/or perf data could provide
>> valuable inputs to guide the search.
> 
> I've not used the perf-tools before. There seem to be quite a few of 
> them, and it's not obvious to me what you want me to run. Any suggestions?

sudo perf record -p $(pidof emacs)
...
^C
sudo perf report

bug#66288: 29.1; Performance regression using pipe for subprocess

[Gregory Heytings]

>
> The usual suspect -- long-line optimizations --
>

"Usual suspect"???

The culprit here is a947c10d90.

bug#66288: 29.1; Performance regression using pipe for subprocess

[Dmitry Gutov]

On 03/10/2023 23:58, Gregory Heytings wrote:
> 
>>
>> The usual suspect -- long-line optimizations --
>>
> 
> "Usual suspect"???
> 
> The culprit here is a947c10d90.

If it is indeed the problem change (though I wonder about the exact 
mechanics for the slowdown this noticeable), then instead of outright 
reverting it, I suggest trying out patch 0002 from this set:

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=66020#61

bug#66288: 29.1; Performance regression using pipe for subprocess

[Chris Hanson]

I applied patch 0002* as you suggested, and that did the trick!

It's now very fast again, and measures the same as 28.2.

Thanks!

*0002-Remember-the-value-of-read_process_output_max-when-a.patch

On 10/3/23 17:26, Dmitry Gutov wrote:
> On 03/10/2023 23:58, Gregory Heytings wrote:
>>
>>>
>>> The usual suspect -- long-line optimizations --
>>>
>>
>> "Usual suspect"???
>>
>> The culprit here is a947c10d90.
> 
> If it is indeed the problem change (though I wonder about the exact 
> mechanics for the slowdown this noticeable), then instead of outright 
> reverting it, I suggest trying out patch 0002 from this set:
> 
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=66020#61

bug#66288: 29.1; Performance regression using pipe for subprocess

[Gerd Möllmann]

Gregory Heytings <gregory@heytings.org> writes:

> The culprit here is a947c10d90.

👍

That also explains why I don't see anything on macOS.

bug#66288: 29.1; Performance regression using pipe for subprocess

[Eli Zaretskii]

> Date: Tue, 3 Oct 2023 20:33:29 -0400
> Cc: gerd.moellmann@gmail.com, 66288@debbugs.gnu.org
> From: Chris Hanson <cph@chris-hanson.org>
> 
> I applied patch 0002* as you suggested, and that did the trick!
> 
> It's now very fast again, and measures the same as 28.2.
> 
> Thanks!
> 
> *0002-Remember-the-value-of-read_process_output_max-when-a.patch

Thanks for testing.

However, I'm reluctant to install that patch on the release branch.
First, it includes at least one (mostly harmless) error: the call to
fcntl with F_GETPIPE_SZ should use only 2 arguments, not 3.  More
importantly, it is too invasive to my palate, and is still in the
process of patch review.

Given that this is the culprit (thanks, Gregory, for the bisection!),
I conclude that the problem which caused this issue is because modern
Linux kernels use 16 pages (64KB) as the default pipe capacity,
whereas that fcntl call reduced it to just one page of 4KB.
Therefore, one simple change is for xscheme.el to make the value of
read-process-output-max to 64KB.  Chris, can you test that, using the
unmodified 29.1 sources?

A perhaps better change is the one below, which realizes that the
fcntl call was added to create_process for fixing bug#55737 so as to
allow _enlarging_ the pipe size when very large reads are required; it
was never meant to _reduce_ the default size of the pipe:

diff --git a/src/process.c b/src/process.c
index 67d1d3e..8cffc42 100644
--- a/src/process.c
+++ b/src/process.c
@@ -2189,8 +2189,14 @@ create_process (Lisp_Object process, char **new_argv, Lisp_Object current_dir)
       inchannel = p->open_fd[READ_FROM_SUBPROCESS];
       forkout = p->open_fd[SUBPROCESS_STDOUT];
 
-#if defined(GNU_LINUX) && defined(F_SETPIPE_SZ)
-      fcntl (inchannel, F_SETPIPE_SZ, read_process_output_max);
+#if defined(GNU_LINUX) && defined(F_SETPIPE_SZ) && defined(F_GETPIPE_SZ)
+      /* If they requested larger reads than the default system pipe
+         capacity, enlarge the capacity to match the request.  */
+      if (read_process_output_max > fcntl (inchannel, F_GETPIPE_SZ))
+	{
+	  int readmax = clip_to_bounds (1, read_process_output_max. INT_MAX);
+	  fcntl (inchannel, F_SETPIPE_SZ, readmax);
+	}
 #endif
     }
 
Paul, any suggestions or comments?

bug#66288: 29.1; Performance regression using pipe for subprocess

[Eli Zaretskii]

> Date: Tue, 03 Oct 2023 20:58:17 +0000
> From: Gregory Heytings <gregory@heytings.org>
> cc: Dmitry Gutov <dmitry@gutov.dev>, gerd.moellmann@gmail.com, 
>     66288@debbugs.gnu.org, cph@chris-hanson.org
> 
> > The usual suspect -- long-line optimizations --
> >
> 
> "Usual suspect"???

What other significant changes in redisplay, with a potential effect
on performance, were made in Emacs 29?

> The culprit here is a947c10d90.

Thanks.

bug#66288: 29.1; Performance regression using pipe for subprocess

[Gregory Heytings]

>
> Therefore, one simple change is for xscheme.el to make the value of 
> read-process-output-max to 64KB.  Chris, can you test that, using the 
> unmodified 29.1 sources?
>

I'm not Chris, but that works indeed.

>
> A perhaps better change is the one below, which realizes that the fcntl 
> call was added to create_process for fixing bug#55737 so as to allow 
> _enlarging_ the pipe size when very large reads are required; it was 
> never meant to _reduce_ the default size of the pipe:
>

Indeed.

bug#66288: 29.1; Performance regression using pipe for subprocess

[Dmitry Gutov]

On 04/10/2023 09:52, Eli Zaretskii wrote:
> A perhaps better change is the one below, which realizes that the
> fcntl call was added to create_process for fixing bug#55737 so as to
> allow_enlarging_  the pipe size when very large reads are required; it
> was never meant to_reduce_  the default size of the pipe:

Right.

It does most of what's in my patch, and for the rest, okay, let's wait 
for the further review.

bug#66288: 29.1; Performance regression using pipe for subprocess

[Chris Hanson]

On 10/4/23 02:52, Eli Zaretskii wrote:

> Therefore, one simple change is for xscheme.el to make the value of
> read-process-output-max to 64KB.  Chris, can you test that, using the
> unmodified 29.1 sources?

I added a binding of (read-process-output-max 65536) around the call to 
start-process and it solved the problem.

Thank you.

bug#66288: 29.1; Performance regression using pipe for subprocess

[Paul Eggert]

On 10/3/23 23:52, Eli Zaretskii wrote:


> +#if defined(GNU_LINUX) && defined(F_SETPIPE_SZ) && defined(F_GETPIPE_SZ)

A small point: no need to check whether GNU_LINUX is defined; any system 
having these two Linux macros should be Linux-compatible.

> +      /* If they requested larger reads than the default system pipe
> +         capacity, enlarge the capacity to match the request.  */
> +      if (read_process_output_max > fcntl (inchannel, F_GETPIPE_SZ))
> +	{
> +	  int readmax = clip_to_bounds (1, read_process_output_max. INT_MAX);

"." -> "," of course.

> +	  fcntl (inchannel, F_SETPIPE_SZ, readmax);

This call can fail if you aren't root and you exceed the system limit in 
/proc/sys/fs/pipe-max-size. So I suggest that if this fnctl fails with 
EPERM, trying it again after clipping to that limit.

bug#66288: 29.1; Performance regression using pipe for subprocess

[Dmitry Gutov]

On 05/10/2023 01:49, Paul Eggert wrote:
>> +      fcntl (inchannel, F_SETPIPE_SZ, readmax);
> 
> This call can fail if you aren't root and you exceed the system limit in 
> /proc/sys/fs/pipe-max-size. So I suggest that if this fnctl fails with 
> EPERM, trying it again after clipping to that limit.

Perhaps we'd rather fail loudly, so the user is aware that their 
customized value cannot be applied.

bug#66288: 29.1; Performance regression using pipe for subprocess

[Eli Zaretskii]

> Date: Wed, 4 Oct 2023 15:49:00 -0700
> Cc: dmitry@gutov.dev, gregory@heytings.org, gerd.moellmann@gmail.com,
>  66288@debbugs.gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> 
> On 10/3/23 23:52, Eli Zaretskii wrote:
> 
> 
> > +#if defined(GNU_LINUX) && defined(F_SETPIPE_SZ) && defined(F_GETPIPE_SZ)
> 
> A small point: no need to check whether GNU_LINUX is defined; any system 
> having these two Linux macros should be Linux-compatible.
> 
> > +      /* If they requested larger reads than the default system pipe
> > +         capacity, enlarge the capacity to match the request.  */
> > +      if (read_process_output_max > fcntl (inchannel, F_GETPIPE_SZ))
> > +	{
> > +	  int readmax = clip_to_bounds (1, read_process_output_max. INT_MAX);
> 
> "." -> "," of course.
> 
> > +	  fcntl (inchannel, F_SETPIPE_SZ, readmax);
> 
> This call can fail if you aren't root and you exceed the system limit in 
> /proc/sys/fs/pipe-max-size. So I suggest that if this fnctl fails with 
> EPERM, trying it again after clipping to that limit.

Thanks.

I thought to ignore EPERM in this case, at least for the release
branch, since we already have that problem there.  And even for
master, what are the downsides of ignoring EPERM here? accessing
/proc/sys/fs/pipe-max-size is a bit of a hassle (and AFAIU does make
this less portable, since we need support for /proc).

bug#66288: 29.1; Performance regression using pipe for subprocess

[Eli Zaretskii]

> Date: Thu, 5 Oct 2023 01:54:43 +0300
> Cc: gregory@heytings.org, gerd.moellmann@gmail.com, 66288@debbugs.gnu.org
> From: Dmitry Gutov <dmitry@gutov.dev>
> 
> On 05/10/2023 01:49, Paul Eggert wrote:
> >> +      fcntl (inchannel, F_SETPIPE_SZ, readmax);
> > 
> > This call can fail if you aren't root and you exceed the system limit in 
> > /proc/sys/fs/pipe-max-size. So I suggest that if this fnctl fails with 
> > EPERM, trying it again after clipping to that limit.
> 
> Perhaps we'd rather fail loudly, so the user is aware that their 
> customized value cannot be applied.

Why not silently?  The clip_to_bounds call can already silently change
the original request anyway.

bug#66288: 29.1; Performance regression using pipe for subprocess

[Dmitry Gutov]

On 05/10/2023 08:50, Eli Zaretskii wrote:
>> Date: Thu, 5 Oct 2023 01:54:43 +0300
>> Cc:gregory@heytings.org,gerd.moellmann@gmail.com,66288@debbugs.gnu.org
>> From: Dmitry Gutov<dmitry@gutov.dev>
>>
>> On 05/10/2023 01:49, Paul Eggert wrote:
>>>> +      fcntl (inchannel, F_SETPIPE_SZ, readmax);
>>> This call can fail if you aren't root and you exceed the system limit in
>>> /proc/sys/fs/pipe-max-size. So I suggest that if this fnctl fails with
>>> EPERM, trying it again after clipping to that limit.
>> Perhaps we'd rather fail loudly, so the user is aware that their
>> customized value cannot be applied.
> Why not silently?  The clip_to_bounds call can already silently change
> the original request anyway.

clip_to_bounds clips to 4294967296 or higher, if I'm not mistaken. And 
my current system-wide limit is 1048576.

I usually like to know when my applied configuration is not used (or 
use-able). Anyway, it's not a hard requirement (not for emacs-29 anyway).

bug#66288: 29.1; Performance regression using pipe for subprocess

[Eli Zaretskii]

> Date: Thu, 5 Oct 2023 13:48:55 +0300
> Cc: eggert@cs.ucla.edu, cph@chris-hanson.org, gregory@heytings.org,
>  gerd.moellmann@gmail.com, 66288@debbugs.gnu.org
> From: Dmitry Gutov <dmitry@gutov.dev>
> 
> On 05/10/2023 08:50, Eli Zaretskii wrote:
> >> Date: Thu, 5 Oct 2023 01:54:43 +0300
> >> Cc:gregory@heytings.org,gerd.moellmann@gmail.com,66288@debbugs.gnu.org
> >> From: Dmitry Gutov<dmitry@gutov.dev>
> >>
> >> On 05/10/2023 01:49, Paul Eggert wrote:
> >>>> +      fcntl (inchannel, F_SETPIPE_SZ, readmax);
> >>> This call can fail if you aren't root and you exceed the system limit in
> >>> /proc/sys/fs/pipe-max-size. So I suggest that if this fnctl fails with
> >>> EPERM, trying it again after clipping to that limit.
> >> Perhaps we'd rather fail loudly, so the user is aware that their
> >> customized value cannot be applied.
> > Why not silently?  The clip_to_bounds call can already silently change
> > the original request anyway.
> 
> clip_to_bounds clips to 4294967296 or higher, if I'm not mistaken. And 
> my current system-wide limit is 1048576.
> 
> I usually like to know when my applied configuration is not used (or 
> use-able). Anyway, it's not a hard requirement (not for emacs-29 anyway).

OK, I installed the change on the emacs-29 branch, and I'm therefore
closing this bug.