Re: Tag peeling peculiarities

Michael Haggerty [off-list ref] writes:

What is stored in ref_value.peeled?  Is it the peeled version of
ref_value.sha1, or is it the peeled version of the associated refname?
Because they are not necessarily the same thing: an entry in the packed
ref_cache *might* be overridden by a loose reference with the same
refname but a different SHA1 which peels to a different value.

The way it should work is that we look for loose ones first and then
only if we do not find a loose one we use packed one, when asked for
a ref by name.  The .sha1 and .peeled fields for a single ref_entry
struct must be consistent with each other, even though you might
have got a ref_value by reading the packed-refs file and another
ref_value by reading loose one and have both in core.  When you have
both packed and loose, the former should not be used at all, and it
certainly should not be pointed by current_ref.

Stepping back a bit, I suspect that it may be worth evaluating to
see if it still makes sense to keep the current_ref optimization
that was introduced in 0ae91be0e1fa (Optimize peel_ref for the
current ref of a for_each_ref callback, 2008-02-24).  Back then, it
was fairly expensive to find a cached_ref entry by name, because it
was implemented as two linked lists (one for loose, one for packed)
and we would have had to traverse them to answer the question.

Since then, e9c4c11165e4 (refs: Use binary search to lookup refs
faster, 2011-09-29) introduced ref_array to make it more efficient
to find a ref_entry by name, and your more recent series that
includes bc5fd6d3c29f (refs.c: reorder definitions more logically,
2012-04-10) further touched that codepath for enumeration in a
subtree.

I am not sure if that last change helped or hurt the performance of
a single ref lookup, but we can certainly say that the performance
characteristics of read_ref_full() is very different in today's code
and the current_ref optimization may no longer be of much value.