Re: performance problem: "git commit filename"

Linus Torvalds [off-list ref] writes:

On Sun, 13 Jan 2008, Junio C Hamano wrote:

quoted

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:

quoted

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?

You are right.  We call this with sha1 that may not necessarily
be the same as ce->sha1.

The code should probably be something like:

	ce = active_cache[pos];

	/*
         * Even if ce matches the work tree, it is not what we can
	 * reuse for sha1, if the hash is different or not a
         * regular blob.
         */
	if (hashcmp(sha1, ce->sha1) || !S_ISREG(ntohl(ce->st_mode))
		return 0;
	/*
         * Does ce actually match the work tree?  If so we can reuse.
         */
	if (ce_uptodate(ce) ||
	    (!lstat(name, &st) && !ce_match_stat(ce, &st, 0)))
		return 1;
	return 0;

The expression inside the latter if () condition should probably
be the new abstraction at the level of ce_modified().

Currently ce_modified() assumes that lstat(2) is cheap and the
callers have called it on paths they are interested in already.