Re: [PATCH] fix threaded grep for machines with only one cpu
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Feb 17, 2010, 01:01 UTC
- Message-ID
- <alpine.DEB.1.00.1002170200370.20986@pacific.mpi-cbg.de>
- In-Reply-To
- <7viq9wzq8g.fsf@alter.siamese.dyndns.org>
Hi,
On Tue, 16 Feb 2010, Junio C Hamano wrote:
Show 35 quoted lines
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> >> diff --git a/builtin-grep.c b/builtin-grep.c
> >> index 26d4deb..5c1545e 100644
> >> --- a/builtin-grep.c
> >> +++ b/builtin-grep.c
> >> @@ -81,8 +81,8 @@ static pthread_mutex_t read_sha1_mutex;
> >>
> >> #define grep_lock() pthread_mutex_lock(&grep_mutex)
> >> #define grep_unlock() pthread_mutex_unlock(&grep_mutex)
> >> -#define read_sha1_lock() pthread_mutex_lock(&read_sha1_mutex)
> >> -#define read_sha1_unlock() pthread_mutex_unlock(&read_sha1_mutex)
> >> +#define read_sha1_lock() do { if (use_threads) pthread_mutex_lock(&read_sha1_mutex); } while (0)
> >> +#define read_sha1_unlock() do { if (use_threads) pthread_mutex_unlock(&read_sha1_mutex); } while (0)
> >
> > This is inconsistent. Just look at the code above and tell me why it is so
> > different.
>
> It is because grep_mutex is protected by "use_threads" very high in the
> callchain and do not need nor want extra if().
>
> But I think this is much cleaner. The patch replaces the one you are
> replying to (i.e. read_sha1_{lock,unlock}() are unconditional).
>
> -- >8 --
> Subject: Fix use of mutex in threaded grep
>
> The program can decide at runtime not to use threading even if the support
> is compiled in. In such a case, mutexes are not necessary and left
> uninitialized. But the code incorrectly tried to take and release the
> read_sha1_mutex unconditionally.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> Acked-by: Fredrik Kuivinen <frekui@gmail.com>
> ---Yes, this one looks much, much nicer.
Ciao, Dscho