> Johannes Sixt <j6t@xxxxxxxx> writes:
> This is the pessimization that I am talking about. I would not mind at all if
> it were only for the attribute subsystem, but the proposed patch would
> pessimize *all* uses of pthread_mutex_lock.
It would only pessimize *uninitialized* mutexes? For initialized mutexes
the added burden is super cheap (one additional condition).
The positive aspect of this way the patch proposes would be that any
future contributor not knowing the details of how to do mutexes right
on Windows, would not totally break the whole system, i.e. this seems
to be more maintainable in the future as it reduces the friction between
pthreads mutexes and the way we can do things in Git in a platform
independent way
On Wed, Oct 26, 2016 at 11:33 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>>
>> Lazy on-demand initialization as needed, perhaps? The on-demand
>> initialization mechanism may become no-op on some platforms that can
>> do static initialization.
>
> Ah, I think I misunderstood your "please rewrite". Did you mean to
> add "void attr_start(void)" helper function to attr.c that does
> series of pthread_mutex_init() calls as needed?
Well one init for now.
> That function can
> be called from main() of platforms that cannot statically initialize
> mutices,
By main you mean the main() in common-main.c or cmd_main in git.c ?
Those both look like the wrong place. Of course it would work adding it
there, but it smells like a maintenance nightmare.
And then we would modify the attr_start command depending on the
platform, i.e.
#ifdef WIN32
void attr_start(void)
{
pthread_mutex_init(..);
}
#else
void attr_start(void)
{
/* nothing as it is statically init'd */
}
#endif
> while on other platforms it can be a no-op as long as the
> variables are statically initialized? If so, that would not pessimize
> any platform, I would think.
I would think this pessimizes ALL platforms from a maintenance perspective
(but what do I know about maintaining stuff as an eager young contributor ;)
So I am willing to go that route, though I do not quite understand where exactly
you'd expect me to put the initializer as all places I can think of
are not the right
place.
Thanks,
Stefan