[PATCH v2 05/12] builtin/gc: make repack arguments self-contained
From: Patrick Steinhardt <hidden>
Date: 2026-07-13 05:52:29
Subsystem:
the rest · Maintainer:
Linus Torvalds
When optimizing the object database most of the heavy-lifting is done by git-repack(1). The arguments we pass to this function are assembled in global scope, which is hard to follow. Refactor the logic by moving the vector into `maintenance_task_odb()`. While that means we have to pass more arguments to this function, it has the upside that the logic becomes self-contained without any kind of global interdependencies. This is a pure refactoring with no intended functional change. Signed-off-by: Patrick Steinhardt <redacted> --- builtin/gc.c | 156 ++++++++++++++++++++++++++++------------------------------- 1 file changed, 75 insertions(+), 81 deletions(-)
diff --git a/builtin/gc.c b/builtin/gc.c
index 2ff98fa727..25a59caea6 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c@@ -661,7 +661,7 @@ static void add_repack_incremental_option(struct strvec *args) strvec_push(args, "--no-write-bitmap-index"); } -static int need_to_gc(struct gc_config *cfg, struct strvec *repack_args) +static int need_to_gc(struct gc_config *cfg) { /* * Setting gc.auto to 0 or negative can disable the
@@ -669,46 +669,8 @@ static int need_to_gc(struct gc_config *cfg, struct strvec *repack_args) */ if (cfg->gc_auto_threshold <= 0) return 0; - - /* - * If there are too many loose objects, but not too many - * packs, we run "repack -d -l". If there are too many packs, - * we run "repack -A -d -l". Otherwise we tell the caller - * there is no need. - */ - if (too_many_packs(cfg)) { - struct string_list keep_pack = STRING_LIST_INIT_NODUP; - - if (cfg->big_pack_threshold) { - find_base_packs(&keep_pack, cfg->big_pack_threshold); - if (keep_pack.nr >= cfg->gc_auto_pack_limit) { - cfg->big_pack_threshold = 0; - 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(cfg, 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(cfg, &keep_pack, repack_args); - string_list_clear(&keep_pack, 0); - } else if (too_many_loose_objects(cfg->gc_auto_threshold)) - add_repack_incremental_option(repack_args); - else + if (!too_many_packs(cfg) && !too_many_loose_objects(cfg->gc_auto_threshold)) return 0; - return 1; }
@@ -841,7 +803,8 @@ static int gc_foreground_tasks(struct maintenance_run_opts *opts, static int maintenance_task_odb(struct maintenance_run_opts *opts, struct gc_config *cfg, - struct strvec *repack_args) + int keep_largest_pack, + int aggressive) { struct child_process repack_cmd = CHILD_PROCESS_INIT; int ret;
@@ -851,9 +814,75 @@ static int maintenance_task_odb(struct maintenance_run_opts *opts, repack_cmd.git_cmd = 1; repack_cmd.odb_to_close = the_repository->objects; - strvec_pushv(&repack_cmd.args, repack_args->v); + + strvec_pushl(&repack_cmd.args, "repack", "-d", "-l", NULL); + if (aggressive) { + strvec_push(&repack_cmd.args, "-f"); + if (cfg->aggressive_depth > 0) + strvec_pushf(&repack_cmd.args, "--depth=%d", cfg->aggressive_depth); + if (cfg->aggressive_window > 0) + strvec_pushf(&repack_cmd.args, "--window=%d", cfg->aggressive_window); + } + if (opts->quiet) + strvec_push(&repack_cmd.args, "-q"); + + /* + * There's three cases we need to consider: + * + * - If we're invoked without `--auto` we'll need to perform a full + * repack. + * + * - If we're invoked with `--auto` and there's too many packs, then + * we perform a full repack, as well. + * + * - Otherwise we perform an incremental repack. + */ + if (!opts->auto_flag) { + struct string_list keep_pack = STRING_LIST_INIT_NODUP; + + if (keep_largest_pack != -1) { + if (keep_largest_pack) + find_base_packs(&keep_pack, 0); + } else if (cfg->big_pack_threshold) { + find_base_packs(&keep_pack, cfg->big_pack_threshold); + } + + add_repack_all_option(cfg, &keep_pack, &repack_cmd.args); + string_list_clear(&keep_pack, 0); + } else if (too_many_packs(cfg)) { + struct string_list keep_pack = STRING_LIST_INIT_NODUP; + + if (cfg->big_pack_threshold) { + find_base_packs(&keep_pack, cfg->big_pack_threshold); + if (keep_pack.nr >= cfg->gc_auto_pack_limit) { + cfg->big_pack_threshold = 0; + 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(cfg, 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(cfg, &keep_pack, &repack_cmd.args); + string_list_clear(&keep_pack, 0); + } else { + add_repack_incremental_option(&repack_cmd.args); + } + if (run_command(&repack_cmd)) { - ret = error(FAILED_RUN, repack_args->v[0]); + ret = error(FAILED_RUN, repack_cmd.args.v[0]); goto out; }
@@ -899,7 +928,6 @@ int cmd_gc(int argc, int keep_largest_pack = -1; int skip_foreground_tasks = 0; timestamp_t dummy; - struct strvec repack_args = STRVEC_INIT; struct maintenance_run_opts opts = MAINTENANCE_RUN_OPTS_INIT; struct gc_config cfg = GC_CONFIG_INIT; const char *prune_expire_sentinel = "sentinel";
@@ -939,8 +967,6 @@ int cmd_gc(int argc, show_usage_with_options_if_asked(argc, argv, builtin_gc_usage, builtin_gc_options); - strvec_pushl(&repack_args, "repack", "-d", "-l", NULL); - gc_config(&cfg); if (parse_expiry_date(cfg.gc_log_expire, &gc_log_expire_time))
@@ -961,16 +987,6 @@ int cmd_gc(int argc, if (cfg.prune_expire && parse_expiry_date(cfg.prune_expire, &dummy)) die(_("failed to parse prune expiry value %s"), cfg.prune_expire); - if (aggressive) { - strvec_push(&repack_args, "-f"); - if (cfg.aggressive_depth > 0) - strvec_pushf(&repack_args, "--depth=%d", cfg.aggressive_depth); - if (cfg.aggressive_window > 0) - strvec_pushf(&repack_args, "--window=%d", cfg.aggressive_window); - } - if (opts.quiet) - strvec_push(&repack_args, "-q"); - if (opts.auto_flag) { if (cfg.detach_auto && opts.detach < 0) opts.detach = 1;
@@ -978,8 +994,7 @@ int cmd_gc(int argc, /* * Auto-gc should be least intrusive as possible. */ - if (!need_to_gc(&cfg, &repack_args) || - run_hooks(the_repository, "pre-auto-gc")) { + if (!need_to_gc(&cfg) || run_hooks(the_repository, "pre-auto-gc")) { ret = 0; goto out; }
@@ -991,18 +1006,6 @@ int cmd_gc(int argc, fprintf(stderr, _("Auto packing the repository for optimum performance.\n")); fprintf(stderr, _("See \"git help gc\" for manual housekeeping.\n")); } - } else { - struct string_list keep_pack = STRING_LIST_INIT_NODUP; - - if (keep_largest_pack != -1) { - if (keep_largest_pack) - find_base_packs(&keep_pack, 0); - } else if (cfg.big_pack_threshold) { - find_base_packs(&keep_pack, cfg.big_pack_threshold); - } - - add_repack_all_option(&cfg, &keep_pack, &repack_args); - string_list_clear(&keep_pack, 0); } if (opts.detach > 0) {
@@ -1065,7 +1068,7 @@ int cmd_gc(int argc, if (maintenance_task_rerere_gc(&opts, &cfg)) die(FAILED_RUN, "rerere"); - if (maintenance_task_odb(&opts, &cfg, &repack_args)) + if (maintenance_task_odb(&opts, &cfg, keep_largest_pack, aggressive)) die(NULL); report_garbage = report_pack_garbage;
@@ -1088,7 +1091,6 @@ int cmd_gc(int argc, out: maintenance_run_opts_release(&opts); - strvec_clear(&repack_args); gc_config_release(&cfg); return 0; }
@@ -1291,15 +1293,7 @@ static int maintenance_task_gc_background(struct maintenance_run_opts *opts, static int gc_condition(struct gc_config *cfg) { - /* - * Note that it's fine to drop the repack arguments here, as we execute - * git-gc(1) as a separate child process anyway. So it knows to compute - * these arguments again. - */ - struct strvec repack_args = STRVEC_INIT; - int ret = need_to_gc(cfg, &repack_args); - strvec_clear(&repack_args); - return ret; + return need_to_gc(cfg); } static int prune_packed(struct maintenance_run_opts *opts)
--
2.55.0.313.g8d093f411d.dirty