On 5/18/21 8:11 AM, Jeff King wrote:
On Tue, May 18, 2021 at 07:21:33AM -0400, Jeff Hostetler wrote:
In the simple-ipc API, there's an explicit "async" interface. But it's
not clear to me how rich it expects the communication with the caller to
be (i.e., whether we could get away with the fork() trick here). Or if
it would be OK for the threading to remain an implementation detail,
with one "worker" upon whom we wait for completion.
TBH I forgot that we still support NO_PTHREAD systems.
I seem to remember that we got rid of some of the non-pthread
stub functions at one point, but I'm fuzzy on the details.
You're probably thinking of when we got rid of a bunch of #ifdef code
paths in index-pack, and replaced it with stubs that turn the pthread
calls into "do nothing" (so all the ugly stuff is in thread-utils.h
now). But we still very much support systems that don't handle pthreads
at all.
WRT to "simple ipc" (and future "builtin fsmonitor"), it's heavily
threaded. There's no point in trying to fake it with forks.
The server side of simple ipc implements a thread pool. And
the builtin fsmonitor will use a thread to monitor FS events
and that thread pool to respond to clients. All driven from a
shared queue of events.
It would be a major overhaul to do all that without threads
-- and even that assumes that nonstop has a sufficient file
system notification mechanism.
OK, that matches my guess from a brief look at the code. Thanks for
confirming.
So, yes, we should ifdef it out as Peff suggests.
The patch I sent wasn't really tested beyond confirming that "make
NO_PTHREADS=1" finished compiling (and that test-tool simple-ipc
barfed appropriately at runtime).
Do you want to pick it up from there and produce a polished patch? I
think we should deal with this prior to the v2.32.0 release (and thanks
Randall for testing and finding it during the -rc0 period).
-Peff
yeah, i'll take it from here and get a patch out today.
Thanks!
Jeff
