Re: [PATCH] refs/files: keep root refs out of loose ref cache
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Oct 15, 2026, 17:31 UTC
- Message-ID
- <xmqqmrseooor.fsf@gitster.g>
- In-Reply-To
- <atBraYoY1SVwnezN@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
Show 27 quoted lines
>> +/*
>> + * 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?