On Tue, Sep 22, 2015 at 2:41 PM, Junio C Hamano <[email protected]> wrote:
> Stefan Beller <[email protected]> writes:
>
>> On Tue, Sep 22, 2015 at 12:53 PM, Junio C Hamano <[email protected]> wrote:
>>> Stefan Beller <[email protected]> writes:
>>>
>>>> So how would you find out when we are done?
>>>
>>> while (1) {
>>> if (we have available slot)
>>> ask to start a new one;
>>> if (nobody is running anymore)
>>> break;
>>> collect the output from running ones;
>>> drain the output from the foreground one;
>>> cull the finished process;
>>> }
>>>
>>
>> Personally I do not like the break; in the middle of
>> the loop, but that's personal preference, I'd guess.
>> I'll also move the condition for (we have available slot)
>> back inside the called function.
>>
>> So I am thinking about using this in the reroll instead:
>>
>> run_processes_parallel_start_as_needed(&pp);
>> while (pp.nr_processes > 0) {
>> run_processes_parallel_buffer_stderr(&pp);
>> run_processes_parallel_output(&pp);
>> run_processes_parallel_collect_finished(&pp);
>> run_processes_parallel_start_as_needed(&pp);
>> }
>
> If you truly think the latter is easier to follow its logic (with
> the duplicated call to the same function only to avoid break that
> makes it clear why we are quitting the loop,
I dislike having the call twice, too.
> and without the
> explicit "start only if we can afford to"), then I have to say that
> our design tastes are fundamentally incompatible.
Well the "start only if we can afford to" is not as easy as just
> if (we have available slot)
> ask to start a new one;
because that's only half the condition. If we don't start a new one
as the callback get_next_task returned without a new task.
So it becomes
while (pp.nr_processes > 0) {
while (pp->nr_processes < pp->max_processes &&
start_one_process(&pp))
; /* nothing */
if (!pp.nr_processes)
break;
buffer(..);
output_live_child(..);
cull_finished(..);
}
I'll think about that.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
