Thread (2 messages) flat view 2 messages, 2 authors, 2026-01-22

Re: [PATCH v7 10/12] run-command: poll child stdin in addition to stdout

From: Junio C Hamano <hidden>
Date: 2026-01-21 23:11:26

Adrian Ratiu [off-list ref] writes:
quoted hunk
Child input feeding might hit the 100ms output poll timeout as a
side-effect of the ungroup=0 design when feeding multiple children
in parallel and buffering their outputs.

This throttles the write throughtput as reported by Kristoffer.

Peff also noted that the parent might block if the write pipe is full
and cause a deadlock if both parent + child wait for one another.

Thus we refactor the run-command I/O loop so it polls on both child
input and output fds to eliminate the risk of artificial 100ms
latencies and unnecessarily blocking the main process.

This ensures that parallel hooks are fed data ASAP while maintaining
responsiveness for (sideband) output.

It's worth noting that in our current design, sequential execution
is not affected by this because it still uses the ungroup=1 behavior.

Reported-by: Kristoffer Haugsbakk <redacted>
Suggested-by: Jeff King <redacted>
Signed-off-by: Adrian Ratiu <redacted>
---
 run-command.c | 61 ++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 43 insertions(+), 18 deletions(-)
diff --git a/run-command.c b/run-command.c
index aaf0e4ecee..dfd2aeda07 100644
--- a/run-command.c
+++ b/run-command.c
@@ -1562,7 +1562,7 @@ static void pp_init(struct parallel_processes *pp,
 
 	CALLOC_ARRAY(pp->children, n);
 	if (!opts->ungroup)
-		CALLOC_ARRAY(pp->pfd, n);
+		CALLOC_ARRAY(pp->pfd, n * 2);
 
 	for (size_t i = 0; i < n; i++) {
 		strbuf_init(&pp->children[i].err, 0);
@@ -1707,21 +1707,52 @@ static void pp_buffer_stdin(struct parallel_processes *pp,
 	}
 }
 
-static void pp_buffer_stderr(struct parallel_processes *pp,
-			     const struct run_process_parallel_opts *opts,
-			     int output_timeout)
+static void pp_buffer_io(struct parallel_processes *pp,
+			 const struct run_process_parallel_opts *opts,
+			 int timeout)
 {
-	while (poll(pp->pfd, opts->processes, output_timeout) < 0) {
+	/* for each potential child slot, prepare two pollfd entries */
+	for (size_t i = 0; i < opts->processes; i++) {
+		if (child_is_working(&pp->children[i]) &&
+		    pp->children[i].process.err > 0) {
I find it a bit odd to insist that .err must be positive integer for
us to decide including it in the polled fd set.  Are we safe to
assume that .err file descriptor in use can never be 0 because we do
not close stdin in the main process anyway?
+			pp->pfd[2*i].fd = pp->children[i].process.err;
+			pp->pfd[2*i].events = POLLIN | POLLHUP;
+		} else {
+			pp->pfd[2*i].fd = -1;
+		}
+
+		if (child_is_receiving_input(&pp->children[i])) {
+			pp->pfd[2*i+1].fd = pp->children[i].process.in;
+			pp->pfd[2*i+1].events = POLLOUT;
+		} else {
+			pp->pfd[2*i+1].fd = -1;
+		}
+	}
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help