From 14aaf5c9d889a4988ffc64b39fe38bd19b930a50 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 23 Sep 2025 12:17:03 +0200 Subject: odb: move packfile map into `struct packfile_store` The object database tracks a map of packfiles by their respective paths, which is used to figure out whether a given packfile has already been loaded. With the introduction of the `struct packfile_store` we have a better place to host this list though. Move the map accordingly. `pack_map_entry_cmp()` isn't used anywhere but in "packfile.c" anymore after this change, so we convert it to a static function, as well. Note that we also drop the `inline` hint: the function is used as a callback function exclusively, and callbacks cannot be inlined. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- midx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'midx.c') diff --git a/midx.c b/midx.c index 7726c13d7e..e96970efbf 100644 --- a/midx.c +++ b/midx.c @@ -460,7 +460,7 @@ int prepare_midx_pack(struct multi_pack_index *m, strbuf_addbuf(&key, &pack_name); strbuf_strip_suffix(&key, ".idx"); strbuf_addstr(&key, ".pack"); - p = hashmap_get_entry_from_hash(&r->objects->pack_map, + p = hashmap_get_entry_from_hash(&r->objects->packfiles->map, strhash(key.buf), key.buf, struct packed_git, packmap_ent); if (!p) { -- cgit v1.2.3 From fe835b0ca0ba4d6968cd2d1f824c178547934792 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 23 Sep 2025 12:17:04 +0200 Subject: odb: move MRU list of packfiles into `struct packfile_store` The object database tracks the list of packfiles in most-recently-used order, which is mostly used to favor reading from packfiles that contain most of the objects that we're currently accessing. With the introduction of the `struct packfile_store` we have a better place to host this list though. Move the list accordingly. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- midx.c | 2 +- odb.c | 2 -- odb.h | 4 ---- packfile.c | 11 ++++++----- packfile.h | 3 +++ 5 files changed, 10 insertions(+), 12 deletions(-) (limited to 'midx.c') diff --git a/midx.c b/midx.c index e96970efbf..91c7b3917d 100644 --- a/midx.c +++ b/midx.c @@ -468,7 +468,7 @@ int prepare_midx_pack(struct multi_pack_index *m, m->source->local); if (p) { install_packed_git(r, p); - list_add_tail(&p->mru, &r->objects->packed_git_mru); + list_add_tail(&p->mru, &r->objects->packfiles->mru); } } diff --git a/odb.c b/odb.c index 737d98c911..32e982bf0b 100644 --- a/odb.c +++ b/odb.c @@ -997,7 +997,6 @@ struct object_database *odb_new(struct repository *repo) memset(o, 0, sizeof(*o)); o->repo = repo; o->packfiles = packfile_store_new(o); - INIT_LIST_HEAD(&o->packed_git_mru); pthread_mutex_init(&o->replace_mutex, NULL); string_list_init_dup(&o->submodule_source_paths); return o; @@ -1035,7 +1034,6 @@ void odb_clear(struct object_database *o) free((char *) o->cached_objects[i].value.buf); FREE_AND_NULL(o->cached_objects); - INIT_LIST_HEAD(&o->packed_git_mru); close_object_store(o); packfile_store_free(o->packfiles); o->packfiles = NULL; diff --git a/odb.h b/odb.h index b79e7280c1..3044b6a661 100644 --- a/odb.h +++ b/odb.h @@ -3,7 +3,6 @@ #include "hashmap.h" #include "object.h" -#include "list.h" #include "oidset.h" #include "oidmap.h" #include "string-list.h" @@ -138,9 +137,6 @@ struct object_database { * Should only be accessed directly by packfile.c and midx.c. */ struct packfile_store *packfiles; - /* A most-recently-used ordered version of the packed_git list. */ - struct list_head packed_git_mru; - struct { struct packed_git **packs; unsigned flags; diff --git a/packfile.c b/packfile.c index 17e0b8ab27..861d7ffd6f 100644 --- a/packfile.c +++ b/packfile.c @@ -1017,10 +1017,10 @@ static void prepare_packed_git_mru(struct repository *r) { struct packed_git *p; - INIT_LIST_HEAD(&r->objects->packed_git_mru); + INIT_LIST_HEAD(&r->objects->packfiles->mru); for (p = r->objects->packfiles->packs; p; p = p->next) - list_add_tail(&p->mru, &r->objects->packed_git_mru); + list_add_tail(&p->mru, &r->objects->packfiles->mru); } static void prepare_packed_git(struct repository *r) @@ -1095,7 +1095,7 @@ struct packed_git *get_all_packs(struct repository *r) struct list_head *get_packed_git_mru(struct repository *r) { prepare_packed_git(r); - return &r->objects->packed_git_mru; + return &r->objects->packfiles->mru; } unsigned long unpack_object_header_buffer(const unsigned char *buf, @@ -2078,10 +2078,10 @@ int find_pack_entry(struct repository *r, const struct object_id *oid, struct pa if (!r->objects->packfiles->packs) return 0; - list_for_each(pos, &r->objects->packed_git_mru) { + list_for_each(pos, &r->objects->packfiles->mru) { struct packed_git *p = list_entry(pos, struct packed_git, mru); if (!p->multi_pack_index && fill_pack_entry(oid, e, p)) { - list_move(&p->mru, &r->objects->packed_git_mru); + list_move(&p->mru, &r->objects->packfiles->mru); return 1; } } @@ -2347,6 +2347,7 @@ struct packfile_store *packfile_store_new(struct object_database *odb) struct packfile_store *store; CALLOC_ARRAY(store, 1); store->odb = odb; + INIT_LIST_HEAD(&store->mru); hashmap_init(&store->map, pack_map_entry_cmp, NULL, 0); return store; } diff --git a/packfile.h b/packfile.h index 9bbef51164..d48d46cc1b 100644 --- a/packfile.h +++ b/packfile.h @@ -64,6 +64,9 @@ struct packfile_store { */ struct packed_git *packs; + /* A most-recently-used ordered version of the packs list. */ + struct list_head mru; + /* * A map of packfile names to packed_git structs for tracking which * packs have been loaded already. -- cgit v1.2.3 From f6f236d926915411eca28cb1c47619fdacf6eafb Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 23 Sep 2025 12:17:09 +0200 Subject: packfile: refactor `install_packed_git()` to work on packfile store The `install_packed_git()` functions adds a packfile to a specific object store. Refactor it to accept a packfile store instead of a repository to clarify its scope. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/fast-import.c | 2 +- builtin/index-pack.c | 2 +- http.c | 2 +- http.h | 2 +- midx.c | 2 +- packfile.c | 11 ++++++----- packfile.h | 9 +++++++-- 7 files changed, 18 insertions(+), 12 deletions(-) (limited to 'midx.c') diff --git a/builtin/fast-import.c b/builtin/fast-import.c index 2c35f9345d..e9d82b31c3 100644 --- a/builtin/fast-import.c +++ b/builtin/fast-import.c @@ -901,7 +901,7 @@ static void end_packfile(void) if (!new_p) die("core git rejected index %s", idx_name); all_packs[pack_id] = new_p; - install_packed_git(the_repository, new_p); + packfile_store_add_pack(the_repository->objects->packfiles, new_p); free(idx_name); /* Print the boundary */ diff --git a/builtin/index-pack.c b/builtin/index-pack.c index f91c301bba..ed490dfad4 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -1645,7 +1645,7 @@ static void final(const char *final_pack_name, const char *curr_pack_name, p = add_packed_git(the_repository, final_index_name, strlen(final_index_name), 0); if (p) - install_packed_git(the_repository, p); + packfile_store_add_pack(the_repository->objects->packfiles, p); } if (!from_stdin) { diff --git a/http.c b/http.c index 98853d6483..af2120b64c 100644 --- a/http.c +++ b/http.c @@ -2541,7 +2541,7 @@ void http_install_packfile(struct packed_git *p, lst = &((*lst)->next); *lst = (*lst)->next; - install_packed_git(the_repository, p); + packfile_store_add_pack(the_repository->objects->packfiles, p); } struct http_pack_request *new_http_pack_request( diff --git a/http.h b/http.h index 36202139f4..e5a5380c6c 100644 --- a/http.h +++ b/http.h @@ -210,7 +210,7 @@ int finish_http_pack_request(struct http_pack_request *preq); void release_http_pack_request(struct http_pack_request *preq); /* - * Remove p from the given list, and invoke install_packed_git() on it. + * Remove p from the given list, and invoke packfile_store_add_pack() on it. * * This is a convenience function for users that have obtained a list of packs * from http_get_info_packs() and have chosen a specific pack to fetch. diff --git a/midx.c b/midx.c index 91c7b3917d..69c44be71c 100644 --- a/midx.c +++ b/midx.c @@ -467,7 +467,7 @@ int prepare_midx_pack(struct multi_pack_index *m, p = add_packed_git(r, pack_name.buf, pack_name.len, m->source->local); if (p) { - install_packed_git(r, p); + packfile_store_add_pack(r->objects->packfiles, p); list_add_tail(&p->mru, &r->objects->packfiles->mru); } } diff --git a/packfile.c b/packfile.c index 950b98aac5..af806aba09 100644 --- a/packfile.c +++ b/packfile.c @@ -779,16 +779,17 @@ struct packed_git *add_packed_git(struct repository *r, const char *path, return p; } -void install_packed_git(struct repository *r, struct packed_git *pack) +void packfile_store_add_pack(struct packfile_store *store, + struct packed_git *pack) { if (pack->pack_fd != -1) pack_open_fds++; - pack->next = r->objects->packfiles->packs; - r->objects->packfiles->packs = pack; + pack->next = store->packs; + store->packs = pack; hashmap_entry_init(&pack->packmap_ent, strhash(pack->pack_name)); - hashmap_add(&r->objects->packfiles->map, &pack->packmap_ent); + hashmap_add(&store->map, &pack->packmap_ent); } void (*report_garbage)(unsigned seen_bits, const char *path); @@ -904,7 +905,7 @@ static void prepare_pack(const char *full_name, size_t full_name_len, if (!hashmap_get(&data->r->objects->packfiles->map, &hent, pack_name)) { p = add_packed_git(data->r, full_name, full_name_len, data->local); if (p) - install_packed_git(data->r, p); + packfile_store_add_pack(data->r->objects->packfiles, p); } free(pack_name); } diff --git a/packfile.h b/packfile.h index a85ff607fe..ba4b0cef9c 100644 --- a/packfile.h +++ b/packfile.h @@ -120,6 +120,13 @@ void packfile_store_close(struct packfile_store *store); */ void packfile_store_reprepare(struct packfile_store *store); +/* + * Add the pack to the store so that contained objects become accessible via + * the store. This moves ownership into the store. + */ +void packfile_store_add_pack(struct packfile_store *store, + struct packed_git *pack); + struct pack_window { struct pack_window *next; unsigned char *base; @@ -196,8 +203,6 @@ int for_each_packed_object(struct repository *repo, each_packed_object_fn cb, #define PACKDIR_FILE_GARBAGE 4 extern void (*report_garbage)(unsigned seen_bits, const char *path); -void install_packed_git(struct repository *r, struct packed_git *pack); - struct packed_git *get_packed_git(struct repository *r); struct list_head *get_packed_git_mru(struct repository *r); struct multi_pack_index *get_multi_pack_index(struct odb_source *source); -- cgit v1.2.3 From d67530f6bbe56f1951b8fd2fcdaae255bf552e2d Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 23 Sep 2025 12:17:10 +0200 Subject: packfile: introduce function to load and add packfiles We have a recurring pattern where we essentially perform an upsert of a packfile in case it isn't yet known by the packfile store. The logic to do so is non-trivial as we have to reconstruct the packfile's key, check the map of packfiles, then create the new packfile and finally add it to the store. Introduce a new function that does this dance for us. Refactor callsites to use it. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/fast-import.c | 4 ++-- builtin/index-pack.c | 10 +++------- midx.c | 23 ++++------------------- packfile.c | 44 +++++++++++++++++++++++++++++++------------- packfile.h | 8 ++++++++ 5 files changed, 48 insertions(+), 41 deletions(-) (limited to 'midx.c') diff --git a/builtin/fast-import.c b/builtin/fast-import.c index e9d82b31c3..a26e79689d 100644 --- a/builtin/fast-import.c +++ b/builtin/fast-import.c @@ -897,11 +897,11 @@ static void end_packfile(void) idx_name = keep_pack(create_index()); /* Register the packfile with core git's machinery. */ - new_p = add_packed_git(pack_data->repo, idx_name, strlen(idx_name), 1); + new_p = packfile_store_load_pack(pack_data->repo->objects->packfiles, + idx_name, 1); if (!new_p) die("core git rejected index %s", idx_name); all_packs[pack_id] = new_p; - packfile_store_add_pack(the_repository->objects->packfiles, new_p); free(idx_name); /* Print the boundary */ diff --git a/builtin/index-pack.c b/builtin/index-pack.c index ed490dfad4..2b78ba7fe4 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -1640,13 +1640,9 @@ static void final(const char *final_pack_name, const char *curr_pack_name, rename_tmp_packfile(&final_index_name, curr_index_name, &index_name, hash, "idx", 1); - if (do_fsck_object) { - struct packed_git *p; - p = add_packed_git(the_repository, final_index_name, - strlen(final_index_name), 0); - if (p) - packfile_store_add_pack(the_repository->objects->packfiles, p); - } + if (do_fsck_object) + packfile_store_load_pack(the_repository->objects->packfiles, + final_index_name, 0); if (!from_stdin) { printf("%s\n", hash_to_hex(hash)); diff --git a/midx.c b/midx.c index 69c44be71c..3faeaf2f8f 100644 --- a/midx.c +++ b/midx.c @@ -443,7 +443,6 @@ int prepare_midx_pack(struct multi_pack_index *m, { struct repository *r = m->source->odb->repo; struct strbuf pack_name = STRBUF_INIT; - struct strbuf key = STRBUF_INIT; struct packed_git *p; pack_int_id = midx_for_pack(&m, pack_int_id); @@ -455,25 +454,11 @@ int prepare_midx_pack(struct multi_pack_index *m, strbuf_addf(&pack_name, "%s/pack/%s", m->source->path, m->pack_names[pack_int_id]); - - /* pack_map holds the ".pack" name, but we have the .idx */ - strbuf_addbuf(&key, &pack_name); - strbuf_strip_suffix(&key, ".idx"); - strbuf_addstr(&key, ".pack"); - p = hashmap_get_entry_from_hash(&r->objects->packfiles->map, - strhash(key.buf), key.buf, - struct packed_git, packmap_ent); - if (!p) { - p = add_packed_git(r, pack_name.buf, pack_name.len, - m->source->local); - if (p) { - packfile_store_add_pack(r->objects->packfiles, p); - list_add_tail(&p->mru, &r->objects->packfiles->mru); - } - } - + p = packfile_store_load_pack(r->objects->packfiles, + pack_name.buf, m->source->local); + if (p) + list_add_tail(&p->mru, &r->objects->packfiles->mru); strbuf_release(&pack_name); - strbuf_release(&key); if (!p) { m->packs[pack_int_id] = MIDX_PACK_ERROR; diff --git a/packfile.c b/packfile.c index af806aba09..9224ca424c 100644 --- a/packfile.c +++ b/packfile.c @@ -792,6 +792,33 @@ void packfile_store_add_pack(struct packfile_store *store, hashmap_add(&store->map, &pack->packmap_ent); } +struct packed_git *packfile_store_load_pack(struct packfile_store *store, + const char *idx_path, int local) +{ + struct strbuf key = STRBUF_INIT; + struct packed_git *p; + + /* + * We're being called with the path to the index file, but `pack_map` + * holds the path to the packfile itself. + */ + strbuf_addstr(&key, idx_path); + strbuf_strip_suffix(&key, ".idx"); + strbuf_addstr(&key, ".pack"); + + p = hashmap_get_entry_from_hash(&store->map, strhash(key.buf), key.buf, + struct packed_git, packmap_ent); + if (!p) { + p = add_packed_git(store->odb->repo, idx_path, + strlen(idx_path), local); + if (p) + packfile_store_add_pack(store, p); + } + + strbuf_release(&key); + return p; +} + void (*report_garbage)(unsigned seen_bits, const char *path); static void report_helper(const struct string_list *list, @@ -891,23 +918,14 @@ static void prepare_pack(const char *full_name, size_t full_name_len, const char *file_name, void *_data) { struct prepare_pack_data *data = (struct prepare_pack_data *)_data; - struct packed_git *p; size_t base_len = full_name_len; if (strip_suffix_mem(full_name, &base_len, ".idx") && !(data->m && midx_contains_pack(data->m, file_name))) { - struct hashmap_entry hent; - char *pack_name = xstrfmt("%.*s.pack", (int)base_len, full_name); - unsigned int hash = strhash(pack_name); - hashmap_entry_init(&hent, hash); - - /* Don't reopen a pack we already have. */ - if (!hashmap_get(&data->r->objects->packfiles->map, &hent, pack_name)) { - p = add_packed_git(data->r, full_name, full_name_len, data->local); - if (p) - packfile_store_add_pack(data->r->objects->packfiles, p); - } - free(pack_name); + char *trimmed_path = xstrndup(full_name, full_name_len); + packfile_store_load_pack(data->r->objects->packfiles, + trimmed_path, data->local); + free(trimmed_path); } if (!report_garbage) diff --git a/packfile.h b/packfile.h index ba4b0cef9c..fcefcbbef6 100644 --- a/packfile.h +++ b/packfile.h @@ -127,6 +127,14 @@ void packfile_store_reprepare(struct packfile_store *store); void packfile_store_add_pack(struct packfile_store *store, struct packed_git *pack); +/* + * Open the packfile and add it to the store if it isn't yet known. Returns + * either the newly opened packfile or the preexisting packfile. Returns a + * `NULL` pointer in case the packfile could not be opened. + */ +struct packed_git *packfile_store_load_pack(struct packfile_store *store, + const char *idx_path, int local); + struct pack_window { struct pack_window *next; unsigned char *base; -- cgit v1.2.3 From ab8aff4a6b2a1d5aa79deeb64bdeecc0234b4ddf Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 23 Sep 2025 12:17:11 +0200 Subject: packfile: move `get_multi_pack_index()` into "midx.c" The `get_multi_pack_index()` function is declared and implemented in the packfile subsystem, even though it really belongs into the multi-pack index subsystem. The reason for this is likely that it needs to call `packfile_store_prepare()`, which is not exposed by the packfile system. In a subsequent commit we're about to add another caller outside of the packfile system though, so we'll have to expose the function anyway. Do so now already and move `get_multi_pack_index()` into the MIDX subsystem. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- midx.c | 6 ++++++ midx.h | 1 + packfile.c | 8 +------- packfile.h | 10 +++++++++- 4 files changed, 17 insertions(+), 8 deletions(-) (limited to 'midx.c') diff --git a/midx.c b/midx.c index 3faeaf2f8f..1d6269f957 100644 --- a/midx.c +++ b/midx.c @@ -93,6 +93,12 @@ static int midx_read_object_offsets(const unsigned char *chunk_start, return 0; } +struct multi_pack_index *get_multi_pack_index(struct odb_source *source) +{ + packfile_store_prepare(source->odb->packfiles); + return source->midx; +} + static struct multi_pack_index *load_multi_pack_index_one(struct odb_source *source, const char *midx_name) { diff --git a/midx.h b/midx.h index e241d2d690..6e54d73503 100644 --- a/midx.h +++ b/midx.h @@ -94,6 +94,7 @@ void get_midx_chain_filename(struct odb_source *source, struct strbuf *out); void get_split_midx_filename_ext(struct odb_source *source, struct strbuf *buf, const unsigned char *hash, const char *ext); +struct multi_pack_index *get_multi_pack_index(struct odb_source *source); struct multi_pack_index *load_multi_pack_index(struct odb_source *source); int prepare_midx_pack(struct multi_pack_index *m, uint32_t pack_int_id); struct packed_git *nth_midxed_pack(struct multi_pack_index *m, diff --git a/packfile.c b/packfile.c index 9224ca424c..7a9193e5ef 100644 --- a/packfile.c +++ b/packfile.c @@ -1003,7 +1003,7 @@ static void packfile_store_prepare_mru(struct packfile_store *store) list_add_tail(&p->mru, &store->mru); } -static void packfile_store_prepare(struct packfile_store *store) +void packfile_store_prepare(struct packfile_store *store) { struct odb_source *source; @@ -1033,12 +1033,6 @@ struct packed_git *get_packed_git(struct repository *r) return r->objects->packfiles->packs; } -struct multi_pack_index *get_multi_pack_index(struct odb_source *source) -{ - packfile_store_prepare(source->odb->packfiles); - return source->midx; -} - struct packed_git *get_all_packs(struct repository *r) { packfile_store_prepare(r->objects->packfiles); diff --git a/packfile.h b/packfile.h index fcefcbbef6..a9e561ac39 100644 --- a/packfile.h +++ b/packfile.h @@ -112,6 +112,15 @@ void packfile_store_free(struct packfile_store *store); */ void packfile_store_close(struct packfile_store *store); +/* + * Prepare the packfile store by loading packfiles and multi-pack indices for + * all alternates. This becomes a no-op if the store is already prepared. + * + * It shouldn't typically be necessary to call this function directly, as + * functions that access the store know to prepare it. + */ +void packfile_store_prepare(struct packfile_store *store); + /* * Clear the packfile caches and try to look up any new packfiles that have * appeared since last preparing the packfiles store. @@ -213,7 +222,6 @@ extern void (*report_garbage)(unsigned seen_bits, const char *path); struct packed_git *get_packed_git(struct repository *r); struct list_head *get_packed_git_mru(struct repository *r); -struct multi_pack_index *get_multi_pack_index(struct odb_source *source); struct packed_git *get_all_packs(struct repository *r); /* -- cgit v1.2.3