Subject: Re: [PATCH 05/16] odb: move MRU list of packfiles into `struct packfile_store`

From: Patrick Steinhardt <[email-masked]>
Date: Thu, 21 Aug 2025 08:40:30 +0200

On Wed, Aug 20, 2025 at 03:20:08PM -0400, Jeff King wrote:
> On Wed, Aug 20, 2025 at 05:44:36AM -0700, Karthik Nayak wrote:
> 
> > Question: for my understanding, so we maintain a list of `packed_git`
> > packfiles in `packfile_store.packs` and then the same list is
> > available in a MRU form in `packfile_store.mru`?
> > 
> > I assume this is to optimize searches to use the mru form? Is there a
> > reason to not pick the mru list?
> 
> Yeah, I believe you should find the same packfiles in the linked list
> currently formed by packed_git.next and the MRU list. When I introduced
> the MRU list long ago, I cowardly did not want to take the risk that
> somebody depended on the order of the original list, and left it in
> place.
> 
> I think it would _probably_ be OK to just keep a single list (and
> manipulate it to keep the MRU property). But whoever does should take a
> close look and make sure that is true. The biggest risk I can think of
> is that there could be some code iterating over the packfiles, coupled
> with object lookups in their loop body. If those lookups reorder the
> list, that would screw up the iteration.
> 
> IMHO it is probably better to do a change like that outside of Patrick's
> series. There already is a lot going on with moving fields around, and
> consolidating the lists can happen on top (and would be made easier for
> having pulled it out into two adjacent lists).

There's in fact been a couple sites where we _didn't_ add packfiles to
the MRU list: "builtin/fast-import.c", "builtin/index-pack.c",
"packfile.c" via `prepare_pack()`. This is no longer going to be the
case at the end of this patch series as we in a later patch adjust
`packfile_store_store_add()` to handle this for us.

But I agree, this is something I'd rather want to push into a subsequent
patch series. I've already got one cooking where I change how the lists
are getting handled, so that's a good opportunity to do so.

Patrick