From 27bed41ebb11ffdfcb79cf74fd4122ac41420f26 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 15 Jul 2026 08:22:31 +0200 Subject: [PATCH 1/9] odb/source-packed: improve lookup when enumerating objects When iterating through objects of a packed source that have a specific prefix we do so via two different methods: - When a multi-pack index is available we use that one to efficiently loop through all objects. - We then loop through all packfiles that aren't covered by a multi-pack index. Regardless of which mechanism we use, we then iterate through all the objects indexed by the respective data structure. Curiously though, while we use the indices for enumerating the objects, we completely ignore it for the actual object lookup. Instead, we call into the generic `odb_source_read_object_info()` function, which will itself consult the indices to figure out where the object in question even lives. This has two consequences: - It's inefficient, as we basically have to figure out the position of the object a second time. - It's subtly wrong, as it may now happen that a specific object will be looked up via a different pack in case it exists multiple times. This is unlikely to have any real-world consequences, but it's still the wrong thing to do. Fix the issue by using `packed_object_info()` directly. While at it, rename the `store` variable to `source`. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- odb/source-packed.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/odb/source-packed.c b/odb/source-packed.c index 0edea5356d..9cfa02b7a2 100644 --- a/odb/source-packed.c +++ b/odb/source-packed.c @@ -143,7 +143,7 @@ static bool should_exclude_pack(struct packed_git *p, enum odb_for_each_object_f } static int for_each_prefixed_object_in_midx( - struct odb_source_packed *store, + struct odb_source_packed *source, struct multi_pack_index *m, const struct odb_for_each_object_options *opts, struct odb_source_packed_for_each_object_wrapper_data *data) @@ -170,6 +170,7 @@ static int for_each_prefixed_object_in_midx( */ for (i = first; i < num; i++) { const struct object_id *current = NULL; + struct packed_git *pack; struct object_id oid; current = nth_midxed_object_oid(&oid, m, i); @@ -177,9 +178,8 @@ static int for_each_prefixed_object_in_midx( if (!match_hash(len, opts->prefix->hash, current->hash)) break; - if (opts->flags) { + if (opts->flags || data->request) { uint32_t pack_id = nth_midxed_pack_int_id(m, i); - struct packed_git *pack; if (prepare_midx_pack(m, pack_id)) { pack_errors = true; @@ -193,9 +193,9 @@ static int for_each_prefixed_object_in_midx( if (data->request) { struct object_info oi = *data->request; + off_t offset = nth_midxed_offset(m, i); - ret = odb_source_read_object_info(&store->base, current, - &oi, 0); + ret = packed_object_info(source, pack, offset, &oi); if (ret) goto out; @@ -219,7 +219,7 @@ out: } static int for_each_prefixed_object_in_pack( - struct odb_source_packed *store, + struct odb_source_packed *source, struct packed_git *p, const struct odb_for_each_object_options *opts, struct odb_source_packed_for_each_object_wrapper_data *data) @@ -246,8 +246,9 @@ static int for_each_prefixed_object_in_pack( if (data->request) { struct object_info oi = *data->request; + off_t offset = nth_packed_object_offset(p, i); - ret = odb_source_read_object_info(&store->base, &oid, &oi, 0); + ret = packed_object_info(source, p, offset, &oi); if (ret) goto out; From 03aaa4f8985ce4813033c1afa36ebec7d7e2a9a1 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 15 Jul 2026 08:22:32 +0200 Subject: [PATCH 2/9] pack-bitmap: mark object filter as `const` The function `for_each_bitmapped_object()` accepts an optional object filter. This filter is never modified by the function, but is not declared as `const`. Fix this. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- pack-bitmap.c | 6 +++--- pack-bitmap.h | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pack-bitmap.c b/pack-bitmap.c index 35774b6f0c..a47c231632 100644 --- a/pack-bitmap.c +++ b/pack-bitmap.c @@ -1976,7 +1976,7 @@ static void filter_bitmap_object_type(struct bitmap_index *bitmap_git, static int filter_bitmap(struct bitmap_index *bitmap_git, struct object_list *tip_objects, struct bitmap *to_filter, - struct list_objects_filter_options *filter) + const struct list_objects_filter_options *filter) { if (!filter || filter->choice == LOFC_DISABLED) return 0; @@ -2027,7 +2027,7 @@ static int filter_bitmap(struct bitmap_index *bitmap_git, return -1; } -static int can_filter_bitmap(struct list_objects_filter_options *filter) +static int can_filter_bitmap(const struct list_objects_filter_options *filter) { return !filter_bitmap(NULL, NULL, NULL, filter); } @@ -2058,7 +2058,7 @@ static void filter_packed_objects_from_bitmap(struct bitmap_index *bitmap_git, } int for_each_bitmapped_object(struct bitmap_index *bitmap_git, - struct list_objects_filter_options *filter, + const struct list_objects_filter_options *filter, show_reachable_fn show_reach, void *payload) { diff --git a/pack-bitmap.h b/pack-bitmap.h index 19a8655457..47935eb24e 100644 --- a/pack-bitmap.h +++ b/pack-bitmap.h @@ -96,7 +96,7 @@ struct list_objects_filter_options; * not supported, `0` otherwise. */ int for_each_bitmapped_object(struct bitmap_index *bitmap_git, - struct list_objects_filter_options *filter, + const struct list_objects_filter_options *filter, show_reachable_fn show_reach, void *payload); From 6f48b8ce56171419f768902b300365c1b6708c96 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 15 Jul 2026 08:22:33 +0200 Subject: [PATCH 3/9] pack-objects: drop unused return value from add_object_entry() This function returns 0/1 to its caller to tell them whether we actually added a new entry (or if we considered it redundant). But nobody has relied on that behavior since 5379a5c5ee (Thin pack generation: optimization., 2006-04-05). The extra return does not hurt much, but it is a bit confusing. We have a sister function, add_object_entry_from_bitmap(), which has the same return value semantics. That function is about to change to always return 0 (not void, because it must conform to a callback function interface). So with that change, we'd have two related functions which both return an "int" but with different semantics. Let's drop the unused "int" return from add_object_entry() entirely, which makes it more clear that the two functions have diverged. Signed-off-by: Jeff King [ps: slightly massaged the commit message] Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/pack-objects.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index ea5eab4cf8..188c4f6d4b 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -1867,8 +1867,8 @@ static const char no_closure_warning[] = N_( "disabling bitmap writing, as some objects are not being packed" ); -static int add_object_entry(const struct object_id *oid, enum object_type type, - const char *name, int exclude) +static void add_object_entry(const struct object_id *oid, enum object_type type, + const char *name, int exclude) { struct packed_git *found_pack = NULL; off_t found_offset = 0; @@ -1876,7 +1876,7 @@ static int add_object_entry(const struct object_id *oid, enum object_type type, display_progress(progress_state, ++nr_seen); if (have_duplicate_entry(oid, exclude)) - return 0; + return; if (!want_object_in_pack(oid, exclude, &found_pack, &found_offset)) { /* The pack is missing an object, so it will not have closure */ @@ -1885,13 +1885,12 @@ static int add_object_entry(const struct object_id *oid, enum object_type type, warning(_(no_closure_warning)); write_bitmap_index = 0; } - return 0; + return; } create_object_entry(oid, type, pack_name_hash_fn(name), exclude, name && no_try_delta(name), found_pack, found_offset); - return 1; } static int add_object_entry_from_bitmap(const struct object_id *oid, From 1ca65ca7b8bce87268900e39315888fd10fc350c Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 15 Jul 2026 08:22:34 +0200 Subject: [PATCH 4/9] pack-bitmap: allow aborting iteration of bitmapped objects In a subsequent commit we'll lift iteration of bitmapped objects into the "packed" backend and make it accessible via `odb_for_each_object()`. The calling convention for that function is that the callback may return a non-zero exit code, and if so we'll abort iteration. This is currently impossible to realize though, as `for_each_bitmapped_object()` will ignore any return value and just churn through all objects completely. This doesn't matter to the callers of `for_each_bitmapped_object()`, as there's only one of them in git-cat-file(1), and the callbacks we pass always return zero. But once we move the logic into the generic infrastructure it becomes a latent bug waiting to happen. Refactor the code so that the return value of the `show_reach` callback is not ignored anymore. Instead, returning a non-zero value will cause us to abort iteration in both `show_objects_for_type()` and in `for_each_bitmapped_object()`. Note though that there's a second user of `show_objects_for_type()` with `traverse_bitmap_commit_list()`, and that function does indeed invoke callbacks that may return non-zero. This non-zero return value never had any effect at all though, and the callbacks that return non-zero values are only ever invoked via `traverse_bitmap_commit_list()`. Consequently, we adapt them to always return 0. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/pack-objects.c | 2 +- builtin/rev-list.c | 2 +- pack-bitmap.c | 31 +++++++++++++++++++++---------- pack-bitmap.h | 3 ++- 4 files changed, 25 insertions(+), 13 deletions(-) diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index 188c4f6d4b..3673b14b89 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -1908,7 +1908,7 @@ static int add_object_entry_from_bitmap(const struct object_id *oid, return 0; create_object_entry(oid, type, name_hash, 0, 0, pack, offset); - return 1; + return 0; } struct pbase_tree_cache { diff --git a/builtin/rev-list.c b/builtin/rev-list.c index 8f63003709..02818b81c6 100644 --- a/builtin/rev-list.c +++ b/builtin/rev-list.c @@ -486,7 +486,7 @@ static int show_object_fast( void *payload UNUSED) { fprintf(stdout, "%s\n", oid_to_hex(oid)); - return 1; + return 0; } static void print_disk_usage(off_t size) diff --git a/pack-bitmap.c b/pack-bitmap.c index a47c231632..eda38a5433 100644 --- a/pack-bitmap.c +++ b/pack-bitmap.c @@ -1695,7 +1695,7 @@ static void init_type_iterator(struct ewah_or_iterator *it, } } -static void show_objects_for_type( +static int show_objects_for_type( struct bitmap_index *bitmap_git, struct bitmap *objects, enum object_type object_type, @@ -1704,6 +1704,7 @@ static void show_objects_for_type( { size_t i = 0; uint32_t offset; + int ret; struct ewah_or_iterator it; eword_t filter; @@ -1749,11 +1750,17 @@ static void show_objects_for_type( hash = bitmap_name_hash(bitmap_git, index_pos); - show_reach(&oid, object_type, 0, hash, pack, ofs, payload); + ret = show_reach(&oid, object_type, 0, hash, pack, ofs, payload); + if (ret) + goto out; } } + ret = 0; + +out: ewah_or_iterator_release(&it); + return ret; } static int in_bitmapped_pack(struct bitmap_index *bitmap_git, @@ -2062,6 +2069,12 @@ int for_each_bitmapped_object(struct bitmap_index *bitmap_git, show_reachable_fn show_reach, void *payload) { + const enum object_type types[] = { + OBJ_COMMIT, + OBJ_TREE, + OBJ_BLOB, + OBJ_TAG, + }; struct bitmap *filtered_bitmap = NULL; uint32_t objects_nr; size_t full_word_count; @@ -2086,14 +2099,12 @@ int for_each_bitmapped_object(struct bitmap_index *bitmap_git, goto out; } - show_objects_for_type(bitmap_git, filtered_bitmap, - OBJ_COMMIT, show_reach, payload); - show_objects_for_type(bitmap_git, filtered_bitmap, - OBJ_TREE, show_reach, payload); - show_objects_for_type(bitmap_git, filtered_bitmap, - OBJ_BLOB, show_reach, payload); - show_objects_for_type(bitmap_git, filtered_bitmap, - OBJ_TAG, show_reach, payload); + for (size_t i = 0; i < ARRAY_SIZE(types); i++) { + ret = show_objects_for_type(bitmap_git, filtered_bitmap, + types[i], show_reach, payload); + if (ret) + goto out; + } ret = 0; out: diff --git a/pack-bitmap.h b/pack-bitmap.h index 47935eb24e..ae8dc491ac 100644 --- a/pack-bitmap.h +++ b/pack-bitmap.h @@ -93,7 +93,8 @@ struct list_objects_filter_options; /* * Filter bitmapped objects and iterate through all resulting objects, * executing `show_reach` for each of them. Returns `-1` in case the filter is - * not supported, `0` otherwise. + * not supported, `0` otherwise. Aborts iteration and bubbles up the return + * value in case `show_reach()` returns non-zero. */ int for_each_bitmapped_object(struct bitmap_index *bitmap_git, const struct list_objects_filter_options *filter, From eaa9807c970254e503cdb2d719d873521de4ed05 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 15 Jul 2026 08:22:35 +0200 Subject: [PATCH 5/9] pack-bitmap: iterate object sources when opening bitmaps When opening a bitmap for a repository we perform two steps: - We first look for a multi-pack index bitmap in any of the object sources connected to the repository. - We then look for a packfile bitmap in any of the packfiles of any of the object sources. Both of these steps thus iterate through object sources themselves, one via `odb_prepare_alternates()` and one via `repo_for_each_pack()`. This layout makes it hard to introduce a way to open the bitmap of one specific object source, which is functionality that we'll require in a subsequent commit. Reverse the loop so that we instead loop through all sources in the outer loop, and then for each source we try to load its bitmap via either the multi-pack index or via a packfile. Note that this changes the precedence of bitmaps in one specific edge case: when an earlier object source only has a packfile bitmap, but a later source has a multi-pack index bitmap, we now pick the packfile bitmap of the earlier source. Previously, a multi-pack index bitmap from any source would have taken precedence over all packfile bitmaps. Given that object sources are ordered such that the local source comes first, this arguably is an improvement, as we now prefer local bitmaps over bitmaps in alternates. Furthermore, we already warn about repositories that have multiple bitmaps, so this setup is broken and thus arguably not worth worrying about too much. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- pack-bitmap.c | 71 +++++++++++++++++++++++---------------------------- 1 file changed, 32 insertions(+), 39 deletions(-) diff --git a/pack-bitmap.c b/pack-bitmap.c index eda38a5433..e32795a595 100644 --- a/pack-bitmap.c +++ b/pack-bitmap.c @@ -680,60 +680,53 @@ static int load_bitmap(struct repository *r, struct bitmap_index *bitmap_git, return 0; } -static int open_pack_bitmap(struct repository *r, - struct bitmap_index *bitmap_git) +static int open_bitmap_for_source(struct odb_source_packed *source, + struct bitmap_index *bitmap_git) { - struct packed_git *p; - int ret = -1; + struct multi_pack_index *midx = get_multi_pack_index(source); + struct packfile_list_entry *e; + bool found = false; - repo_for_each_pack(r, p) { - if (open_pack_bitmap_1(bitmap_git, p) == 0) { - ret = 0; - /* - * The only reason to keep looking is to report - * duplicates. - */ - if (!trace2_is_enabled()) - break; - } + if (midx && !open_midx_bitmap_1(bitmap_git, midx)) + found = true; + + for (e = packfile_store_get_packs(source); e; e = e->next) { + /* + * When tracing is enabled we want to keep looking to report + * duplicates even if we have already found a bitmap. + */ + if (found && !trace2_is_enabled()) + break; + + if (!open_pack_bitmap_1(bitmap_git, e->pack)) + found = true; } - return ret; + return found ? 0 : -1; } -static int open_midx_bitmap(struct repository *r, - struct bitmap_index *bitmap_git) +static int open_bitmap(struct repository *r, + struct bitmap_index *bitmap_git) { struct odb_source *source; - int ret = -1; + bool found = false; assert(!bitmap_git->map); odb_prepare_alternates(r->objects); for (source = r->objects->sources; source; source = source->next) { struct odb_source_files *files = odb_source_files_downcast(source); - struct multi_pack_index *midx = get_multi_pack_index(files->packed); - if (midx && !open_midx_bitmap_1(bitmap_git, midx)) - ret = 0; + + if (!open_bitmap_for_source(files->packed, bitmap_git)) + found = true; + + /* + * The only reason to keep looking after having found a bitmap + * is to report duplicates. + */ + if (found && !trace2_is_enabled()) + break; } - return ret; -} - -static int open_bitmap(struct repository *r, - struct bitmap_index *bitmap_git) -{ - int found; - - assert(!bitmap_git->map); - - found = !open_midx_bitmap(r, bitmap_git); - - /* - * these will all be skipped if we opened a midx bitmap; but run it - * anyway if tracing is enabled to report the duplicates - */ - if (!found || trace2_is_enabled()) - found |= !open_pack_bitmap(r, bitmap_git); return found ? 0 : -1; } From db95bfc121aa9725e8128d7b1dd73330c0beb7f9 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 15 Jul 2026 08:22:36 +0200 Subject: [PATCH 6/9] pack-bitmap: drop `_1` suffix from functions that open bitmaps In the preceding commit we've refactored how we open bitmaps. As part of the refactoring we have consolidated `open_pack_bitmap()` as well as `open_midx_bitmap()` into `open_bitmap_for_source()`. Consequently, we only have their `open_pack_bitmap_1()` and `open_midx_bitmap_1()` variants left over, where the `_1` suffix doesn't really make much sense anymore. Drop the suffix. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- pack-bitmap.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/pack-bitmap.c b/pack-bitmap.c index e32795a595..72c8ae3228 100644 --- a/pack-bitmap.c +++ b/pack-bitmap.c @@ -460,8 +460,8 @@ char *pack_bitmap_filename(struct packed_git *p) return xstrfmt("%.*s.bitmap", (int)len, p->pack_name); } -static int open_midx_bitmap_1(struct bitmap_index *bitmap_git, - struct multi_pack_index *midx) +static int open_midx_bitmap(struct bitmap_index *bitmap_git, + struct multi_pack_index *midx) { struct stat st; char *bitmap_name = midx_bitmap_filename(midx); @@ -539,7 +539,7 @@ cleanup: return -1; } -static int open_pack_bitmap_1(struct bitmap_index *bitmap_git, struct packed_git *packfile) +static int open_pack_bitmap(struct bitmap_index *bitmap_git, struct packed_git *packfile) { int fd; struct stat st; @@ -603,7 +603,7 @@ static int load_reverse_index(struct repository *r, struct bitmap_index *bitmap_ /* * The multi-pack-index's .rev file is already loaded via - * open_pack_bitmap_1(). + * open_pack_bitmap(). * * But we still need to open the individual pack .rev files, * since we will need to make use of them in pack-objects. @@ -687,7 +687,7 @@ static int open_bitmap_for_source(struct odb_source_packed *source, struct packfile_list_entry *e; bool found = false; - if (midx && !open_midx_bitmap_1(bitmap_git, midx)) + if (midx && !open_midx_bitmap(bitmap_git, midx)) found = true; for (e = packfile_store_get_packs(source); e; e = e->next) { @@ -698,7 +698,7 @@ static int open_bitmap_for_source(struct odb_source_packed *source, if (found && !trace2_is_enabled()) break; - if (!open_pack_bitmap_1(bitmap_git, e->pack)) + if (!open_pack_bitmap(bitmap_git, e->pack)) found = true; } @@ -746,7 +746,7 @@ struct bitmap_index *prepare_midx_bitmap_git(struct multi_pack_index *midx) { struct bitmap_index *bitmap_git = xcalloc(1, sizeof(*bitmap_git)); - if (!open_midx_bitmap_1(bitmap_git, midx)) + if (!open_midx_bitmap(bitmap_git, midx)) return bitmap_git; free_bitmap_index(bitmap_git); From 8cd7ee7b0d12fce9449ea08fe394e18dc97fe059 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 15 Jul 2026 08:22:37 +0200 Subject: [PATCH 7/9] pack-bitmap: introduce function to open bitmap for a single source The function `prepare_bitmap_git()` opens the first bitmap it can find in any of the object sources connected to the repository. In a subsequent commit, the "packed" object database backend will learn to use bitmaps to answer object filters when enumerating objects. That backend operates on a single object source though, so using a bitmap that potentially belongs to a different source would be wrong: - The source would yield objects that are not part of the source itself. - The object source info would be attributed to the wrong source. - With multiple sources, each source would enumerate the same bitmap another time. Introduce a new function `prepare_bitmap_git_for_source()` that only opens bitmaps belonging to the given object source. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- pack-bitmap.c | 12 ++++++++++++ pack-bitmap.h | 2 ++ 2 files changed, 14 insertions(+) diff --git a/pack-bitmap.c b/pack-bitmap.c index 72c8ae3228..09ba15d26b 100644 --- a/pack-bitmap.c +++ b/pack-bitmap.c @@ -753,6 +753,18 @@ struct bitmap_index *prepare_midx_bitmap_git(struct multi_pack_index *midx) return NULL; } +struct bitmap_index *prepare_bitmap_git_for_source(struct odb_source_packed *source) +{ + struct bitmap_index *bitmap_git = xcalloc(1, sizeof(*bitmap_git)); + + if (!open_bitmap_for_source(source, bitmap_git) && + !load_bitmap(source->base.odb->repo, bitmap_git, 0)) + return bitmap_git; + + free_bitmap_index(bitmap_git); + return NULL; +} + int bitmap_index_contains_pack(struct bitmap_index *bitmap, struct packed_git *pack) { for (; bitmap; bitmap = bitmap->base) { diff --git a/pack-bitmap.h b/pack-bitmap.h index ae8dc491ac..9f20fb6e56 100644 --- a/pack-bitmap.h +++ b/pack-bitmap.h @@ -9,6 +9,7 @@ #include "string-list.h" struct commit; +struct odb_source_packed; struct repository; struct rev_info; @@ -68,6 +69,7 @@ struct bitmapped_pack { struct bitmap_index *prepare_bitmap_git(struct repository *r); struct bitmap_index *prepare_midx_bitmap_git(struct multi_pack_index *midx); +struct bitmap_index *prepare_bitmap_git_for_source(struct odb_source_packed *source); /* * Given a bitmap index, determine whether it contains the pack either directly From 204daf5e5c6f931352ddde16236e9705075d15bf Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 15 Jul 2026 08:22:38 +0200 Subject: [PATCH 8/9] odb: introduce object filters to `odb_for_each_object()` The function `for_each_bitmapped_object()` can be used to iterate through all objects covered by a bitmap. The benefit of this function is that it allows the caller to efficiently handle some object filters. For example, this can be used to filter out objects of a specific type with some simple bitmap operations. But callers are currently required to manually wire up the use of bitmaps though, and to do so they have to reach into internals of a given object database source. Introduce a new `struct odb_for_each_object_options::filter` field so that the interface becomes generic. When set, then a backend may optionally use the filter to skip some objects that it would have otherwise yielded. Note that the respective backends are free to ignore this field if they cannot meaningfully optimize for a given filter, and consequently callers need to verify whether they actually want the returned objects. While annoying, we cannot easily lift this restriction anyway as the object filter infrastructure supports some filters that cannot be answered by the object database alone. An alternative might be to limit the filters to only those that _can_ be answered by backends. But ultimately, the filters that can be answered efficiently by the "packed" backend are completely disjunct from those that can be answered by the "loose" backend, and consequently the set of filters supported by all backends would be empty. Furthermore, it would require us to make assumptions about capabilities of future backends, which may be able to efficiently handle more filters than current ones. So in the end, this alternative would only limit us artificially. Implement the logic for the "packed" source. Note that we use the new function `prepare_bitmap_git_for_source()` to open the bitmap: as the backend operates on a single object source, we must only use bitmaps that belong to that specific source. Otherwise we might yield objects that are not part of the source at all, and with multiple sources we would enumerate the same bitmap once per source. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- odb.h | 12 +++++++++ odb/source-packed.c | 62 +++++++++++++++++++++++++++++++++++++++++++++ pack-bitmap.c | 3 +-- pack-bitmap.h | 3 +++ 4 files changed, 78 insertions(+), 2 deletions(-) diff --git a/odb.h b/odb.h index a1e222f605..67d0b34942 100644 --- a/odb.h +++ b/odb.h @@ -8,6 +8,7 @@ #include "thread-utils.h" struct cached_object_entry; +struct list_objects_filter_options; struct odb_source_inmemory; struct packed_git; struct repository; @@ -490,6 +491,17 @@ struct odb_for_each_object_options { */ const struct object_id *prefix; size_t prefix_hex_len; + + /* + * Optional object filter that allows backends to skip yielding + * objects that are excluded by the filter as an optimization. The + * filter is a best-effort hint: backends may use it to skip + * excluded objects (e.g. by consulting a reachability bitmap), but + * are also free to ignore it entirely and yield every object. As a + * consequence, callers must re-apply the filter on yielded objects + * if they require strict filtering semantics. + */ + const struct list_objects_filter_options *filter; }; /* diff --git a/odb/source-packed.c b/odb/source-packed.c index 9cfa02b7a2..4777395053 100644 --- a/odb/source-packed.c +++ b/odb/source-packed.c @@ -3,11 +3,13 @@ #include "chdir-notify.h" #include "dir.h" #include "git-zlib.h" +#include "list-objects-filter-options.h" #include "mergesort.h" #include "midx.h" #include "odb/source-packed.h" #include "odb/streaming.h" #include "packfile.h" +#include "pack-bitmap.h" static int find_pack_entry(struct odb_source_packed *store, const struct object_id *oid, @@ -315,6 +317,37 @@ out: return ret; } +struct bitmapped_for_each_object_data { + struct odb_source_packed *packed; + const struct object_info *request; + const struct odb_for_each_object_options *opts; + odb_for_each_object_cb cb; + void *cb_data; +}; + +static int bitmapped_for_each_object(const struct object_id *oid, + enum object_type type UNUSED, + int flags UNUSED, + uint32_t hash UNUSED, + struct packed_git *pack, + off_t offset, + void *cb_data) +{ + struct bitmapped_for_each_object_data *data = cb_data; + + if (should_exclude_pack(pack, data->opts->flags)) + return 0; + + if (data->request) { + struct object_info oi = *data->request; + if (packed_object_info(data->packed, pack, offset, &oi) < 0) + return -1; + return data->cb(oid, &oi, data->cb_data); + } + + return data->cb(oid, NULL, data->cb_data); +} + static int odb_source_packed_for_each_object(struct odb_source *source, const struct object_info *request, odb_for_each_object_cb cb, @@ -328,12 +361,33 @@ static int odb_source_packed_for_each_object(struct odb_source *source, .cb = cb, .cb_data = cb_data, }; + struct bitmap_index *bitmap = NULL; struct packfile_list_entry *e; int pack_errors = 0, ret; if (opts->prefix) return odb_source_packed_for_each_prefixed_object(packed, opts, &data); + if (opts->filter && + opts->filter->choice != LOFC_DISABLED && + can_filter_bitmap(opts->filter)) + bitmap = prepare_bitmap_git_for_source(packed); + if (bitmap) { + struct bitmapped_for_each_object_data bitmap_data = { + .packed = packed, + .request = request, + .opts = opts, + .cb = cb, + .cb_data = cb_data, + }; + + ret = for_each_bitmapped_object(bitmap, opts->filter, + bitmapped_for_each_object, + &bitmap_data); + if (ret) + goto out; + } + packed->skip_mru_updates = true; for (e = packfile_store_get_packs(packed); e; e = e->next) { @@ -342,6 +396,13 @@ static int odb_source_packed_for_each_object(struct odb_source *source, if (should_exclude_pack(p, opts->flags)) continue; + /* + * Objects covered by the bitmap have already been yielded + * above; skip them here to avoid duplicates. + */ + if (bitmap && bitmap_index_contains_pack(bitmap, p)) + continue; + if (open_pack_index(p)) { pack_errors = 1; continue; @@ -357,6 +418,7 @@ static int odb_source_packed_for_each_object(struct odb_source *source, out: packed->skip_mru_updates = false; + free_bitmap_index(bitmap); if (!ret && pack_errors) ret = -1; diff --git a/pack-bitmap.c b/pack-bitmap.c index 09ba15d26b..f55a0859ea 100644 --- a/pack-bitmap.c +++ b/pack-bitmap.c @@ -2039,12 +2039,11 @@ static int filter_bitmap(struct bitmap_index *bitmap_git, return -1; } -static int can_filter_bitmap(const struct list_objects_filter_options *filter) +bool can_filter_bitmap(const struct list_objects_filter_options *filter) { return !filter_bitmap(NULL, NULL, NULL, filter); } - static void filter_packed_objects_from_bitmap(struct bitmap_index *bitmap_git, struct bitmap *result) { diff --git a/pack-bitmap.h b/pack-bitmap.h index 9f20fb6e56..1385027c1f 100644 --- a/pack-bitmap.h +++ b/pack-bitmap.h @@ -92,6 +92,9 @@ int test_bitmap_pseudo_merge_objects(struct repository *r, uint32_t n); struct list_objects_filter_options; +/* Check whether the filter can be computed via the bitmap. */ +bool can_filter_bitmap(const struct list_objects_filter_options *filter); + /* * Filter bitmapped objects and iterate through all resulting objects, * executing `show_reach` for each of them. Returns `-1` in case the filter is From bfa86e6a3a80b910f0875350d747f7a831694b18 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 15 Jul 2026 08:22:39 +0200 Subject: [PATCH 9/9] builtin/cat-file: filter objects via object database When batching all objects, git-cat-file(1) reaches into the internals of the object database and manually manages bitmaps to apply object filters. This creates coupling between the command and the internals of the respective backend. Refactor git-cat-file(1) to use the new object filter option when batching all objects. This significantly simplifies the logic and ensures that we don't have to reach into internals of the "files" source anymore. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- builtin/cat-file.c | 76 +++++----------------------------------------- 1 file changed, 7 insertions(+), 69 deletions(-) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index b4b99a73da..1458dd76d6 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -20,7 +20,6 @@ #include "userdiff.h" #include "oid-array.h" #include "packfile.h" -#include "pack-bitmap.h" #include "object-file.h" #include "object-name.h" #include "odb.h" @@ -844,28 +843,6 @@ static int batch_one_object_oi(const struct object_id *oid, return payload->callback(oid, NULL, 0, payload->payload); } -static int batch_one_object_packed(const struct object_id *oid, - struct packed_git *pack, - uint32_t pos, - void *_payload) -{ - struct for_each_object_payload *payload = _payload; - return payload->callback(oid, pack, nth_packed_object_offset(pack, pos), - payload->payload); -} - -static int batch_one_object_bitmapped(const struct object_id *oid, - enum object_type type UNUSED, - int flags UNUSED, - uint32_t hash UNUSED, - struct packed_git *pack, - off_t offset, - void *_payload) -{ - struct for_each_object_payload *payload = _payload; - return payload->callback(oid, pack, offset, payload->payload); -} - static void batch_each_object(struct batch_options *opt, for_each_object_fn callback, unsigned flags, @@ -875,56 +852,17 @@ static void batch_each_object(struct batch_options *opt, .callback = callback, .payload = _payload, }; + struct odb_source_info source_info; + struct object_info oi = { + .source_infop = &source_info, + }; struct odb_for_each_object_options opts = { .flags = flags, + .filter = &opt->objects_filter, }; - struct bitmap_index *bitmap = NULL; - struct odb_source *source; - /* - * TODO: we still need to tap into implementation details of the object - * database sources. Ideally, we should extend `odb_for_each_object()` - * to handle object filters itself so that we can move the filtering - * logic into the individual sources. - */ - odb_prepare_alternates(the_repository->objects); - for (source = the_repository->objects->sources; source; source = source->next) { - struct odb_source_files *files = odb_source_files_downcast(source); - int ret = odb_source_for_each_object(&files->loose->base, NULL, batch_one_object_oi, - &payload, &opts); - if (ret) - break; - } - - if (opt->objects_filter.choice != LOFC_DISABLED && - (bitmap = prepare_bitmap_git(the_repository)) && - !for_each_bitmapped_object(bitmap, &opt->objects_filter, - batch_one_object_bitmapped, &payload)) { - struct packed_git *pack; - - repo_for_each_pack(the_repository, pack) { - if (bitmap_index_contains_pack(bitmap, pack) || - open_pack_index(pack)) - continue; - for_each_object_in_pack(pack, batch_one_object_packed, - &payload, flags); - } - } else { - struct odb_source_info source_info; - struct object_info oi = { - .source_infop = &source_info, - }; - - for (source = the_repository->objects->sources; source; source = source->next) { - struct odb_source_files *files = odb_source_files_downcast(source); - int ret = odb_source_for_each_object(&files->packed->base, &oi, - batch_one_object_oi, &payload, &opts); - if (ret) - break; - } - } - - free_bitmap_index(bitmap); + odb_for_each_object_ext(the_repository->objects, &oi, + batch_one_object_oi, &payload, &opts); } static int batch_objects(struct batch_options *opt)