Re: [PATCH] refs/files: keep root refs out of loose ref cache

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

>> +/*
>> + * Create a separate ref cache containing only the root refs. These are kept
>> + * out of the loose ref cache, which may already exist and is shared by
>> + * iterations that should not include them.
>> + */
>
> This raises the question whether an alternative fix could have been to
> teach the cache itself to skip over root refs in case the caller is not
> interested in them. The answer may very well be a "no", but I would be
> curious to learn why you settled on the proposed fix instead.
> ...
>> -	loose_iter = cache_ref_iterator_begin(get_loose_ref_cache(refs, flags),
>> +	loose_iter = cache_ref_iterator_begin(get_loose_ref_cache(refs),
>>  					      prefix, ref_store->repo, 1);
>>  
>> +	if (flags & REFS_FOR_EACH_INCLUDE_ROOT_REFS) {
>> +		root_refs = create_root_ref_cache(refs);
>> +		loose_iter = overlay_ref_iterator_begin(
>> +				cache_ref_iterator_begin(root_refs, prefix,
>> +							 ref_store->repo, 0),
>> +				loose_iter);
>> +	}
>
> It's a bit weird that we recreate the root ref "cache" for every single
> ref iterator we create that asks for root refs. Is there a reason why
> you have decided to not actually cache the root refs the same way as we
> cache the loose refs?

Curious.

So an alternative would be to have two caches, the primary one for
things under refs/ hierarchy and the other one for those outside,
and callers can choose to use only the primary one or use both of
them?