Re: [PATCHv4 06/14] run-command: add an asynchronous parallel child processor

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Stefan Beller <sbeller@xxxxxxxxxx> writes:

> +static void pp_buffer_stderr(struct parallel_processes *pp)
> +{
> +	int i;
> +
> +	while ((i = poll(pp->pfd, pp->max_processes, 100)) < 0) {
> +		if (errno == EINTR)
> +			continue;
> +		pp_cleanup(pp);
> +		die_errno("poll");
> +	}
> +
> +	/* Buffer output from all pipes. */
> +	for (i = 0; i < pp->max_processes; i++) {
> +		if (pp->children[i].in_use &&
> +		    pp->pfd[i].revents & POLLIN)
> +			if (strbuf_read_once(&pp->children[i].err,
> +					     pp->children[i].process.err, 0) < 0)
> +				if (errno != EAGAIN)
> +					die_errno("read");
> +	}
> +}

I think it is a good thing that the caller is passing the whole pp
to this function.  One thing you may want to consider is to adjust
the poll(2) timeout longer when the process slots are full.

There is nothing you can gain by returning early due to timeout
without doing anything from this function when you know you cannot
start a new process (here, I am assuming that your poll(2) would be
unblocked for a disconnect when one of the processes exits, letting
you return and letting the caller call collect_finished(), which in
turn would allow us to make some progress).

On the other hand, during the early ramp-up period, you may want to
use a shorter poll(2) timeout to give the caller a chance to spawn
more processes sooner.  But that falls into performance tuning that
can and should be left to a later follow-up patch after we get the
basic machinery right.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]