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) diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index ea5eab4cf8..3673b14b89 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, @@ -1909,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/odb.h b/odb.h index 94754643d2..7995bed97b 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; @@ -502,6 +503,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 8d9ce197cc..d00aec5a45 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, @@ -143,7 +145,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 +172,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 +180,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 +195,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 +221,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 +248,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; @@ -314,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, @@ -327,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) { @@ -341,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; @@ -356,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 1fc3e62d5e..d8dc4ae8d1 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); @@ -543,7 +543,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; @@ -607,7 +607,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. @@ -684,60 +684,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(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(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; } @@ -757,7 +750,19 @@ 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); + 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); @@ -1699,7 +1704,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, @@ -1708,6 +1713,7 @@ static void show_objects_for_type( { size_t i = 0; uint32_t offset; + int ret; struct ewah_or_iterator it; eword_t filter; @@ -1753,11 +1759,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, @@ -1980,7 +1992,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; @@ -2031,12 +2043,11 @@ static int filter_bitmap(struct bitmap_index *bitmap_git, return -1; } -static int can_filter_bitmap(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) { @@ -2062,10 +2073,16 @@ 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) { + 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; @@ -2090,14 +2107,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 19a8655457..1385027c1f 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 @@ -90,13 +92,17 @@ 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 - * 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, - struct list_objects_filter_options *filter, + const struct list_objects_filter_options *filter, show_reachable_fn show_reach, void *payload);