Re: performance problem: "git commit filename"

[Linus Torvalds <torvalds@linux-foundation.org>]

On Sun, 13 Jan 2008, Junio C Hamano wrote:
> 
> I've reworked the patch, and in the kernel repository, a
> single-path commit after touching that path now calls 23k
> lstat(2).  It used to call 46k lstat(2) after your fix.

Hmm. This part of it looks incorrect:

> diff --git a/diff.c b/diff.c
> index b18c140..62d0c06 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -1510,6 +1510,10 @@ static int reuse_worktree_file(const char *name, const unsigned char *sha1, int
>  	if (pos < 0)
>  		return 0;
>  	ce = active_cache[pos];
> +
> +	if (ce_uptodate(ce))
> +		return 1;
> +
>  	if ((lstat(name, &st) < 0) ||
>  	    !S_ISREG(st.st_mode) || /* careful! */
>  	    ce_match_stat(ce, &st, 0) ||

Isn't this wrong? I think it also needs to check that ce->sha1 matches the 
right SHA1, because even if the lstat() information may be fine, if the 
SHA1 doesn't match what we want, we still shouldn't use the checked-out 
copy, of course.

The old code continues with a

	   hashcmp(sha1, ce->sha1))
		return 0;

in that if-statement that is partially visible in the context, and it's 
that hashcmp() that got incorrectly cut off from the logic.

(Of course, maybe we never call this function unless we've already checked 
that the cache-entry SHA1 matches, but if so, that subsequent hashcmp 
should just be removed instead).

		Linus