Re: [PATCH] fix overslow :/no-such-string-ever-existed diagnostics

From: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date: May 10, 2011, 20:41 UTC
Message-ID: <vpqfwomffuy.fsf@bauges.imag.fr>
In-Reply-To: <7vzkmuidk1.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> On my box, the command spends ~1.8 seconds without the patch to make the
> report; with the patch it spends ~1.12 seconds.

Nice improvement.

>  * Yes, we often say "this is an error path and paying the price in
>    performance for a better diagnosis is worth", and it is correct.
>    But it is not an excuse to spend unnecessary cycles.

Right. When I introduced the second call with gently=0, I was thinking of cases where the function is fast, but :/blah has to walk through history, and we don't want that twice (especially since the conclusion is to give generic and not-so-helpfull message!).

>    We may want to add a guard at the beginning of die_verify_filename() to
>    omit the extra call to get_sha1_with_mode_1(only_to_die=1) when arg
>    looks like a magic pathspec, i.e. ":" followed by anything !isalnum().

Is this a complement or an alternative? It seems to me that your other patch makes this first one useless (in the sense that the second call is always cheap), and avoids complexifying the code of get_sha1_with_mode_1 for the first call.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/