All of lore.kernel.org
 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: 13+ 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]
2026-10-21  2:27     ` Junio C Hamano
2026-10-21  5:36       ` Patrick Steinhardt

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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.