From: Junio C Hamano Date: Thu, 15 Oct 2026 17:31:48 GMT Subject: Re: [PATCH] refs/files: keep root refs out of loose ref cache Message-ID: In-Reply-To: Patrick Steinhardt 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?