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

From: Junio C Hamano <[email-masked]>
Date: Wed, 23 Sep 2015 07:59:36 -0700

Junio C Hamano <[email-masked]> writes:

> Stefan Beller <[email-masked]> 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.
> ...
> 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.

Just to make sure there is no misunderstanding, just like I prefer
"start one" over "start as many as possible" in order to give
scheduling decision to the calling loop, I would expect that such a
follow-up performance tuning would be done by adding another field
to pp tht lets the caller of this function, the overall scheduling
loop, to tweak the timeout used, without this function making the
decision locallly.

Thanks.