Skip to content

Commit c9107d2

Browse files
pks-tgitster
authored andcommitted
packfile: refactor misleading code when unusing pack windows
The function `unuse_one_window()` is responsible for unmapping one of the packfile windows, which is done when we have exceeded the allowed number of window. The function receives a `struct packed_git` as input, which serves as an additional packfile that should be considered to be closed. If not given, we seemingly skip that and instead go through all of the repository's packfiles. The conditional that checks whether we have a packfile though does not make much sense anymore, as we dereference the packfile regardless of whether or not it is a `NULL` pointer to derive the repository's packfile store. The function was originally introduced via f0e17e8 (pack: move release_pack_memory(), 2017-08-18), and here we indeed had a caller that passed a `NULL` pointer. That caller was later removed via 9827d4c (packfile: drop release_pack_memory(), 2019-08-12), so starting with that commit we always pass a `struct packed_git`. In 9c5ce06 (packfile: use `repository` from `packed_git` directly, 2024-12-03) we then inadvertently started to rely on the fact that the pointer is never `NULL` because we use it now to identify the repository. Arguably, it didn't really make sense in the first place that the caller provides a packfile, as the selected window would have been overridden anyway by the subsequent loop over all packfiles if there was an older window. So the overall logic is quite misleading overall. The only case where it _could_ make a difference is when there were two packfiles with the same `last_used` value, but that case doesn't ever happen because the `pack_used_ctr` is strictly increasing. Refactor the code so that we instead pass in the object database to help make the code less misleading. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent 0ff5227 commit c9107d2

File tree

1 file changed

+5
-6
lines changed

1 file changed

+5
-6
lines changed

packfile.c

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -355,16 +355,15 @@ static void scan_windows(struct packed_git *p,
355355
}
356356
}
357357

358-
static int unuse_one_window(struct packed_git *current)
358+
static int unuse_one_window(struct object_database *odb)
359359
{
360360
struct packfile_list_entry *e;
361361
struct packed_git *lru_p = NULL;
362362
struct pack_window *lru_w = NULL, *lru_l = NULL;
363363

364-
if (current)
365-
scan_windows(current, &lru_p, &lru_w, &lru_l);
366-
for (e = current->repo->objects->packfiles->packs.head; e; e = e->next)
364+
for (e = odb->packfiles->packs.head; e; e = e->next)
367365
scan_windows(e->pack, &lru_p, &lru_w, &lru_l);
366+
368367
if (lru_p) {
369368
munmap(lru_w->base, lru_w->len);
370369
pack_mapped -= lru_w->len;
@@ -740,8 +739,8 @@ unsigned char *use_pack(struct packed_git *p,
740739
win->len = (size_t)len;
741740
pack_mapped += win->len;
742741

743-
while (settings->packed_git_limit < pack_mapped
744-
&& unuse_one_window(p))
742+
while (settings->packed_git_limit < pack_mapped &&
743+
unuse_one_window(p->repo->objects))
745744
; /* nothing */
746745
win->base = xmmap_gently(NULL, win->len,
747746
PROT_READ, MAP_PRIVATE,

0 commit comments

Comments
 (0)