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

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

quoted

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