Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API

From: Andrzej K. Haczewski <ahaczewski@gmail.com>
Date: Nov 4, 2009, 21:16 UTC
Message-ID: <16cee31f0911041316n20fc9f12s6595dadc813d8f46@mail.gmail.com>
In-Reply-To: <alpine.LFD.2.00.0911041247250.10340@xanadu.home>

2009/11/4 Nicolas Pitre <nico@fluxnic.net>:

Show 9 quoted lines

> On Wed, 4 Nov 2009, Andrzej K. Haczewski wrote:
>
>> +     NO_STATIC_PTHREADS_INIT = YesPlease
>
> Let's not go that route please.  If Windows can't get away without
> runtime initializations then let's use them on all platforms.  There is
> no gain in exploding the code path combinations here wrt testing
> coverage.
>

I don't like that approach either, but I was frighten of Junio being anal about static inits ;).

Let's make it clear: has anyone have any objections that I add explicit initialization of mutexes and condition variables for POSIX also?

>> +static THREAD_RET_TYPE threaded_find_deltas(void *arg)
>
> Why can't you just cast the function pointer in your pthread_create
> wrapper instead?  No one cares about the returned value anyway.

Because of calling convention - I'd have to cast cdecl function as stdcall function, which would change the function call clean up (in cdecl caller is responsible for unwinding stack, stdcall callee; the effect - double stack unwinding).

Show 12 quoted lines

>> @@ -2327,6 +2354,8 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
>>  #ifdef THREADED_DELTA_SEARCH
>>       if (!delta_search_threads)      /* --threads=0 means autodetect */
>>               delta_search_threads = online_cpus();
>> +
>> +     init_threaded_delta_search();
>
> What about doing this at the beginning of ll_find_deltas() instead?
> And similarly for cleanup_threaded_delta_search(): call it right before
> leaving ll_find_deltas().  This way thread issues would remain more
> localized.  In fact I'd move the whole thing above in ll_find_deltas()
> as well (separately from this patch though).

Sounds sensible, but I'd wait for the NO_STATIC_PTHREADS_INIT verdict.

-- Andrzej