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 <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Derrick Stolee committed Apr 29, 2019 at 09:18 UTC af96fe3392fb078cb5447bcb94f2ed8d79d0a4a8
4 files changed +30 -33
midx.c
+14 -6
@@ -192,10 +192,8 @@ void close_midx(struct multi_pack_index *m)
192 m->fd = -1;
193
194 for (i = 0; i < m->num_packs; i++) {
195 - if (m->packs[i]) {
196 - close_pack(m->packs[i]);
197 - free(m->packs[i]);
198 - }
195 + if (m->packs[i])
196 + m->packs[i]->multi_pack_index = 0;
197 }
198 FREE_AND_NULL(m->packs);
199 FREE_AND_NULL(m->pack_names);
@@ -204,6 +202,7 @@ void close_midx(struct multi_pack_index *m)
202 int prepare_midx_pack(struct repository *r, struct multi_pack_index *m, uint32_t pack_int_id)
203 {
204 struct strbuf pack_name = STRBUF_INIT;
205 + struct packed_git *p;
206
207 if (pack_int_id >= m->num_packs)
208 die(_("bad pack-int-id: %u (%u total packs)"),
@@ -215,9 +214,18 @@ int prepare_midx_pack(struct repository *r, struct multi_pack_index *m, uint32_t
214 strbuf_addf(&pack_name, "%s/pack/%s", m->object_dir,
215 m->pack_names[pack_int_id]);
216
218 - m->packs[pack_int_id] = add_packed_git(pack_name.buf, pack_name.len, m->local);
217 + p = add_packed_git(pack_name.buf, pack_name.len, m->local);
218 strbuf_release(&pack_name);
220 - return !m->packs[pack_int_id];
219 +
220 + if (!p)
221 + return 1;
222 +
223 + p->multi_pack_index = 1;
224 + m->packs[pack_int_id] = p;
225 + install_packed_git(r, p);
226 + list_add_tail(&p->mru, &r->objects->packed_git_mru);
227 +
228 + return 0;
229 }
230
231 int bsearch_midx(const struct object_id *oid, struct multi_pack_index *m, uint32_t *result)
object-store.h
+2 -7
@@ -76,7 +76,8 @@ struct packed_git {
76 pack_keep_in_core:1,
77 freshened:1,
78 do_not_close:1,
79 - pack_promisor:1;
79 + pack_promisor:1,
80 + multi_pack_index:1;
81 unsigned char hash[GIT_MAX_RAWSZ];
82 struct revindex_entry *revindex;
83 /* something like ".git/objects/pack/xxxxx.pack" */
@@ -128,12 +129,6 @@ struct raw_object_store {
129 /* A most-recently-used ordered version of the packed_git list. */
130 struct list_head packed_git_mru;
131
131 - /*
132 - * A linked list containing all packfiles, starting with those
133 - * contained in the multi_pack_index.
134 - */
135 - struct packed_git *all_packs;
136 -
132 /*
133 * A fast, rough count of the number of objects in the repository.
134 * These two fields are not meant for direct access. Use
packfile.c
+8 -20
@@ -994,8 +994,6 @@ static void prepare_packed_git(struct repository *r)
994 }
995 rearrange_packed_git(r);
996
997 - r->objects->all_packs = NULL;
998 -
997 prepare_packed_git_mru(r);
998 r->objects->packed_git_initialized = 1;
999 }
@@ -1026,26 +1024,16 @@ struct multi_pack_index *get_multi_pack_index(struct repository *r)
1024
1025 struct packed_git *get_all_packs(struct repository *r)
1026 {
1029 - prepare_packed_git(r);
1030 -
1031 - if (!r->objects->all_packs) {
1032 - struct packed_git *p = r->objects->packed_git;
1033 - struct multi_pack_index *m;
1034 -
1035 - for (m = r->objects->multi_pack_index; m; m = m->next) {
1036 - uint32_t i;
1037 - for (i = 0; i < m->num_packs; i++) {
1038 - if (!prepare_midx_pack(r, m, i)) {
1039 - m->packs[i]->next = p;
1040 - p = m->packs[i];
1041 - }
1042 - }
1043 - }
1027 + struct multi_pack_index *m;
1028
1045 - r->objects->all_packs = p;
1029 + prepare_packed_git(r);
1030 + for (m = r->objects->multi_pack_index; m; m = m->next) {
1031 + uint32_t i;
1032 + for (i = 0; i < m->num_packs; i++)
1033 + prepare_midx_pack(r, m, i);
1034 }
1035
1048 - return r->objects->all_packs;
1036 + return r->objects->packed_git;
1037 }
1038
1039 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
1992
1993 list_for_each(pos, &r->objects->packed_git_mru) {
1994 struct packed_git *p = list_entry(pos, struct packed_git, mru);
2007 - if (fill_pack_entry(oid, e, p)) {
1995 + if (!p->multi_pack_index && fill_pack_entry(oid, e, p)) {
1996 list_move(&p->mru, &r->objects->packed_git_mru);
1997 return 1;
1998 }
sha1-name.c
+6
@@ -157,6 +157,9 @@ static void unique_in_pack(struct packed_git *p,
157 uint32_t num, i, first = 0;
158 const struct object_id *current = NULL;
159
160 + if (p->multi_pack_index)
161 + return;
162 +
163 if (open_pack_index(p) || !p->num_objects)
164 return;
165
@@ -589,6 +592,9 @@ static void find_abbrev_len_for_pack(struct packed_git *p,
592 struct object_id oid;
593 const struct object_id *mad_oid;
594
595 + if (p->multi_pack_index)
596 + return;
597 +
598 if (open_pack_index(p) || !p->num_objects)
599 return;
600