Git development
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: Andy Koppe <andy.koppe@gmail.com>
Cc: git@vger.kernel.org, Karthik Nayak <karthik.188@gmail.com>
Subject: Re: [PATCH v3] refs/files: keep root refs out of loose ref cache
Date: Tue, 20 Oct 2026 08:42:00 +0200	[thread overview]
Message-ID: <atcNOMxaz2WxDIA1@pks.im> (raw)
In-Reply-To: <20261019203427.2637569-1-andy.koppe@gmail.com>

On Mon, Oct 19, 2026 at 09:34:20PM +0100, Andy Koppe wrote:
> With the files backend, root refs such as HEAD or ORIG_HEAD are only
> added to the loose ref cache if REFS_FOR_EACH_INCLUDE_ROOT_REFS is
> passed when the cache is created. If the cache has already been created
> for an earlier ref iterator, root refs are silently omitted. Conversely,
> once they have been added, any later iterator also yields them, whether
> or not they were requested. This has been the case since d0f00c1ac1
> (refs: introduce `refs_for_each_include_root_refs()`, 2024-02-23).
> 
> For example, `git for-each-ref --include-root-refs --merged=main` omits
> root refs that are merged into main, because looking up the given commit
> loads the replace refs, which populates the loose ref cache first. The
> reftable backend is not affected, as it filters root refs for each
> iterator rather than caching them.
> 
> Fix this by keeping the root refs out of the loose ref cache. Instead,
> read them into a separate, temporary ref cache whenever an iterator is
> requested with REFS_FOR_EACH_INCLUDE_ROOT_REFS, and overlay an iterator
> over that cache on the loose ref iterator. The cache is freed along with
> the iterator.
> 
> Alternatively, the root refs could always be added to the loose ref
> cache and skipped during iteration unless requested. But that would mean
> reading them in every command that uses the loose ref cache, even though
> only two call sites actually request them. Also, keeping the root ref
> cache for reuse by later iterators is not worthwhile, because those two
> call sites only request the root refs once per command.

Thanks for spelling out alternative implementations and why we didn't
use them.

> Signed-off-by: Andy Koppe <andy.koppe@gmail.com>
> ---
> Changes from v1:
> - Amend commit message for clarity and to discuss alternatives.
> - Move new create_root_ref_cache() below get_loose_ref_cache() because
>   its comment references the loose ref cache and because it makes for
>   a cleaner diff. Tighten up the comment.
> 
> Changes from v2:
> - Drop fixes to documentation of REFS_FOR_EACH_INCLUDE_ROOT_REFS and
>   --include-root-refs added in v2. I'll create a separate patch with
>   those as well as other fixes for outdated uses of "pseudoref".
> 
> The range diff shows the changes from v1, because they're of more
> interest than dropping the documentation changes I'd added in v2.

Thanks for this. By the way, I would recommend to have a look at b4,
which handles all of the bookkeeping for you.

In any case, this patch looks good to me. Thanks!

Patrick

      reply	other threads:[~2026-10-20  6:42 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-14 15:32 [PATCH] refs/files: keep root refs out of loose ref cache Andy Koppe
2026-10-15  5:58 ` Patrick Steinhardt
2026-10-15 17:31   ` Junio C Hamano
2026-10-15 19:43     ` Andy Koppe
2026-10-16  7:00       ` Patrick Steinhardt
2026-10-16 15:05         ` Junio C Hamano
2026-10-17 20:04 ` [PATCH v2] " Andy Koppe
2026-10-19  5:57   ` Patrick Steinhardt
2026-10-19 21:30     ` Andy Koppe
2026-10-19 20:34 ` [PATCH v3] " Andy Koppe
2026-10-20  6:42   ` Patrick Steinhardt [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=atcNOMxaz2WxDIA1@pks.im \
    --to=ps@pks.im \
    --cc=andy.koppe@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=karthik.188@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox