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

Previous Next

Full log

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

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

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

Previous Next

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