Re: [PATCH] packfile: avoid access(3p) calls for missing packs

From: Jeff King <peff@peff.net>
Date: May 23, 2025, 02:03 UTC
Message-ID: <20250523020308.GA559000@coredump.intra.peff.net>
In-Reply-To: <aC/JDrF5uJh9XxXj@nand.local>

On Thu, May 22, 2025 at 09:02:06PM -0400, Taylor Blau wrote:

Show 17 quoted lines

> > I don't recall offhand whether reprepare_packed_git() will open the new
> > midx. If it doesn't, we'd still find the object via the actual pack
> > .idx, but we may end up consulting the now-stale midx over and over (so
> > we'll get the right answer, but there may be some room for
> > optimization).
> 
> This all sounds right to me. And we do end up loading a new MIDX via
> reprepare_packed_git(): that function calls prepare_packed_git() (which
> doesn't immediately return, since we just zero'd out the
> r->objects->packed_git_initialized flag). We then enumerate the ODBs,
> calling prepare_multi_pack_index_one() and prepare_packed_git_one() for
> each.
> 
> From there we end up in load_multi_pack_index(), which gets a freshened
> view of the MIDX file. If all goes well there, then
> prepare_multi_pack_index_one() assigns it to
> r->objects->multi_pack_index as expected.

One thing that puzzled me here is how prepare_multi_pack_index_one() deals with an existing r->objects->multi_pack_index:

          m = load_multi_pack_index(r, object_dir, local);
  
          if (m) {
                  struct multi_pack_index *mp = r->objects->multi_pack_index;
                  if (mp) {
                          m->next = mp->next;
                          mp->next = m;
                  } else
                          r->objects->multi_pack_index = m;
                  return 1;
          }

I don't see reprepare_packed_git() zero-ing out the existing midx pointer. So would we hit that "if (mp)" block, in which case we seem to create a loop of next pointers? But the old midx would still be the first one in the loop. Would we still consult it?

Or am I just reading this totally wrong?

-Peff