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
