Re: [PATCH 2/5] thread-utils: add a threaded task queue

[Stefan Beller <sbeller@google.com>]

On Thu, Aug 27, 2015 at 5:59 AM, Johannes Schindelin
<johannes.schindelin@gmx.de> wrote:
> Hi Stefan,
>
> On 2015-08-27 02:52, Stefan Beller wrote:
>
>> diff --git a/run-command.c b/run-command.c
>> index 28e1d55..cb15cd9 100644
>> --- a/run-command.c
>> +++ b/run-command.c
>> @@ -668,6 +668,22 @@ int git_atexit(void (*handler)(void))
>>
>>  #endif
>>
>> +void setup_main_thread(void)
>> [...]
>>
>> diff --git a/thread-utils.c b/thread-utils.c
>> index a2135e0..b45ab92 100644
>> --- a/thread-utils.c
>> +++ b/thread-utils.c
>> [...]
>> +/* FIXME: deduplicate this code with run-command.c */
>> +static void setup_main_thread(void)
>
> Do you remember off-hand why the code could not be moved to thread-utils.c wholesale? Just curious.

The code in run-command has a few things regarding the struct async
handling in there,
which we don't  need/want. I just realized there is some duplicate
code, but I couldn't cut
it clearly out.

>
>> +#else /* NO_PTHREADS */
>> +
>> +struct task_queue {
>> +     int early_return;
>> +};
>> +
>> +struct task_queue *create_task_queue(unsigned max_threads)
>> +{
>> +     struct task_queue *tq = xmalloc(sizeof(*tq));
>> +
>> +     tq->early_return = 0;
>> +}
>> +
>> +void add_task(struct task_queue *tq,
>> +           int (*fct)(struct task_queue *tq, void *task),
>
> Might make sense to typedef this... Maybe task_t?
>
>> +           void *task)
>> +{
>> +     if (tq->early_return)
>> +             return;
>
> Ah, so "early_return" actually means "interrupted" or "canceled"?

The early_return is meant to return early in case of an error in some thread.
In the threaded version, the `dispatcher` is executed in each thread.
It gets its
new tasks via `next_task`, which takes the early_return value from the thread,
ORs it into the early_return of the task queue (which all threads have
access to).
So by the ORing into the task queues early_return the signal to abort
early is propagated to a place all threads have access to. And in case
that value is
set, the `next_task` will return NULL as an indication to cleanup and
pthread_exit.

>
> I guess I will have to set aside some time to wrap my head around the way tasks are handled here, in particular how the two `early_return` variables (`dispatcher()`'s local variable and the field in the `task_queue`) interact.
>
> Thanks!
> Dscho