GNU bug report logs - #79334
[PATCH] Don't release thread select lock unnecessarily

Previous Next

Full log

Message #29 received at 79334 <at> debbugs.gnu.org (full text, mbox):

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 79334 <at> debbugs.gnu.org, eggert <at> cs.ucla.edu, Dmitry Gutov <dmitry <at> gutov.dev>
Subject: Re: bug#79334: [PATCH] Don't release thread select lock unnecessarily
Date: Fri, 29 Aug 2025 11:43:21 -0400

[Message part 1 (text/plain, inline)]

On Fri, Aug 29, 2025, 11:41 AM Eli Zaretskii <eliz <at> gnu.org> wrote:

> > From: Spencer Baugh <sbaugh <at> janestreet.com>
> > Cc: 79334 <at> debbugs.gnu.org,  eggert <at> cs.ucla.edu,  dmitry <at> gutov.dev
> > Date: Fri, 29 Aug 2025 10:49:34 -0400
> >
> > Eli Zaretskii <eliz <at> gnu.org> writes:
> >
> > >> Primarily: Calling thread_select here complicates reasoning about the
> > >> code.  We should not release the lock when not necessary, because it
> > >> makes it harder to think about possible thread interleavings.  And
> it's
> > >> not necessary here because this pselect call returns immediately.
> > >
> > > How do we define "necessary" in this context?
> >
> > It's necessary to release the lock around long pselect calls because
> > otherwise other threads will not run while Emacs is waiting for input,
> > which means they probably won't run at all.
> >
> > >> (The only reason the pselect call wouldn't return immediately is if we
> > >> released the lock and then switched to another thread)
> > >
> > > You seem to assume that a call to thread_select is only necessary when
> > > some potentially prolonged operation or wait is expected?  But I'm not
> > > sure this is correct, because that could prevent other threads from
> > > running for too long, and force programmers to inject sleep-for
> > > etc. in their programs.  By allowing thread switches as frequently as
> > > possible lets other threads more opportunities to run (to some extent
> > > simulating preemptive scheduling), which I think is a Good Thing
> > > overall, no?
> >
> > Three points:
> >
> > - Most importantly: We can only allow thread switches at known safe
> >   points.  It's not clear that this is a safe point.  (Actually, we know
> >   it currently isn't, because of the status_notify issue.  But I suspect
> >   there may be other issues which are harder to analyze)
>
> This reason will be removed once we fix status_notify.
>
> > - Adding more thread switches is not necessarily a good thing.  It can
> >   degrade whole-system performance, because context switching is slow.
> >
> > - wait_reading_process_output already does a thread switch around the
> >   pselect, so it's not important for one to happen here.
>
> You basically give the same single reason in several different
> wordings.
>
> I see your point, but my experience with threads in Emacs is the
> opposite: there seem to be too few thread switch opportunities, so
> many programs don't work as expected unless one sprinkles sleep-for in
> various places.  For example, try to run something in a non-main
> thread and then attempt to interact with Emacs in the main thread.
>
> IOW, it is quite easy for a Lisp program to hog the global lock and
> starve the other threads, so each additional thread-switch opportunity
> is IME a blessing, not a curse.
>

Okay, so then how about we delete this unsafe thread_select and put some
thread-yield calls around wait_reading_process_output?

>

[Message part 2 (text/html, inline)]

Previous Next

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