git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Luciano Rocha <luciano@eurotux.com>,
	Pieter de Bie <pdebie@ai.rug.nl>,
	git@vger.kernel.org
Subject: Re: [PATCH 01/02/RFC] implement a stat cache
Date: Mon, 21 Apr 2008 11:27:29 -0700	[thread overview]
Message-ID: <7v3apfawry.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: <alpine.LFD.1.10.0804201959590.2779@woody.linux-foundation.org> (Linus Torvalds's message of "Sun, 20 Apr 2008 20:15:20 -0700 (PDT)")

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

> 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.

  parent reply	other threads:[~2008-04-21 18:28 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-04-19 19:28 Git performance on OS X Pieter de Bie
2008-04-19 21:22 ` Linus Torvalds
2008-04-19 21:29   ` Linus Torvalds
2008-04-19 22:08     ` Pieter de Bie
2008-04-20 16:17     ` David Kastrup
2008-04-19 21:54 ` Linus Torvalds
2008-04-19 22:00   ` Pieter de Bie
2008-04-19 22:39     ` Linus Torvalds
2008-04-20  4:14       ` Junio C Hamano
2008-04-20 11:13       ` [PATCH 01/02/RFC] implement a stat cache Luciano Rocha
2008-04-20 11:15         ` [PATCH 02/02/RFC] make use of the " Luciano Rocha
2008-04-20 11:18         ` [PATCH 01/02/RFC] implement a " Luciano Rocha
2008-04-20 16:03         ` Linus Torvalds
2008-04-20 22:04           ` Luciano Rocha
2008-04-20 22:29             ` Linus Torvalds
2008-04-20 23:07               ` Linus Torvalds
2008-04-21  0:53                 ` Dmitry Potapov
2008-04-21  8:41                   ` Johan Herland
2008-04-21  1:21                 ` Junio C Hamano
2008-04-21  3:15                   ` Linus Torvalds
2008-04-21  3:20                     ` Linus Torvalds
2008-04-21 18:27                     ` Junio C Hamano [this message]
2008-04-21 19:09                       ` Linus Torvalds
2008-04-21 20:06                         ` Junio C Hamano
2008-04-21 10:04               ` David Kastrup
2008-04-19 22:44     ` Git performance on OS X Jakub Narebski
2008-04-19 22:50       ` Linus Torvalds
2008-04-19 22:54         ` Linus Torvalds
2008-04-19 23:10           ` Pieter de Bie
2008-04-19 23:26             ` Linus Torvalds
2008-04-19 23:35               ` Roman Shaposhnik
2008-04-19 23:57                 ` Pieter de Bie
2008-04-20  0:06                 ` Linus Torvalds
2008-04-20  0:21                   ` Roman Shaposhnik
2008-04-19 23:56               ` Pieter de Bie
2008-04-20  0:31                 ` Linus Torvalds
2008-04-20  1:23                   ` Dmitry Potapov
2008-04-20 16:22                   ` David Kastrup
2008-04-19 23:04         ` Linus Torvalds

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7v3apfawry.fsf@gitster.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=luciano@eurotux.com \
    --cc=pdebie@ai.rug.nl \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).