@leroysheep / rag-system / commits / 7d663dd83b

builtin/gc: move geometric repacking into `odb_optimize()`

We have two major object database optimization strategies: - The legacy strategy used by git-gc(1), which absorbs loose objects into packfiles, and eventually merges all packfiles once we have too many of them. - The more recent "geometric" strategy used by git-maintenance(1), which merges packfiles using a geometric sequence. These two strategies are still using completely separate code paths. In a subsequent commit we'll want to make both strategies pluggable though. Prepare for this change by merging the "geometric" strategy into `odb_optimize()`. This also allows us to reuse some of the logic we have in that function. Note that this change requires us to adapt tests because we're now using "-q" instead of "--quiet". Naturally though, these invocations are of course equivalent to one another. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Jul 13, 2026 at 07:52 UTC 7d663dd83ba2ae459178c98f96c0761d044698b4
2 files changed +96 -93
builtin/gc.c
+87 -84
index 17490106fc..c8504f4456 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -593,6 +593,11 @@ static int keep_one_pack(struct string_list_item *item, void *data) return 0; } +enum odb_optimize_strategy { + ODB_OPTIMIZE_INCREMENTAL, + ODB_OPTIMIZE_GEOMETRIC, +}; + enum odb_optimize_flags { /* Enable verbose logging and progress reporting. */ ODB_OPTIMIZE_VERBOSE = (1 << 0), @@ -605,6 +610,7 @@ enum odb_optimize_flags { }; struct odb_optimize_options { + enum odb_optimize_strategy strategy; enum odb_optimize_flags flags; const char *prune_expire; const char *expire_to; @@ -858,49 +864,87 @@ static int odb_optimize(struct object_database *odb, * * - Otherwise we perform an incremental repack. */ - if (!(opts->flags & ODB_OPTIMIZE_AUTO)) { - struct string_list keep_pack = STRING_LIST_INIT_NODUP; - - if (opts->keep_largest_pack != -1) { - if (opts->keep_largest_pack) - find_base_packs(&keep_pack, 0); - } else if (big_pack_threshold) { - find_base_packs(&keep_pack, big_pack_threshold); - } - - add_repack_all_option(opts, &keep_pack, &repack_cmd.args); - string_list_clear(&keep_pack, 0); - } else { - if (too_many_packs(gc_auto_pack_limit)) { + switch (opts->strategy) { + case ODB_OPTIMIZE_INCREMENTAL: + if (!(opts->flags & ODB_OPTIMIZE_AUTO)) { struct string_list keep_pack = STRING_LIST_INIT_NODUP; - if (big_pack_threshold) { - find_base_packs(&keep_pack, big_pack_threshold); - if (keep_pack.nr >= gc_auto_pack_limit) { - string_list_clear(&keep_pack, 0); + if (opts->keep_largest_pack != -1) { + if (opts->keep_largest_pack) find_base_packs(&keep_pack, 0); - } - } else { - struct packed_git *p = find_base_packs(&keep_pack, 0); - uint64_t mem_have, mem_want; - - mem_have = total_ram(); - mem_want = estimate_repack_memory(p); - - /* - * Only allow 1/2 of memory for pack-objects, leave - * the rest for the OS and other processes in the - * system. - */ - if (!mem_have || mem_want < mem_have / 2) - string_list_clear(&keep_pack, 0); + } else if (big_pack_threshold) { + find_base_packs(&keep_pack, big_pack_threshold); } add_repack_all_option(opts, &keep_pack, &repack_cmd.args); string_list_clear(&keep_pack, 0); } else { - add_repack_incremental_option(&repack_cmd.args); + if (too_many_packs(gc_auto_pack_limit)) { + struct string_list keep_pack = STRING_LIST_INIT_NODUP; + + if (big_pack_threshold) { + find_base_packs(&keep_pack, big_pack_threshold); + if (keep_pack.nr >= gc_auto_pack_limit) { + string_list_clear(&keep_pack, 0); + find_base_packs(&keep_pack, 0); + } + } else { + struct packed_git *p = find_base_packs(&keep_pack, 0); + uint64_t mem_have, mem_want; + + mem_have = total_ram(); + mem_want = estimate_repack_memory(p); + + /* + * Only allow 1/2 of memory for pack-objects, leave + * the rest for the OS and other processes in the + * system. + */ + if (!mem_have || mem_want < mem_have / 2) + string_list_clear(&keep_pack, 0); + } + + add_repack_all_option(opts, &keep_pack, &repack_cmd.args); + string_list_clear(&keep_pack, 0); + } else { + add_repack_incremental_option(&repack_cmd.args); + } } + + break; + case ODB_OPTIMIZE_GEOMETRIC: { + struct pack_geometry geometry = { + .split_factor = 2, + }; + struct pack_objects_args po_args = { + .local = 1, + }; + struct existing_packs existing_packs = EXISTING_PACKS_INIT; + struct string_list kept_packs = STRING_LIST_INIT_DUP; + + repo_config_get_int(the_repository, "maintenance.geometric-repack.splitFactor", + &geometry.split_factor); + + existing_packs.repo = the_repository; + existing_packs_collect(&existing_packs, &kept_packs); + pack_geometry_init(&geometry, &existing_packs, &po_args); + pack_geometry_split(&geometry); + + if (geometry.split < geometry.pack_nr) { + strvec_pushf(&repack_cmd.args, "--geometric=%d", + geometry.split_factor); + } else { + add_repack_all_option(opts, NULL, &repack_cmd.args); + } + if (the_repository->settings.core_multi_pack_index) + strvec_push(&repack_cmd.args, "--write-midx"); + + existing_packs_release(&existing_packs); + pack_geometry_release(&geometry); + break; + } + default: + die("unknown maintenance strategy '%d'", opts->strategy); } if (run_command(&repack_cmd)) { @@ -908,7 +952,8 @@ static int odb_optimize(struct object_database *odb, goto out; } - if (opts->prune_expire) { + /* Geometric repacking uses cruft packs, so we don't have to prune separately. */ + if (opts->strategy != ODB_OPTIMIZE_GEOMETRIC && opts->prune_expire) { struct child_process prune_cmd = CHILD_PROCESS_INIT; strvec_pushl(&prune_cmd.args, "prune", "--expire", NULL); @@ -943,6 +988,7 @@ static int maintenance_task_odb(struct maintenance_run_opts *opts, int aggressive) { struct odb_optimize_options odb_opts = { + .strategy = ODB_OPTIMIZE_INCREMENTAL, .keep_largest_pack = keep_largest_pack, OPTIMIZE_FIELDS_FROM_GC_CONFIG(cfg, aggressive), }; @@ -1624,58 +1670,15 @@ static int maintenance_task_incremental_repack(struct maintenance_run_opts *opts static int maintenance_task_geometric_repack(struct maintenance_run_opts *opts, struct gc_config *cfg) { - struct pack_geometry geometry = { - .split_factor = 2, - }; - struct pack_objects_args po_args = { - .local = 1, + struct odb_optimize_options odb_opts = { + .strategy = ODB_OPTIMIZE_GEOMETRIC, + OPTIMIZE_FIELDS_FROM_GC_CONFIG(cfg, 0), }; - struct existing_packs existing_packs = EXISTING_PACKS_INIT; - struct string_list kept_packs = STRING_LIST_INIT_DUP; - struct child_process child = CHILD_PROCESS_INIT; - int ret; - - repo_config_get_int(the_repository, "maintenance.geometric-repack.splitFactor", - &geometry.split_factor); - - existing_packs.repo = the_repository; - existing_packs_collect(&existing_packs, &kept_packs); - pack_geometry_init(&geometry, &existing_packs, &po_args); - pack_geometry_split(&geometry); - - child.git_cmd = 1; - child.odb_to_close = the_repository->objects; - - strvec_pushl(&child.args, "repack", "-d", "-l", NULL); - if (geometry.split < geometry.pack_nr) { - strvec_pushf(&child.args, "--geometric=%d", - geometry.split_factor); - } else { - struct odb_optimize_options odb_opts = { - OPTIMIZE_FIELDS_FROM_GC_CONFIG(cfg, 0), - }; - if (!opts->quiet) - odb_opts.flags |= ODB_OPTIMIZE_VERBOSE; - - add_repack_all_option(&odb_opts, NULL, &child.args); - } - if (opts->quiet) - strvec_push(&child.args, "--quiet"); - if (the_repository->settings.core_multi_pack_index) - strvec_push(&child.args, "--write-midx"); - - if (run_command(&child)) { - ret = error(_("failed to perform geometric repack")); - goto out; - } - - ret = 0; + if (!opts->quiet) + odb_opts.flags |= ODB_OPTIMIZE_VERBOSE; -out: - existing_packs_release(&existing_packs); - pack_geometry_release(&geometry); - return ret; + return odb_optimize(the_repository->objects, &odb_opts); } static int geometric_repack_auto_condition(struct gc_config *cfg UNUSED)
t/t7900-maintenance.sh
+9 -9
index 2d52e7918a..6d87da2ae4 100755 --- a/t/t7900-maintenance.sh +++ b/t/t7900-maintenance.sh @@ -574,8 +574,8 @@ run_and_verify_geometric_pack () { rm -f "trace2.txt" && GIT_TRACE2_EVENT="$(pwd)/trace2.txt" \ git maintenance run --task=geometric-repack 2>/dev/null && - test_subcommand git repack -d -l --geometric=2 \ - --quiet --write-midx <trace2.txt && + test_subcommand git repack -d -l -q --geometric=2 \ + --write-midx <trace2.txt && # Verify that the number of packfiles matches our expectation. ls -l .git/objects/pack/*.pack >packfiles && @@ -606,8 +606,8 @@ test_expect_success 'geometric repacking task' ' # The initial repack causes an all-into-one repack. GIT_TRACE2_EVENT="$(pwd)/initial-repack.txt" \ git maintenance run --task=geometric-repack 2>/dev/null && - test_subcommand git repack -d -l --cruft --cruft-expiration=2.weeks.ago \ - --quiet --write-midx <initial-repack.txt && + test_subcommand git repack -d -l -q --cruft --cruft-expiration=2.weeks.ago \ + --write-midx <initial-repack.txt && # Repacking should now cause a no-op geometric repack because # no packfiles need to be combined. @@ -627,8 +627,8 @@ test_expect_success 'geometric repacking task' ' # an all-into-one-repack. GIT_TRACE2_EVENT="$(pwd)/all-into-one-repack.txt" \ git maintenance run --task=geometric-repack 2>/dev/null && - test_subcommand git repack -d -l --cruft --cruft-expiration=2.weeks.ago \ - --quiet --write-midx <all-into-one-repack.txt && + test_subcommand git repack -d -l -q --cruft --cruft-expiration=2.weeks.ago \ + --write-midx <all-into-one-repack.txt && # The geometric repack soaks up unreachable objects. echo blob-1 | git hash-object -w --stdin -t blob && @@ -662,8 +662,8 @@ test_expect_success 'geometric repacking task' ' run_and_verify_geometric_pack 3 && GIT_TRACE2_EVENT="$(pwd)/cruft-repack.txt" \ git maintenance run --task=geometric-repack 2>/dev/null && - test_subcommand git repack -d -l --cruft --cruft-expiration=2.weeks.ago \ - --quiet --write-midx <cruft-repack.txt && + test_subcommand git repack -d -l -q --cruft --cruft-expiration=2.weeks.ago \ + --write-midx <cruft-repack.txt && ls .git/objects/pack/*.pack >packs && test_line_count = 2 packs && ls .git/objects/pack/*.mtimes >cruft && @@ -754,7 +754,7 @@ test_expect_success 'geometric repacking honors configured split factor' ' test_geometric_repack_needed false splitFactor=2 && test_geometric_repack_needed true splitFactor=3 && - test_subcommand git repack -d -l --geometric=3 --quiet --write-midx <trace2.txt + test_subcommand git repack -d -l -q --geometric=3 --write-midx <trace2.txt ) '