Re: [PATCH 2/2] lookup_commit_reference_gently: do not read non-{tag,commit}

From: Jeff King <peff@peff.net>
Date: May 31, 2013, 16:00 UTC
Message-ID: <20130531160020.GA1365@sigill.intra.peff.net>
In-Reply-To: <87sj138tcp.fsf@linux-k42r.v.cablecom.net>

On Fri, May 31, 2013 at 10:08:06AM +0200, Thomas Rast wrote:

Show 11 quoted lines

> > Have you measured the impact of this on normal operations? During a
> > traversal, we spend a measurable amount of time looking up commits in
> > packfiles, and this would presumably double it.
> 
> I don't think so, but admittedly I didn't measure it.
> 
> The reason why it's unlikely is that this is specific to
> lookup_commit_reference_gently, which according to some grepping is
> usually done on refs or values that refs might have; e.g. on the old&new
> sides of a fetch in remote.c, or in many places in the callback of some
> variant of for_each_ref.

Yeah, I saw that the "_gently" form backs some of the other forms (non-gently, lookup_commit_or_die) and was worried that we would use it as part of the revision traversal to find parents. But we don't, of course; we use lookup_commit, because we would not accept a parent that is a tag pointing to a commit.

So I think it probably won't matter in any sane case.

> Of course if you have a ridiculously large number of refs (and I gather
> _you_ do), this will hurt somewhat in the usual case, but speed up the
> case where there is a ref (usually a lightweight tag) directly pointing
> at a large blob.

In my large-ref cases, there are often a lot of duplicate refs anyway (e.g., many forks of a project having the same tags). So usually the right thing there is to use lookup_object to see if we have the object already anyway. parse_object has this optimization, but we can add it into sha1_object_info, too, if it turns out to be a problem.

-Peff