all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: Spencer Baugh <sbaugh@janestreet.com>
Cc: emacs-devel@gnu.org
Subject: Re: call-process should not block process filters from running
Date: Sun, 02 Jul 2023 08:45:42 +0300	[thread overview]
Message-ID: <837criq321.fsf@gnu.org> (raw)
In-Reply-To: <iermt0fphkc.fsf@janestreet.com> (message from Spencer Baugh on Sat, 01 Jul 2023 15:17:39 -0400)

> From: Spencer Baugh <sbaugh@janestreet.com>
> Date: Sat, 01 Jul 2023 15:17:39 -0400
> 
> > We read from pipe in chunks, and my idea was to leave the reading code
> > intact, just call wait_reading_process_output each iteration through
> > the reading loop.  Long-running call-process calls spend their time in
> > this loop, reading the process output that takes long time to produce.
> > So that's where I envision most of the gains will come from.
> 
> That's certainly better than nothing, but that won't run other Lisp
> while the call-process process is not producing output.  Just as one
> degenerate example, call-process on "sleep 30" won't run any Lisp.  More
> realistically, expensive computations which take a while to produce
> output ("factor $BIGNUM" for example) will not run much Lisp.  From
> looking at call-process usage, I think most are of that form.  So I
> don't think this will solve the problem.

My suggestion is to try first and only reject this idea if it indeed
is not useful in practice.

The "sleep 30" example is not interesting, IMO: Emacs has sleep-for
which will do the same while still reading from subprocesses and
letting timers run, so if a Lisp program calls "sleep 30" it probably
actually wants Emacs to do nothing during that time.

So IMO we should find interesting practical use cases which run
long-running programs via call-process, and see what this idea gives
us in those cases, before deciding whether it makes sense.  To see
whether the idea "holds water", it is not necessary to implement it in
its entirety: it is enough to count the number of iterations through
the loop that reads the output (perhaps also examining the effect of
making CALLPROC_BUFFER_SIZE_MIN smaller), because this will tell us
how many opportunities for calling wait_reading_process_output we will
have.

> > As for waitpid, I'd initially leave that alone, and only add the
> > wait_reading_process_output to the loop that protects that from
> > signals.  IME, by the time we get to waitpid after reading the last
> > portion of the process output, the process have either already exited
> > or will do so almost instantly.  If we discover this is not so, we
> > could later replace waitpid with some wait with timeout in a loop.
> 
> I see.  But if that's the case, then that's not really any better than
> just calling reading input and running other Lisp after that loop is
> done, which of course we already do.  Because, if waitpid returns
> immediately anyway, it doesn't really matter if we run Lisp before it or
> run Lisp after it.

That's what I'm saying, yes.  But if we find important cases where a
program sends EOF to its stdout and then proceeds with some lengthy
processing (which then forces us to wait in waitpid for a long time),
we could emulate waitpid using different APIs that wait with timeouts.

> > I didn't mean to throw away the reading loop and waitpid, because
> > doing so will require to write a lot of non-trivial C code, and goes
> > against what I consider to be the main advantage of my idea.
> 
> Yes, I understand that you don't want to introduce bugs into
> call-process.  IMO that's why making a completely new function instead
> of modifying call-process is a better bet.

It might be a better bet from the POV of not breaking call-process,
but it still runs the risk that the new API will have bugs which
call-process doesn't.  If we want the new API to be attractive, this
risk is still important and real.

> But if you would prefer a modification to call-process I'm quite happy
> to write non-trivial C code, as long as the problem actually gets
> solved.

I prefer that we try the minimal change I described, and only consider
more complicated solutions if that idea turns out to be not useful
enough.



  reply	other threads:[~2023-07-02  5:45 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-27 21:55 call-process should not block process filters from running Spencer Baugh
2023-06-28 11:39 ` Mattias Engdegård
2023-06-28 11:56 ` Po Lu
2023-06-28 12:08   ` Spencer Baugh
2023-06-28 13:17     ` Po Lu
2023-06-28 12:52 ` Eli Zaretskii
2023-06-28 13:27   ` Spencer Baugh
2023-06-28 13:34     ` Eli Zaretskii
2023-07-01 18:24     ` Spencer Baugh
2023-07-01 18:59       ` Eli Zaretskii
2023-07-01 19:17         ` Spencer Baugh
2023-07-02  5:45           ` Eli Zaretskii [this message]
2023-07-03  0:02             ` sbaugh
2023-07-03 10:00               ` Po Lu
2023-07-03 17:53                 ` sbaugh
2023-07-03 18:51                   ` Eli Zaretskii
2023-07-03 20:28                     ` sbaugh
2023-07-04  4:12                       ` Po Lu
2023-07-04 11:25                         ` Eli Zaretskii
2023-07-04 12:42                         ` sbaugh
2023-07-04 13:42                           ` Michael Albinus
2023-07-04 14:16                             ` sbaugh
2023-07-05  6:36                               ` Michael Albinus
2023-07-04 11:10                       ` Eli Zaretskii
2023-07-04 12:20                         ` sbaugh
2023-07-04 13:09                           ` Eli Zaretskii
2023-07-04 13:37                             ` sbaugh
2023-07-04 13:25                           ` Po Lu
2023-07-04  1:04                     ` sbaugh
2023-07-04  4:09                       ` Po Lu
2023-07-04 12:27                         ` sbaugh
2023-07-04 13:22                           ` Po Lu
2023-07-04 13:51                             ` sbaugh
2023-07-04 16:38                               ` Eli Zaretskii
2023-07-04 16:53                                 ` sbaugh
2023-07-04 17:14                                   ` Eli Zaretskii
2023-07-04 16:49               ` Dmitry Gutov
2023-07-04 18:12                 ` sbaugh
2023-07-05 18:53                   ` Dmitry Gutov
2023-07-06  2:24                     ` sbaugh
2023-07-06  8:06                       ` Michael Albinus
2023-07-08 15:54                         ` sbaugh
2023-07-09  9:04                           ` Michael Albinus

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=837criq321.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=emacs-devel@gnu.org \
    --cc=sbaugh@janestreet.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.