From af96fe3392fb078cb5447bcb94f2ed8d79d0a4a8 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Mon, 29 Apr 2019 09:18:56 -0700 Subject: midx: add packs to packed_git linked list The multi-pack-index allows searching for objects across multiple packs using one object list. The original design gains many of these performance benefits by keeping the packs in the multi-pack-index out of the packed_git list. Unfortunately, this has one major drawback. If the multi-pack-index covers thousands of packs, and a command loads many of those packs, then we can hit the limit for open file descriptors. The close_one_pack() method is used to limit this resource, but it only looks at the packed_git list, and uses an LRU cache to prevent thrashing. Instead of complicating this close_one_pack() logic to include direct references to the multi-pack-index, simply add the packs opened by the multi-pack-index to the packed_git list. This immediately solves the file-descriptor limit problem, but requires some extra steps to avoid performance issues or other problems: 1. Create a multi_pack_index bit in the packed_git struct that is one if and only if the pack was loaded from a multi-pack-index. 2. Skip packs with the multi_pack_index bit when doing object lookups and abbreviations. These algorithms already check the multi-pack-index before the packed_git struct. This has a very small performance hit, as we need to walk more packed_git structs. This is acceptable, since these operations run binary search on the other packs, so this walk-and-ignore logic is very fast by comparison. 3. When closing a multi-pack-index file, do not close its packs, as those packs will be closed using close_all_packs(). In some cases, such as 'git repack', we run 'close_midx()' without also closing the packs, so we need to un-set the multi_pack_index bit in those packs. This is necessary, and caught by running t6501-freshen-objects.sh with GIT_TEST_MULTI_PACK_INDEX=1. To manually test this change, I inserted trace2 logging into close_pack_fd() and set pack_max_fds to 10, then ran 'git rev-list --all --objects' on a copy of the Git repo with 300+ pack-files and a multi-pack-index. The logs verified the packs are closed as we read them beyond the file descriptor limit. Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano --- packfile.c | 28 ++++++++-------------------- 1 file changed, 8 insertions(+), 20 deletions(-) (limited to 'packfile.c') diff --git a/packfile.c b/packfile.c index 7b94a14726..060de420d1 100644 --- a/packfile.c +++ b/packfile.c @@ -994,8 +994,6 @@ static void prepare_packed_git(struct repository *r) } rearrange_packed_git(r); - r->objects->all_packs = NULL; - prepare_packed_git_mru(r); r->objects->packed_git_initialized = 1; } @@ -1026,26 +1024,16 @@ struct multi_pack_index *get_multi_pack_index(struct repository *r) struct packed_git *get_all_packs(struct repository *r) { - prepare_packed_git(r); - - if (!r->objects->all_packs) { - struct packed_git *p = r->objects->packed_git; - struct multi_pack_index *m; - - for (m = r->objects->multi_pack_index; m; m = m->next) { - uint32_t i; - for (i = 0; i < m->num_packs; i++) { - if (!prepare_midx_pack(r, m, i)) { - m->packs[i]->next = p; - p = m->packs[i]; - } - } - } + struct multi_pack_index *m; - r->objects->all_packs = p; + prepare_packed_git(r); + for (m = r->objects->multi_pack_index; m; m = m->next) { + uint32_t i; + for (i = 0; i < m->num_packs; i++) + prepare_midx_pack(r, m, i); } - return r->objects->all_packs; + return r->objects->packed_git; } struct list_head *get_packed_git_mru(struct repository *r) @@ -2004,7 +1992,7 @@ int find_pack_entry(struct repository *r, const struct object_id *oid, struct pa list_for_each(pos, &r->objects->packed_git_mru) { struct packed_git *p = list_entry(pos, struct packed_git, mru); - if (fill_pack_entry(oid, e, p)) { + if (!p->multi_pack_index && fill_pack_entry(oid, e, p)) { list_move(&p->mru, &r->objects->packed_git_mru); return 1; } -- cgit v1.3