Re: [PATCH v3] fetch-pack.c: use oidset to check existence of loose object

From: Takuto Ikuta <tikuta@chromium.org>
Date: Mar 14, 2018, 06:26 UTC
Message-ID: <CALNjmMqqa1i0=ekqQfNZ5DmA+h7mOGzZcF-RB1A6+Rm8vw80pQ@mail.gmail.com>
In-Reply-To: <xmqqo9jr3lrt.fsf@gitster-ct.c.googlers.com>

2018年3月14日(水) 2:53 Junio C Hamano <gitster@pobox.com>:

> Takuto Ikuta <tikuta@chromium.org> writes:

Show 9 quoted lines

> >> During fetch, everything_local() tries to mark common part by
> >> walking the refs the other side advertised upon the first contact,
> >> so it is correct that the number of checks is not reduced in a fetch
> >> that does not fetch many refs, but the number of remote-tracking refs
> >> you have has no effect, so I doubt such a rephrasing would make the
> >> description more understandable.  "When fetching from a repository
> >> with large number of refs" is probably what you want to say, no?
> >
> > For refs existing in local repository, everything_local looks to be

able to find

Show 5 quoted lines

> > corresponding object from packed or loose objects. And if it exists,
> > I think, cost of lstat(2) is relatively smaller than other operations.
> > But for remote refs, everything_local fails to find it from packed
> > object (this check is fast)
> > and it tries to find loose object by using lstat(2), and this fails and

slow.

> > My patch is to skip this lstat(2) to non-existing ref objects for

repositories

> > having large number of remote refs.

> This still does not make sense to me, and I suspect that I am
> misreading you.  In what sense are you using the word "repository"
> and "remote refs"?

Yes, I think I did not understand the terms correctly. I meant "repository" for repository in remote servers, and "remote refs" for refs shown by `git ls-remote`.

Show 16 quoted lines

> Imagine this situation.  I have a local repository A, I fetch from a
> remote repository B but in my repository A, I do *not* use
> remote-tracking refs to remember what the last values of refs at
> repository B.  Now when I try to fetch a single ref from B into A,
> many refs B advertises point at objects A has never heard about, and
> that triggers many lstat(2) that yields ENOENT that is slow.  Your
> patch is to optimize this so that we learn these objects do not
> exist locally without running many lstat(2) to objects and helps
> repositories (like my repository A) when fetching from a repository
> with large number of refs (like the repository B).  It does not
> matter how many "remote refs" receiving repository (e.g. my A) has,
> and it does not matter how many "remote refs" sending repository
> (e.g. my B) has---whether it is refs/remotes/origin/foo
> (i.e. "remote") or refs/heads/foo (i.e. "local"), a ref at B that
> points at an object that is missing at A will cause the same
> lstat(2) at A without your change.

> That is why I think "When fetching from a repository with large
> number of refs" is what you meant, not "fetching into a repository
> with large number of remote refs" nor "fetching from a repository
> with large number of remote refs".

Thank you for explanation. And yes, that is exactly I want to do in my patch. Fixed description in v6.

Thanks.