Re: [PATCHv3 06/13] run-command: add an asynchronous parallel child processor

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

 



On Tue, Sep 22, 2015 at 2:41 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Stefan Beller <sbeller@xxxxxxxxxx> writes:
>
>> On Tue, Sep 22, 2015 at 12:53 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>>> Stefan Beller <sbeller@xxxxxxxxxx> 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 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]