David Turner [off-list ref] writes:
On Tue, 2014-11-18 at 12:55 -0800, Junio C Hamano wrote:
quoted
I vaguely recall that the reason why we dropped it was because it
was too much code churn in an area that was being worked on in
parallel, but you may need to go back to the list archive for
details.
OK, in that case I'll try to remember to reroll it once the rest of the
refs stuff lands.
Sure. But I would much prefer to see us explore an arch independent
optimisation of the caller before starting to micro-optimize a leaf
function.
It is not check_refname_format() that is the real problem. It's the
fact that we do O(# of refs) work whenever we have to access the
packed-refs file. check_refname_format() is part of that, surely,
but so is reading the file, creating all of the refname structs in
memory, etc. (credit to peff@).
On Tue, Nov 18, 2014 at 01:26:56PM -0800, Junio C Hamano wrote:
It is not check_refname_format() that is the real problem. It's the
fact that we do O(# of refs) work whenever we have to access the
packed-refs file. check_refname_format() is part of that, surely,
but so is reading the file, creating all of the refname structs in
memory, etc. (credit to peff@).
Yeah, I'd agree very much with that. I am not sure if I am cc'd here
because of my general complaining about packed-refs, or if I have said
something clever on the subject.
I did implement at one point a packed-refs reader that does a binary
search on the mmap'd packed-refs file, and can return a single value or
even locate the first entry matching a prefix (like "refs/tags/") and
iterate until we're out of the prefix. Unfortunately this runs very
contrary to the caching design of the refs.c code. It is focused on
caching _loose_ refs, where we may read an outer directory (like
"refs/"), and would like to avoid descending into an inner directory
(likes "refs/foo/") unless we are interested in what is in it. But
caching partial reads of packed-refs like this is "inside out"; we might
read all of "refs/tags/*", but have no clue what else is in "refs/". So
integrating it into refs.c would take pretty major surgery.
-Peff