Re: [PATCH 01/02/RFC] implement a stat cache

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 21, 2008, 18:27 UTC
Message-ID: <7v3apfawry.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <alpine.LFD.1.10.0804201959590.2779@woody.linux-foundation.org>

Linus Torvalds <torvalds@linux-foundation.org> writes:

Show 14 quoted lines

> Here's a trial balloon patch that totally revamps how that whole function 
> works. Instead of passing in a "symlink_cache" thing that it modifies for 
> the caller, it just has its totally *internal* cache of where it found the 
> last symlink, and what the last directory it found last time was.
>
> So now the logic becomes:
>
>  - if a pathname that is passed in matches the last known symlink prefix, 
>    we don't even need to do anything else - it is known to have a symlink 
>    prefix.
>
>  - if the pathname that is passed in matches the last known directory 
>    prefix, we start looking just from that point onward (since we know 
>    that the leading part is a directory without symlinks)

That makes sense.

> Caveat: I do think we should add a way to invalidate the pathname caches 
> when we turn a symlink into a directory or vice versa, so this patch isn't 
> really complete as-is, but I think it's a good start.

True.

There are a few patches in flight that are not in 'master' (Dmitry quoted one of them), that use more has_symlink_leading_path() calls. In retrospect, the function was misnamed. It describes what it checks (i.e. "does the path have leading component that is a symlink?") but I probably should have named it after what it really wants to tell (i.e. "lstat(2) says this exists, but does it really, from the point of view of git?")

Doesn't it become very tempting to replace lstat() calls we make to check the status of a work tree path, with a function git_wtstat() that is:

        int git_wtstat(const char *path, struct stat *st)
        {
                int status = lstat(path, st);

                if (status)
                        return status;

                if (!has_symlink_leading_path(path, strlen(path)))
                        return 0;

                /*
                 * As far as git is concerned, this does not exist in
                 * the work tree!
                 */
                errno = ENOENT;
                return -1;
        }

This unfortunately is not enough to hide the need for has_symlink calls from outside callers. When we check out a new path "a/b/c/d/e", for example, if we naively checked if we creat(2) "a/b/c/d/e" (and otherwise we try the equivalent of "mkdir -p"), we would be tricked by a symlink "a/b" that points at some random place that has "c/d" subdirectory in it, and we need to unlink "a/b" first, and the above git_wtstat() does not really help such codepath.