packfile: track packs via the MRU list exclusively

We track packfiles via two different lists: - `struct packfile_store::packs` is a list that sorts local packs first. In addition, these packs are sorted so that younger packs are sorted towards the front. - `struct packfile_store::mru` is a list that sorts packs so that most-recently used packs are at the front. The reasoning behind the ordering in the `packs` list is that younger objects stored in the local object store tend to be accessed more frequently, and that is certainly true for some cases. But there are going to be lots of cases where that isn't true. Especially when traversing history it is likely that one needs to access many older objects, and due to our housekeeping it is very likely that almost all of those older objects will be contained in one large pack that is oldest. So whether or not the ordering makes sense really depends on the use case at hand. A flexible approach like our MRU list addresses that need, as it will sort packs towards the front that are accessed all the time. Intuitively, this approach is thus able to satisfy more use cases more efficiently. This reasoning casts some doubt on whether or not it really makes sense to track packs via two different lists. It causes confusion, and it is not clear whether there are use cases where the `packs` list really is such an obvious choice. Merge these two lists into one most-recently-used list. Note that there is one important edge case: `for_each_packed_object()` uses the MRU list to iterate through packs, and then it lists each object in those packs. This would have the effect that we now sort the current pack towards the front, thus modifying the list of packfiles we are iterating over, with the consequence that we'll see an infinite loop. This edge case is worked around by introducing a new field that allows us to skip updating the MRU. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Oct 30, 2025 at 11:38 UTC c31bad4f7dcf3e04ae22e7d4a1059fd628acf1a2
3 files changed +26 -32
builtin/pack-objects.c
+2 -2
@@ -1748,11 +1748,11 @@ static int want_object_in_pack_mtime(const struct object_id *oid,
1748 }
1749 }
1750
1751 - for (e = the_repository->objects->packfiles->mru.head; e; e = e->next) {
1751 + for (e = the_repository->objects->packfiles->packs.head; e; e = e->next) {
1752 struct packed_git *p = e->pack;
1753 want = want_object_in_pack_one(p, oid, exclude, found_pack, found_offset, found_mtime);
1754 if (!exclude && want > 0)
1755 - packfile_list_prepend(&the_repository->objects->packfiles->mru, p);
1755 + packfile_list_prepend(&the_repository->objects->packfiles->packs, p);
1756 if (want != -1)
1757 return want;
1758 }
packfile.c
+7 -20
@@ -870,9 +870,7 @@ void packfile_store_add_pack(struct packfile_store *store,
870 if (pack->pack_fd != -1)
871 pack_open_fds++;
872
873 - packfile_list_prepend(&store->packs, pack);
874 - packfile_list_append(&store->mru, pack);
875 -
873 + packfile_list_append(&store->packs, pack);
874 strmap_put(&store->packs_by_path, pack->pack_name, pack);
875 }
876
@@ -1077,14 +1075,6 @@ static int sort_pack(const struct packfile_list_entry *a,
1075 return -1;
1076 }
1077
1080 -static void packfile_store_prepare_mru(struct packfile_store *store)
1081 -{
1082 - packfile_list_clear(&store->mru);
1083 -
1084 - for (struct packfile_list_entry *e = store->packs.head; e; e = e->next)
1085 - packfile_list_append(&store->mru, e->pack);
1086 -}
1087 -
1078 void packfile_store_prepare(struct packfile_store *store)
1079 {
1080 struct odb_source *source;
@@ -1103,7 +1093,6 @@ void packfile_store_prepare(struct packfile_store *store)
1093 if (!e->next)
1094 store->packs.tail = e;
1095
1106 - packfile_store_prepare_mru(store);
1096 store->initialized = true;
1097 }
1098
@@ -1128,12 +1117,6 @@ struct packfile_list_entry *packfile_store_get_packs(struct packfile_store *stor
1117 return store->packs.head;
1118 }
1119
1131 -struct packfile_list_entry *packfile_store_get_packs_mru(struct packfile_store *store)
1132 -{
1133 - packfile_store_prepare(store);
1134 - return store->mru.head;
1135 -}
1136 -
1120 /*
1121 * Give a fast, rough count of the number of objects in the repository. This
1122 * ignores loose objects completely. If you have a lot of them, then either
@@ -2134,11 +2117,12 @@ int find_pack_entry(struct repository *r, const struct object_id *oid, struct pa
2117 if (!r->objects->packfiles->packs.head)
2118 return 0;
2119
2137 - for (l = r->objects->packfiles->mru.head; l; l = l->next) {
2120 + for (l = r->objects->packfiles->packs.head; l; l = l->next) {
2121 struct packed_git *p = l->pack;
2122
2123 if (!p->multi_pack_index && fill_pack_entry(oid, e, p)) {
2141 - packfile_list_prepend(&r->objects->packfiles->mru, p);
2124 + if (!r->objects->packfiles->skip_mru_updates)
2125 + packfile_list_prepend(&r->objects->packfiles->packs, p);
2126 return 1;
2127 }
2128 }
@@ -2270,6 +2254,7 @@ int for_each_packed_object(struct repository *repo, each_packed_object_fn cb,
2254 int r = 0;
2255 int pack_errors = 0;
2256
2257 + repo->objects->packfiles->skip_mru_updates = true;
2258 repo_for_each_pack(repo, p) {
2259 if ((flags & FOR_EACH_OBJECT_LOCAL_ONLY) && !p->pack_local)
2260 continue;
@@ -2290,6 +2275,8 @@ int for_each_packed_object(struct repository *repo, each_packed_object_fn cb,
2275 if (r)
2276 break;
2277 }
2278 + repo->objects->packfiles->skip_mru_updates = false;
2279 +
2280 return r ? r : pack_errors;
2281 }
2282
packfile.h
+17 -10
@@ -79,8 +79,8 @@ struct packfile_store {
79 struct object_database *odb;
80
81 /*
82 - * The list of packfiles in the order in which they are being added to
83 - * the store.
82 + * The list of packfiles in the order in which they have been most
83 + * recently used.
84 */
85 struct packfile_list packs;
86
@@ -98,9 +98,6 @@ struct packfile_store {
98 unsigned flags;
99 } kept_cache;
100
101 - /* A most-recently-used ordered version of the packs list. */
102 - struct packfile_list mru;
103 -
101 /*
102 * A map of packfile names to packed_git structs for tracking which
103 * packs have been loaded already.
@@ -112,6 +109,21 @@ struct packfile_store {
109 * packs.
110 */
111 bool initialized;
112 +
113 + /*
114 + * Usually, packfiles will be reordered to the front of the `packs`
115 + * list whenever an object is looked up via them. This has the effect
116 + * that packs that contain a lot of accessed objects will be located
117 + * towards the front.
118 + *
119 + * This is usually desireable, but there are exceptions. One exception
120 + * is when the looking up multiple objects in a loop for each packfile.
121 + * In that case, we may easily end up with an infinite loop as the
122 + * packfiles get reordered to the front repeatedly.
123 + *
124 + * Setting this field to `true` thus disables these reorderings.
125 + */
126 + bool skip_mru_updates;
127 };
128
129 /*
@@ -171,11 +183,6 @@ void packfile_store_add_pack(struct packfile_store *store,
183 */
184 struct packfile_list_entry *packfile_store_get_packs(struct packfile_store *store);
185
174 -/*
175 - * Get all packs in most-recently-used order.
176 - */
177 -struct packfile_list_entry *packfile_store_get_packs_mru(struct packfile_store *store);
178 -
186 /*
187 * Open the packfile and add it to the store if it isn't yet known. Returns
188 * either the newly opened packfile or the preexisting packfile. Returns a