packfile: fix perf regression with many packs
Sincemain589127caa7(packfile: move list of packs into the packfile store, 2025-10-30), there is a performance regression when many packfiles need to be loaded: `packfile_store_add_pack()` now calls `packfile_list_remove_internal()` to detect whether the packfile was _already_ in the list, and if so, move it to the end of the list. This function linearly scans the existing list before every insertion. Newly loading N packs therefore has complexity O(N²). In one reported use case (https://github.com/microsoft/git/issues/970), N equals 37,815 and caused a slow-down of a simple `git rev-parse --short HEAD` (which is regularly executed as part of `GIT_PS1`) from 0.4s to 4.5s. Let's fix this by establishing a fast path for known-new packfiles. The keen reader will note that there is currently only a single, "known-new" caller of the `packfile_list_append()` function, and wonder why not simply remove this check whether the packfile already exists in the list? Originally, when above-mentioned commit introduced that logic, there was a second caller in `prepare_midx()`, which would have required that check, but that caller was removed in6aff1f25a0(packfile: always add packfiles to MRU when adding a pack, 2025-10-30). Still, the function is declared in a header file, and to avoid any problems with in-flight or downstream callers, it is safer to extend the signature to be explicit whether or not to skip that check. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
parent
f85a7e6620
commit
e4621a0169
|
|
@ -57,11 +57,12 @@ void packfile_list_prepend(struct packfile_list *list, struct packed_git *pack)
|
||||||
list->tail = entry;
|
list->tail = entry;
|
||||||
}
|
}
|
||||||
|
|
||||||
void packfile_list_append(struct packfile_list *list, struct packed_git *pack)
|
void packfile_list_append(struct packfile_list *list, struct packed_git *pack,
|
||||||
|
int skip_dup_check)
|
||||||
{
|
{
|
||||||
struct packfile_list_entry *entry;
|
struct packfile_list_entry *entry;
|
||||||
|
|
||||||
entry = packfile_list_remove_internal(list, pack);
|
entry = skip_dup_check ? NULL : packfile_list_remove_internal(list, pack);
|
||||||
if (!entry) {
|
if (!entry) {
|
||||||
entry = xmalloc(sizeof(*entry));
|
entry = xmalloc(sizeof(*entry));
|
||||||
entry->pack = pack;
|
entry->pack = pack;
|
||||||
|
|
|
||||||
|
|
@ -15,7 +15,8 @@ struct packfile_list_entry {
|
||||||
void packfile_list_clear(struct packfile_list *list);
|
void packfile_list_clear(struct packfile_list *list);
|
||||||
void packfile_list_remove(struct packfile_list *list, struct packed_git *pack);
|
void packfile_list_remove(struct packfile_list *list, struct packed_git *pack);
|
||||||
void packfile_list_prepend(struct packfile_list *list, struct packed_git *pack);
|
void packfile_list_prepend(struct packfile_list *list, struct packed_git *pack);
|
||||||
void packfile_list_append(struct packfile_list *list, struct packed_git *pack);
|
void packfile_list_append(struct packfile_list *list, struct packed_git *pack,
|
||||||
|
int skip_dup_check);
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Find the pack within the "packs" list whose index contains the object
|
* Find the pack within the "packs" list whose index contains the object
|
||||||
|
|
|
||||||
|
|
@ -781,7 +781,7 @@ void packfile_store_add_pack(struct odb_source_packed *store,
|
||||||
if (pack->pack_fd != -1)
|
if (pack->pack_fd != -1)
|
||||||
pack_open_fds++;
|
pack_open_fds++;
|
||||||
|
|
||||||
packfile_list_append(&store->packs, pack);
|
packfile_list_append(&store->packs, pack, 1);
|
||||||
strmap_put(&store->packs_by_path, pack->pack_name, pack);
|
strmap_put(&store->packs_by_path, pack->pack_name, pack);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -141,4 +141,8 @@ test_perf "load 10,000 packs" '
|
||||||
git rev-parse --verify "HEAD^{commit}"
|
git rev-parse --verify "HEAD^{commit}"
|
||||||
'
|
'
|
||||||
|
|
||||||
|
test_perf "abbreviate with 10,000 packs" '
|
||||||
|
git rev-parse --short HEAD
|
||||||
|
'
|
||||||
|
|
||||||
test_done
|
test_done
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue