On Wed, 22 Jul 2009, Junio C Hamano wrote:
>
> Hmm, we now have to remember what this patch did, if we ever wanted to
> introduce negative refs later (see ef06b91 do_for_each_ref: perform the
> same sanity check for leftovers., 2006-11-18). Not exactly nice to spread
> the codepaths that need to be updated. Is the cold cache performance of
> "git branch" to list your local branches that important?
Hmm. I do think that 7.5s is _way_ too long to wait for something as
simple as "what branches do I have?".
And yes, it's also an operation that I'd expect to be quite possibly the
first one you do when moving to a new repo, so cold-cache is realistic.
And the 'rawref' thing is exactly the same as the 'ref' version, except it
doesn't do the null_sha1 check and the 'has_sha1-file()' check.
And since git branch will do something _better_ than the 'has_sha1_file()'
check (by virtue of actually looking up the commit), I don't think that
part is an issue. So the only issue is the is_null_sha1() thing.
And quite frankly, while the null-sha1 check may make sense, the way the
flag is named right now (DO_FOR_EACH_INCLUDE_BROKEN), I think we might be
better off re-thinking things later if we ever end up caring. That
'is_null_sha1()' check should possibly be under a separate flag.
That said, while I think my patch was the simplest and most
the problem could certainly have been fixed differently.
For example, instead of using 'for_each_ref()' and then splitting them by
kind with that "detect kind" loop, it could instead have done two loops,
ie
if (kinds & REF_LOCAL_BRANCH)
for_each_ref_in("refs/heads/", append_local, &ref_list);
if (kinds & REF_REMOTE_BRANCH)
for_each_ref_in("refs/remotes/", append_remote, &ref_list);
and avoided the other refs we aren't interested in _that_ way instead.
But it would be a bigger and involved patch. It gets really messy too (I
tried), because when you use 'for_each_ref_in()' it removes the prefix as
it goes along, but then the code in builtin-branch.c wants the prefix
after all.
Linus
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html