[PATCH 00/11] odb: make optimizations pluggable

[PATCH 00/11] odb: make optimizations pluggable

[Patrick Steinhardt]

Hi,

this patch series converts object housekeeping to become pluggable.
There isn't really anything else to say about this.

The series is built on top of f85a7e6620 (Start Git 2.56 cycle,
2026-07-06).

Thanks!

Patrick

---
Patrick Steinhardt (11):
      odb: run "pre-auto-gc" hook for all maintenance tasks
      builtin/gc: move worktree and rerere tasks before object optimizations
      builtin/gc: extract object database optimizations into separate function
      builtin/gc: make repack arguments self-contained
      builtin/gc: inline config values specific to the "files" backend
      builtin/gc: introduce object database optimization options
      builtin/gc: move geometric repacking into `odb_optimize()`
      builtin/gc: introduce `odb_optimize_required()`
      builtin/gc: refactor ODB optimizations to operate on "files" source
      builtin/gc: fix signedness issues in ODB-related functionality
      odb: make optimizations pluggable

 builtin/gc.c           | 534 ++++++++-----------------------------------------
 odb.c                  |  12 ++
 odb.h                  |  45 +++++
 odb/source-files.c     | 470 +++++++++++++++++++++++++++++++++++++++++++
 odb/source-files.h     |  15 ++
 odb/source.h           |  36 ++++
 t/t7900-maintenance.sh | 143 ++++++++++++-
 7 files changed, 789 insertions(+), 466 deletions(-)


---
base-commit: f85a7e662054a7b0d9070e432508831afa214b47
change-id: 20260612-b4-pks-odb-optimize-3426c57e5c30

[PATCH 01/11] odb: run "pre-auto-gc" hook for all maintenance tasks

[Patrick Steinhardt]

The "pre-auto-gc" hook is supposed to run before auto-maintenance
starts. The intent of this is to give users the ability to intercept
running maintenance in case there's for example an event that is not
supposed to run in parallel with repository maintenance.

This hook runs via `need_to_gc()`, which is invoked via two paths:

  - It is called directly by git-gc(1).

  - It is called indirectly by git-maintenance(1) via the "gc" task.

While the former makes sense, the latter is somewhat off. While the hook
is indeed strongly tied to gc'ing a repository, the original intent of
the hook is rather to inhibit any kind of automated garbage collection.
That noticeably also includes all the other maintenance tasks that our
new infrastructure may run, but those aren't getting intercepted at all.
The move towards our new maintenance strategy has thus somewhat neutered
the effectiveness of the hook.

Fix this issue by running the hook before the first auto-maintenance
task that would run as determined by the tasks's auto condition. Note
that this requires us to lift the call to `run_hooks()` out of
`needs_to_gc()`, as the hook would otherwise potentially run multiple
times.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/gc.c           |  35 ++++++++++----
 t/t7900-maintenance.sh | 121 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 147 insertions(+), 9 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index d32af422af..77d0a5c948 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -709,8 +709,6 @@ static int need_to_gc(struct gc_config *cfg, struct strvec *repack_args)
 	else
 		return 0;
 
-	if (run_hooks(the_repository, "pre-auto-gc"))
-		return 0;
 	return 1;
 }
 
@@ -933,7 +931,8 @@ int cmd_gc(int argc,
 		/*
 		 * Auto-gc should be least intrusive as possible.
 		 */
-		if (!need_to_gc(&cfg, &repack_args)) {
+		if (!need_to_gc(&cfg, &repack_args) ||
+		    run_hooks(the_repository, "pre-auto-gc")) {
 			ret = 0;
 			goto out;
 		}
@@ -1755,11 +1754,18 @@ enum task_phase {
 	TASK_PHASE_BACKGROUND,
 };
 
+enum auto_gc_hook_result {
+	AUTO_GC_HOOK_UNDECIDED = 0,
+	AUTO_GC_HOOK_RUN = 1,
+	AUTO_GC_HOOK_SKIP = 2,
+};
+
 static int maybe_run_task(const struct maintenance_task *task,
 			  struct repository *repo,
 			  struct maintenance_run_opts *opts,
 			  struct gc_config *cfg,
-			  enum task_phase phase)
+			  enum task_phase phase,
+			  enum auto_gc_hook_result *auto_gc_hook_result)
 {
 	int foreground = (phase == TASK_PHASE_FOREGROUND);
 	maintenance_task_fn fn = foreground ? task->foreground : task->background;
@@ -1768,9 +1774,19 @@ static int maybe_run_task(const struct maintenance_task *task,
 
 	if (!fn)
 		return 0;
-	if (opts->auto_flag &&
-	    (!task->auto_condition || !task->auto_condition(cfg)))
-		return 0;
+	if (opts->auto_flag) {
+		if (*auto_gc_hook_result == AUTO_GC_HOOK_SKIP)
+			return 0;
+
+		if (!task->auto_condition || !task->auto_condition(cfg))
+			return 0;
+
+		if (*auto_gc_hook_result == AUTO_GC_HOOK_UNDECIDED)
+			*auto_gc_hook_result = run_hooks(repo, "pre-auto-gc") ?
+				AUTO_GC_HOOK_SKIP : AUTO_GC_HOOK_RUN;
+		if (*auto_gc_hook_result == AUTO_GC_HOOK_SKIP)
+			return 0;
+	}
 
 	trace2_region_enter(region, task->name, repo);
 	if (fn(opts, cfg)) {
@@ -1789,6 +1805,7 @@ static int maintenance_run_tasks(struct maintenance_run_opts *opts,
 	struct lock_file lk;
 	struct repository *r = the_repository;
 	char *lock_path = xstrfmt("%s/maintenance", r->objects->sources->path);
+	enum auto_gc_hook_result auto_gc_hook_result = AUTO_GC_HOOK_UNDECIDED;
 
 	if (hold_lock_file_for_update(&lk, lock_path, LOCK_NO_DEREF) < 0) {
 		/*
@@ -1808,7 +1825,7 @@ static int maintenance_run_tasks(struct maintenance_run_opts *opts,
 
 	for (size_t i = 0; i < opts->tasks_nr; i++)
 		if (maybe_run_task(&tasks[opts->tasks[i]], r, opts, cfg,
-				   TASK_PHASE_FOREGROUND))
+				   TASK_PHASE_FOREGROUND, &auto_gc_hook_result))
 			result = 1;
 
 	/* Failure to daemonize is ok, we'll continue in foreground. */
@@ -1820,7 +1837,7 @@ static int maintenance_run_tasks(struct maintenance_run_opts *opts,
 
 	for (size_t i = 0; i < opts->tasks_nr; i++)
 		if (maybe_run_task(&tasks[opts->tasks[i]], r, opts, cfg,
-				   TASK_PHASE_BACKGROUND))
+				   TASK_PHASE_BACKGROUND, &auto_gc_hook_result))
 			result = 1;
 
 	rollback_lock_file(&lk);
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index d7f82e1bec..1212b306b6 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -740,6 +740,127 @@ test_expect_success 'geometric repacking honors configured split factor' '
 	)
 '
 
+test_expect_success 'pre-auto-gc hook runs exactly once' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		write_script .git/hooks/pre-auto-gc <<-\EOF &&
+		echo hook >>hook.log
+		EOF
+
+		# Satisfy the auto condition for multiple tasks, both in the
+		# foreground and in the background phase.
+		git config set maintenance.reflog-expire.auto -1 &&
+		git config set maintenance.geometric-repack.auto -1 &&
+		git config set maintenance.rerere-gc.auto -1 &&
+
+		GIT_TRACE2_EVENT="$(pwd)/trace2.txt" \
+			git maintenance run --auto 2>/dev/null &&
+
+		# The successful hook does not inhibit any of the tasks...
+		test_subcommand git reflog expire --all <trace2.txt &&
+		test_subcommand_flex git repack <trace2.txt &&
+		test_subcommand git rerere gc <trace2.txt &&
+		# ... but it must only have been executed a single time.
+		test_line_count = 1 hook.log
+	)
+'
+
+test_expect_success 'pre-auto-gc hook can inhibit geometric strategy' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		write_script .git/hooks/pre-auto-gc <<-\EOF &&
+		echo hook >>hook.log
+		exit 1
+		EOF
+
+		git config set maintenance.reflog-expire.auto -1 &&
+		git config set maintenance.geometric-repack.auto -1 &&
+		git config set maintenance.rerere-gc.auto -1 &&
+
+		# Maintenance would be required...
+		git maintenance is-needed --auto &&
+
+		GIT_TRACE2_EVENT="$(pwd)/trace2.txt" \
+			git maintenance run --auto 2>/dev/null &&
+
+		# ... but the failing hook inhibits all tasks. The hook itself
+		# is expected to be the only child process being spawned, and
+		# it must only run a single time.
+		test_grep "child_start.*pre-auto-gc" trace2.txt &&
+		test_subcommand_flex ! git trace2 &&
+		test_line_count = 1 hook.log
+	)
+'
+
+test_expect_success 'pre-auto-gc hook can inhibit gc strategy' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		write_script .git/hooks/pre-auto-gc <<-\EOF &&
+		echo hook >>hook.log
+		exit 1
+		EOF
+
+		git config set maintenance.strategy gc &&
+		git config set maintenance.auto false &&
+		git config set gc.auto 3 &&
+
+		test_oid_init &&
+
+		# We need to create two objects whose hashes start with 17
+		# since this is what the gc task counts.
+		test_commit "$(test_oid blob17_1)" &&
+		test_commit "$(test_oid blob17_2)" &&
+
+		# Maintenance would be required...
+		git maintenance is-needed --auto &&
+
+		GIT_TRACE2_EVENT="$(pwd)/trace2.txt" \
+			git maintenance run --auto 2>/dev/null &&
+
+		# ... but the failing hook inhibits all tasks. The hook itself
+		# is expected to be the only child process being spawned, and
+		# it must only run a single time.
+		test_grep "child_start.*pre-auto-gc" trace2.txt &&
+		test_subcommand_flex ! git trace2 &&
+		test_line_count = 1 hook.log
+	)
+'
+
+test_expect_success 'pre-auto-gc hook does not run when no maintenance is needed' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		write_script .git/hooks/pre-auto-gc <<-\EOF &&
+		echo hook >>hook.log
+		EOF
+		test_must_fail git maintenance is-needed --auto &&
+		git maintenance run --auto 2>/dev/null &&
+		test_path_is_missing hook.log
+	)
+'
+
+test_expect_success 'pre-auto-gc hook does not run without --auto' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	test_hook -C repo pre-auto-gc <<-\EOF &&
+	echo hook >>hook.log
+	EOF
+	(
+		cd repo &&
+		GIT_TRACE2_EVENT="$(pwd)/trace2.txt" \
+			git maintenance run 2>/dev/null &&
+		test_grep "\[\"git\",\"repack\"," trace2.txt &&
+		test_path_is_missing hook.log
+	)
+'
+
 test_expect_success 'pack-refs task' '
 	for n in $(test_seq 1 5)
 	do

-- 
2.55.0.141.g00534a21ce.dirty

Re: [PATCH 01/11] odb: run "pre-auto-gc" hook for all maintenance tasks

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> While the former makes sense, the latter is somewhat off. While the hook
> is indeed strongly tied to gc'ing a repository, the original intent of
> the hook is rather to inhibit any kind of automated garbage collection.
> That noticeably also includes all the other maintenance tasks that our
> new infrastructure may run, but those aren't getting intercepted at all.

If we want to halt object collection right now for some reason, it
is likely that for the same reason we may want automated pruning of
old reflog entries, for example.  So I can buy the above reasoning.

> diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
> index d7f82e1bec..1212b306b6 100755
> --- a/t/t7900-maintenance.sh
> +++ b/t/t7900-maintenance.sh
> @@ -740,6 +740,127 @@ test_expect_success 'geometric repacking honors configured split factor' '
>  	)
>  '
>  
> +test_expect_success 'pre-auto-gc hook runs exactly once' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	(
> +		cd repo &&
> +		write_script .git/hooks/pre-auto-gc <<-\EOF &&
> +		echo hook >>hook.log
> +		EOF
> +
> +		# Satisfy the auto condition for multiple tasks, both in the
> +		# foreground and in the background phase.
> +		git config set maintenance.reflog-expire.auto -1 &&
> +		git config set maintenance.geometric-repack.auto -1 &&
> +		git config set maintenance.rerere-gc.auto -1 &&
> +
> +		GIT_TRACE2_EVENT="$(pwd)/trace2.txt" \
> +			git maintenance run --auto 2>/dev/null &&
> +
> +		# The successful hook does not inhibit any of the tasks...
> +		test_subcommand git reflog expire --all <trace2.txt &&
> +		test_subcommand_flex git repack <trace2.txt &&
> +		test_subcommand git rerere gc <trace2.txt &&
> +		# ... but it must only have been executed a single time.
> +		test_line_count = 1 hook.log
> +	)
> +'

Somehow I'd feel better if the hook used a full path to the append
only log file, but it is reasonably clear that these three commands
are unlikely to chdir around, so it may be OK.

Obviously not in scope of this topic, but I wonder if we have a
better way to test these three "housekeeping tasks" have run, than
casting in stone the current implementation that spawns these three
external command as subprocesses.

Re: [PATCH 01/11] odb: run "pre-auto-gc" hook for all maintenance tasks

[Patrick Steinhardt]

On Tue, Jul 07, 2026 at 12:55:31PM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> > diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
> > index d7f82e1bec..1212b306b6 100755
> > --- a/t/t7900-maintenance.sh
> > +++ b/t/t7900-maintenance.sh
> > @@ -740,6 +740,127 @@ test_expect_success 'geometric repacking honors configured split factor' '
> >  	)
> >  '
> >  
> > +test_expect_success 'pre-auto-gc hook runs exactly once' '
> > +	test_when_finished "rm -rf repo" &&
> > +	git init repo &&
> > +	(
> > +		cd repo &&
> > +		write_script .git/hooks/pre-auto-gc <<-\EOF &&
> > +		echo hook >>hook.log
> > +		EOF
> > +
> > +		# Satisfy the auto condition for multiple tasks, both in the
> > +		# foreground and in the background phase.
> > +		git config set maintenance.reflog-expire.auto -1 &&
> > +		git config set maintenance.geometric-repack.auto -1 &&
> > +		git config set maintenance.rerere-gc.auto -1 &&
> > +
> > +		GIT_TRACE2_EVENT="$(pwd)/trace2.txt" \
> > +			git maintenance run --auto 2>/dev/null &&
> > +
> > +		# The successful hook does not inhibit any of the tasks...
> > +		test_subcommand git reflog expire --all <trace2.txt &&
> > +		test_subcommand_flex git repack <trace2.txt &&
> > +		test_subcommand git rerere gc <trace2.txt &&
> > +		# ... but it must only have been executed a single time.
> > +		test_line_count = 1 hook.log
> > +	)
> > +'
> 
> Somehow I'd feel better if the hook used a full path to the append
> only log file, but it is reasonably clear that these three commands
> are unlikely to chdir around, so it may be OK.
> 
> Obviously not in scope of this topic, but I wonder if we have a
> better way to test these three "housekeeping tasks" have run, than
> casting in stone the current implementation that spawns these three
> external command as subprocesses.

Yeah, this is awfully fragile indeed. Since 25914c4fde (maintenance: add
trace2 regions for task execution, 2020-09-17) we already have trace2
markers for each task that's running, so that may indeed be a much
better way to figure out whether the task is running or not.

I've made some local changes, but will hold back with sending a v2 until
there's more feedback.

Thanks!

Patrick

[PATCH 02/11] builtin/gc: move worktree and rerere tasks before object optimizations

[Patrick Steinhardt]

In subsequent patches we'll consolidate all tasks that relate to
maintenance of the object database and move it into the "files" backend.
The relevant code is somewhat scattered though, as several other tasks
are interspersed between.

Refactor the code so that all object database optimizations are grouped
together, which requires us to move worktree pruning and rerere garbage
collection around. In theory, rearranging this code can have an effect
on the object database optimizations:

  - Rerere entries really shouldn't impact garbage collection at all, as
    these entries are not stored in the object database.

  - The index and HEAD reference of pruned worktrees may reference
    objects that become unreachable.

That being said, the impact should be overall rather negligible. If the
user was asking us to prune objects with immediate expiration time then
we might now prune objects that were previously still kept alive by the
worktree. But besides being a very specific edge case, it's arguably not
even the wrong thing to also prune any potentially-unreachable objects
immediately.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/gc.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index 77d0a5c948..8f568003ee 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1011,6 +1011,13 @@ int cmd_gc(int argc,
 	if (opts.detach <= 0 && !skip_foreground_tasks)
 		gc_foreground_tasks(&opts, &cfg);
 
+	if (cfg.prune_worktrees_expire &&
+	    maintenance_task_worktree_prune(&opts, &cfg))
+		die(FAILED_RUN, "worktree");
+
+	if (maintenance_task_rerere_gc(&opts, &cfg))
+		die(FAILED_RUN, "rerere");
+
 	if (!the_repository->repository_format_precious_objects) {
 		struct child_process repack_cmd = CHILD_PROCESS_INIT;
 
@@ -1038,13 +1045,6 @@ int cmd_gc(int argc,
 		}
 	}
 
-	if (cfg.prune_worktrees_expire &&
-	    maintenance_task_worktree_prune(&opts, &cfg))
-		die(FAILED_RUN, "worktree");
-
-	if (maintenance_task_rerere_gc(&opts, &cfg))
-		die(FAILED_RUN, "rerere");
-
 	report_garbage = report_pack_garbage;
 	odb_reprepare(the_repository->objects);
 	if (pack_garbage.nr > 0) {

-- 
2.55.0.141.g00534a21ce.dirty

Re: [PATCH 02/11] builtin/gc: move worktree and rerere tasks before object optimizations

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> In subsequent patches we'll consolidate all tasks that relate to
> maintenance of the object database and move it into the "files" backend.
> The relevant code is somewhat scattered though, as several other tasks
> are interspersed between.
>
> Refactor the code so that all object database optimizations are grouped
> together, which requires us to move worktree pruning and rerere garbage
> collection around. In theory, rearranging this code can have an effect
> on the object database optimizations:
>
>   - Rerere entries really shouldn't impact garbage collection at all, as
>     these entries are not stored in the object database.
>
>   - The index and HEAD reference of pruned worktrees may reference
>     objects that become unreachable.

Over time "gc" (and more prominently, "maintenance") ceased to be
about object database optimization but about general housekeeping
operations to keep your repository healthy.  rerere database,
reflog, packed-refs and reftable compaction are all outside the
scope of object database optimization.  Grouping these inside the
umbrella "gc/maintenance" framework would be a good first step to
make parts of them pluggable.

> That being said, the impact should be overall rather negligible. If the
> user was asking us to prune objects with immediate expiration time then
> we might now prune objects that were previously still kept alive by the
> worktree. But besides being a very specific edge case, it's arguably not
> even the wrong thing to also prune any potentially-unreachable objects
> immediately.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  builtin/gc.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/builtin/gc.c b/builtin/gc.c
> index 77d0a5c948..8f568003ee 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -1011,6 +1011,13 @@ int cmd_gc(int argc,
>  	if (opts.detach <= 0 && !skip_foreground_tasks)
>  		gc_foreground_tasks(&opts, &cfg);
>  
> +	if (cfg.prune_worktrees_expire &&
> +	    maintenance_task_worktree_prune(&opts, &cfg))
> +		die(FAILED_RUN, "worktree");
> +
> +	if (maintenance_task_rerere_gc(&opts, &cfg))
> +		die(FAILED_RUN, "rerere");
> +
>  	if (!the_repository->repository_format_precious_objects) {
>  		struct child_process repack_cmd = CHILD_PROCESS_INIT;
>  
> @@ -1038,13 +1045,6 @@ int cmd_gc(int argc,
>  		}
>  	}
>  
> -	if (cfg.prune_worktrees_expire &&
> -	    maintenance_task_worktree_prune(&opts, &cfg))
> -		die(FAILED_RUN, "worktree");
> -
> -	if (maintenance_task_rerere_gc(&opts, &cfg))
> -		die(FAILED_RUN, "rerere");
> -
>  	report_garbage = report_pack_garbage;
>  	odb_reprepare(the_repository->objects);
>  	if (pack_garbage.nr > 0) {

[PATCH 03/11] builtin/gc: extract object database optimizations into separate function

[Patrick Steinhardt]

Extract the object database optimization logic from `cmd_gc()` into a
new `maintenance_task_odb()` helper function. This is a pure refactoring
with no intended functional change.

Note that the message that notifies the user about too many loose
objects is moved into the new function, as well. It is inherently an
implementation detail of how the "files" source works, and as a
consequence we'll move it around in a later commit, as well. This
reordering means that the warning may now be printed at a different
point in time, but it's not expected that this will have any practical
implications.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/gc.c | 79 +++++++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 49 insertions(+), 30 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index 8f568003ee..2ff98fa727 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -839,6 +839,53 @@ static int gc_foreground_tasks(struct maintenance_run_opts *opts,
 	return 0;
 }
 
+static int maintenance_task_odb(struct maintenance_run_opts *opts,
+				struct gc_config *cfg,
+				struct strvec *repack_args)
+{
+	struct child_process repack_cmd = CHILD_PROCESS_INIT;
+	int ret;
+
+	if (the_repository->repository_format_precious_objects)
+		return 0;
+
+	repack_cmd.git_cmd = 1;
+	repack_cmd.odb_to_close = the_repository->objects;
+	strvec_pushv(&repack_cmd.args, repack_args->v);
+	if (run_command(&repack_cmd)) {
+		ret = error(FAILED_RUN, repack_args->v[0]);
+		goto out;
+	}
+
+	if (cfg->prune_expire) {
+		struct child_process prune_cmd = CHILD_PROCESS_INIT;
+
+		strvec_pushl(&prune_cmd.args, "prune", "--expire", NULL);
+		/* run `git prune` even if using cruft packs */
+		strvec_push(&prune_cmd.args, cfg->prune_expire);
+		if (opts->quiet)
+			strvec_push(&prune_cmd.args, "--no-progress");
+		if (repo_has_promisor_remote(the_repository))
+			strvec_push(&prune_cmd.args,
+				    "--exclude-promisor-objects");
+		prune_cmd.git_cmd = 1;
+
+		if (run_command(&prune_cmd)) {
+			ret = error(FAILED_RUN, prune_cmd.args.v[0]);
+			goto out;
+		}
+	}
+
+	if (opts->auto_flag && too_many_loose_objects(cfg->gc_auto_threshold))
+		warning(_("There are too many unreachable loose objects; "
+			"run 'git prune' to remove them."));
+
+	ret = 0;
+
+out:
+	return ret;
+}
+
 int cmd_gc(int argc,
 	   const char **argv,
 	   const char *prefix,
@@ -1018,32 +1065,8 @@ int cmd_gc(int argc,
 	if (maintenance_task_rerere_gc(&opts, &cfg))
 		die(FAILED_RUN, "rerere");
 
-	if (!the_repository->repository_format_precious_objects) {
-		struct child_process repack_cmd = CHILD_PROCESS_INIT;
-
-		repack_cmd.git_cmd = 1;
-		repack_cmd.odb_to_close = the_repository->objects;
-		strvec_pushv(&repack_cmd.args, repack_args.v);
-		if (run_command(&repack_cmd))
-			die(FAILED_RUN, repack_args.v[0]);
-
-		if (cfg.prune_expire) {
-			struct child_process prune_cmd = CHILD_PROCESS_INIT;
-
-			strvec_pushl(&prune_cmd.args, "prune", "--expire", NULL);
-			/* run `git prune` even if using cruft packs */
-			strvec_push(&prune_cmd.args, cfg.prune_expire);
-			if (opts.quiet)
-				strvec_push(&prune_cmd.args, "--no-progress");
-			if (repo_has_promisor_remote(the_repository))
-				strvec_push(&prune_cmd.args,
-					    "--exclude-promisor-objects");
-			prune_cmd.git_cmd = 1;
-
-			if (run_command(&prune_cmd))
-				die(FAILED_RUN, prune_cmd.args.v[0]);
-		}
-	}
+	if (maintenance_task_odb(&opts, &cfg, &repack_args))
+		die(NULL);
 
 	report_garbage = report_pack_garbage;
 	odb_reprepare(the_repository->objects);
@@ -1057,10 +1080,6 @@ int cmd_gc(int argc,
 					     !opts.quiet && !daemonized ? COMMIT_GRAPH_WRITE_PROGRESS : 0,
 					     NULL);
 
-	if (opts.auto_flag && too_many_loose_objects(cfg.gc_auto_threshold))
-		warning(_("There are too many unreachable loose objects; "
-			"run 'git prune' to remove them."));
-
 	if (!daemonized) {
 		char *path = repo_git_path(the_repository, "gc.log");
 		unlink(path);

-- 
2.55.0.141.g00534a21ce.dirty

Re: [PATCH 03/11] builtin/gc: extract object database optimizations into separate function

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> Extract the object database optimization logic from `cmd_gc()` into a
> new `maintenance_task_odb()` helper function. This is a pure refactoring
> with no intended functional change.
>
> Note that the message that notifies the user about too many loose
> objects is moved into the new function, as well. It is inherently an
> implementation detail of how the "files" source works, and as a
> consequence we'll move it around in a later commit, as well. This
> reordering means that the warning may now be printed at a different
> point in time, but it's not expected that this will have any practical
> implications.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  builtin/gc.c | 79 +++++++++++++++++++++++++++++++++++++-----------------------
>  1 file changed, 49 insertions(+), 30 deletions(-)
>
> diff --git a/builtin/gc.c b/builtin/gc.c
> index 8f568003ee..2ff98fa727 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -839,6 +839,53 @@ static int gc_foreground_tasks(struct maintenance_run_opts *opts,
>  	return 0;
>  }
>  
> +static int maintenance_task_odb(struct maintenance_run_opts *opts,
> +				struct gc_config *cfg,
> +				struct strvec *repack_args)
> +{
> +	struct child_process repack_cmd = CHILD_PROCESS_INIT;
> +	int ret;
> +
> +	if (the_repository->repository_format_precious_objects)
> +		return 0;
> +
> +	repack_cmd.git_cmd = 1;
> +	repack_cmd.odb_to_close = the_repository->objects;
> +	strvec_pushv(&repack_cmd.args, repack_args->v);
> +	if (run_command(&repack_cmd)) {
> +		ret = error(FAILED_RUN, repack_args->v[0]);
> +		goto out;
> +	}
> +
> +	if (cfg->prune_expire) {
> +		struct child_process prune_cmd = CHILD_PROCESS_INIT;
> +
> +		strvec_pushl(&prune_cmd.args, "prune", "--expire", NULL);
> +		/* run `git prune` even if using cruft packs */
> +		strvec_push(&prune_cmd.args, cfg->prune_expire);
> +		if (opts->quiet)
> +			strvec_push(&prune_cmd.args, "--no-progress");
> +		if (repo_has_promisor_remote(the_repository))
> +			strvec_push(&prune_cmd.args,
> +				    "--exclude-promisor-objects");
> +		prune_cmd.git_cmd = 1;
> +
> +		if (run_command(&prune_cmd)) {
> +			ret = error(FAILED_RUN, prune_cmd.args.v[0]);
> +			goto out;
> +		}
> +	}
> +
> +	if (opts->auto_flag && too_many_loose_objects(cfg->gc_auto_threshold))
> +		warning(_("There are too many unreachable loose objects; "
> +			"run 'git prune' to remove them."));
> +
> +	ret = 0;
> +
> +out:
> +	return ret;
> +}
> +
>  int cmd_gc(int argc,
>  	   const char **argv,
>  	   const char *prefix,
> @@ -1018,32 +1065,8 @@ int cmd_gc(int argc,
>  	if (maintenance_task_rerere_gc(&opts, &cfg))
>  		die(FAILED_RUN, "rerere");
>  
> -	if (!the_repository->repository_format_precious_objects) {
> -		struct child_process repack_cmd = CHILD_PROCESS_INIT;
> -
> -		repack_cmd.git_cmd = 1;
> -		repack_cmd.odb_to_close = the_repository->objects;
> -		strvec_pushv(&repack_cmd.args, repack_args.v);
> -		if (run_command(&repack_cmd))
> -			die(FAILED_RUN, repack_args.v[0]);
> -
> -		if (cfg.prune_expire) {
> -			struct child_process prune_cmd = CHILD_PROCESS_INIT;
> -
> -			strvec_pushl(&prune_cmd.args, "prune", "--expire", NULL);
> -			/* run `git prune` even if using cruft packs */
> -			strvec_push(&prune_cmd.args, cfg.prune_expire);
> -			if (opts.quiet)
> -				strvec_push(&prune_cmd.args, "--no-progress");
> -			if (repo_has_promisor_remote(the_repository))
> -				strvec_push(&prune_cmd.args,
> -					    "--exclude-promisor-objects");
> -			prune_cmd.git_cmd = 1;
> -
> -			if (run_command(&prune_cmd))
> -				die(FAILED_RUN, prune_cmd.args.v[0]);
> -		}
> -	}
> +	if (maintenance_task_odb(&opts, &cfg, &repack_args))
> +		die(NULL);

Instead of giving the "fatal:" message here from this function, the
new code lets the helper function issue an "error:", so we do not
want to say an extra "fatal:" from die(), so this die(NULL) may be a
good thing to do.

[PATCH 04/11] builtin/gc: make repack arguments self-contained

[Patrick Steinhardt]

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 <ps@pks.im>
---
 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.141.g00534a21ce.dirty

[PATCH 05/11] builtin/gc: inline config values specific to the "files" backend

[Patrick Steinhardt]

The `struct gc_config` contains a set of values that we read via the Git
repository's configuration. Several of those values that are consumed by
the object database optimization logic are inherently specific to the
"files" config.

In a later commit we'll make the logic to optimize object databases
pluggable. So by carrying these "files"-backend specific values in the
generic config struct means that other backends would have to worry
about these values, too. This feels somewhat dirty, as implementation-
specific details should live with the backends themselves.

Inline these values directly at the call sites that need them instead.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/gc.c | 115 +++++++++++++++++++++++++++--------------------------------
 1 file changed, 53 insertions(+), 62 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index 25a59caea6..5d445edaa0 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -130,22 +130,11 @@ struct gc_config {
 	unsigned long max_cruft_size;
 	int aggressive_depth;
 	int aggressive_window;
-	int gc_auto_threshold;
-	int gc_auto_pack_limit;
 	int detach_auto;
 	char *gc_log_expire;
 	char *prune_expire;
 	char *prune_worktrees_expire;
-	char *repack_filter;
-	char *repack_filter_to;
 	char *repack_expire_to;
-	unsigned long big_pack_threshold;
-	unsigned long max_delta_cache_size;
-	/*
-	 * Remove this member from gc_config once repo_settings is passed
-	 * through the callchain.
-	 */
-	size_t delta_base_cache_limit;
 };
 
 #define GC_CONFIG_INIT { \
@@ -154,14 +143,10 @@ struct gc_config {
 	.cruft_packs = 1, \
 	.aggressive_depth = 50, \
 	.aggressive_window = 250, \
-	.gc_auto_threshold = 6700, \
-	.gc_auto_pack_limit = 50, \
 	.detach_auto = 1, \
 	.gc_log_expire = xstrdup("1.day.ago"), \
 	.prune_expire = xstrdup("2.weeks.ago"), \
 	.prune_worktrees_expire = xstrdup("3.months.ago"), \
-	.max_delta_cache_size = DEFAULT_DELTA_CACHE_SIZE, \
-	.delta_base_cache_limit = DEFAULT_DELTA_BASE_CACHE_LIMIT, \
 }
 
 static void gc_config_release(struct gc_config *cfg)
@@ -169,15 +154,12 @@ static void gc_config_release(struct gc_config *cfg)
 	free(cfg->gc_log_expire);
 	free(cfg->prune_expire);
 	free(cfg->prune_worktrees_expire);
-	free(cfg->repack_filter);
-	free(cfg->repack_filter_to);
 }
 
 static void gc_config(struct gc_config *cfg)
 {
 	const char *value;
 	char *owned = NULL;
-	unsigned long ulongval;
 
 	if (!repo_config_get_value(the_repository, "gc.packrefs", &value)) {
 		if (value && !strcmp(value, "notbare"))
@@ -192,8 +174,6 @@ static void gc_config(struct gc_config *cfg)
 
 	repo_config_get_int(the_repository, "gc.aggressivewindow", &cfg->aggressive_window);
 	repo_config_get_int(the_repository, "gc.aggressivedepth", &cfg->aggressive_depth);
-	repo_config_get_int(the_repository, "gc.auto", &cfg->gc_auto_threshold);
-	repo_config_get_int(the_repository, "gc.autopacklimit", &cfg->gc_auto_pack_limit);
 	repo_config_get_bool(the_repository, "gc.autodetach", &cfg->detach_auto);
 	repo_config_get_bool(the_repository, "gc.cruftpacks", &cfg->cruft_packs);
 	repo_config_get_ulong(the_repository, "gc.maxcruftsize", &cfg->max_cruft_size);
@@ -213,22 +193,6 @@ static void gc_config(struct gc_config *cfg)
 		cfg->gc_log_expire = owned;
 	}
 
-	repo_config_get_ulong(the_repository, "gc.bigpackthreshold", &cfg->big_pack_threshold);
-	repo_config_get_ulong(the_repository, "pack.deltacachesize", &cfg->max_delta_cache_size);
-
-	if (!repo_config_get_ulong(the_repository, "core.deltabasecachelimit", &ulongval))
-		cfg->delta_base_cache_limit = ulongval;
-
-	if (!repo_config_get_string(the_repository, "gc.repackfilter", &owned)) {
-		free(cfg->repack_filter);
-		cfg->repack_filter = owned;
-	}
-
-	if (!repo_config_get_string(the_repository, "gc.repackfilterto", &owned)) {
-		free(cfg->repack_filter_to);
-		cfg->repack_filter_to = owned;
-	}
-
 	repo_config(the_repository, git_default_config, NULL);
 }
 
@@ -504,12 +468,12 @@ static struct packed_git *find_base_packs(struct string_list *packs,
 	return base;
 }
 
-static int too_many_packs(struct gc_config *cfg)
+static int too_many_packs(int gc_auto_pack_limit)
 {
 	struct packed_git *p;
 	int cnt = 0;
 
-	if (cfg->gc_auto_pack_limit <= 0)
+	if (gc_auto_pack_limit <= 0)
 		return 0;
 
 	repo_for_each_pack(the_repository, p) {
@@ -523,7 +487,7 @@ static int too_many_packs(struct gc_config *cfg)
 		 */
 		cnt++;
 	}
-	return cfg->gc_auto_pack_limit < cnt;
+	return gc_auto_pack_limit < cnt;
 }
 
 static uint64_t total_ram(void)
@@ -571,9 +535,10 @@ static uint64_t total_ram(void)
 	return 0;
 }
 
-static uint64_t estimate_repack_memory(struct gc_config *cfg,
-				       struct packed_git *pack)
+static uint64_t estimate_repack_memory(struct packed_git *pack)
 {
+	unsigned long max_delta_cache_size = DEFAULT_DELTA_CACHE_SIZE;
+	unsigned long delta_base_cache_limit = DEFAULT_DELTA_BASE_CACHE_LIMIT;
 	unsigned long nr_objects;
 	size_t os_cache, heap;
 
@@ -584,6 +549,9 @@ static uint64_t estimate_repack_memory(struct gc_config *cfg,
 	if (!pack || !nr_objects)
 		return 0;
 
+	repo_config_get_ulong(the_repository, "pack.deltacachesize", &max_delta_cache_size);
+	repo_config_get_ulong(the_repository, "core.deltabasecachelimit", &delta_base_cache_limit);
+
 	/*
 	 * First we have to scan through at least one pack.
 	 * Assume enough room in OS file cache to keep the entire pack
@@ -611,9 +579,9 @@ static uint64_t estimate_repack_memory(struct gc_config *cfg,
 	 * read_sha1_file() (either at delta calculation phase, or
 	 * writing phase) also fills up the delta base cache
 	 */
-	heap += cfg->delta_base_cache_limit;
+	heap += delta_base_cache_limit;
 	/* and of course pack-objects has its own delta cache */
-	heap += cfg->max_delta_cache_size;
+	heap += max_delta_cache_size;
 
 	return os_cache + heap;
 }
@@ -629,6 +597,12 @@ static void add_repack_all_option(struct gc_config *cfg,
 				  struct string_list *keep_pack,
 				  struct strvec *args)
 {
+	char *repack_filter = NULL;
+	char *repack_filter_to = NULL;
+
+	repo_config_get_string(the_repository, "gc.repackfilter", &repack_filter);
+	repo_config_get_string(the_repository, "gc.repackfilterto", &repack_filter_to);
+
 	if (cfg->prune_expire && !strcmp(cfg->prune_expire, "now")
 		&& !(cfg->cruft_packs && cfg->repack_expire_to))
 		strvec_push(args, "-a");
@@ -650,10 +624,13 @@ static void add_repack_all_option(struct gc_config *cfg,
 	if (keep_pack)
 		for_each_string_list(keep_pack, keep_one_pack, args);
 
-	if (cfg->repack_filter && *cfg->repack_filter)
-		strvec_pushf(args, "--filter=%s", cfg->repack_filter);
-	if (cfg->repack_filter_to && *cfg->repack_filter_to)
-		strvec_pushf(args, "--filter-to=%s", cfg->repack_filter_to);
+	if (repack_filter && *repack_filter)
+		strvec_pushf(args, "--filter=%s", repack_filter);
+	if (repack_filter_to && *repack_filter_to)
+		strvec_pushf(args, "--filter-to=%s", repack_filter_to);
+
+	free(repack_filter);
+	free(repack_filter_to);
 }
 
 static void add_repack_incremental_option(struct strvec *args)
@@ -661,16 +638,24 @@ 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)
+static int need_to_gc(struct repository *repo)
 {
+	int gc_auto_threshold = 6700;
+	int gc_auto_pack_limit = 50;
+
+	repo_config_get_int(repo, "gc.auto", &gc_auto_threshold);
+	repo_config_get_int(repo, "gc.autopacklimit", &gc_auto_pack_limit);
+
 	/*
 	 * Setting gc.auto to 0 or negative can disable the
 	 * automatic gc.
 	 */
-	if (cfg->gc_auto_threshold <= 0)
+	if (gc_auto_threshold <= 0)
 		return 0;
-	if (!too_many_packs(cfg) && !too_many_loose_objects(cfg->gc_auto_threshold))
+	if (!too_many_packs(gc_auto_pack_limit) &&
+	    !too_many_loose_objects(gc_auto_threshold))
 		return 0;
+
 	return 1;
 }
 
@@ -807,8 +792,15 @@ static int maintenance_task_odb(struct maintenance_run_opts *opts,
 				int aggressive)
 {
 	struct child_process repack_cmd = CHILD_PROCESS_INIT;
+	unsigned long big_pack_threshold = 0;
+	int gc_auto_threshold = 6700;
+	int gc_auto_pack_limit = 50;
 	int ret;
 
+	repo_config_get_int(the_repository, "gc.auto", &gc_auto_threshold);
+	repo_config_get_int(the_repository, "gc.autopacklimit", &gc_auto_pack_limit);
+	repo_config_get_ulong(the_repository, "gc.bigpackthreshold", &big_pack_threshold);
+
 	if (the_repository->repository_format_precious_objects)
 		return 0;
 
@@ -843,19 +835,18 @@ static int maintenance_task_odb(struct maintenance_run_opts *opts,
 		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);
+		} else if (big_pack_threshold) {
+			find_base_packs(&keep_pack, 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)) {
+	} else if (too_many_packs(gc_auto_pack_limit)) {
 		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;
+		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);
 			}
@@ -864,7 +855,7 @@ static int maintenance_task_odb(struct maintenance_run_opts *opts,
 			uint64_t mem_have, mem_want;
 
 			mem_have = total_ram();
-			mem_want = estimate_repack_memory(cfg, p);
+			mem_want = estimate_repack_memory(p);
 
 			/*
 			 * Only allow 1/2 of memory for pack-objects, leave
@@ -905,7 +896,7 @@ static int maintenance_task_odb(struct maintenance_run_opts *opts,
 		}
 	}
 
-	if (opts->auto_flag && too_many_loose_objects(cfg->gc_auto_threshold))
+	if (opts->auto_flag && too_many_loose_objects(gc_auto_threshold))
 		warning(_("There are too many unreachable loose objects; "
 			"run 'git prune' to remove them."));
 
@@ -994,7 +985,7 @@ int cmd_gc(int argc,
 		/*
 		 * Auto-gc should be least intrusive as possible.
 		 */
-		if (!need_to_gc(&cfg) || run_hooks(the_repository, "pre-auto-gc")) {
+		if (!need_to_gc(the_repository) || run_hooks(the_repository, "pre-auto-gc")) {
 			ret = 0;
 			goto out;
 		}
@@ -1291,9 +1282,9 @@ static int maintenance_task_gc_background(struct maintenance_run_opts *opts,
 	return run_command(&child);
 }
 
-static int gc_condition(struct gc_config *cfg)
+static int gc_condition(struct gc_config *cfg UNUSED)
 {
-	return need_to_gc(cfg);
+	return need_to_gc(the_repository);
 }
 
 static int prune_packed(struct maintenance_run_opts *opts)

-- 
2.55.0.141.g00534a21ce.dirty

[PATCH 06/11] builtin/gc: introduce object database optimization options

[Patrick Steinhardt]

Introduce `struct odb_optimize_options` to decouple the options that are
specific to optimizing the object database from `struct gc_config`. This
structure will be moved into the object database layer in a subsequent
commit.

Note that there are a small set of backend-specific options in this
structure. In an ideal world those of course wouldn't exist, but as
we're introducing the object database abstractions retroactively we are
somewhat forced to keep them.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/gc.c | 181 +++++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 120 insertions(+), 61 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index 5d445edaa0..17490106fc 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -593,7 +593,39 @@ static int keep_one_pack(struct string_list_item *item, void *data)
 	return 0;
 }
 
-static void add_repack_all_option(struct gc_config *cfg,
+enum odb_optimize_flags {
+	/* Enable verbose logging and progress reporting. */
+	ODB_OPTIMIZE_VERBOSE = (1 << 0),
+
+	/* Perform auto-maintenance, only optimizing objects as required. */
+	ODB_OPTIMIZE_AUTO = (1 << 1),
+
+	/* Recompute existing deltas. */
+	ODB_OPTIMIZE_NO_REUSE_DELTAS = (1 << 2),
+};
+
+struct odb_optimize_options {
+	enum odb_optimize_flags flags;
+	const char *prune_expire;
+	const char *expire_to;
+	int depth;
+	int window;
+
+	/* Backend-specific options. */
+	int keep_largest_pack;
+	int cruft_packs;
+	unsigned long max_cruft_size;
+};
+
+#define OPTIMIZE_FIELDS_FROM_GC_CONFIG(cfg, aggressive) \
+	.prune_expire = (cfg)->prune_expire, \
+	.expire_to = (cfg)->repack_expire_to, \
+	.cruft_packs = (cfg)->cruft_packs, \
+	.max_cruft_size = (cfg)->max_cruft_size, \
+	.window = (aggressive) ? (cfg)->aggressive_window : 0, \
+	.depth = (aggressive) ? (cfg)->aggressive_depth : 0
+
+static void add_repack_all_option(const struct odb_optimize_options *opts,
 				  struct string_list *keep_pack,
 				  struct strvec *args)
 {
@@ -603,22 +635,22 @@ static void add_repack_all_option(struct gc_config *cfg,
 	repo_config_get_string(the_repository, "gc.repackfilter", &repack_filter);
 	repo_config_get_string(the_repository, "gc.repackfilterto", &repack_filter_to);
 
-	if (cfg->prune_expire && !strcmp(cfg->prune_expire, "now")
-		&& !(cfg->cruft_packs && cfg->repack_expire_to))
+	if (opts->prune_expire && !strcmp(opts->prune_expire, "now") &&
+	    !(opts->cruft_packs && opts->expire_to))
 		strvec_push(args, "-a");
-	else if (cfg->cruft_packs) {
+	else if (opts->cruft_packs) {
 		strvec_push(args, "--cruft");
-		if (cfg->prune_expire)
-			strvec_pushf(args, "--cruft-expiration=%s", cfg->prune_expire);
-		if (cfg->max_cruft_size)
+		if (opts->prune_expire)
+			strvec_pushf(args, "--cruft-expiration=%s", opts->prune_expire);
+		if (opts->max_cruft_size)
 			strvec_pushf(args, "--max-cruft-size=%lu",
-				     cfg->max_cruft_size);
-		if (cfg->repack_expire_to)
-			strvec_pushf(args, "--expire-to=%s", cfg->repack_expire_to);
+				     opts->max_cruft_size);
+		if (opts->expire_to)
+			strvec_pushf(args, "--expire-to=%s", opts->expire_to);
 	} else {
 		strvec_push(args, "-A");
-		if (cfg->prune_expire)
-			strvec_pushf(args, "--unpack-unreachable=%s", cfg->prune_expire);
+		if (opts->prune_expire)
+			strvec_pushf(args, "--unpack-unreachable=%s", opts->prune_expire);
 	}
 
 	if (keep_pack)
@@ -786,10 +818,8 @@ static int gc_foreground_tasks(struct maintenance_run_opts *opts,
 	return 0;
 }
 
-static int maintenance_task_odb(struct maintenance_run_opts *opts,
-				struct gc_config *cfg,
-				int keep_largest_pack,
-				int aggressive)
+static int odb_optimize(struct object_database *odb,
+			const struct odb_optimize_options *opts)
 {
 	struct child_process repack_cmd = CHILD_PROCESS_INIT;
 	unsigned long big_pack_threshold = 0;
@@ -801,21 +831,20 @@ static int maintenance_task_odb(struct maintenance_run_opts *opts,
 	repo_config_get_int(the_repository, "gc.autopacklimit", &gc_auto_pack_limit);
 	repo_config_get_ulong(the_repository, "gc.bigpackthreshold", &big_pack_threshold);
 
-	if (the_repository->repository_format_precious_objects)
+	if (odb->repo->repository_format_precious_objects)
 		return 0;
 
 	repack_cmd.git_cmd = 1;
 	repack_cmd.odb_to_close = the_repository->objects;
 
 	strvec_pushl(&repack_cmd.args, "repack", "-d", "-l", NULL);
-	if (aggressive) {
+	if (opts->flags & ODB_OPTIMIZE_NO_REUSE_DELTAS)
 		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)
+	if (opts->depth > 0)
+		strvec_pushf(&repack_cmd.args, "--depth=%d", opts->depth);
+	if (opts->window > 0)
+		strvec_pushf(&repack_cmd.args, "--window=%d", opts->window);
+	if (!(opts->flags & ODB_OPTIMIZE_VERBOSE))
 		strvec_push(&repack_cmd.args, "-q");
 
 	/*
@@ -829,47 +858,49 @@ static int maintenance_task_odb(struct maintenance_run_opts *opts,
 	 *
 	 *   - Otherwise we perform an incremental repack.
 	 */
-	if (!opts->auto_flag) {
+	if (!(opts->flags & ODB_OPTIMIZE_AUTO)) {
 		struct string_list keep_pack = STRING_LIST_INIT_NODUP;
 
-		if (keep_largest_pack != -1) {
-			if (keep_largest_pack)
+		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(cfg, &keep_pack, &repack_cmd.args);
+		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)) {
-		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 {
+		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);
 			}
-		} 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);
 		}
-
-		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)) {
@@ -877,13 +908,13 @@ static int maintenance_task_odb(struct maintenance_run_opts *opts,
 		goto out;
 	}
 
-	if (cfg->prune_expire) {
+	if (opts->prune_expire) {
 		struct child_process prune_cmd = CHILD_PROCESS_INIT;
 
 		strvec_pushl(&prune_cmd.args, "prune", "--expire", NULL);
 		/* run `git prune` even if using cruft packs */
-		strvec_push(&prune_cmd.args, cfg->prune_expire);
-		if (opts->quiet)
+		strvec_push(&prune_cmd.args, opts->prune_expire);
+		if (!(opts->flags & ODB_OPTIMIZE_VERBOSE))
 			strvec_push(&prune_cmd.args, "--no-progress");
 		if (repo_has_promisor_remote(the_repository))
 			strvec_push(&prune_cmd.args,
@@ -896,7 +927,7 @@ static int maintenance_task_odb(struct maintenance_run_opts *opts,
 		}
 	}
 
-	if (opts->auto_flag && too_many_loose_objects(gc_auto_threshold))
+	if (opts->flags & ODB_OPTIMIZE_AUTO && too_many_loose_objects(gc_auto_threshold))
 		warning(_("There are too many unreachable loose objects; "
 			"run 'git prune' to remove them."));
 
@@ -906,6 +937,26 @@ static int maintenance_task_odb(struct maintenance_run_opts *opts,
 	return ret;
 }
 
+static int maintenance_task_odb(struct maintenance_run_opts *opts,
+				struct gc_config *cfg,
+				int keep_largest_pack,
+				int aggressive)
+{
+	struct odb_optimize_options odb_opts = {
+		.keep_largest_pack = keep_largest_pack,
+		OPTIMIZE_FIELDS_FROM_GC_CONFIG(cfg, aggressive),
+	};
+
+	if (opts->auto_flag)
+		odb_opts.flags |= ODB_OPTIMIZE_AUTO;
+	if (!opts->quiet)
+		odb_opts.flags |= ODB_OPTIMIZE_VERBOSE;
+	if (aggressive)
+		odb_opts.flags |= ODB_OPTIMIZE_NO_REUSE_DELTAS;
+
+	return odb_optimize(the_repository->objects, &odb_opts);
+}
+
 int cmd_gc(int argc,
 	   const char **argv,
 	   const char *prefix,
@@ -1596,11 +1647,19 @@ static int maintenance_task_geometric_repack(struct maintenance_run_opts *opts,
 	child.odb_to_close = the_repository->objects;
 
 	strvec_pushl(&child.args, "repack", "-d", "-l", NULL);
-	if (geometry.split < geometry.pack_nr)
+	if (geometry.split < geometry.pack_nr) {
 		strvec_pushf(&child.args, "--geometric=%d",
 			     geometry.split_factor);
-	else
-		add_repack_all_option(cfg, NULL, &child.args);
+	} 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)

-- 
2.55.0.141.g00534a21ce.dirty

[PATCH 07/11] builtin/gc: move geometric repacking into `odb_optimize()`

[Patrick Steinhardt]

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>
---
 builtin/gc.c           | 171 +++++++++++++++++++++++++------------------------
 t/t7900-maintenance.sh |  22 +++----
 2 files changed, 98 insertions(+), 95 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
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)
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 1212b306b6..e0251064c7 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -556,8 +556,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 &&
@@ -588,8 +588,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.
@@ -609,8 +609,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 &&
@@ -644,8 +644,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 &&
@@ -736,7 +736,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
 	)
 '
 
@@ -1167,7 +1167,7 @@ test_expect_success 'maintenance.strategy is respected' '
 		test_strategy geometric <<-\EOF &&
 		git pack-refs --all --prune
 		git reflog expire --all
-		git repack -d -l --geometric=2 --quiet --write-midx
+		git repack -d -l -q --geometric=2 --write-midx
 		git commit-graph write --split --reachable --no-progress
 		git worktree prune --expire 3.months.ago
 		git rerere gc
@@ -1176,7 +1176,7 @@ test_expect_success 'maintenance.strategy is respected' '
 		test_strategy geometric --schedule=weekly <<-\EOF
 		git pack-refs --all --prune
 		git reflog expire --all
-		git repack -d -l --geometric=2 --quiet --write-midx
+		git repack -d -l -q --geometric=2 --write-midx
 		git commit-graph write --split --reachable --no-progress
 		git worktree prune --expire 3.months.ago
 		git rerere gc

-- 
2.55.0.141.g00534a21ce.dirty

[PATCH 08/11] builtin/gc: introduce `odb_optimize_required()`

[Patrick Steinhardt]

When invoking either git-gc(1) or git-maintenance(1) with the "--auto"
flag then we only perform those maintenance tasks that are actually
required. This logic is inherently an implementation detail of the
object database backend that's in use. But the logic is scattered around
multiple different functions, which makes it hard to make the logic
pluggable.

Introduce a new `odb_optimize_required()` function that allows us to
check these conditions in a generic way.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/gc.c | 160 ++++++++++++++++++++++++++++++++++-------------------------
 1 file changed, 92 insertions(+), 68 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index c8504f4456..e119930adc 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -676,25 +676,84 @@ static void add_repack_incremental_option(struct strvec *args)
 	strvec_push(args, "--no-write-bitmap-index");
 }
 
-static int need_to_gc(struct repository *repo)
+static bool odb_optimize_required(struct object_database *odb,
+				  const struct odb_optimize_options *opts)
 {
-	int gc_auto_threshold = 6700;
-	int gc_auto_pack_limit = 50;
+	switch (opts->strategy) {
+	case ODB_OPTIMIZE_INCREMENTAL: {
+		int gc_auto_threshold = 6700;
+		int gc_auto_pack_limit = 50;
 
-	repo_config_get_int(repo, "gc.auto", &gc_auto_threshold);
-	repo_config_get_int(repo, "gc.autopacklimit", &gc_auto_pack_limit);
+		repo_config_get_int(odb->repo, "gc.auto", &gc_auto_threshold);
+		repo_config_get_int(odb->repo, "gc.autopacklimit", &gc_auto_pack_limit);
 
-	/*
-	 * Setting gc.auto to 0 or negative can disable the
-	 * automatic gc.
-	 */
-	if (gc_auto_threshold <= 0)
-		return 0;
-	if (!too_many_packs(gc_auto_pack_limit) &&
-	    !too_many_loose_objects(gc_auto_threshold))
-		return 0;
+		/*
+		 * Setting gc.auto to 0 or negative can disable the
+		 * automatic gc.
+		 */
+		if (gc_auto_threshold <= 0)
+			return false;
+		if (!too_many_packs(gc_auto_pack_limit) &&
+		    !too_many_loose_objects(gc_auto_threshold))
+			return false;
 
-	return 1;
+		return true;
+	}
+	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;
+		int auto_value = 100;
+		bool ret;
+
+		repo_config_get_int(odb->repo, "maintenance.geometric-repack.auto",
+				    &auto_value);
+		if (!auto_value)
+			return false;
+		if (auto_value < 0)
+			return true;
+
+		repo_config_get_int(odb->repo, "maintenance.geometric-repack.splitFactor",
+				    &geometry.split_factor);
+
+		existing_packs.repo = odb->repo;
+		existing_packs_collect(&existing_packs, &kept_packs);
+		pack_geometry_init(&geometry, &existing_packs, &po_args);
+		pack_geometry_split(&geometry);
+
+		/*
+		 * When we'd merge at least two packs with one another we always
+		 * perform the repack.
+		 */
+		if (geometry.split) {
+			ret = true;
+			goto out;
+		}
+
+		/*
+		 * Otherwise, we estimate the number of loose objects to determine
+		 * whether we want to create a new packfile or not.
+		 */
+		if (too_many_loose_objects(auto_value)) {
+			ret = true;
+			goto out;
+		}
+
+		ret = false;
+
+	out:
+		existing_packs_release(&existing_packs);
+		pack_geometry_release(&geometry);
+		return ret;
+	}
+	default:
+		BUG("unknown maintenance strategy '%d'", opts->strategy);
+	}
 }
 
 /* return NULL on success, else hostname running the gc */
@@ -1076,13 +1135,19 @@ int cmd_gc(int argc,
 		die(_("failed to parse prune expiry value %s"), cfg.prune_expire);
 
 	if (opts.auto_flag) {
+		struct odb_optimize_options optimize_opts = {
+			.strategy = ODB_OPTIMIZE_INCREMENTAL,
+			OPTIMIZE_FIELDS_FROM_GC_CONFIG(&cfg, 0),
+		};
+
 		if (cfg.detach_auto && opts.detach < 0)
 			opts.detach = 1;
 
 		/*
 		 * Auto-gc should be least intrusive as possible.
 		 */
-		if (!need_to_gc(the_repository) || run_hooks(the_repository, "pre-auto-gc")) {
+		if (!odb_optimize_required(the_repository->objects, &optimize_opts) ||
+		    run_hooks(the_repository, "pre-auto-gc")) {
 			ret = 0;
 			goto out;
 		}
@@ -1379,9 +1444,13 @@ static int maintenance_task_gc_background(struct maintenance_run_opts *opts,
 	return run_command(&child);
 }
 
-static int gc_condition(struct gc_config *cfg UNUSED)
+static int gc_condition(struct gc_config *cfg)
 {
-	return need_to_gc(the_repository);
+	struct odb_optimize_options opts = {
+		.strategy = ODB_OPTIMIZE_INCREMENTAL,
+		OPTIMIZE_FIELDS_FROM_GC_CONFIG(cfg, 0),
+	};
+	return odb_optimize_required(the_repository->objects, &opts);
 }
 
 static int prune_packed(struct maintenance_run_opts *opts)
@@ -1681,58 +1750,13 @@ static int maintenance_task_geometric_repack(struct maintenance_run_opts *opts,
 	return odb_optimize(the_repository->objects, &odb_opts);
 }
 
-static int geometric_repack_auto_condition(struct gc_config *cfg UNUSED)
+static int geometric_repack_auto_condition(struct gc_config *cfg)
 {
-	struct pack_geometry geometry = {
-		.split_factor = 2,
-	};
-	struct pack_objects_args po_args = {
-		.local = 1,
+	struct odb_optimize_options 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;
-	int auto_value = 100;
-	int ret;
-
-	repo_config_get_int(the_repository, "maintenance.geometric-repack.auto",
-			    &auto_value);
-	if (!auto_value)
-		return 0;
-	if (auto_value < 0)
-		return 1;
-
-	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);
-
-	/*
-	 * When we'd merge at least two packs with one another we always
-	 * perform the repack.
-	 */
-	if (geometry.split) {
-		ret = 1;
-		goto out;
-	}
-
-	/*
-	 * Otherwise, we estimate the number of loose objects to determine
-	 * whether we want to create a new packfile or not.
-	 */
-	if (too_many_loose_objects(auto_value)) {
-		ret = 1;
-		goto out;
-	}
-
-	ret = 0;
-
-out:
-	existing_packs_release(&existing_packs);
-	pack_geometry_release(&geometry);
-	return ret;
+	return odb_optimize_required(the_repository->objects, &opts);
 }
 
 typedef int (*maintenance_task_fn)(struct maintenance_run_opts *opts,

-- 
2.55.0.141.g00534a21ce.dirty

[PATCH 09/11] builtin/gc: refactor ODB optimizations to operate on "files" source

[Patrick Steinhardt]

We have a couple of functions that are implementation details of how the
"files" object database source performs optimizations. These functions
often use global state like `the_repository` and implicitly derive the
source they are supposed to optimize.

Refactor these interfaces to accept a "files" source directly. This will
make it easier to move around the whole logic into "odb/source-files.c"
in a subsequent step.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/gc.c | 79 +++++++++++++++++++++++++++++++-----------------------------
 1 file changed, 41 insertions(+), 38 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index e119930adc..3207182488 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -428,9 +428,8 @@ static int rerere_gc_condition(struct gc_config *cfg UNUSED)
 	return should_gc;
 }
 
-static int too_many_loose_objects(int limit)
+static int too_many_loose_objects(struct odb_source_files *files, int limit)
 {
-	struct odb_source_files *files = odb_source_files_downcast(the_repository->objects->sources);
 	/*
 	 * This is weird, but stems from legacy behaviour: the GC auto
 	 * threshold was always essentially interpreted as if it was rounded up
@@ -446,19 +445,21 @@ static int too_many_loose_objects(int limit)
 	return loose_count > auto_threshold;
 }
 
-static struct packed_git *find_base_packs(struct string_list *packs,
+static struct packed_git *find_base_packs(struct odb_source_files *files,
+					  struct string_list *packs,
 					  unsigned long limit)
 {
-	struct packed_git *p, *base = NULL;
+	struct packfile_list_entry *e;
+	struct packed_git *base = NULL;
 
-	repo_for_each_pack(the_repository, p) {
-		if (!p->pack_local || p->is_cruft)
+	for (e = packfile_store_get_packs(files->packed); e; e = e->next) {
+		if (e->pack->is_cruft)
 			continue;
 		if (limit) {
-			if (p->pack_size >= limit)
-				string_list_append(packs, p->pack_name);
-		} else if (!base || base->pack_size < p->pack_size) {
-			base = p;
+			if (e->pack->pack_size >= limit)
+				string_list_append(packs, e->pack->pack_name);
+		} else if (!base || base->pack_size < e->pack->pack_size) {
+			base = e->pack;
 		}
 	}
 
@@ -468,18 +469,16 @@ static struct packed_git *find_base_packs(struct string_list *packs,
 	return base;
 }
 
-static int too_many_packs(int gc_auto_pack_limit)
+static int too_many_packs(struct odb_source_files *files, int gc_auto_pack_limit)
 {
-	struct packed_git *p;
+	struct packfile_list_entry *e;
 	int cnt = 0;
 
 	if (gc_auto_pack_limit <= 0)
 		return 0;
 
-	repo_for_each_pack(the_repository, p) {
-		if (!p->pack_local)
-			continue;
-		if (p->pack_keep)
+	for (e = packfile_store_get_packs(files->packed); e; e = e->next) {
+		if (e->pack->pack_keep)
 			continue;
 		/*
 		 * Perhaps check the size of the pack and count only
@@ -535,15 +534,16 @@ static uint64_t total_ram(void)
 	return 0;
 }
 
-static uint64_t estimate_repack_memory(struct packed_git *pack)
+static uint64_t estimate_repack_memory(struct odb_source_files *files,
+				       struct packed_git *pack)
 {
 	unsigned long max_delta_cache_size = DEFAULT_DELTA_CACHE_SIZE;
 	unsigned long delta_base_cache_limit = DEFAULT_DELTA_BASE_CACHE_LIMIT;
 	unsigned long nr_objects;
 	size_t os_cache, heap;
 
-	if (odb_count_objects(the_repository->objects,
-			      ODB_COUNT_OBJECTS_APPROXIMATE, &nr_objects) < 0)
+	if (odb_source_count_objects(&files->base, ODB_COUNT_OBJECTS_APPROXIMATE,
+				     &nr_objects) < 0)
 		return 0;
 
 	if (!pack || !nr_objects)
@@ -679,6 +679,8 @@ static void add_repack_incremental_option(struct strvec *args)
 static bool odb_optimize_required(struct object_database *odb,
 				  const struct odb_optimize_options *opts)
 {
+	struct odb_source_files *files = odb_source_files_downcast(odb->sources);
+
 	switch (opts->strategy) {
 	case ODB_OPTIMIZE_INCREMENTAL: {
 		int gc_auto_threshold = 6700;
@@ -693,8 +695,8 @@ static bool odb_optimize_required(struct object_database *odb,
 		 */
 		if (gc_auto_threshold <= 0)
 			return false;
-		if (!too_many_packs(gc_auto_pack_limit) &&
-		    !too_many_loose_objects(gc_auto_threshold))
+		if (!too_many_packs(files, gc_auto_pack_limit) &&
+		    !too_many_loose_objects(files, gc_auto_threshold))
 			return false;
 
 		return true;
@@ -739,7 +741,7 @@ static bool odb_optimize_required(struct object_database *odb,
 		 * Otherwise, we estimate the number of loose objects to determine
 		 * whether we want to create a new packfile or not.
 		 */
-		if (too_many_loose_objects(auto_value)) {
+		if (too_many_loose_objects(files, auto_value)) {
 			ret = true;
 			goto out;
 		}
@@ -886,21 +888,22 @@ static int gc_foreground_tasks(struct maintenance_run_opts *opts,
 static int odb_optimize(struct object_database *odb,
 			const struct odb_optimize_options *opts)
 {
+	struct odb_source_files *files = odb_source_files_downcast(odb->sources);
 	struct child_process repack_cmd = CHILD_PROCESS_INIT;
 	unsigned long big_pack_threshold = 0;
 	int gc_auto_threshold = 6700;
 	int gc_auto_pack_limit = 50;
 	int ret;
 
-	repo_config_get_int(the_repository, "gc.auto", &gc_auto_threshold);
-	repo_config_get_int(the_repository, "gc.autopacklimit", &gc_auto_pack_limit);
-	repo_config_get_ulong(the_repository, "gc.bigpackthreshold", &big_pack_threshold);
+	repo_config_get_int(odb->repo, "gc.auto", &gc_auto_threshold);
+	repo_config_get_int(odb->repo, "gc.autopacklimit", &gc_auto_pack_limit);
+	repo_config_get_ulong(odb->repo, "gc.bigpackthreshold", &big_pack_threshold);
 
 	if (odb->repo->repository_format_precious_objects)
 		return 0;
 
 	repack_cmd.git_cmd = 1;
-	repack_cmd.odb_to_close = the_repository->objects;
+	repack_cmd.odb_to_close = odb->repo->objects;
 
 	strvec_pushl(&repack_cmd.args, "repack", "-d", "-l", NULL);
 	if (opts->flags & ODB_OPTIMIZE_NO_REUSE_DELTAS)
@@ -930,29 +933,29 @@ static int odb_optimize(struct object_database *odb,
 
 			if (opts->keep_largest_pack != -1) {
 				if (opts->keep_largest_pack)
-					find_base_packs(&keep_pack, 0);
+					find_base_packs(files, &keep_pack, 0);
 			} else if (big_pack_threshold) {
-				find_base_packs(&keep_pack, big_pack_threshold);
+				find_base_packs(files, &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)) {
+			if (too_many_packs(files, 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);
+					find_base_packs(files, &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);
+						find_base_packs(files, &keep_pack, 0);
 					}
 				} else {
-					struct packed_git *p = find_base_packs(&keep_pack, 0);
+					struct packed_git *p = find_base_packs(files, &keep_pack, 0);
 					uint64_t mem_have, mem_want;
 
 					mem_have = total_ram();
-					mem_want = estimate_repack_memory(p);
+					mem_want = estimate_repack_memory(files, p);
 
 					/*
 					 * Only allow 1/2 of memory for pack-objects, leave
@@ -981,10 +984,10 @@ static int odb_optimize(struct object_database *odb,
 		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",
+		repo_config_get_int(odb->repo, "maintenance.geometric-repack.splitFactor",
 				    &geometry.split_factor);
 
-		existing_packs.repo = the_repository;
+		existing_packs.repo = odb->repo;
 		existing_packs_collect(&existing_packs, &kept_packs);
 		pack_geometry_init(&geometry, &existing_packs, &po_args);
 		pack_geometry_split(&geometry);
@@ -995,7 +998,7 @@ static int odb_optimize(struct object_database *odb,
 		} else {
 			add_repack_all_option(opts, NULL, &repack_cmd.args);
 		}
-		if (the_repository->settings.core_multi_pack_index)
+		if (odb->repo->settings.core_multi_pack_index)
 			strvec_push(&repack_cmd.args, "--write-midx");
 
 		existing_packs_release(&existing_packs);
@@ -1020,7 +1023,7 @@ static int odb_optimize(struct object_database *odb,
 		strvec_push(&prune_cmd.args, opts->prune_expire);
 		if (!(opts->flags & ODB_OPTIMIZE_VERBOSE))
 			strvec_push(&prune_cmd.args, "--no-progress");
-		if (repo_has_promisor_remote(the_repository))
+		if (repo_has_promisor_remote(odb->repo))
 			strvec_push(&prune_cmd.args,
 				    "--exclude-promisor-objects");
 		prune_cmd.git_cmd = 1;
@@ -1031,7 +1034,7 @@ static int odb_optimize(struct object_database *odb,
 		}
 	}
 
-	if (opts->flags & ODB_OPTIMIZE_AUTO && too_many_loose_objects(gc_auto_threshold))
+	if (opts->flags & ODB_OPTIMIZE_AUTO && too_many_loose_objects(files, gc_auto_threshold))
 		warning(_("There are too many unreachable loose objects; "
 			"run 'git prune' to remove them."));
 

-- 
2.55.0.141.g00534a21ce.dirty

[PATCH 10/11] builtin/gc: fix signedness issues in ODB-related functionality

[Patrick Steinhardt]

There are a couple of signedness issues in ODB-related functionality.
These are not a problem because we disable -Wsign-compare in this file,
but once we move these functions into "odb/source-files.c" they will
result in warnings.

Fix those issues:

  - In `too_many_loose_objects()` we receive a signed limit, but compare
    it with the unsigned actual number of loose objects. This is fixed
    by bailing out immediately when the limit is smaller than or equal
    to zero, which we also do similarly in other places. The warning is
    then squelched via a cast.

  - In `find_base_packs()` we compare the signed size of the pack
    against the unsigned limit. As the pack size is always going to be a
    positive file size it's safe to cast it to an unsigned value.

  - In `odb_optimize()` we compare the unsigned `keep_pack.nr` value
    against the signed `gc_auto_pack_limit`. We only reach this code
    when `too_many_packs()` returns true-ish, and that can only happen
    when `gc_auto_pack_limit > 0`. Consequently, we can fix the warning
    by casting the limit to an unsigned value.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/gc.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index 3207182488..8cf3781313 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -430,19 +430,21 @@ static int rerere_gc_condition(struct gc_config *cfg UNUSED)
 
 static int too_many_loose_objects(struct odb_source_files *files, int limit)
 {
-	/*
-	 * This is weird, but stems from legacy behaviour: the GC auto
-	 * threshold was always essentially interpreted as if it was rounded up
-	 * to the next multiple 256 of, so we retain this behaviour for now.
-	 */
-	int auto_threshold = DIV_ROUND_UP(limit, 256) * 256;
 	unsigned long loose_count;
 
+	if (limit <= 0)
+		return 0;
+
 	if (odb_source_count_objects(&files->loose->base, ODB_COUNT_OBJECTS_APPROXIMATE,
 				     &loose_count) < 0)
 		return 0;
 
-	return loose_count > auto_threshold;
+	/*
+	 * This is weird, but stems from legacy behaviour: the GC auto
+	 * threshold was always essentially interpreted as if it was rounded up
+	 * to the next multiple 256 of, so we retain this behaviour for now.
+	 */
+	return loose_count > (DIV_ROUND_UP(((unsigned long) limit), 256) * 256);
 }
 
 static struct packed_git *find_base_packs(struct odb_source_files *files,
@@ -456,7 +458,7 @@ static struct packed_git *find_base_packs(struct odb_source_files *files,
 		if (e->pack->is_cruft)
 			continue;
 		if (limit) {
-			if (e->pack->pack_size >= limit)
+			if ((uintmax_t) e->pack->pack_size >= limit)
 				string_list_append(packs, e->pack->pack_name);
 		} else if (!base || base->pack_size < e->pack->pack_size) {
 			base = e->pack;
@@ -946,7 +948,7 @@ static int odb_optimize(struct object_database *odb,
 
 				if (big_pack_threshold) {
 					find_base_packs(files, &keep_pack, big_pack_threshold);
-					if (keep_pack.nr >= gc_auto_pack_limit) {
+					if (keep_pack.nr >= (unsigned long) gc_auto_pack_limit) {
 						string_list_clear(&keep_pack, 0);
 						find_base_packs(files, &keep_pack, 0);
 					}

-- 
2.55.0.141.g00534a21ce.dirty

[PATCH 11/11] odb: make optimizations pluggable

[Patrick Steinhardt]

Move `odb_optimize()` and `odb_optimize_required()` from "builtin/gc.c"
into the "files" source and wire them up via newly introduced vtable
pointers for the object database sources. This makes the logic pluggable
and thus allows other backends to have their own, custom implementation.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/gc.c       | 490 +----------------------------------------------------
 odb.c              |  12 ++
 odb.h              |  45 +++++
 odb/source-files.c | 470 ++++++++++++++++++++++++++++++++++++++++++++++++++
 odb/source-files.h |  15 ++
 odb/source.h       |  36 ++++
 6 files changed, 579 insertions(+), 489 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index 8cf3781313..ac1a21e912 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -30,16 +30,11 @@
 #include "commit-graph.h"
 #include "packfile.h"
 #include "object-file.h"
-#include "pack.h"
-#include "pack-objects.h"
+#include "odb.h"
 #include "path.h"
 #include "reflog.h"
-#include "repack.h"
 #include "rerere.h"
 #include "revision.h"
-#include "blob.h"
-#include "tree.h"
-#include "promisor-remote.h"
 #include "refs.h"
 #include "remote.h"
 #include "exec-cmd.h"
@@ -428,203 +423,6 @@ static int rerere_gc_condition(struct gc_config *cfg UNUSED)
 	return should_gc;
 }
 
-static int too_many_loose_objects(struct odb_source_files *files, int limit)
-{
-	unsigned long loose_count;
-
-	if (limit <= 0)
-		return 0;
-
-	if (odb_source_count_objects(&files->loose->base, ODB_COUNT_OBJECTS_APPROXIMATE,
-				     &loose_count) < 0)
-		return 0;
-
-	/*
-	 * This is weird, but stems from legacy behaviour: the GC auto
-	 * threshold was always essentially interpreted as if it was rounded up
-	 * to the next multiple 256 of, so we retain this behaviour for now.
-	 */
-	return loose_count > (DIV_ROUND_UP(((unsigned long) limit), 256) * 256);
-}
-
-static struct packed_git *find_base_packs(struct odb_source_files *files,
-					  struct string_list *packs,
-					  unsigned long limit)
-{
-	struct packfile_list_entry *e;
-	struct packed_git *base = NULL;
-
-	for (e = packfile_store_get_packs(files->packed); e; e = e->next) {
-		if (e->pack->is_cruft)
-			continue;
-		if (limit) {
-			if ((uintmax_t) e->pack->pack_size >= limit)
-				string_list_append(packs, e->pack->pack_name);
-		} else if (!base || base->pack_size < e->pack->pack_size) {
-			base = e->pack;
-		}
-	}
-
-	if (base)
-		string_list_append(packs, base->pack_name);
-
-	return base;
-}
-
-static int too_many_packs(struct odb_source_files *files, int gc_auto_pack_limit)
-{
-	struct packfile_list_entry *e;
-	int cnt = 0;
-
-	if (gc_auto_pack_limit <= 0)
-		return 0;
-
-	for (e = packfile_store_get_packs(files->packed); e; e = e->next) {
-		if (e->pack->pack_keep)
-			continue;
-		/*
-		 * Perhaps check the size of the pack and count only
-		 * very small ones here?
-		 */
-		cnt++;
-	}
-	return gc_auto_pack_limit < cnt;
-}
-
-static uint64_t total_ram(void)
-{
-#if defined(HAVE_SYSINFO)
-	struct sysinfo si;
-
-	if (!sysinfo(&si)) {
-		uint64_t total = si.totalram;
-
-		if (si.mem_unit > 1)
-			total *= (uint64_t)si.mem_unit;
-		return total;
-	}
-#elif defined(HAVE_BSD_SYSCTL) && (defined(HW_MEMSIZE) || defined(HW_PHYSMEM) || defined(HW_PHYSMEM64))
-	uint64_t physical_memory;
-	int mib[2];
-	size_t length;
-
-	mib[0] = CTL_HW;
-# if defined(HW_MEMSIZE)
-	mib[1] = HW_MEMSIZE;
-# elif defined(HW_PHYSMEM64)
-	mib[1] = HW_PHYSMEM64;
-# else
-	mib[1] = HW_PHYSMEM;
-# endif
-	length = sizeof(physical_memory);
-	if (!sysctl(mib, 2, &physical_memory, &length, NULL, 0)) {
-		if (length == 4) {
-			uint32_t mem;
-
-			if (!sysctl(mib, 2, &mem, &length, NULL, 0))
-				physical_memory = mem;
-		}
-		return physical_memory;
-	}
-#elif defined(GIT_WINDOWS_NATIVE)
-	MEMORYSTATUSEX memInfo;
-
-	memInfo.dwLength = sizeof(MEMORYSTATUSEX);
-	if (GlobalMemoryStatusEx(&memInfo))
-		return memInfo.ullTotalPhys;
-#endif
-	return 0;
-}
-
-static uint64_t estimate_repack_memory(struct odb_source_files *files,
-				       struct packed_git *pack)
-{
-	unsigned long max_delta_cache_size = DEFAULT_DELTA_CACHE_SIZE;
-	unsigned long delta_base_cache_limit = DEFAULT_DELTA_BASE_CACHE_LIMIT;
-	unsigned long nr_objects;
-	size_t os_cache, heap;
-
-	if (odb_source_count_objects(&files->base, ODB_COUNT_OBJECTS_APPROXIMATE,
-				     &nr_objects) < 0)
-		return 0;
-
-	if (!pack || !nr_objects)
-		return 0;
-
-	repo_config_get_ulong(the_repository, "pack.deltacachesize", &max_delta_cache_size);
-	repo_config_get_ulong(the_repository, "core.deltabasecachelimit", &delta_base_cache_limit);
-
-	/*
-	 * First we have to scan through at least one pack.
-	 * Assume enough room in OS file cache to keep the entire pack
-	 * or we may accidentally evict data of other processes from
-	 * the cache.
-	 */
-	os_cache = pack->pack_size + pack->index_size;
-	/* then pack-objects needs lots more for book keeping */
-	heap = sizeof(struct object_entry) * nr_objects;
-	/*
-	 * internal rev-list --all --objects takes up some memory too,
-	 * let's say half of it is for blobs
-	 */
-	heap += sizeof(struct blob) * nr_objects / 2;
-	/*
-	 * and the other half is for trees (commits and tags are
-	 * usually insignificant)
-	 */
-	heap += sizeof(struct tree) * nr_objects / 2;
-	/* and then obj_hash[], underestimated in fact */
-	heap += sizeof(struct object *) * nr_objects;
-	/* revindex is used also */
-	heap += (sizeof(off_t) + sizeof(uint32_t)) * nr_objects;
-	/*
-	 * read_sha1_file() (either at delta calculation phase, or
-	 * writing phase) also fills up the delta base cache
-	 */
-	heap += delta_base_cache_limit;
-	/* and of course pack-objects has its own delta cache */
-	heap += max_delta_cache_size;
-
-	return os_cache + heap;
-}
-
-static int keep_one_pack(struct string_list_item *item, void *data)
-{
-	struct strvec *args = data;
-	strvec_pushf(args, "--keep-pack=%s", basename(item->string));
-	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),
-
-	/* Perform auto-maintenance, only optimizing objects as required. */
-	ODB_OPTIMIZE_AUTO = (1 << 1),
-
-	/* Recompute existing deltas. */
-	ODB_OPTIMIZE_NO_REUSE_DELTAS = (1 << 2),
-};
-
-struct odb_optimize_options {
-	enum odb_optimize_strategy strategy;
-	enum odb_optimize_flags flags;
-	const char *prune_expire;
-	const char *expire_to;
-	int depth;
-	int window;
-
-	/* Backend-specific options. */
-	int keep_largest_pack;
-	int cruft_packs;
-	unsigned long max_cruft_size;
-};
-
 #define OPTIMIZE_FIELDS_FROM_GC_CONFIG(cfg, aggressive) \
 	.prune_expire = (cfg)->prune_expire, \
 	.expire_to = (cfg)->repack_expire_to, \
@@ -633,133 +431,6 @@ struct odb_optimize_options {
 	.window = (aggressive) ? (cfg)->aggressive_window : 0, \
 	.depth = (aggressive) ? (cfg)->aggressive_depth : 0
 
-static void add_repack_all_option(const struct odb_optimize_options *opts,
-				  struct string_list *keep_pack,
-				  struct strvec *args)
-{
-	char *repack_filter = NULL;
-	char *repack_filter_to = NULL;
-
-	repo_config_get_string(the_repository, "gc.repackfilter", &repack_filter);
-	repo_config_get_string(the_repository, "gc.repackfilterto", &repack_filter_to);
-
-	if (opts->prune_expire && !strcmp(opts->prune_expire, "now") &&
-	    !(opts->cruft_packs && opts->expire_to))
-		strvec_push(args, "-a");
-	else if (opts->cruft_packs) {
-		strvec_push(args, "--cruft");
-		if (opts->prune_expire)
-			strvec_pushf(args, "--cruft-expiration=%s", opts->prune_expire);
-		if (opts->max_cruft_size)
-			strvec_pushf(args, "--max-cruft-size=%lu",
-				     opts->max_cruft_size);
-		if (opts->expire_to)
-			strvec_pushf(args, "--expire-to=%s", opts->expire_to);
-	} else {
-		strvec_push(args, "-A");
-		if (opts->prune_expire)
-			strvec_pushf(args, "--unpack-unreachable=%s", opts->prune_expire);
-	}
-
-	if (keep_pack)
-		for_each_string_list(keep_pack, keep_one_pack, args);
-
-	if (repack_filter && *repack_filter)
-		strvec_pushf(args, "--filter=%s", repack_filter);
-	if (repack_filter_to && *repack_filter_to)
-		strvec_pushf(args, "--filter-to=%s", repack_filter_to);
-
-	free(repack_filter);
-	free(repack_filter_to);
-}
-
-static void add_repack_incremental_option(struct strvec *args)
-{
-	strvec_push(args, "--no-write-bitmap-index");
-}
-
-static bool odb_optimize_required(struct object_database *odb,
-				  const struct odb_optimize_options *opts)
-{
-	struct odb_source_files *files = odb_source_files_downcast(odb->sources);
-
-	switch (opts->strategy) {
-	case ODB_OPTIMIZE_INCREMENTAL: {
-		int gc_auto_threshold = 6700;
-		int gc_auto_pack_limit = 50;
-
-		repo_config_get_int(odb->repo, "gc.auto", &gc_auto_threshold);
-		repo_config_get_int(odb->repo, "gc.autopacklimit", &gc_auto_pack_limit);
-
-		/*
-		 * Setting gc.auto to 0 or negative can disable the
-		 * automatic gc.
-		 */
-		if (gc_auto_threshold <= 0)
-			return false;
-		if (!too_many_packs(files, gc_auto_pack_limit) &&
-		    !too_many_loose_objects(files, gc_auto_threshold))
-			return false;
-
-		return true;
-	}
-	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;
-		int auto_value = 100;
-		bool ret;
-
-		repo_config_get_int(odb->repo, "maintenance.geometric-repack.auto",
-				    &auto_value);
-		if (!auto_value)
-			return false;
-		if (auto_value < 0)
-			return true;
-
-		repo_config_get_int(odb->repo, "maintenance.geometric-repack.splitFactor",
-				    &geometry.split_factor);
-
-		existing_packs.repo = odb->repo;
-		existing_packs_collect(&existing_packs, &kept_packs);
-		pack_geometry_init(&geometry, &existing_packs, &po_args);
-		pack_geometry_split(&geometry);
-
-		/*
-		 * When we'd merge at least two packs with one another we always
-		 * perform the repack.
-		 */
-		if (geometry.split) {
-			ret = true;
-			goto out;
-		}
-
-		/*
-		 * Otherwise, we estimate the number of loose objects to determine
-		 * whether we want to create a new packfile or not.
-		 */
-		if (too_many_loose_objects(files, auto_value)) {
-			ret = true;
-			goto out;
-		}
-
-		ret = false;
-
-	out:
-		existing_packs_release(&existing_packs);
-		pack_geometry_release(&geometry);
-		return ret;
-	}
-	default:
-		BUG("unknown maintenance strategy '%d'", opts->strategy);
-	}
-}
-
 /* return NULL on success, else hostname running the gc */
 static const char *lock_repo_for_gc(int force, pid_t* ret_pid)
 {
@@ -887,165 +558,6 @@ static int gc_foreground_tasks(struct maintenance_run_opts *opts,
 	return 0;
 }
 
-static int odb_optimize(struct object_database *odb,
-			const struct odb_optimize_options *opts)
-{
-	struct odb_source_files *files = odb_source_files_downcast(odb->sources);
-	struct child_process repack_cmd = CHILD_PROCESS_INIT;
-	unsigned long big_pack_threshold = 0;
-	int gc_auto_threshold = 6700;
-	int gc_auto_pack_limit = 50;
-	int ret;
-
-	repo_config_get_int(odb->repo, "gc.auto", &gc_auto_threshold);
-	repo_config_get_int(odb->repo, "gc.autopacklimit", &gc_auto_pack_limit);
-	repo_config_get_ulong(odb->repo, "gc.bigpackthreshold", &big_pack_threshold);
-
-	if (odb->repo->repository_format_precious_objects)
-		return 0;
-
-	repack_cmd.git_cmd = 1;
-	repack_cmd.odb_to_close = odb->repo->objects;
-
-	strvec_pushl(&repack_cmd.args, "repack", "-d", "-l", NULL);
-	if (opts->flags & ODB_OPTIMIZE_NO_REUSE_DELTAS)
-		strvec_push(&repack_cmd.args, "-f");
-	if (opts->depth > 0)
-		strvec_pushf(&repack_cmd.args, "--depth=%d", opts->depth);
-	if (opts->window > 0)
-		strvec_pushf(&repack_cmd.args, "--window=%d", opts->window);
-	if (!(opts->flags & ODB_OPTIMIZE_VERBOSE))
-		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.
-	 */
-	switch (opts->strategy) {
-	case ODB_OPTIMIZE_INCREMENTAL:
-		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(files, &keep_pack, 0);
-			} else if (big_pack_threshold) {
-				find_base_packs(files, &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(files, gc_auto_pack_limit)) {
-				struct string_list keep_pack = STRING_LIST_INIT_NODUP;
-
-				if (big_pack_threshold) {
-					find_base_packs(files, &keep_pack, big_pack_threshold);
-					if (keep_pack.nr >= (unsigned long) gc_auto_pack_limit) {
-						string_list_clear(&keep_pack, 0);
-						find_base_packs(files, &keep_pack, 0);
-					}
-				} else {
-					struct packed_git *p = find_base_packs(files, &keep_pack, 0);
-					uint64_t mem_have, mem_want;
-
-					mem_have = total_ram();
-					mem_want = estimate_repack_memory(files, 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(odb->repo, "maintenance.geometric-repack.splitFactor",
-				    &geometry.split_factor);
-
-		existing_packs.repo = odb->repo;
-		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 (odb->repo->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)) {
-		ret = error(FAILED_RUN, repack_cmd.args.v[0]);
-		goto out;
-	}
-
-	/* 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);
-		/* run `git prune` even if using cruft packs */
-		strvec_push(&prune_cmd.args, opts->prune_expire);
-		if (!(opts->flags & ODB_OPTIMIZE_VERBOSE))
-			strvec_push(&prune_cmd.args, "--no-progress");
-		if (repo_has_promisor_remote(odb->repo))
-			strvec_push(&prune_cmd.args,
-				    "--exclude-promisor-objects");
-		prune_cmd.git_cmd = 1;
-
-		if (run_command(&prune_cmd)) {
-			ret = error(FAILED_RUN, prune_cmd.args.v[0]);
-			goto out;
-		}
-	}
-
-	if (opts->flags & ODB_OPTIMIZE_AUTO && too_many_loose_objects(files, gc_auto_threshold))
-		warning(_("There are too many unreachable loose objects; "
-			"run 'git prune' to remove them."));
-
-	ret = 0;
-
-out:
-	return ret;
-}
-
 static int maintenance_task_odb(struct maintenance_run_opts *opts,
 				struct gc_config *cfg,
 				int keep_largest_pack,
diff --git a/odb.c b/odb.c
index 7d555be09f..89660981fe 100644
--- a/odb.c
+++ b/odb.c
@@ -1003,6 +1003,18 @@ int odb_write_object_stream(struct object_database *odb,
 	return odb_source_write_object_stream(odb->sources, stream, len, oid);
 }
 
+int odb_optimize(struct object_database *odb,
+		 const struct odb_optimize_options *opts)
+{
+	return odb_source_optimize(odb->sources, opts);
+}
+
+bool odb_optimize_required(struct object_database *odb,
+			   const struct odb_optimize_options *opts)
+{
+	return odb_source_optimize_required(odb->sources, opts);
+}
+
 struct object_database *odb_new(struct repository *repo,
 				const char *primary_source,
 				const char *secondary_sources)
diff --git a/odb.h b/odb.h
index 3834a0dcbf..7e1c85c22e 100644
--- a/odb.h
+++ b/odb.h
@@ -117,6 +117,51 @@ struct object_database *odb_new(struct repository *repo,
 /* Free the object database and release all resources. */
 void odb_free(struct object_database *o);
 
+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),
+
+	/* Perform auto-maintenance, only optimizing objects as required. */
+	ODB_OPTIMIZE_AUTO = (1 << 1),
+
+	/* Recompute existing deltas. */
+	ODB_OPTIMIZE_NO_REUSE_DELTAS = (1 << 2),
+};
+
+struct odb_optimize_options {
+	enum odb_optimize_strategy strategy;
+	enum odb_optimize_flags flags;
+	const char *prune_expire;
+	const char *expire_to;
+	int depth;
+	int window;
+
+	/* Backend-specific options. */
+	int keep_largest_pack;
+	int cruft_packs;
+	unsigned long max_cruft_size;
+};
+
+/*
+ * Optimize the object database. Returns 0 on success, a negative error code
+ * otherwise.
+ */
+int odb_optimize(struct object_database *odb,
+		 const struct odb_optimize_options *opts);
+
+/*
+ * Check whether optimization of the object database is required given the
+ * provided options. Returns true if optimization should be performed, false
+ * otherwise.
+ */
+bool odb_optimize_required(struct object_database *odb,
+			   const struct odb_optimize_options *opts);
+
 /*
  * Close the object database and all of its sources so that any held resources
  * will be released. The database can still be used after closing it, in which
diff --git a/odb/source-files.c b/odb/source-files.c
index bbd1784b33..82cf61da4a 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -1,6 +1,8 @@
 #include "git-compat-util.h"
 #include "abspath.h"
+#include "blob.h"
 #include "chdir-notify.h"
+#include "config.h"
 #include "gettext.h"
 #include "lockfile.h"
 #include "object-file.h"
@@ -8,8 +10,16 @@
 #include "odb/source.h"
 #include "odb/source-files.h"
 #include "odb/source-loose.h"
+#include "pack-objects.h"
 #include "packfile.h"
+#include "path.h"
+#include "promisor-remote.h"
+#include "repack.h"
+#include "run-command.h"
 #include "strbuf.h"
+#include "string-list.h"
+#include "strvec.h"
+#include "tree.h"
 #include "write-or-die.h"
 
 static void odb_source_files_reparent(const char *name UNUSED,
@@ -260,6 +270,464 @@ static int odb_source_files_write_alternate(struct odb_source *source,
 	return ret;
 }
 
+static int too_many_loose_objects(struct odb_source_files *files, int limit)
+{
+	unsigned long loose_count;
+
+	if (limit <= 0)
+		return 0;
+
+	if (odb_source_count_objects(&files->loose->base, ODB_COUNT_OBJECTS_APPROXIMATE,
+				     &loose_count) < 0)
+		return 0;
+
+	/*
+	 * This is weird, but stems from legacy behaviour: the GC auto
+	 * threshold was always essentially interpreted as if it was rounded up
+	 * to the next multiple 256 of, so we retain this behaviour for now.
+	 */
+	return loose_count > (DIV_ROUND_UP(((unsigned long) limit), 256) * 256);
+}
+
+static struct packed_git *find_base_packs(struct odb_source_files *files,
+					  struct string_list *packs,
+					  unsigned long limit)
+{
+	struct packfile_list_entry *e;
+	struct packed_git *base = NULL;
+
+	for (e = packfile_store_get_packs(files->packed); e; e = e->next) {
+		if (e->pack->is_cruft)
+			continue;
+		if (limit) {
+			if ((uintmax_t) e->pack->pack_size >= limit)
+				string_list_append(packs, e->pack->pack_name);
+		} else if (!base || base->pack_size < e->pack->pack_size) {
+			base = e->pack;
+		}
+	}
+
+	if (base)
+		string_list_append(packs, base->pack_name);
+
+	return base;
+}
+
+static int too_many_packs(struct odb_source_files *files, int gc_auto_pack_limit)
+{
+	struct packfile_list_entry *e;
+	int cnt = 0;
+
+	if (gc_auto_pack_limit <= 0)
+		return 0;
+
+	for (e = packfile_store_get_packs(files->packed); e; e = e->next) {
+		if (e->pack->pack_keep)
+			continue;
+		/*
+		 * Perhaps check the size of the pack and count only
+		 * very small ones here?
+		 */
+		cnt++;
+	}
+	return gc_auto_pack_limit < cnt;
+}
+
+static uint64_t total_ram(void)
+{
+#if defined(HAVE_SYSINFO)
+	struct sysinfo si;
+
+	if (!sysinfo(&si)) {
+		uint64_t total = si.totalram;
+
+		if (si.mem_unit > 1)
+			total *= (uint64_t)si.mem_unit;
+		return total;
+	}
+#elif defined(HAVE_BSD_SYSCTL) && (defined(HW_MEMSIZE) || defined(HW_PHYSMEM) || defined(HW_PHYSMEM64))
+	uint64_t physical_memory;
+	int mib[2];
+	size_t length;
+
+	mib[0] = CTL_HW;
+# if defined(HW_MEMSIZE)
+	mib[1] = HW_MEMSIZE;
+# elif defined(HW_PHYSMEM64)
+	mib[1] = HW_PHYSMEM64;
+# else
+	mib[1] = HW_PHYSMEM;
+# endif
+	length = sizeof(physical_memory);
+	if (!sysctl(mib, 2, &physical_memory, &length, NULL, 0)) {
+		if (length == 4) {
+			uint32_t mem;
+
+			if (!sysctl(mib, 2, &mem, &length, NULL, 0))
+				physical_memory = mem;
+		}
+		return physical_memory;
+	}
+#elif defined(GIT_WINDOWS_NATIVE)
+	MEMORYSTATUSEX memInfo;
+
+	memInfo.dwLength = sizeof(MEMORYSTATUSEX);
+	if (GlobalMemoryStatusEx(&memInfo))
+		return memInfo.ullTotalPhys;
+#endif
+	return 0;
+}
+
+static uint64_t estimate_repack_memory(struct odb_source_files *files,
+				       struct packed_git *pack)
+{
+	unsigned long max_delta_cache_size = DEFAULT_DELTA_CACHE_SIZE;
+	unsigned long delta_base_cache_limit = DEFAULT_DELTA_BASE_CACHE_LIMIT;
+	unsigned long nr_objects;
+	size_t os_cache, heap;
+
+	if (odb_source_count_objects(&files->base, ODB_COUNT_OBJECTS_APPROXIMATE,
+				     &nr_objects) < 0)
+		return 0;
+
+	if (!pack || !nr_objects)
+		return 0;
+
+	repo_config_get_ulong(files->base.odb->repo, "pack.deltacachesize",
+			      &max_delta_cache_size);
+	repo_config_get_ulong(files->base.odb->repo, "core.deltabasecachelimit",
+			      &delta_base_cache_limit);
+
+	/*
+	 * First we have to scan through at least one pack.
+	 * Assume enough room in OS file cache to keep the entire pack
+	 * or we may accidentally evict data of other processes from
+	 * the cache.
+	 */
+	os_cache = pack->pack_size + pack->index_size;
+	/* then pack-objects needs lots more for book keeping */
+	heap = sizeof(struct object_entry) * nr_objects;
+	/*
+	 * internal rev-list --all --objects takes up some memory too,
+	 * let's say half of it is for blobs
+	 */
+	heap += sizeof(struct blob) * nr_objects / 2;
+	/*
+	 * and the other half is for trees (commits and tags are
+	 * usually insignificant)
+	 */
+	heap += sizeof(struct tree) * nr_objects / 2;
+	/* and then obj_hash[], underestimated in fact */
+	heap += sizeof(struct object *) * nr_objects;
+	/* revindex is used also */
+	heap += (sizeof(off_t) + sizeof(uint32_t)) * nr_objects;
+	/*
+	 * read_sha1_file() (either at delta calculation phase, or
+	 * writing phase) also fills up the delta base cache
+	 */
+	heap += delta_base_cache_limit;
+	/* and of course pack-objects has its own delta cache */
+	heap += max_delta_cache_size;
+
+	return os_cache + heap;
+}
+
+static int keep_one_pack(struct string_list_item *item, void *data)
+{
+	struct strvec *args = data;
+	strvec_pushf(args, "--keep-pack=%s", basename(item->string));
+	return 0;
+}
+
+static void add_repack_all_option(struct repository *repo,
+				  const struct odb_optimize_options *opts,
+				  struct string_list *keep_pack,
+				  struct strvec *args)
+{
+	char *repack_filter = NULL;
+	char *repack_filter_to = NULL;
+
+	repo_config_get_string(repo, "gc.repackfilter", &repack_filter);
+	repo_config_get_string(repo, "gc.repackfilterto", &repack_filter_to);
+
+	if (opts->prune_expire && !strcmp(opts->prune_expire, "now") &&
+	    !(opts->cruft_packs && opts->expire_to))
+		strvec_push(args, "-a");
+	else if (opts->cruft_packs) {
+		strvec_push(args, "--cruft");
+		if (opts->prune_expire)
+			strvec_pushf(args, "--cruft-expiration=%s", opts->prune_expire);
+		if (opts->max_cruft_size)
+			strvec_pushf(args, "--max-cruft-size=%lu",
+				     opts->max_cruft_size);
+		if (opts->expire_to)
+			strvec_pushf(args, "--expire-to=%s", opts->expire_to);
+	} else {
+		strvec_push(args, "-A");
+		if (opts->prune_expire)
+			strvec_pushf(args, "--unpack-unreachable=%s", opts->prune_expire);
+	}
+
+	if (keep_pack)
+		for_each_string_list(keep_pack, keep_one_pack, args);
+
+	if (repack_filter && *repack_filter)
+		strvec_pushf(args, "--filter=%s", repack_filter);
+	if (repack_filter_to && *repack_filter_to)
+		strvec_pushf(args, "--filter-to=%s", repack_filter_to);
+
+	free(repack_filter);
+	free(repack_filter_to);
+}
+
+static void add_repack_incremental_option(struct strvec *args)
+{
+	strvec_push(args, "--no-write-bitmap-index");
+}
+
+bool odb_source_files_optimize_required(struct odb_source *source,
+					const struct odb_optimize_options *opts)
+{
+	struct odb_source_files *files = odb_source_files_downcast(source);
+	struct repository *repo = source->odb->repo;
+
+	switch (opts->strategy) {
+	case ODB_OPTIMIZE_INCREMENTAL: {
+		int gc_auto_threshold = 6700;
+		int gc_auto_pack_limit = 50;
+
+		repo_config_get_int(repo, "gc.auto", &gc_auto_threshold);
+		repo_config_get_int(repo, "gc.autopacklimit", &gc_auto_pack_limit);
+
+		/*
+		 * Setting gc.auto to 0 or negative can disable the
+		 * automatic gc.
+		 */
+		if (gc_auto_threshold <= 0)
+			return false;
+		if (!too_many_packs(files, gc_auto_pack_limit) &&
+		    !too_many_loose_objects(files, gc_auto_threshold))
+			return false;
+
+		return true;
+	}
+	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;
+		int auto_value = 100;
+		bool ret;
+
+		repo_config_get_int(repo, "maintenance.geometric-repack.auto",
+				    &auto_value);
+		if (!auto_value)
+			return false;
+		if (auto_value < 0)
+			return true;
+
+		repo_config_get_int(repo, "maintenance.geometric-repack.splitFactor",
+				    &geometry.split_factor);
+
+		existing_packs.repo = repo;
+		existing_packs_collect(&existing_packs, &kept_packs);
+		pack_geometry_init(&geometry, &existing_packs, &po_args);
+		pack_geometry_split(&geometry);
+
+		/*
+		 * When we'd merge at least two packs with one another we always
+		 * perform the repack.
+		 */
+		if (geometry.split) {
+			ret = true;
+			goto out;
+		}
+
+		/*
+		 * Otherwise, we estimate the number of loose objects to determine
+		 * whether we want to create a new packfile or not.
+		 */
+		if (too_many_loose_objects(files, auto_value)) {
+			ret = true;
+			goto out;
+		}
+
+		ret = false;
+
+	out:
+		existing_packs_release(&existing_packs);
+		pack_geometry_release(&geometry);
+		return ret;
+	}
+	default:
+		BUG("unknown maintenance strategy '%d'", opts->strategy);
+	}
+}
+
+int odb_source_files_optimize(struct odb_source *source,
+			      const struct odb_optimize_options *opts)
+{
+	struct odb_source_files *files = odb_source_files_downcast(source);
+	struct repository *repo = source->odb->repo;
+	struct child_process repack_cmd = CHILD_PROCESS_INIT;
+	unsigned long big_pack_threshold = 0;
+	int gc_auto_threshold = 6700;
+	int gc_auto_pack_limit = 50;
+	int ret;
+
+	repo_config_get_int(repo, "gc.auto", &gc_auto_threshold);
+	repo_config_get_int(repo, "gc.autopacklimit", &gc_auto_pack_limit);
+	repo_config_get_ulong(repo, "gc.bigpackthreshold", &big_pack_threshold);
+
+	if (repo->repository_format_precious_objects)
+		return 0;
+
+	repack_cmd.git_cmd = 1;
+	repack_cmd.odb_to_close = repo->objects;
+
+	strvec_pushl(&repack_cmd.args, "repack", "-d", "-l", NULL);
+	if (opts->flags & ODB_OPTIMIZE_NO_REUSE_DELTAS)
+		strvec_push(&repack_cmd.args, "-f");
+	if (opts->depth > 0)
+		strvec_pushf(&repack_cmd.args, "--depth=%d", opts->depth);
+	if (opts->window > 0)
+		strvec_pushf(&repack_cmd.args, "--window=%d", opts->window);
+	if (!(opts->flags & ODB_OPTIMIZE_VERBOSE))
+		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.
+	 */
+	switch (opts->strategy) {
+	case ODB_OPTIMIZE_INCREMENTAL:
+		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(files, &keep_pack, 0);
+			} else if (big_pack_threshold) {
+				find_base_packs(files, &keep_pack, big_pack_threshold);
+			}
+
+			add_repack_all_option(repo, opts, &keep_pack, &repack_cmd.args);
+			string_list_clear(&keep_pack, 0);
+		} else {
+			if (too_many_packs(files, gc_auto_pack_limit)) {
+				struct string_list keep_pack = STRING_LIST_INIT_NODUP;
+
+				if (big_pack_threshold) {
+					find_base_packs(files, &keep_pack, big_pack_threshold);
+					if (keep_pack.nr >= (unsigned long) gc_auto_pack_limit) {
+						string_list_clear(&keep_pack, 0);
+						find_base_packs(files, &keep_pack, 0);
+					}
+				} else {
+					struct packed_git *p = find_base_packs(files, &keep_pack, 0);
+					uint64_t mem_have, mem_want;
+
+					mem_have = total_ram();
+					mem_want = estimate_repack_memory(files, 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(repo, 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(repo, "maintenance.geometric-repack.splitFactor",
+				    &geometry.split_factor);
+
+		existing_packs.repo = repo;
+		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(repo, opts, NULL, &repack_cmd.args);
+		}
+		if (repo->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)) {
+		ret = error("failed to run %s", repack_cmd.args.v[0]);
+		goto out;
+	}
+
+	/* 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);
+		/* run `git prune` even if using cruft packs */
+		strvec_push(&prune_cmd.args, opts->prune_expire);
+		if (!(opts->flags & ODB_OPTIMIZE_VERBOSE))
+			strvec_push(&prune_cmd.args, "--no-progress");
+		if (repo_has_promisor_remote(repo))
+			strvec_push(&prune_cmd.args,
+				    "--exclude-promisor-objects");
+		prune_cmd.git_cmd = 1;
+
+		if (run_command(&prune_cmd)) {
+			ret = error("failed to run %s", prune_cmd.args.v[0]);
+			goto out;
+		}
+	}
+
+	if (opts->flags & ODB_OPTIMIZE_AUTO && too_many_loose_objects(files, gc_auto_threshold))
+		warning(_("There are too many unreachable loose objects; "
+			"run 'git prune' to remove them."));
+
+	ret = 0;
+
+out:
+	return ret;
+}
+
 struct odb_source_files *odb_source_files_new(struct object_database *odb,
 					      const char *path,
 					      bool local)
@@ -285,6 +753,8 @@ struct odb_source_files *odb_source_files_new(struct object_database *odb,
 	files->base.begin_transaction = odb_source_files_begin_transaction;
 	files->base.read_alternates = odb_source_files_read_alternates;
 	files->base.write_alternate = odb_source_files_write_alternate;
+	files->base.optimize = odb_source_files_optimize;
+	files->base.optimize_required = odb_source_files_optimize_required;
 
 	/*
 	 * Ideally, we would only ever store absolute paths in the source. This
diff --git a/odb/source-files.h b/odb/source-files.h
index d7ac3c1c81..044242bc36 100644
--- a/odb/source-files.h
+++ b/odb/source-files.h
@@ -21,6 +21,21 @@ struct odb_source_files *odb_source_files_new(struct object_database *odb,
 					      const char *path,
 					      bool local);
 
+/*
+ * Optimize the files object database source by repacking loose objects and
+ * packfiles as needed. Returns 0 on success, a negative error code otherwise.
+ */
+int odb_source_files_optimize(struct odb_source *source,
+			      const struct odb_optimize_options *opts);
+
+/*
+ * Check whether optimization of the files object database source is required
+ * given the provided options. Returns true if optimization should be
+ * performed, false otherwise.
+ */
+bool odb_source_files_optimize_required(struct odb_source *source,
+					const struct odb_optimize_options *opts);
+
 /*
  * Cast the given object database source to the files backend. This will cause
  * a BUG in case the source doesn't use this backend.
diff --git a/odb/source.h b/odb/source.h
index 8767708c9c..88a48ba3c3 100644
--- a/odb/source.h
+++ b/odb/source.h
@@ -258,6 +258,21 @@ struct odb_source {
 	 */
 	int (*write_alternate)(struct odb_source *source,
 			       const char *alternate);
+
+	/*
+	 * This callback is expected to optimize the object database source.
+	 * Returns 0 on success, a negative error code otherwise.
+	 */
+	int (*optimize)(struct odb_source *source,
+			const struct odb_optimize_options *opts);
+
+	/*
+	 * This callback is expected to check whether optimization of the
+	 * object database source is required given the provided options.
+	 * Returns true if optimization should be performed, false otherwise.
+	 */
+	bool (*optimize_required)(struct odb_source *source,
+				  const struct odb_optimize_options *opts);
 };
 
 /*
@@ -475,4 +490,25 @@ static inline int odb_source_begin_transaction(struct odb_source *source,
 	return source->begin_transaction(source, out);
 }
 
+/*
+ * Optimize the object database source. Returns 0 on success, a negative error
+ * code otherwise.
+ */
+static inline int odb_source_optimize(struct odb_source *source,
+				      const struct odb_optimize_options *opts)
+{
+	return source->optimize(source, opts);
+}
+
+/*
+ * Check whether optimization of the object database source is required given
+ * the provided options. Returns true if optimization should be performed,
+ * false otherwise.
+ */
+static inline bool odb_source_optimize_required(struct odb_source *source,
+						const struct odb_optimize_options *opts)
+{
+	return source->optimize_required(source, opts);
+}
+
 #endif

-- 
2.55.0.141.g00534a21ce.dirty

[PATCH v2 00/12] odb: make optimizations pluggable

[Patrick Steinhardt]

Hi,

this patch series converts object housekeeping to become pluggable.
There isn't really anything else to say about this.

The series is built on top of f85a7e6620 (Start Git 2.56 cycle,
2026-07-06).

Changes in v2:
  - Make tests in t7900 a bit more robust by not checking for exact
    commands, but instead by checking for executed tasks.
  - Link to v1: https://patch.msgid.link/20260707-b4-pks-odb-optimize-v1-0-aae607667be4@pks.im

Thanks!

Patrick

---
Patrick Steinhardt (12):
      t7900: simplify how we check for maintenance tasks
      odb: run "pre-auto-gc" hook for all maintenance tasks
      builtin/gc: move worktree and rerere tasks before object optimizations
      builtin/gc: extract object database optimizations into separate function
      builtin/gc: make repack arguments self-contained
      builtin/gc: inline config values specific to the "files" backend
      builtin/gc: introduce object database optimization options
      builtin/gc: move geometric repacking into `odb_optimize()`
      builtin/gc: introduce `odb_optimize_required()`
      builtin/gc: refactor ODB optimizations to operate on "files" source
      builtin/gc: fix signedness issues in ODB-related functionality
      odb: make optimizations pluggable

 builtin/gc.c           | 534 ++++++++-----------------------------------------
 odb.c                  |  12 ++
 odb.h                  |  45 +++++
 odb/source-files.c     | 470 +++++++++++++++++++++++++++++++++++++++++++
 odb/source-files.h     |  15 ++
 odb/source.h           |  36 ++++
 t/t7900-maintenance.sh | 338 +++++++++++++++++++++----------
 7 files changed, 894 insertions(+), 556 deletions(-)

Range-diff versus v1:

 -:  ---------- >  1:  b05988c150 t7900: simplify how we check for maintenance tasks
 1:  df9ed71c10 !  2:  734d3c6d17 odb: run "pre-auto-gc" hook for all maintenance tasks
    @@ t/t7900-maintenance.sh: test_expect_success 'geometric repacking honors configur
     +			git maintenance run --auto 2>/dev/null &&
     +
     +		# The successful hook does not inhibit any of the tasks...
    -+		test_subcommand git reflog expire --all <trace2.txt &&
    -+		test_subcommand_flex git repack <trace2.txt &&
    -+		test_subcommand git rerere gc <trace2.txt &&
    ++		test_maintenance_tasks trace2.txt <<-\EOF &&
    ++		reflog-expire foreground
    ++		geometric-repack
    ++		rerere-gc
    ++		EOF
     +		# ... but it must only have been executed a single time.
     +		test_line_count = 1 hook.log
     +	)
    @@ t/t7900-maintenance.sh: test_expect_success 'geometric repacking honors configur
     +		# is expected to be the only child process being spawned, and
     +		# it must only run a single time.
     +		test_grep "child_start.*pre-auto-gc" trace2.txt &&
    -+		test_subcommand_flex ! git trace2 &&
    ++		test_maintenance_tasks trace2.txt <<-\EOF &&
    ++		EOF
     +		test_line_count = 1 hook.log
     +	)
     +'
    @@ t/t7900-maintenance.sh: test_expect_success 'geometric repacking honors configur
     +		# is expected to be the only child process being spawned, and
     +		# it must only run a single time.
     +		test_grep "child_start.*pre-auto-gc" trace2.txt &&
    ++		test_maintenance_tasks trace2.txt <<-\EOF &&
    ++		EOF
     +		test_subcommand_flex ! git trace2 &&
     +		test_line_count = 1 hook.log
     +	)
 2:  ba358cede1 =  3:  a61f01a1c1 builtin/gc: move worktree and rerere tasks before object optimizations
 3:  ffbaf71f46 =  4:  3d6ea9927b builtin/gc: extract object database optimizations into separate function
 4:  7b568dcd0c =  5:  de2c2c084d builtin/gc: make repack arguments self-contained
 5:  e18465b4b2 =  6:  0b4f6a553d builtin/gc: inline config values specific to the "files" backend
 6:  4ee5a61e44 =  7:  e231c437ba builtin/gc: introduce object database optimization options
 7:  2b428d4516 !  8:  b015c35c8a builtin/gc: move geometric repacking into `odb_optimize()`
    @@ t/t7900-maintenance.sh: test_expect_success 'geometric repacking honors configur
      	)
      '
      
    -@@ t/t7900-maintenance.sh: test_expect_success 'maintenance.strategy is respected' '
    - 		test_strategy geometric <<-\EOF &&
    - 		git pack-refs --all --prune
    - 		git reflog expire --all
    --		git repack -d -l --geometric=2 --quiet --write-midx
    -+		git repack -d -l -q --geometric=2 --write-midx
    - 		git commit-graph write --split --reachable --no-progress
    - 		git worktree prune --expire 3.months.ago
    - 		git rerere gc
    -@@ t/t7900-maintenance.sh: test_expect_success 'maintenance.strategy is respected' '
    - 		test_strategy geometric --schedule=weekly <<-\EOF
    - 		git pack-refs --all --prune
    - 		git reflog expire --all
    --		git repack -d -l --geometric=2 --quiet --write-midx
    -+		git repack -d -l -q --geometric=2 --write-midx
    - 		git commit-graph write --split --reachable --no-progress
    - 		git worktree prune --expire 3.months.ago
    - 		git rerere gc
 8:  79c3d77210 =  9:  426a06b349 builtin/gc: introduce `odb_optimize_required()`
 9:  15f65ab0bf = 10:  cfb6014c30 builtin/gc: refactor ODB optimizations to operate on "files" source
10:  9035d7d679 = 11:  a478e0e0b3 builtin/gc: fix signedness issues in ODB-related functionality
11:  8fa84c3aa0 = 12:  5383b9027c odb: make optimizations pluggable

---
base-commit: f85a7e662054a7b0d9070e432508831afa214b47
change-id: 20260612-b4-pks-odb-optimize-3426c57e5c30

[PATCH v2 01/12] t7900: simplify how we check for maintenance tasks

[Patrick Steinhardt]

We have several tests in t7900 that verify whether specific maintenance
tasks did or did not run. This is done rather ad-hoc by checking for
spawned Git commands, which is awfully fragile:

  - We have to adjust tests whenever arguments to the spawned Git
    commands change.

  - We don't have a way to verify that negative matches are still
    working as expected.

  - We rely on maintenance tasks spawning a Git command in the first
    place.

We can do much better though, as we already have trace2 regions for each
of the maintenance tasks. Introduce a helper function that extracts all
such regions so that we can get a direct list of all maintenance tasks
that a certain command ran.

Adapt tests that care about whether or not a specific task ran to use
this new helper. Note that many tests still use `test_subcommand`
though, as they really care about the exact command that was executed.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t7900-maintenance.sh | 194 ++++++++++++++++++++++++++-----------------------
 1 file changed, 102 insertions(+), 92 deletions(-)

diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index d7f82e1bec..129829f1f4 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -23,6 +23,12 @@ test_xmllint () {
 	fi
 }
 
+test_maintenance_tasks () {
+	cat >expect &&
+	sed -ne "s/.*\"region_enter\".*\"category\":\"maintenance\([^\"]*\)\".*\"label\":\"\([^\"][^\"]*\)\".*/\2\1/p" "$1" >actual &&
+	test_cmp expect actual
+}
+
 test_lazy_prereq SYSTEMD_ANALYZE '
 	systemd-analyze verify /lib/systemd/system/basic.target
 '
@@ -180,8 +186,9 @@ test_expect_success 'maintenance.<task>.enabled' '
 	git config maintenance.gc.enabled false &&
 	git config maintenance.commit-graph.enabled true &&
 	GIT_TRACE2_EVENT="$(pwd)/run-config.txt" git maintenance run 2>err &&
-	test_subcommand ! git gc --quiet <run-config.txt &&
-	test_subcommand git commit-graph write --split --reachable --no-progress <run-config.txt
+	test_maintenance_tasks run-config.txt <<-\EOF
+	commit-graph
+	EOF
 '
 
 test_expect_success 'run --task=<task>' '
@@ -189,16 +196,20 @@ test_expect_success 'run --task=<task>' '
 		git maintenance run --task=commit-graph 2>/dev/null &&
 	GIT_TRACE2_EVENT="$(pwd)/run-gc.txt" \
 		git maintenance run --task=gc 2>/dev/null &&
-	GIT_TRACE2_EVENT="$(pwd)/run-commit-graph.txt" \
-		git maintenance run --task=commit-graph 2>/dev/null &&
 	GIT_TRACE2_EVENT="$(pwd)/run-both.txt" \
 		git maintenance run --task=commit-graph --task=gc 2>/dev/null &&
-	test_subcommand ! git gc --quiet --no-detach --skip-foreground-tasks <run-commit-graph.txt &&
-	test_subcommand git gc --quiet --no-detach --skip-foreground-tasks <run-gc.txt &&
-	test_subcommand git gc --quiet --no-detach --skip-foreground-tasks <run-both.txt &&
-	test_subcommand git commit-graph write --split --reachable --no-progress <run-commit-graph.txt &&
-	test_subcommand ! git commit-graph write --split --reachable --no-progress <run-gc.txt &&
-	test_subcommand git commit-graph write --split --reachable --no-progress <run-both.txt
+	test_maintenance_tasks run-commit-graph.txt <<-\EOF &&
+	commit-graph
+	EOF
+	test_maintenance_tasks run-gc.txt <<-\EOF &&
+	gc foreground
+	gc
+	EOF
+	test_maintenance_tasks run-both.txt <<-\EOF
+	gc foreground
+	commit-graph
+	gc
+	EOF
 '
 
 test_expect_success 'core.commitGraph=false prevents write process' '
@@ -235,12 +246,19 @@ test_expect_success 'commit-graph auto condition' '
 	GIT_TRACE2_EVENT="$(pwd)/cg-two-satisfied.txt" \
 		git -c maintenance.commit-graph.auto=2 $COMMAND &&
 
-	COMMIT_GRAPH_WRITE="git commit-graph write --split --reachable --no-progress" &&
-	test_subcommand ! $COMMIT_GRAPH_WRITE <cg-no.txt &&
-	test_subcommand $COMMIT_GRAPH_WRITE <cg-negative-means-yes.txt &&
-	test_subcommand ! $COMMIT_GRAPH_WRITE <cg-zero-means-no.txt &&
-	test_subcommand $COMMIT_GRAPH_WRITE <cg-one-satisfied.txt &&
-	test_subcommand $COMMIT_GRAPH_WRITE <cg-two-satisfied.txt
+	test_maintenance_tasks cg-no.txt <<-\EOF &&
+	EOF
+	test_maintenance_tasks cg-negative-means-yes.txt <<-\EOF &&
+	commit-graph
+	EOF
+	test_maintenance_tasks cg-zero-means-no.txt <<-\EOF &&
+	EOF
+	test_maintenance_tasks cg-one-satisfied.txt <<-\EOF &&
+	commit-graph
+	EOF
+	test_maintenance_tasks cg-two-satisfied.txt <<-\EOF
+	commit-graph
+	EOF
 '
 
 test_expect_success 'commit-graph auto condition with merges' '
@@ -910,24 +928,28 @@ test_expect_success '--schedule inheritance weekly -> daily -> hourly' '
 
 	GIT_TRACE2_EVENT="$(pwd)/hourly.txt" \
 		git maintenance run --schedule=hourly 2>/dev/null &&
-	test_subcommand git prune-packed --quiet <hourly.txt &&
-	test_subcommand ! git commit-graph write --split --reachable \
-		--no-progress <hourly.txt &&
-	test_subcommand ! git multi-pack-index write --no-progress <hourly.txt &&
+	test_maintenance_tasks hourly.txt <<-\EOF &&
+	prefetch
+	loose-objects
+	EOF
 
 	GIT_TRACE2_EVENT="$(pwd)/daily.txt" \
 		git maintenance run --schedule=daily 2>/dev/null &&
-	test_subcommand git prune-packed --quiet <daily.txt &&
-	test_subcommand git commit-graph write --split --reachable \
-		--no-progress <daily.txt &&
-	test_subcommand ! git multi-pack-index write --no-progress <daily.txt &&
+	test_maintenance_tasks daily.txt <<-\EOF &&
+	prefetch
+	loose-objects
+	commit-graph
+	EOF
 
 	GIT_TRACE2_EVENT="$(pwd)/weekly.txt" \
 		git maintenance run --schedule=weekly 2>/dev/null &&
-	test_subcommand git prune-packed --quiet <weekly.txt &&
-	test_subcommand git commit-graph write --split --reachable \
-		--no-progress <weekly.txt &&
-	test_subcommand git multi-pack-index write --no-progress <weekly.txt
+	test_maintenance_tasks weekly.txt <<-\EOF
+	pack-refs foreground
+	prefetch
+	loose-objects
+	incremental-repack
+	commit-graph
+	EOF
 '
 
 test_expect_success 'maintenance.strategy inheritance' '
@@ -946,29 +968,25 @@ test_expect_success 'maintenance.strategy inheritance' '
 	GIT_TRACE2_EVENT="$(pwd)/incremental-weekly.txt" \
 		git maintenance run --schedule=weekly --quiet &&
 
-	test_subcommand git commit-graph write --split --reachable \
-		--no-progress <incremental-hourly.txt &&
-	test_subcommand ! git prune-packed --quiet <incremental-hourly.txt &&
-	test_subcommand ! git multi-pack-index write --no-progress \
-		<incremental-hourly.txt &&
-	test_subcommand ! git pack-refs --all --prune \
-		<incremental-hourly.txt &&
-
-	test_subcommand git commit-graph write --split --reachable \
-		--no-progress <incremental-daily.txt &&
-	test_subcommand git prune-packed --quiet <incremental-daily.txt &&
-	test_subcommand git multi-pack-index write --no-progress \
-		<incremental-daily.txt &&
-	test_subcommand ! git pack-refs --all --prune \
-		<incremental-daily.txt &&
-
-	test_subcommand git commit-graph write --split --reachable \
-		--no-progress <incremental-weekly.txt &&
-	test_subcommand git prune-packed --quiet <incremental-weekly.txt &&
-	test_subcommand git multi-pack-index write --no-progress \
-		<incremental-weekly.txt &&
-	test_subcommand git pack-refs --all --prune \
-		<incremental-weekly.txt &&
+	test_maintenance_tasks incremental-hourly.txt <<-\EOF &&
+	prefetch
+	commit-graph
+	EOF
+
+	test_maintenance_tasks incremental-daily.txt <<-\EOF &&
+	prefetch
+	loose-objects
+	incremental-repack
+	commit-graph
+	EOF
+
+	test_maintenance_tasks incremental-weekly.txt <<-\EOF &&
+	pack-refs foreground
+	prefetch
+	loose-objects
+	incremental-repack
+	commit-graph
+	EOF
 
 	# Modify defaults
 	git config maintenance.commit-graph.schedule daily &&
@@ -980,30 +998,26 @@ test_expect_success 'maintenance.strategy inheritance' '
 	GIT_TRACE2_EVENT="$(pwd)/modified-daily.txt" \
 		git maintenance run --schedule=daily --quiet &&
 
-	test_subcommand ! git commit-graph write --split --reachable \
-		--no-progress <modified-hourly.txt &&
-	test_subcommand git prune-packed --quiet <modified-hourly.txt &&
-	test_subcommand ! git multi-pack-index write --no-progress \
-		<modified-hourly.txt &&
+	test_maintenance_tasks modified-hourly.txt <<-\EOF &&
+	prefetch
+	loose-objects
+	EOF
 
-	test_subcommand git commit-graph write --split --reachable \
-		--no-progress <modified-daily.txt &&
-	test_subcommand git prune-packed --quiet <modified-daily.txt &&
-	test_subcommand ! git multi-pack-index write --no-progress \
-		<modified-daily.txt
+	test_maintenance_tasks modified-daily.txt <<-\EOF
+	prefetch
+	loose-objects
+	commit-graph
+	EOF
 '
 
 test_strategy () {
 	STRATEGY="$1"
 	shift
 
-	cat >expect &&
 	rm -f trace2.txt &&
 	GIT_TRACE2_EVENT="$(pwd)/trace2.txt" \
 		git -c maintenance.strategy=$STRATEGY maintenance run --quiet "$@" &&
-	sed -n 's/{"event":"child_start","sid":"[^/"]*",.*,"argv":\["\(.*\)\"]}/\1/p' <trace2.txt |
-		sed 's/","/ /g'  >actual
-	test_cmp expect actual
+	test_maintenance_tasks trace2.txt
 }
 
 test_expect_success 'maintenance.strategy is respected' '
@@ -1017,48 +1031,44 @@ test_expect_success 'maintenance.strategy is respected' '
 		test_grep "unknown maintenance strategy: .unknown." err &&
 
 		test_strategy incremental <<-\EOF &&
-		git pack-refs --all --prune
-		git reflog expire --all
-		git gc --quiet --no-detach --skip-foreground-tasks
+		gc foreground
+		gc
 		EOF
 
 		test_strategy incremental --schedule=weekly <<-\EOF &&
-		git pack-refs --all --prune
-		git prune-packed --quiet
-		git multi-pack-index write --no-progress
-		git multi-pack-index expire --no-progress
-		git multi-pack-index repack --no-progress --batch-size=1
-		git commit-graph write --split --reachable --no-progress
+		pack-refs foreground
+		prefetch
+		loose-objects
+		incremental-repack
+		commit-graph
 		EOF
 
 		test_strategy gc <<-\EOF &&
-		git pack-refs --all --prune
-		git reflog expire --all
-		git gc --quiet --no-detach --skip-foreground-tasks
+		gc foreground
+		gc
 		EOF
 
 		test_strategy gc --schedule=weekly <<-\EOF &&
-		git pack-refs --all --prune
-		git reflog expire --all
-		git gc --quiet --no-detach --skip-foreground-tasks
+		gc foreground
+		gc
 		EOF
 
 		test_strategy geometric <<-\EOF &&
-		git pack-refs --all --prune
-		git reflog expire --all
-		git repack -d -l --geometric=2 --quiet --write-midx
-		git commit-graph write --split --reachable --no-progress
-		git worktree prune --expire 3.months.ago
-		git rerere gc
+		pack-refs foreground
+		reflog-expire foreground
+		geometric-repack
+		commit-graph
+		worktree-prune
+		rerere-gc
 		EOF
 
 		test_strategy geometric --schedule=weekly <<-\EOF
-		git pack-refs --all --prune
-		git reflog expire --all
-		git repack -d -l --geometric=2 --quiet --write-midx
-		git commit-graph write --split --reachable --no-progress
-		git worktree prune --expire 3.months.ago
-		git rerere gc
+		pack-refs foreground
+		reflog-expire foreground
+		geometric-repack
+		commit-graph
+		worktree-prune
+		rerere-gc
 		EOF
 	)
 '

-- 
2.55.0.313.g8d093f411d.dirty

[PATCH v2 02/12] odb: run "pre-auto-gc" hook for all maintenance tasks

[Patrick Steinhardt]

The "pre-auto-gc" hook is supposed to run before auto-maintenance
starts. The intent of this is to give users the ability to intercept
running maintenance in case there's for example an event that is not
supposed to run in parallel with repository maintenance.

This hook runs via `need_to_gc()`, which is invoked via two paths:

  - It is called directly by git-gc(1).

  - It is called indirectly by git-maintenance(1) via the "gc" task.

While the former makes sense, the latter is somewhat off. While the hook
is indeed strongly tied to gc'ing a repository, the original intent of
the hook is rather to inhibit any kind of automated garbage collection.
That noticeably also includes all the other maintenance tasks that our
new infrastructure may run, but those aren't getting intercepted at all.
The move towards our new maintenance strategy has thus somewhat neutered
the effectiveness of the hook.

Fix this issue by running the hook before the first auto-maintenance
task that would run as determined by the tasks's auto condition. Note
that this requires us to lift the call to `run_hooks()` out of
`needs_to_gc()`, as the hook would otherwise potentially run multiple
times.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/gc.c           |  35 ++++++++++----
 t/t7900-maintenance.sh | 126 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 152 insertions(+), 9 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index d32af422af..77d0a5c948 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -709,8 +709,6 @@ static int need_to_gc(struct gc_config *cfg, struct strvec *repack_args)
 	else
 		return 0;
 
-	if (run_hooks(the_repository, "pre-auto-gc"))
-		return 0;
 	return 1;
 }
 
@@ -933,7 +931,8 @@ int cmd_gc(int argc,
 		/*
 		 * Auto-gc should be least intrusive as possible.
 		 */
-		if (!need_to_gc(&cfg, &repack_args)) {
+		if (!need_to_gc(&cfg, &repack_args) ||
+		    run_hooks(the_repository, "pre-auto-gc")) {
 			ret = 0;
 			goto out;
 		}
@@ -1755,11 +1754,18 @@ enum task_phase {
 	TASK_PHASE_BACKGROUND,
 };
 
+enum auto_gc_hook_result {
+	AUTO_GC_HOOK_UNDECIDED = 0,
+	AUTO_GC_HOOK_RUN = 1,
+	AUTO_GC_HOOK_SKIP = 2,
+};
+
 static int maybe_run_task(const struct maintenance_task *task,
 			  struct repository *repo,
 			  struct maintenance_run_opts *opts,
 			  struct gc_config *cfg,
-			  enum task_phase phase)
+			  enum task_phase phase,
+			  enum auto_gc_hook_result *auto_gc_hook_result)
 {
 	int foreground = (phase == TASK_PHASE_FOREGROUND);
 	maintenance_task_fn fn = foreground ? task->foreground : task->background;
@@ -1768,9 +1774,19 @@ static int maybe_run_task(const struct maintenance_task *task,
 
 	if (!fn)
 		return 0;
-	if (opts->auto_flag &&
-	    (!task->auto_condition || !task->auto_condition(cfg)))
-		return 0;
+	if (opts->auto_flag) {
+		if (*auto_gc_hook_result == AUTO_GC_HOOK_SKIP)
+			return 0;
+
+		if (!task->auto_condition || !task->auto_condition(cfg))
+			return 0;
+
+		if (*auto_gc_hook_result == AUTO_GC_HOOK_UNDECIDED)
+			*auto_gc_hook_result = run_hooks(repo, "pre-auto-gc") ?
+				AUTO_GC_HOOK_SKIP : AUTO_GC_HOOK_RUN;
+		if (*auto_gc_hook_result == AUTO_GC_HOOK_SKIP)
+			return 0;
+	}
 
 	trace2_region_enter(region, task->name, repo);
 	if (fn(opts, cfg)) {
@@ -1789,6 +1805,7 @@ static int maintenance_run_tasks(struct maintenance_run_opts *opts,
 	struct lock_file lk;
 	struct repository *r = the_repository;
 	char *lock_path = xstrfmt("%s/maintenance", r->objects->sources->path);
+	enum auto_gc_hook_result auto_gc_hook_result = AUTO_GC_HOOK_UNDECIDED;
 
 	if (hold_lock_file_for_update(&lk, lock_path, LOCK_NO_DEREF) < 0) {
 		/*
@@ -1808,7 +1825,7 @@ static int maintenance_run_tasks(struct maintenance_run_opts *opts,
 
 	for (size_t i = 0; i < opts->tasks_nr; i++)
 		if (maybe_run_task(&tasks[opts->tasks[i]], r, opts, cfg,
-				   TASK_PHASE_FOREGROUND))
+				   TASK_PHASE_FOREGROUND, &auto_gc_hook_result))
 			result = 1;
 
 	/* Failure to daemonize is ok, we'll continue in foreground. */
@@ -1820,7 +1837,7 @@ static int maintenance_run_tasks(struct maintenance_run_opts *opts,
 
 	for (size_t i = 0; i < opts->tasks_nr; i++)
 		if (maybe_run_task(&tasks[opts->tasks[i]], r, opts, cfg,
-				   TASK_PHASE_BACKGROUND))
+				   TASK_PHASE_BACKGROUND, &auto_gc_hook_result))
 			result = 1;
 
 	rollback_lock_file(&lk);
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 129829f1f4..2d52e7918a 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -758,6 +758,132 @@ test_expect_success 'geometric repacking honors configured split factor' '
 	)
 '
 
+test_expect_success 'pre-auto-gc hook runs exactly once' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		write_script .git/hooks/pre-auto-gc <<-\EOF &&
+		echo hook >>hook.log
+		EOF
+
+		# Satisfy the auto condition for multiple tasks, both in the
+		# foreground and in the background phase.
+		git config set maintenance.reflog-expire.auto -1 &&
+		git config set maintenance.geometric-repack.auto -1 &&
+		git config set maintenance.rerere-gc.auto -1 &&
+
+		GIT_TRACE2_EVENT="$(pwd)/trace2.txt" \
+			git maintenance run --auto 2>/dev/null &&
+
+		# The successful hook does not inhibit any of the tasks...
+		test_maintenance_tasks trace2.txt <<-\EOF &&
+		reflog-expire foreground
+		geometric-repack
+		rerere-gc
+		EOF
+		# ... but it must only have been executed a single time.
+		test_line_count = 1 hook.log
+	)
+'
+
+test_expect_success 'pre-auto-gc hook can inhibit geometric strategy' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		write_script .git/hooks/pre-auto-gc <<-\EOF &&
+		echo hook >>hook.log
+		exit 1
+		EOF
+
+		git config set maintenance.reflog-expire.auto -1 &&
+		git config set maintenance.geometric-repack.auto -1 &&
+		git config set maintenance.rerere-gc.auto -1 &&
+
+		# Maintenance would be required...
+		git maintenance is-needed --auto &&
+
+		GIT_TRACE2_EVENT="$(pwd)/trace2.txt" \
+			git maintenance run --auto 2>/dev/null &&
+
+		# ... but the failing hook inhibits all tasks. The hook itself
+		# is expected to be the only child process being spawned, and
+		# it must only run a single time.
+		test_grep "child_start.*pre-auto-gc" trace2.txt &&
+		test_maintenance_tasks trace2.txt <<-\EOF &&
+		EOF
+		test_line_count = 1 hook.log
+	)
+'
+
+test_expect_success 'pre-auto-gc hook can inhibit gc strategy' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		write_script .git/hooks/pre-auto-gc <<-\EOF &&
+		echo hook >>hook.log
+		exit 1
+		EOF
+
+		git config set maintenance.strategy gc &&
+		git config set maintenance.auto false &&
+		git config set gc.auto 3 &&
+
+		test_oid_init &&
+
+		# We need to create two objects whose hashes start with 17
+		# since this is what the gc task counts.
+		test_commit "$(test_oid blob17_1)" &&
+		test_commit "$(test_oid blob17_2)" &&
+
+		# Maintenance would be required...
+		git maintenance is-needed --auto &&
+
+		GIT_TRACE2_EVENT="$(pwd)/trace2.txt" \
+			git maintenance run --auto 2>/dev/null &&
+
+		# ... but the failing hook inhibits all tasks. The hook itself
+		# is expected to be the only child process being spawned, and
+		# it must only run a single time.
+		test_grep "child_start.*pre-auto-gc" trace2.txt &&
+		test_maintenance_tasks trace2.txt <<-\EOF &&
+		EOF
+		test_subcommand_flex ! git trace2 &&
+		test_line_count = 1 hook.log
+	)
+'
+
+test_expect_success 'pre-auto-gc hook does not run when no maintenance is needed' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	(
+		cd repo &&
+		write_script .git/hooks/pre-auto-gc <<-\EOF &&
+		echo hook >>hook.log
+		EOF
+		test_must_fail git maintenance is-needed --auto &&
+		git maintenance run --auto 2>/dev/null &&
+		test_path_is_missing hook.log
+	)
+'
+
+test_expect_success 'pre-auto-gc hook does not run without --auto' '
+	test_when_finished "rm -rf repo" &&
+	git init repo &&
+	test_hook -C repo pre-auto-gc <<-\EOF &&
+	echo hook >>hook.log
+	EOF
+	(
+		cd repo &&
+		GIT_TRACE2_EVENT="$(pwd)/trace2.txt" \
+			git maintenance run 2>/dev/null &&
+		test_grep "\[\"git\",\"repack\"," trace2.txt &&
+		test_path_is_missing hook.log
+	)
+'
+
 test_expect_success 'pack-refs task' '
 	for n in $(test_seq 1 5)
 	do

-- 
2.55.0.313.g8d093f411d.dirty

[PATCH v2 03/12] builtin/gc: move worktree and rerere tasks before object optimizations

[Patrick Steinhardt]

In subsequent patches we'll consolidate all tasks that relate to
maintenance of the object database and move it into the "files" backend.
The relevant code is somewhat scattered though, as several other tasks
are interspersed between.

Refactor the code so that all object database optimizations are grouped
together, which requires us to move worktree pruning and rerere garbage
collection around. In theory, rearranging this code can have an effect
on the object database optimizations:

  - Rerere entries really shouldn't impact garbage collection at all, as
    these entries are not stored in the object database.

  - The index and HEAD reference of pruned worktrees may reference
    objects that become unreachable.

That being said, the impact should be overall rather negligible. If the
user was asking us to prune objects with immediate expiration time then
we might now prune objects that were previously still kept alive by the
worktree. But besides being a very specific edge case, it's arguably not
even the wrong thing to also prune any potentially-unreachable objects
immediately.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/gc.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index 77d0a5c948..8f568003ee 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1011,6 +1011,13 @@ int cmd_gc(int argc,
 	if (opts.detach <= 0 && !skip_foreground_tasks)
 		gc_foreground_tasks(&opts, &cfg);
 
+	if (cfg.prune_worktrees_expire &&
+	    maintenance_task_worktree_prune(&opts, &cfg))
+		die(FAILED_RUN, "worktree");
+
+	if (maintenance_task_rerere_gc(&opts, &cfg))
+		die(FAILED_RUN, "rerere");
+
 	if (!the_repository->repository_format_precious_objects) {
 		struct child_process repack_cmd = CHILD_PROCESS_INIT;
 
@@ -1038,13 +1045,6 @@ int cmd_gc(int argc,
 		}
 	}
 
-	if (cfg.prune_worktrees_expire &&
-	    maintenance_task_worktree_prune(&opts, &cfg))
-		die(FAILED_RUN, "worktree");
-
-	if (maintenance_task_rerere_gc(&opts, &cfg))
-		die(FAILED_RUN, "rerere");
-
 	report_garbage = report_pack_garbage;
 	odb_reprepare(the_repository->objects);
 	if (pack_garbage.nr > 0) {

-- 
2.55.0.313.g8d093f411d.dirty

[PATCH v2 04/12] builtin/gc: extract object database optimizations into separate function

[Patrick Steinhardt]

Extract the object database optimization logic from `cmd_gc()` into a
new `maintenance_task_odb()` helper function. This is a pure refactoring
with no intended functional change.

Note that the message that notifies the user about too many loose
objects is moved into the new function, as well. It is inherently an
implementation detail of how the "files" source works, and as a
consequence we'll move it around in a later commit, as well. This
reordering means that the warning may now be printed at a different
point in time, but it's not expected that this will have any practical
implications.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/gc.c | 79 +++++++++++++++++++++++++++++++++++++-----------------------
 1 file changed, 49 insertions(+), 30 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index 8f568003ee..2ff98fa727 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -839,6 +839,53 @@ static int gc_foreground_tasks(struct maintenance_run_opts *opts,
 	return 0;
 }
 
+static int maintenance_task_odb(struct maintenance_run_opts *opts,
+				struct gc_config *cfg,
+				struct strvec *repack_args)
+{
+	struct child_process repack_cmd = CHILD_PROCESS_INIT;
+	int ret;
+
+	if (the_repository->repository_format_precious_objects)
+		return 0;
+
+	repack_cmd.git_cmd = 1;
+	repack_cmd.odb_to_close = the_repository->objects;
+	strvec_pushv(&repack_cmd.args, repack_args->v);
+	if (run_command(&repack_cmd)) {
+		ret = error(FAILED_RUN, repack_args->v[0]);
+		goto out;
+	}
+
+	if (cfg->prune_expire) {
+		struct child_process prune_cmd = CHILD_PROCESS_INIT;
+
+		strvec_pushl(&prune_cmd.args, "prune", "--expire", NULL);
+		/* run `git prune` even if using cruft packs */
+		strvec_push(&prune_cmd.args, cfg->prune_expire);
+		if (opts->quiet)
+			strvec_push(&prune_cmd.args, "--no-progress");
+		if (repo_has_promisor_remote(the_repository))
+			strvec_push(&prune_cmd.args,
+				    "--exclude-promisor-objects");
+		prune_cmd.git_cmd = 1;
+
+		if (run_command(&prune_cmd)) {
+			ret = error(FAILED_RUN, prune_cmd.args.v[0]);
+			goto out;
+		}
+	}
+
+	if (opts->auto_flag && too_many_loose_objects(cfg->gc_auto_threshold))
+		warning(_("There are too many unreachable loose objects; "
+			"run 'git prune' to remove them."));
+
+	ret = 0;
+
+out:
+	return ret;
+}
+
 int cmd_gc(int argc,
 	   const char **argv,
 	   const char *prefix,
@@ -1018,32 +1065,8 @@ int cmd_gc(int argc,
 	if (maintenance_task_rerere_gc(&opts, &cfg))
 		die(FAILED_RUN, "rerere");
 
-	if (!the_repository->repository_format_precious_objects) {
-		struct child_process repack_cmd = CHILD_PROCESS_INIT;
-
-		repack_cmd.git_cmd = 1;
-		repack_cmd.odb_to_close = the_repository->objects;
-		strvec_pushv(&repack_cmd.args, repack_args.v);
-		if (run_command(&repack_cmd))
-			die(FAILED_RUN, repack_args.v[0]);
-
-		if (cfg.prune_expire) {
-			struct child_process prune_cmd = CHILD_PROCESS_INIT;
-
-			strvec_pushl(&prune_cmd.args, "prune", "--expire", NULL);
-			/* run `git prune` even if using cruft packs */
-			strvec_push(&prune_cmd.args, cfg.prune_expire);
-			if (opts.quiet)
-				strvec_push(&prune_cmd.args, "--no-progress");
-			if (repo_has_promisor_remote(the_repository))
-				strvec_push(&prune_cmd.args,
-					    "--exclude-promisor-objects");
-			prune_cmd.git_cmd = 1;
-
-			if (run_command(&prune_cmd))
-				die(FAILED_RUN, prune_cmd.args.v[0]);
-		}
-	}
+	if (maintenance_task_odb(&opts, &cfg, &repack_args))
+		die(NULL);
 
 	report_garbage = report_pack_garbage;
 	odb_reprepare(the_repository->objects);
@@ -1057,10 +1080,6 @@ int cmd_gc(int argc,
 					     !opts.quiet && !daemonized ? COMMIT_GRAPH_WRITE_PROGRESS : 0,
 					     NULL);
 
-	if (opts.auto_flag && too_many_loose_objects(cfg.gc_auto_threshold))
-		warning(_("There are too many unreachable loose objects; "
-			"run 'git prune' to remove them."));
-
 	if (!daemonized) {
 		char *path = repo_git_path(the_repository, "gc.log");
 		unlink(path);

-- 
2.55.0.313.g8d093f411d.dirty

[PATCH v2 05/12] builtin/gc: make repack arguments self-contained

[Patrick Steinhardt]

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 <ps@pks.im>
---
 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

[PATCH v2 06/12] builtin/gc: inline config values specific to the "files" backend

[Patrick Steinhardt]

The `struct gc_config` contains a set of values that we read via the Git
repository's configuration. Several of those values that are consumed by
the object database optimization logic are inherently specific to the
"files" config.

In a later commit we'll make the logic to optimize object databases
pluggable. So by carrying these "files"-backend specific values in the
generic config struct means that other backends would have to worry
about these values, too. This feels somewhat dirty, as implementation-
specific details should live with the backends themselves.

Inline these values directly at the call sites that need them instead.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/gc.c | 115 +++++++++++++++++++++++++++--------------------------------
 1 file changed, 53 insertions(+), 62 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index 25a59caea6..5d445edaa0 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -130,22 +130,11 @@ struct gc_config {
 	unsigned long max_cruft_size;
 	int aggressive_depth;
 	int aggressive_window;
-	int gc_auto_threshold;
-	int gc_auto_pack_limit;
 	int detach_auto;
 	char *gc_log_expire;
 	char *prune_expire;
 	char *prune_worktrees_expire;
-	char *repack_filter;
-	char *repack_filter_to;
 	char *repack_expire_to;
-	unsigned long big_pack_threshold;
-	unsigned long max_delta_cache_size;
-	/*
-	 * Remove this member from gc_config once repo_settings is passed
-	 * through the callchain.
-	 */
-	size_t delta_base_cache_limit;
 };
 
 #define GC_CONFIG_INIT { \
@@ -154,14 +143,10 @@ struct gc_config {
 	.cruft_packs = 1, \
 	.aggressive_depth = 50, \
 	.aggressive_window = 250, \
-	.gc_auto_threshold = 6700, \
-	.gc_auto_pack_limit = 50, \
 	.detach_auto = 1, \
 	.gc_log_expire = xstrdup("1.day.ago"), \
 	.prune_expire = xstrdup("2.weeks.ago"), \
 	.prune_worktrees_expire = xstrdup("3.months.ago"), \
-	.max_delta_cache_size = DEFAULT_DELTA_CACHE_SIZE, \
-	.delta_base_cache_limit = DEFAULT_DELTA_BASE_CACHE_LIMIT, \
 }
 
 static void gc_config_release(struct gc_config *cfg)
@@ -169,15 +154,12 @@ static void gc_config_release(struct gc_config *cfg)
 	free(cfg->gc_log_expire);
 	free(cfg->prune_expire);
 	free(cfg->prune_worktrees_expire);
-	free(cfg->repack_filter);
-	free(cfg->repack_filter_to);
 }
 
 static void gc_config(struct gc_config *cfg)
 {
 	const char *value;
 	char *owned = NULL;
-	unsigned long ulongval;
 
 	if (!repo_config_get_value(the_repository, "gc.packrefs", &value)) {
 		if (value && !strcmp(value, "notbare"))
@@ -192,8 +174,6 @@ static void gc_config(struct gc_config *cfg)
 
 	repo_config_get_int(the_repository, "gc.aggressivewindow", &cfg->aggressive_window);
 	repo_config_get_int(the_repository, "gc.aggressivedepth", &cfg->aggressive_depth);
-	repo_config_get_int(the_repository, "gc.auto", &cfg->gc_auto_threshold);
-	repo_config_get_int(the_repository, "gc.autopacklimit", &cfg->gc_auto_pack_limit);
 	repo_config_get_bool(the_repository, "gc.autodetach", &cfg->detach_auto);
 	repo_config_get_bool(the_repository, "gc.cruftpacks", &cfg->cruft_packs);
 	repo_config_get_ulong(the_repository, "gc.maxcruftsize", &cfg->max_cruft_size);
@@ -213,22 +193,6 @@ static void gc_config(struct gc_config *cfg)
 		cfg->gc_log_expire = owned;
 	}
 
-	repo_config_get_ulong(the_repository, "gc.bigpackthreshold", &cfg->big_pack_threshold);
-	repo_config_get_ulong(the_repository, "pack.deltacachesize", &cfg->max_delta_cache_size);
-
-	if (!repo_config_get_ulong(the_repository, "core.deltabasecachelimit", &ulongval))
-		cfg->delta_base_cache_limit = ulongval;
-
-	if (!repo_config_get_string(the_repository, "gc.repackfilter", &owned)) {
-		free(cfg->repack_filter);
-		cfg->repack_filter = owned;
-	}
-
-	if (!repo_config_get_string(the_repository, "gc.repackfilterto", &owned)) {
-		free(cfg->repack_filter_to);
-		cfg->repack_filter_to = owned;
-	}
-
 	repo_config(the_repository, git_default_config, NULL);
 }
 
@@ -504,12 +468,12 @@ static struct packed_git *find_base_packs(struct string_list *packs,
 	return base;
 }
 
-static int too_many_packs(struct gc_config *cfg)
+static int too_many_packs(int gc_auto_pack_limit)
 {
 	struct packed_git *p;
 	int cnt = 0;
 
-	if (cfg->gc_auto_pack_limit <= 0)
+	if (gc_auto_pack_limit <= 0)
 		return 0;
 
 	repo_for_each_pack(the_repository, p) {
@@ -523,7 +487,7 @@ static int too_many_packs(struct gc_config *cfg)
 		 */
 		cnt++;
 	}
-	return cfg->gc_auto_pack_limit < cnt;
+	return gc_auto_pack_limit < cnt;
 }
 
 static uint64_t total_ram(void)
@@ -571,9 +535,10 @@ static uint64_t total_ram(void)
 	return 0;
 }
 
-static uint64_t estimate_repack_memory(struct gc_config *cfg,
-				       struct packed_git *pack)
+static uint64_t estimate_repack_memory(struct packed_git *pack)
 {
+	unsigned long max_delta_cache_size = DEFAULT_DELTA_CACHE_SIZE;
+	unsigned long delta_base_cache_limit = DEFAULT_DELTA_BASE_CACHE_LIMIT;
 	unsigned long nr_objects;
 	size_t os_cache, heap;
 
@@ -584,6 +549,9 @@ static uint64_t estimate_repack_memory(struct gc_config *cfg,
 	if (!pack || !nr_objects)
 		return 0;
 
+	repo_config_get_ulong(the_repository, "pack.deltacachesize", &max_delta_cache_size);
+	repo_config_get_ulong(the_repository, "core.deltabasecachelimit", &delta_base_cache_limit);
+
 	/*
 	 * First we have to scan through at least one pack.
 	 * Assume enough room in OS file cache to keep the entire pack
@@ -611,9 +579,9 @@ static uint64_t estimate_repack_memory(struct gc_config *cfg,
 	 * read_sha1_file() (either at delta calculation phase, or
 	 * writing phase) also fills up the delta base cache
 	 */
-	heap += cfg->delta_base_cache_limit;
+	heap += delta_base_cache_limit;
 	/* and of course pack-objects has its own delta cache */
-	heap += cfg->max_delta_cache_size;
+	heap += max_delta_cache_size;
 
 	return os_cache + heap;
 }
@@ -629,6 +597,12 @@ static void add_repack_all_option(struct gc_config *cfg,
 				  struct string_list *keep_pack,
 				  struct strvec *args)
 {
+	char *repack_filter = NULL;
+	char *repack_filter_to = NULL;
+
+	repo_config_get_string(the_repository, "gc.repackfilter", &repack_filter);
+	repo_config_get_string(the_repository, "gc.repackfilterto", &repack_filter_to);
+
 	if (cfg->prune_expire && !strcmp(cfg->prune_expire, "now")
 		&& !(cfg->cruft_packs && cfg->repack_expire_to))
 		strvec_push(args, "-a");
@@ -650,10 +624,13 @@ static void add_repack_all_option(struct gc_config *cfg,
 	if (keep_pack)
 		for_each_string_list(keep_pack, keep_one_pack, args);
 
-	if (cfg->repack_filter && *cfg->repack_filter)
-		strvec_pushf(args, "--filter=%s", cfg->repack_filter);
-	if (cfg->repack_filter_to && *cfg->repack_filter_to)
-		strvec_pushf(args, "--filter-to=%s", cfg->repack_filter_to);
+	if (repack_filter && *repack_filter)
+		strvec_pushf(args, "--filter=%s", repack_filter);
+	if (repack_filter_to && *repack_filter_to)
+		strvec_pushf(args, "--filter-to=%s", repack_filter_to);
+
+	free(repack_filter);
+	free(repack_filter_to);
 }
 
 static void add_repack_incremental_option(struct strvec *args)
@@ -661,16 +638,24 @@ 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)
+static int need_to_gc(struct repository *repo)
 {
+	int gc_auto_threshold = 6700;
+	int gc_auto_pack_limit = 50;
+
+	repo_config_get_int(repo, "gc.auto", &gc_auto_threshold);
+	repo_config_get_int(repo, "gc.autopacklimit", &gc_auto_pack_limit);
+
 	/*
 	 * Setting gc.auto to 0 or negative can disable the
 	 * automatic gc.
 	 */
-	if (cfg->gc_auto_threshold <= 0)
+	if (gc_auto_threshold <= 0)
 		return 0;
-	if (!too_many_packs(cfg) && !too_many_loose_objects(cfg->gc_auto_threshold))
+	if (!too_many_packs(gc_auto_pack_limit) &&
+	    !too_many_loose_objects(gc_auto_threshold))
 		return 0;
+
 	return 1;
 }
 
@@ -807,8 +792,15 @@ static int maintenance_task_odb(struct maintenance_run_opts *opts,
 				int aggressive)
 {
 	struct child_process repack_cmd = CHILD_PROCESS_INIT;
+	unsigned long big_pack_threshold = 0;
+	int gc_auto_threshold = 6700;
+	int gc_auto_pack_limit = 50;
 	int ret;
 
+	repo_config_get_int(the_repository, "gc.auto", &gc_auto_threshold);
+	repo_config_get_int(the_repository, "gc.autopacklimit", &gc_auto_pack_limit);
+	repo_config_get_ulong(the_repository, "gc.bigpackthreshold", &big_pack_threshold);
+
 	if (the_repository->repository_format_precious_objects)
 		return 0;
 
@@ -843,19 +835,18 @@ static int maintenance_task_odb(struct maintenance_run_opts *opts,
 		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);
+		} else if (big_pack_threshold) {
+			find_base_packs(&keep_pack, 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)) {
+	} else if (too_many_packs(gc_auto_pack_limit)) {
 		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;
+		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);
 			}
@@ -864,7 +855,7 @@ static int maintenance_task_odb(struct maintenance_run_opts *opts,
 			uint64_t mem_have, mem_want;
 
 			mem_have = total_ram();
-			mem_want = estimate_repack_memory(cfg, p);
+			mem_want = estimate_repack_memory(p);
 
 			/*
 			 * Only allow 1/2 of memory for pack-objects, leave
@@ -905,7 +896,7 @@ static int maintenance_task_odb(struct maintenance_run_opts *opts,
 		}
 	}
 
-	if (opts->auto_flag && too_many_loose_objects(cfg->gc_auto_threshold))
+	if (opts->auto_flag && too_many_loose_objects(gc_auto_threshold))
 		warning(_("There are too many unreachable loose objects; "
 			"run 'git prune' to remove them."));
 
@@ -994,7 +985,7 @@ int cmd_gc(int argc,
 		/*
 		 * Auto-gc should be least intrusive as possible.
 		 */
-		if (!need_to_gc(&cfg) || run_hooks(the_repository, "pre-auto-gc")) {
+		if (!need_to_gc(the_repository) || run_hooks(the_repository, "pre-auto-gc")) {
 			ret = 0;
 			goto out;
 		}
@@ -1291,9 +1282,9 @@ static int maintenance_task_gc_background(struct maintenance_run_opts *opts,
 	return run_command(&child);
 }
 
-static int gc_condition(struct gc_config *cfg)
+static int gc_condition(struct gc_config *cfg UNUSED)
 {
-	return need_to_gc(cfg);
+	return need_to_gc(the_repository);
 }
 
 static int prune_packed(struct maintenance_run_opts *opts)

-- 
2.55.0.313.g8d093f411d.dirty

[PATCH v2 07/12] builtin/gc: introduce object database optimization options

[Patrick Steinhardt]

Introduce `struct odb_optimize_options` to decouple the options that are
specific to optimizing the object database from `struct gc_config`. This
structure will be moved into the object database layer in a subsequent
commit.

Note that there are a small set of backend-specific options in this
structure. In an ideal world those of course wouldn't exist, but as
we're introducing the object database abstractions retroactively we are
somewhat forced to keep them.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/gc.c | 181 +++++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 120 insertions(+), 61 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index 5d445edaa0..17490106fc 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -593,7 +593,39 @@ static int keep_one_pack(struct string_list_item *item, void *data)
 	return 0;
 }
 
-static void add_repack_all_option(struct gc_config *cfg,
+enum odb_optimize_flags {
+	/* Enable verbose logging and progress reporting. */
+	ODB_OPTIMIZE_VERBOSE = (1 << 0),
+
+	/* Perform auto-maintenance, only optimizing objects as required. */
+	ODB_OPTIMIZE_AUTO = (1 << 1),
+
+	/* Recompute existing deltas. */
+	ODB_OPTIMIZE_NO_REUSE_DELTAS = (1 << 2),
+};
+
+struct odb_optimize_options {
+	enum odb_optimize_flags flags;
+	const char *prune_expire;
+	const char *expire_to;
+	int depth;
+	int window;
+
+	/* Backend-specific options. */
+	int keep_largest_pack;
+	int cruft_packs;
+	unsigned long max_cruft_size;
+};
+
+#define OPTIMIZE_FIELDS_FROM_GC_CONFIG(cfg, aggressive) \
+	.prune_expire = (cfg)->prune_expire, \
+	.expire_to = (cfg)->repack_expire_to, \
+	.cruft_packs = (cfg)->cruft_packs, \
+	.max_cruft_size = (cfg)->max_cruft_size, \
+	.window = (aggressive) ? (cfg)->aggressive_window : 0, \
+	.depth = (aggressive) ? (cfg)->aggressive_depth : 0
+
+static void add_repack_all_option(const struct odb_optimize_options *opts,
 				  struct string_list *keep_pack,
 				  struct strvec *args)
 {
@@ -603,22 +635,22 @@ static void add_repack_all_option(struct gc_config *cfg,
 	repo_config_get_string(the_repository, "gc.repackfilter", &repack_filter);
 	repo_config_get_string(the_repository, "gc.repackfilterto", &repack_filter_to);
 
-	if (cfg->prune_expire && !strcmp(cfg->prune_expire, "now")
-		&& !(cfg->cruft_packs && cfg->repack_expire_to))
+	if (opts->prune_expire && !strcmp(opts->prune_expire, "now") &&
+	    !(opts->cruft_packs && opts->expire_to))
 		strvec_push(args, "-a");
-	else if (cfg->cruft_packs) {
+	else if (opts->cruft_packs) {
 		strvec_push(args, "--cruft");
-		if (cfg->prune_expire)
-			strvec_pushf(args, "--cruft-expiration=%s", cfg->prune_expire);
-		if (cfg->max_cruft_size)
+		if (opts->prune_expire)
+			strvec_pushf(args, "--cruft-expiration=%s", opts->prune_expire);
+		if (opts->max_cruft_size)
 			strvec_pushf(args, "--max-cruft-size=%lu",
-				     cfg->max_cruft_size);
-		if (cfg->repack_expire_to)
-			strvec_pushf(args, "--expire-to=%s", cfg->repack_expire_to);
+				     opts->max_cruft_size);
+		if (opts->expire_to)
+			strvec_pushf(args, "--expire-to=%s", opts->expire_to);
 	} else {
 		strvec_push(args, "-A");
-		if (cfg->prune_expire)
-			strvec_pushf(args, "--unpack-unreachable=%s", cfg->prune_expire);
+		if (opts->prune_expire)
+			strvec_pushf(args, "--unpack-unreachable=%s", opts->prune_expire);
 	}
 
 	if (keep_pack)
@@ -786,10 +818,8 @@ static int gc_foreground_tasks(struct maintenance_run_opts *opts,
 	return 0;
 }
 
-static int maintenance_task_odb(struct maintenance_run_opts *opts,
-				struct gc_config *cfg,
-				int keep_largest_pack,
-				int aggressive)
+static int odb_optimize(struct object_database *odb,
+			const struct odb_optimize_options *opts)
 {
 	struct child_process repack_cmd = CHILD_PROCESS_INIT;
 	unsigned long big_pack_threshold = 0;
@@ -801,21 +831,20 @@ static int maintenance_task_odb(struct maintenance_run_opts *opts,
 	repo_config_get_int(the_repository, "gc.autopacklimit", &gc_auto_pack_limit);
 	repo_config_get_ulong(the_repository, "gc.bigpackthreshold", &big_pack_threshold);
 
-	if (the_repository->repository_format_precious_objects)
+	if (odb->repo->repository_format_precious_objects)
 		return 0;
 
 	repack_cmd.git_cmd = 1;
 	repack_cmd.odb_to_close = the_repository->objects;
 
 	strvec_pushl(&repack_cmd.args, "repack", "-d", "-l", NULL);
-	if (aggressive) {
+	if (opts->flags & ODB_OPTIMIZE_NO_REUSE_DELTAS)
 		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)
+	if (opts->depth > 0)
+		strvec_pushf(&repack_cmd.args, "--depth=%d", opts->depth);
+	if (opts->window > 0)
+		strvec_pushf(&repack_cmd.args, "--window=%d", opts->window);
+	if (!(opts->flags & ODB_OPTIMIZE_VERBOSE))
 		strvec_push(&repack_cmd.args, "-q");
 
 	/*
@@ -829,47 +858,49 @@ static int maintenance_task_odb(struct maintenance_run_opts *opts,
 	 *
 	 *   - Otherwise we perform an incremental repack.
 	 */
-	if (!opts->auto_flag) {
+	if (!(opts->flags & ODB_OPTIMIZE_AUTO)) {
 		struct string_list keep_pack = STRING_LIST_INIT_NODUP;
 
-		if (keep_largest_pack != -1) {
-			if (keep_largest_pack)
+		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(cfg, &keep_pack, &repack_cmd.args);
+		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)) {
-		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 {
+		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);
 			}
-		} 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);
 		}
-
-		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)) {
@@ -877,13 +908,13 @@ static int maintenance_task_odb(struct maintenance_run_opts *opts,
 		goto out;
 	}
 
-	if (cfg->prune_expire) {
+	if (opts->prune_expire) {
 		struct child_process prune_cmd = CHILD_PROCESS_INIT;
 
 		strvec_pushl(&prune_cmd.args, "prune", "--expire", NULL);
 		/* run `git prune` even if using cruft packs */
-		strvec_push(&prune_cmd.args, cfg->prune_expire);
-		if (opts->quiet)
+		strvec_push(&prune_cmd.args, opts->prune_expire);
+		if (!(opts->flags & ODB_OPTIMIZE_VERBOSE))
 			strvec_push(&prune_cmd.args, "--no-progress");
 		if (repo_has_promisor_remote(the_repository))
 			strvec_push(&prune_cmd.args,
@@ -896,7 +927,7 @@ static int maintenance_task_odb(struct maintenance_run_opts *opts,
 		}
 	}
 
-	if (opts->auto_flag && too_many_loose_objects(gc_auto_threshold))
+	if (opts->flags & ODB_OPTIMIZE_AUTO && too_many_loose_objects(gc_auto_threshold))
 		warning(_("There are too many unreachable loose objects; "
 			"run 'git prune' to remove them."));
 
@@ -906,6 +937,26 @@ static int maintenance_task_odb(struct maintenance_run_opts *opts,
 	return ret;
 }
 
+static int maintenance_task_odb(struct maintenance_run_opts *opts,
+				struct gc_config *cfg,
+				int keep_largest_pack,
+				int aggressive)
+{
+	struct odb_optimize_options odb_opts = {
+		.keep_largest_pack = keep_largest_pack,
+		OPTIMIZE_FIELDS_FROM_GC_CONFIG(cfg, aggressive),
+	};
+
+	if (opts->auto_flag)
+		odb_opts.flags |= ODB_OPTIMIZE_AUTO;
+	if (!opts->quiet)
+		odb_opts.flags |= ODB_OPTIMIZE_VERBOSE;
+	if (aggressive)
+		odb_opts.flags |= ODB_OPTIMIZE_NO_REUSE_DELTAS;
+
+	return odb_optimize(the_repository->objects, &odb_opts);
+}
+
 int cmd_gc(int argc,
 	   const char **argv,
 	   const char *prefix,
@@ -1596,11 +1647,19 @@ static int maintenance_task_geometric_repack(struct maintenance_run_opts *opts,
 	child.odb_to_close = the_repository->objects;
 
 	strvec_pushl(&child.args, "repack", "-d", "-l", NULL);
-	if (geometry.split < geometry.pack_nr)
+	if (geometry.split < geometry.pack_nr) {
 		strvec_pushf(&child.args, "--geometric=%d",
 			     geometry.split_factor);
-	else
-		add_repack_all_option(cfg, NULL, &child.args);
+	} 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)

-- 
2.55.0.313.g8d093f411d.dirty

[PATCH v2 08/12] builtin/gc: move geometric repacking into `odb_optimize()`

[Patrick Steinhardt]

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>
---
 builtin/gc.c           | 171 +++++++++++++++++++++++++------------------------
 t/t7900-maintenance.sh |  18 +++---
 2 files changed, 96 insertions(+), 93 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
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)
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
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
 	)
 '
 

-- 
2.55.0.313.g8d093f411d.dirty

[PATCH v2 09/12] builtin/gc: introduce `odb_optimize_required()`

[Patrick Steinhardt]

When invoking either git-gc(1) or git-maintenance(1) with the "--auto"
flag then we only perform those maintenance tasks that are actually
required. This logic is inherently an implementation detail of the
object database backend that's in use. But the logic is scattered around
multiple different functions, which makes it hard to make the logic
pluggable.

Introduce a new `odb_optimize_required()` function that allows us to
check these conditions in a generic way.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/gc.c | 160 ++++++++++++++++++++++++++++++++++-------------------------
 1 file changed, 92 insertions(+), 68 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index c8504f4456..e119930adc 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -676,25 +676,84 @@ static void add_repack_incremental_option(struct strvec *args)
 	strvec_push(args, "--no-write-bitmap-index");
 }
 
-static int need_to_gc(struct repository *repo)
+static bool odb_optimize_required(struct object_database *odb,
+				  const struct odb_optimize_options *opts)
 {
-	int gc_auto_threshold = 6700;
-	int gc_auto_pack_limit = 50;
+	switch (opts->strategy) {
+	case ODB_OPTIMIZE_INCREMENTAL: {
+		int gc_auto_threshold = 6700;
+		int gc_auto_pack_limit = 50;
 
-	repo_config_get_int(repo, "gc.auto", &gc_auto_threshold);
-	repo_config_get_int(repo, "gc.autopacklimit", &gc_auto_pack_limit);
+		repo_config_get_int(odb->repo, "gc.auto", &gc_auto_threshold);
+		repo_config_get_int(odb->repo, "gc.autopacklimit", &gc_auto_pack_limit);
 
-	/*
-	 * Setting gc.auto to 0 or negative can disable the
-	 * automatic gc.
-	 */
-	if (gc_auto_threshold <= 0)
-		return 0;
-	if (!too_many_packs(gc_auto_pack_limit) &&
-	    !too_many_loose_objects(gc_auto_threshold))
-		return 0;
+		/*
+		 * Setting gc.auto to 0 or negative can disable the
+		 * automatic gc.
+		 */
+		if (gc_auto_threshold <= 0)
+			return false;
+		if (!too_many_packs(gc_auto_pack_limit) &&
+		    !too_many_loose_objects(gc_auto_threshold))
+			return false;
 
-	return 1;
+		return true;
+	}
+	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;
+		int auto_value = 100;
+		bool ret;
+
+		repo_config_get_int(odb->repo, "maintenance.geometric-repack.auto",
+				    &auto_value);
+		if (!auto_value)
+			return false;
+		if (auto_value < 0)
+			return true;
+
+		repo_config_get_int(odb->repo, "maintenance.geometric-repack.splitFactor",
+				    &geometry.split_factor);
+
+		existing_packs.repo = odb->repo;
+		existing_packs_collect(&existing_packs, &kept_packs);
+		pack_geometry_init(&geometry, &existing_packs, &po_args);
+		pack_geometry_split(&geometry);
+
+		/*
+		 * When we'd merge at least two packs with one another we always
+		 * perform the repack.
+		 */
+		if (geometry.split) {
+			ret = true;
+			goto out;
+		}
+
+		/*
+		 * Otherwise, we estimate the number of loose objects to determine
+		 * whether we want to create a new packfile or not.
+		 */
+		if (too_many_loose_objects(auto_value)) {
+			ret = true;
+			goto out;
+		}
+
+		ret = false;
+
+	out:
+		existing_packs_release(&existing_packs);
+		pack_geometry_release(&geometry);
+		return ret;
+	}
+	default:
+		BUG("unknown maintenance strategy '%d'", opts->strategy);
+	}
 }
 
 /* return NULL on success, else hostname running the gc */
@@ -1076,13 +1135,19 @@ int cmd_gc(int argc,
 		die(_("failed to parse prune expiry value %s"), cfg.prune_expire);
 
 	if (opts.auto_flag) {
+		struct odb_optimize_options optimize_opts = {
+			.strategy = ODB_OPTIMIZE_INCREMENTAL,
+			OPTIMIZE_FIELDS_FROM_GC_CONFIG(&cfg, 0),
+		};
+
 		if (cfg.detach_auto && opts.detach < 0)
 			opts.detach = 1;
 
 		/*
 		 * Auto-gc should be least intrusive as possible.
 		 */
-		if (!need_to_gc(the_repository) || run_hooks(the_repository, "pre-auto-gc")) {
+		if (!odb_optimize_required(the_repository->objects, &optimize_opts) ||
+		    run_hooks(the_repository, "pre-auto-gc")) {
 			ret = 0;
 			goto out;
 		}
@@ -1379,9 +1444,13 @@ static int maintenance_task_gc_background(struct maintenance_run_opts *opts,
 	return run_command(&child);
 }
 
-static int gc_condition(struct gc_config *cfg UNUSED)
+static int gc_condition(struct gc_config *cfg)
 {
-	return need_to_gc(the_repository);
+	struct odb_optimize_options opts = {
+		.strategy = ODB_OPTIMIZE_INCREMENTAL,
+		OPTIMIZE_FIELDS_FROM_GC_CONFIG(cfg, 0),
+	};
+	return odb_optimize_required(the_repository->objects, &opts);
 }
 
 static int prune_packed(struct maintenance_run_opts *opts)
@@ -1681,58 +1750,13 @@ static int maintenance_task_geometric_repack(struct maintenance_run_opts *opts,
 	return odb_optimize(the_repository->objects, &odb_opts);
 }
 
-static int geometric_repack_auto_condition(struct gc_config *cfg UNUSED)
+static int geometric_repack_auto_condition(struct gc_config *cfg)
 {
-	struct pack_geometry geometry = {
-		.split_factor = 2,
-	};
-	struct pack_objects_args po_args = {
-		.local = 1,
+	struct odb_optimize_options 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;
-	int auto_value = 100;
-	int ret;
-
-	repo_config_get_int(the_repository, "maintenance.geometric-repack.auto",
-			    &auto_value);
-	if (!auto_value)
-		return 0;
-	if (auto_value < 0)
-		return 1;
-
-	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);
-
-	/*
-	 * When we'd merge at least two packs with one another we always
-	 * perform the repack.
-	 */
-	if (geometry.split) {
-		ret = 1;
-		goto out;
-	}
-
-	/*
-	 * Otherwise, we estimate the number of loose objects to determine
-	 * whether we want to create a new packfile or not.
-	 */
-	if (too_many_loose_objects(auto_value)) {
-		ret = 1;
-		goto out;
-	}
-
-	ret = 0;
-
-out:
-	existing_packs_release(&existing_packs);
-	pack_geometry_release(&geometry);
-	return ret;
+	return odb_optimize_required(the_repository->objects, &opts);
 }
 
 typedef int (*maintenance_task_fn)(struct maintenance_run_opts *opts,

-- 
2.55.0.313.g8d093f411d.dirty

[PATCH v2 10/12] builtin/gc: refactor ODB optimizations to operate on "files" source

[Patrick Steinhardt]

We have a couple of functions that are implementation details of how the
"files" object database source performs optimizations. These functions
often use global state like `the_repository` and implicitly derive the
source they are supposed to optimize.

Refactor these interfaces to accept a "files" source directly. This will
make it easier to move around the whole logic into "odb/source-files.c"
in a subsequent step.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/gc.c | 79 +++++++++++++++++++++++++++++++-----------------------------
 1 file changed, 41 insertions(+), 38 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index e119930adc..3207182488 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -428,9 +428,8 @@ static int rerere_gc_condition(struct gc_config *cfg UNUSED)
 	return should_gc;
 }
 
-static int too_many_loose_objects(int limit)
+static int too_many_loose_objects(struct odb_source_files *files, int limit)
 {
-	struct odb_source_files *files = odb_source_files_downcast(the_repository->objects->sources);
 	/*
 	 * This is weird, but stems from legacy behaviour: the GC auto
 	 * threshold was always essentially interpreted as if it was rounded up
@@ -446,19 +445,21 @@ static int too_many_loose_objects(int limit)
 	return loose_count > auto_threshold;
 }
 
-static struct packed_git *find_base_packs(struct string_list *packs,
+static struct packed_git *find_base_packs(struct odb_source_files *files,
+					  struct string_list *packs,
 					  unsigned long limit)
 {
-	struct packed_git *p, *base = NULL;
+	struct packfile_list_entry *e;
+	struct packed_git *base = NULL;
 
-	repo_for_each_pack(the_repository, p) {
-		if (!p->pack_local || p->is_cruft)
+	for (e = packfile_store_get_packs(files->packed); e; e = e->next) {
+		if (e->pack->is_cruft)
 			continue;
 		if (limit) {
-			if (p->pack_size >= limit)
-				string_list_append(packs, p->pack_name);
-		} else if (!base || base->pack_size < p->pack_size) {
-			base = p;
+			if (e->pack->pack_size >= limit)
+				string_list_append(packs, e->pack->pack_name);
+		} else if (!base || base->pack_size < e->pack->pack_size) {
+			base = e->pack;
 		}
 	}
 
@@ -468,18 +469,16 @@ static struct packed_git *find_base_packs(struct string_list *packs,
 	return base;
 }
 
-static int too_many_packs(int gc_auto_pack_limit)
+static int too_many_packs(struct odb_source_files *files, int gc_auto_pack_limit)
 {
-	struct packed_git *p;
+	struct packfile_list_entry *e;
 	int cnt = 0;
 
 	if (gc_auto_pack_limit <= 0)
 		return 0;
 
-	repo_for_each_pack(the_repository, p) {
-		if (!p->pack_local)
-			continue;
-		if (p->pack_keep)
+	for (e = packfile_store_get_packs(files->packed); e; e = e->next) {
+		if (e->pack->pack_keep)
 			continue;
 		/*
 		 * Perhaps check the size of the pack and count only
@@ -535,15 +534,16 @@ static uint64_t total_ram(void)
 	return 0;
 }
 
-static uint64_t estimate_repack_memory(struct packed_git *pack)
+static uint64_t estimate_repack_memory(struct odb_source_files *files,
+				       struct packed_git *pack)
 {
 	unsigned long max_delta_cache_size = DEFAULT_DELTA_CACHE_SIZE;
 	unsigned long delta_base_cache_limit = DEFAULT_DELTA_BASE_CACHE_LIMIT;
 	unsigned long nr_objects;
 	size_t os_cache, heap;
 
-	if (odb_count_objects(the_repository->objects,
-			      ODB_COUNT_OBJECTS_APPROXIMATE, &nr_objects) < 0)
+	if (odb_source_count_objects(&files->base, ODB_COUNT_OBJECTS_APPROXIMATE,
+				     &nr_objects) < 0)
 		return 0;
 
 	if (!pack || !nr_objects)
@@ -679,6 +679,8 @@ static void add_repack_incremental_option(struct strvec *args)
 static bool odb_optimize_required(struct object_database *odb,
 				  const struct odb_optimize_options *opts)
 {
+	struct odb_source_files *files = odb_source_files_downcast(odb->sources);
+
 	switch (opts->strategy) {
 	case ODB_OPTIMIZE_INCREMENTAL: {
 		int gc_auto_threshold = 6700;
@@ -693,8 +695,8 @@ static bool odb_optimize_required(struct object_database *odb,
 		 */
 		if (gc_auto_threshold <= 0)
 			return false;
-		if (!too_many_packs(gc_auto_pack_limit) &&
-		    !too_many_loose_objects(gc_auto_threshold))
+		if (!too_many_packs(files, gc_auto_pack_limit) &&
+		    !too_many_loose_objects(files, gc_auto_threshold))
 			return false;
 
 		return true;
@@ -739,7 +741,7 @@ static bool odb_optimize_required(struct object_database *odb,
 		 * Otherwise, we estimate the number of loose objects to determine
 		 * whether we want to create a new packfile or not.
 		 */
-		if (too_many_loose_objects(auto_value)) {
+		if (too_many_loose_objects(files, auto_value)) {
 			ret = true;
 			goto out;
 		}
@@ -886,21 +888,22 @@ static int gc_foreground_tasks(struct maintenance_run_opts *opts,
 static int odb_optimize(struct object_database *odb,
 			const struct odb_optimize_options *opts)
 {
+	struct odb_source_files *files = odb_source_files_downcast(odb->sources);
 	struct child_process repack_cmd = CHILD_PROCESS_INIT;
 	unsigned long big_pack_threshold = 0;
 	int gc_auto_threshold = 6700;
 	int gc_auto_pack_limit = 50;
 	int ret;
 
-	repo_config_get_int(the_repository, "gc.auto", &gc_auto_threshold);
-	repo_config_get_int(the_repository, "gc.autopacklimit", &gc_auto_pack_limit);
-	repo_config_get_ulong(the_repository, "gc.bigpackthreshold", &big_pack_threshold);
+	repo_config_get_int(odb->repo, "gc.auto", &gc_auto_threshold);
+	repo_config_get_int(odb->repo, "gc.autopacklimit", &gc_auto_pack_limit);
+	repo_config_get_ulong(odb->repo, "gc.bigpackthreshold", &big_pack_threshold);
 
 	if (odb->repo->repository_format_precious_objects)
 		return 0;
 
 	repack_cmd.git_cmd = 1;
-	repack_cmd.odb_to_close = the_repository->objects;
+	repack_cmd.odb_to_close = odb->repo->objects;
 
 	strvec_pushl(&repack_cmd.args, "repack", "-d", "-l", NULL);
 	if (opts->flags & ODB_OPTIMIZE_NO_REUSE_DELTAS)
@@ -930,29 +933,29 @@ static int odb_optimize(struct object_database *odb,
 
 			if (opts->keep_largest_pack != -1) {
 				if (opts->keep_largest_pack)
-					find_base_packs(&keep_pack, 0);
+					find_base_packs(files, &keep_pack, 0);
 			} else if (big_pack_threshold) {
-				find_base_packs(&keep_pack, big_pack_threshold);
+				find_base_packs(files, &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)) {
+			if (too_many_packs(files, 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);
+					find_base_packs(files, &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);
+						find_base_packs(files, &keep_pack, 0);
 					}
 				} else {
-					struct packed_git *p = find_base_packs(&keep_pack, 0);
+					struct packed_git *p = find_base_packs(files, &keep_pack, 0);
 					uint64_t mem_have, mem_want;
 
 					mem_have = total_ram();
-					mem_want = estimate_repack_memory(p);
+					mem_want = estimate_repack_memory(files, p);
 
 					/*
 					 * Only allow 1/2 of memory for pack-objects, leave
@@ -981,10 +984,10 @@ static int odb_optimize(struct object_database *odb,
 		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",
+		repo_config_get_int(odb->repo, "maintenance.geometric-repack.splitFactor",
 				    &geometry.split_factor);
 
-		existing_packs.repo = the_repository;
+		existing_packs.repo = odb->repo;
 		existing_packs_collect(&existing_packs, &kept_packs);
 		pack_geometry_init(&geometry, &existing_packs, &po_args);
 		pack_geometry_split(&geometry);
@@ -995,7 +998,7 @@ static int odb_optimize(struct object_database *odb,
 		} else {
 			add_repack_all_option(opts, NULL, &repack_cmd.args);
 		}
-		if (the_repository->settings.core_multi_pack_index)
+		if (odb->repo->settings.core_multi_pack_index)
 			strvec_push(&repack_cmd.args, "--write-midx");
 
 		existing_packs_release(&existing_packs);
@@ -1020,7 +1023,7 @@ static int odb_optimize(struct object_database *odb,
 		strvec_push(&prune_cmd.args, opts->prune_expire);
 		if (!(opts->flags & ODB_OPTIMIZE_VERBOSE))
 			strvec_push(&prune_cmd.args, "--no-progress");
-		if (repo_has_promisor_remote(the_repository))
+		if (repo_has_promisor_remote(odb->repo))
 			strvec_push(&prune_cmd.args,
 				    "--exclude-promisor-objects");
 		prune_cmd.git_cmd = 1;
@@ -1031,7 +1034,7 @@ static int odb_optimize(struct object_database *odb,
 		}
 	}
 
-	if (opts->flags & ODB_OPTIMIZE_AUTO && too_many_loose_objects(gc_auto_threshold))
+	if (opts->flags & ODB_OPTIMIZE_AUTO && too_many_loose_objects(files, gc_auto_threshold))
 		warning(_("There are too many unreachable loose objects; "
 			"run 'git prune' to remove them."));
 

-- 
2.55.0.313.g8d093f411d.dirty

[PATCH v2 11/12] builtin/gc: fix signedness issues in ODB-related functionality

[Patrick Steinhardt]

There are a couple of signedness issues in ODB-related functionality.
These are not a problem because we disable -Wsign-compare in this file,
but once we move these functions into "odb/source-files.c" they will
result in warnings.

Fix those issues:

  - In `too_many_loose_objects()` we receive a signed limit, but compare
    it with the unsigned actual number of loose objects. This is fixed
    by bailing out immediately when the limit is smaller than or equal
    to zero, which we also do similarly in other places. The warning is
    then squelched via a cast.

  - In `find_base_packs()` we compare the signed size of the pack
    against the unsigned limit. As the pack size is always going to be a
    positive file size it's safe to cast it to an unsigned value.

  - In `odb_optimize()` we compare the unsigned `keep_pack.nr` value
    against the signed `gc_auto_pack_limit`. We only reach this code
    when `too_many_packs()` returns true-ish, and that can only happen
    when `gc_auto_pack_limit > 0`. Consequently, we can fix the warning
    by casting the limit to an unsigned value.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/gc.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index 3207182488..8cf3781313 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -430,19 +430,21 @@ static int rerere_gc_condition(struct gc_config *cfg UNUSED)
 
 static int too_many_loose_objects(struct odb_source_files *files, int limit)
 {
-	/*
-	 * This is weird, but stems from legacy behaviour: the GC auto
-	 * threshold was always essentially interpreted as if it was rounded up
-	 * to the next multiple 256 of, so we retain this behaviour for now.
-	 */
-	int auto_threshold = DIV_ROUND_UP(limit, 256) * 256;
 	unsigned long loose_count;
 
+	if (limit <= 0)
+		return 0;
+
 	if (odb_source_count_objects(&files->loose->base, ODB_COUNT_OBJECTS_APPROXIMATE,
 				     &loose_count) < 0)
 		return 0;
 
-	return loose_count > auto_threshold;
+	/*
+	 * This is weird, but stems from legacy behaviour: the GC auto
+	 * threshold was always essentially interpreted as if it was rounded up
+	 * to the next multiple 256 of, so we retain this behaviour for now.
+	 */
+	return loose_count > (DIV_ROUND_UP(((unsigned long) limit), 256) * 256);
 }
 
 static struct packed_git *find_base_packs(struct odb_source_files *files,
@@ -456,7 +458,7 @@ static struct packed_git *find_base_packs(struct odb_source_files *files,
 		if (e->pack->is_cruft)
 			continue;
 		if (limit) {
-			if (e->pack->pack_size >= limit)
+			if ((uintmax_t) e->pack->pack_size >= limit)
 				string_list_append(packs, e->pack->pack_name);
 		} else if (!base || base->pack_size < e->pack->pack_size) {
 			base = e->pack;
@@ -946,7 +948,7 @@ static int odb_optimize(struct object_database *odb,
 
 				if (big_pack_threshold) {
 					find_base_packs(files, &keep_pack, big_pack_threshold);
-					if (keep_pack.nr >= gc_auto_pack_limit) {
+					if (keep_pack.nr >= (unsigned long) gc_auto_pack_limit) {
 						string_list_clear(&keep_pack, 0);
 						find_base_packs(files, &keep_pack, 0);
 					}

-- 
2.55.0.313.g8d093f411d.dirty

Re: [PATCH v2 11/12] builtin/gc: fix signedness issues in ODB-related functionality

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> There are a couple of signedness issues in ODB-related functionality.
> These are not a problem because we disable -Wsign-compare in this file,
> but once we move these functions into "odb/source-files.c" they will
> result in warnings.
>
> Fix those issues:
>
>   - In `too_many_loose_objects()` we receive a signed limit, but compare
>     it with the unsigned actual number of loose objects. This is fixed
>     by bailing out immediately when the limit is smaller than or equal
>     to zero, which we also do similarly in other places. The warning is
>     then squelched via a cast.
>
>   - In `find_base_packs()` we compare the signed size of the pack
>     against the unsigned limit. As the pack size is always going to be a
>     positive file size it's safe to cast it to an unsigned value.
>
>   - In `odb_optimize()` we compare the unsigned `keep_pack.nr` value
>     against the signed `gc_auto_pack_limit`. We only reach this code
>     when `too_many_packs()` returns true-ish, and that can only happen
>     when `gc_auto_pack_limit > 0`. Consequently, we can fix the warning
>     by casting the limit to an unsigned value.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  builtin/gc.c | 20 +++++++++++---------
>  1 file changed, 11 insertions(+), 9 deletions(-)

Yuck.  The -Wsign-compare strikes again X-<.

> diff --git a/builtin/gc.c b/builtin/gc.c
> index 3207182488..8cf3781313 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -430,19 +430,21 @@ static int rerere_gc_condition(struct gc_config *cfg UNUSED)
>  
>  static int too_many_loose_objects(struct odb_source_files *files, int limit)
>  {
> -	/*
> -	 * This is weird, but stems from legacy behaviour: the GC auto
> -	 * threshold was always essentially interpreted as if it was rounded up
> -	 * to the next multiple 256 of, so we retain this behaviour for now.
> -	 */
> -	int auto_threshold = DIV_ROUND_UP(limit, 256) * 256;
>  	unsigned long loose_count;
>  
> +	if (limit <= 0)
> +		return 0;
> +
>  	if (odb_source_count_objects(&files->loose->base, ODB_COUNT_OBJECTS_APPROXIMATE,
>  				     &loose_count) < 0)
>  		return 0;
>  
> -	return loose_count > auto_threshold;
> +	/*
> +	 * This is weird, but stems from legacy behaviour: the GC auto
> +	 * threshold was always essentially interpreted as if it was rounded up
> +	 * to the next multiple 256 of, so we retain this behaviour for now.
> +	 */
> +	return loose_count > (DIV_ROUND_UP(((unsigned long) limit), 256) * 256);
>  }

OK.  It is trivially correct (if rounding up is correct, that is).

> @@ -456,7 +458,7 @@ static struct packed_git *find_base_packs(struct odb_source_files *files,
>  		if (e->pack->is_cruft)
>  			continue;
>  		if (limit) {
> -			if (e->pack->pack_size >= limit)
> +			if ((uintmax_t) e->pack->pack_size >= limit)

Here, just like in too_many_loose_objects(), 'limit' is of type
'unsigned long'.  While it makes sense to convert both sides of
the comparison to an unsigned type, casting only the left side
to a type that differs from the right side puzzles me.

Presumably, the other side is of type 'off_t', which is signed,
explaining the desire to cast it to an unsigned type.  But I am
not sure what happens if 'off_t' is wider than 'unsigned long'.

Otherwise looking good.

[PATCH v2 12/12] odb: make optimizations pluggable

[Patrick Steinhardt]

Move `odb_optimize()` and `odb_optimize_required()` from "builtin/gc.c"
into the "files" source and wire them up via newly introduced vtable
pointers for the object database sources. This makes the logic pluggable
and thus allows other backends to have their own, custom implementation.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/gc.c       | 490 +----------------------------------------------------
 odb.c              |  12 ++
 odb.h              |  45 +++++
 odb/source-files.c | 470 ++++++++++++++++++++++++++++++++++++++++++++++++++
 odb/source-files.h |  15 ++
 odb/source.h       |  36 ++++
 6 files changed, 579 insertions(+), 489 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index 8cf3781313..ac1a21e912 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -30,16 +30,11 @@
 #include "commit-graph.h"
 #include "packfile.h"
 #include "object-file.h"
-#include "pack.h"
-#include "pack-objects.h"
+#include "odb.h"
 #include "path.h"
 #include "reflog.h"
-#include "repack.h"
 #include "rerere.h"
 #include "revision.h"
-#include "blob.h"
-#include "tree.h"
-#include "promisor-remote.h"
 #include "refs.h"
 #include "remote.h"
 #include "exec-cmd.h"
@@ -428,203 +423,6 @@ static int rerere_gc_condition(struct gc_config *cfg UNUSED)
 	return should_gc;
 }
 
-static int too_many_loose_objects(struct odb_source_files *files, int limit)
-{
-	unsigned long loose_count;
-
-	if (limit <= 0)
-		return 0;
-
-	if (odb_source_count_objects(&files->loose->base, ODB_COUNT_OBJECTS_APPROXIMATE,
-				     &loose_count) < 0)
-		return 0;
-
-	/*
-	 * This is weird, but stems from legacy behaviour: the GC auto
-	 * threshold was always essentially interpreted as if it was rounded up
-	 * to the next multiple 256 of, so we retain this behaviour for now.
-	 */
-	return loose_count > (DIV_ROUND_UP(((unsigned long) limit), 256) * 256);
-}
-
-static struct packed_git *find_base_packs(struct odb_source_files *files,
-					  struct string_list *packs,
-					  unsigned long limit)
-{
-	struct packfile_list_entry *e;
-	struct packed_git *base = NULL;
-
-	for (e = packfile_store_get_packs(files->packed); e; e = e->next) {
-		if (e->pack->is_cruft)
-			continue;
-		if (limit) {
-			if ((uintmax_t) e->pack->pack_size >= limit)
-				string_list_append(packs, e->pack->pack_name);
-		} else if (!base || base->pack_size < e->pack->pack_size) {
-			base = e->pack;
-		}
-	}
-
-	if (base)
-		string_list_append(packs, base->pack_name);
-
-	return base;
-}
-
-static int too_many_packs(struct odb_source_files *files, int gc_auto_pack_limit)
-{
-	struct packfile_list_entry *e;
-	int cnt = 0;
-
-	if (gc_auto_pack_limit <= 0)
-		return 0;
-
-	for (e = packfile_store_get_packs(files->packed); e; e = e->next) {
-		if (e->pack->pack_keep)
-			continue;
-		/*
-		 * Perhaps check the size of the pack and count only
-		 * very small ones here?
-		 */
-		cnt++;
-	}
-	return gc_auto_pack_limit < cnt;
-}
-
-static uint64_t total_ram(void)
-{
-#if defined(HAVE_SYSINFO)
-	struct sysinfo si;
-
-	if (!sysinfo(&si)) {
-		uint64_t total = si.totalram;
-
-		if (si.mem_unit > 1)
-			total *= (uint64_t)si.mem_unit;
-		return total;
-	}
-#elif defined(HAVE_BSD_SYSCTL) && (defined(HW_MEMSIZE) || defined(HW_PHYSMEM) || defined(HW_PHYSMEM64))
-	uint64_t physical_memory;
-	int mib[2];
-	size_t length;
-
-	mib[0] = CTL_HW;
-# if defined(HW_MEMSIZE)
-	mib[1] = HW_MEMSIZE;
-# elif defined(HW_PHYSMEM64)
-	mib[1] = HW_PHYSMEM64;
-# else
-	mib[1] = HW_PHYSMEM;
-# endif
-	length = sizeof(physical_memory);
-	if (!sysctl(mib, 2, &physical_memory, &length, NULL, 0)) {
-		if (length == 4) {
-			uint32_t mem;
-
-			if (!sysctl(mib, 2, &mem, &length, NULL, 0))
-				physical_memory = mem;
-		}
-		return physical_memory;
-	}
-#elif defined(GIT_WINDOWS_NATIVE)
-	MEMORYSTATUSEX memInfo;
-
-	memInfo.dwLength = sizeof(MEMORYSTATUSEX);
-	if (GlobalMemoryStatusEx(&memInfo))
-		return memInfo.ullTotalPhys;
-#endif
-	return 0;
-}
-
-static uint64_t estimate_repack_memory(struct odb_source_files *files,
-				       struct packed_git *pack)
-{
-	unsigned long max_delta_cache_size = DEFAULT_DELTA_CACHE_SIZE;
-	unsigned long delta_base_cache_limit = DEFAULT_DELTA_BASE_CACHE_LIMIT;
-	unsigned long nr_objects;
-	size_t os_cache, heap;
-
-	if (odb_source_count_objects(&files->base, ODB_COUNT_OBJECTS_APPROXIMATE,
-				     &nr_objects) < 0)
-		return 0;
-
-	if (!pack || !nr_objects)
-		return 0;
-
-	repo_config_get_ulong(the_repository, "pack.deltacachesize", &max_delta_cache_size);
-	repo_config_get_ulong(the_repository, "core.deltabasecachelimit", &delta_base_cache_limit);
-
-	/*
-	 * First we have to scan through at least one pack.
-	 * Assume enough room in OS file cache to keep the entire pack
-	 * or we may accidentally evict data of other processes from
-	 * the cache.
-	 */
-	os_cache = pack->pack_size + pack->index_size;
-	/* then pack-objects needs lots more for book keeping */
-	heap = sizeof(struct object_entry) * nr_objects;
-	/*
-	 * internal rev-list --all --objects takes up some memory too,
-	 * let's say half of it is for blobs
-	 */
-	heap += sizeof(struct blob) * nr_objects / 2;
-	/*
-	 * and the other half is for trees (commits and tags are
-	 * usually insignificant)
-	 */
-	heap += sizeof(struct tree) * nr_objects / 2;
-	/* and then obj_hash[], underestimated in fact */
-	heap += sizeof(struct object *) * nr_objects;
-	/* revindex is used also */
-	heap += (sizeof(off_t) + sizeof(uint32_t)) * nr_objects;
-	/*
-	 * read_sha1_file() (either at delta calculation phase, or
-	 * writing phase) also fills up the delta base cache
-	 */
-	heap += delta_base_cache_limit;
-	/* and of course pack-objects has its own delta cache */
-	heap += max_delta_cache_size;
-
-	return os_cache + heap;
-}
-
-static int keep_one_pack(struct string_list_item *item, void *data)
-{
-	struct strvec *args = data;
-	strvec_pushf(args, "--keep-pack=%s", basename(item->string));
-	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),
-
-	/* Perform auto-maintenance, only optimizing objects as required. */
-	ODB_OPTIMIZE_AUTO = (1 << 1),
-
-	/* Recompute existing deltas. */
-	ODB_OPTIMIZE_NO_REUSE_DELTAS = (1 << 2),
-};
-
-struct odb_optimize_options {
-	enum odb_optimize_strategy strategy;
-	enum odb_optimize_flags flags;
-	const char *prune_expire;
-	const char *expire_to;
-	int depth;
-	int window;
-
-	/* Backend-specific options. */
-	int keep_largest_pack;
-	int cruft_packs;
-	unsigned long max_cruft_size;
-};
-
 #define OPTIMIZE_FIELDS_FROM_GC_CONFIG(cfg, aggressive) \
 	.prune_expire = (cfg)->prune_expire, \
 	.expire_to = (cfg)->repack_expire_to, \
@@ -633,133 +431,6 @@ struct odb_optimize_options {
 	.window = (aggressive) ? (cfg)->aggressive_window : 0, \
 	.depth = (aggressive) ? (cfg)->aggressive_depth : 0
 
-static void add_repack_all_option(const struct odb_optimize_options *opts,
-				  struct string_list *keep_pack,
-				  struct strvec *args)
-{
-	char *repack_filter = NULL;
-	char *repack_filter_to = NULL;
-
-	repo_config_get_string(the_repository, "gc.repackfilter", &repack_filter);
-	repo_config_get_string(the_repository, "gc.repackfilterto", &repack_filter_to);
-
-	if (opts->prune_expire && !strcmp(opts->prune_expire, "now") &&
-	    !(opts->cruft_packs && opts->expire_to))
-		strvec_push(args, "-a");
-	else if (opts->cruft_packs) {
-		strvec_push(args, "--cruft");
-		if (opts->prune_expire)
-			strvec_pushf(args, "--cruft-expiration=%s", opts->prune_expire);
-		if (opts->max_cruft_size)
-			strvec_pushf(args, "--max-cruft-size=%lu",
-				     opts->max_cruft_size);
-		if (opts->expire_to)
-			strvec_pushf(args, "--expire-to=%s", opts->expire_to);
-	} else {
-		strvec_push(args, "-A");
-		if (opts->prune_expire)
-			strvec_pushf(args, "--unpack-unreachable=%s", opts->prune_expire);
-	}
-
-	if (keep_pack)
-		for_each_string_list(keep_pack, keep_one_pack, args);
-
-	if (repack_filter && *repack_filter)
-		strvec_pushf(args, "--filter=%s", repack_filter);
-	if (repack_filter_to && *repack_filter_to)
-		strvec_pushf(args, "--filter-to=%s", repack_filter_to);
-
-	free(repack_filter);
-	free(repack_filter_to);
-}
-
-static void add_repack_incremental_option(struct strvec *args)
-{
-	strvec_push(args, "--no-write-bitmap-index");
-}
-
-static bool odb_optimize_required(struct object_database *odb,
-				  const struct odb_optimize_options *opts)
-{
-	struct odb_source_files *files = odb_source_files_downcast(odb->sources);
-
-	switch (opts->strategy) {
-	case ODB_OPTIMIZE_INCREMENTAL: {
-		int gc_auto_threshold = 6700;
-		int gc_auto_pack_limit = 50;
-
-		repo_config_get_int(odb->repo, "gc.auto", &gc_auto_threshold);
-		repo_config_get_int(odb->repo, "gc.autopacklimit", &gc_auto_pack_limit);
-
-		/*
-		 * Setting gc.auto to 0 or negative can disable the
-		 * automatic gc.
-		 */
-		if (gc_auto_threshold <= 0)
-			return false;
-		if (!too_many_packs(files, gc_auto_pack_limit) &&
-		    !too_many_loose_objects(files, gc_auto_threshold))
-			return false;
-
-		return true;
-	}
-	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;
-		int auto_value = 100;
-		bool ret;
-
-		repo_config_get_int(odb->repo, "maintenance.geometric-repack.auto",
-				    &auto_value);
-		if (!auto_value)
-			return false;
-		if (auto_value < 0)
-			return true;
-
-		repo_config_get_int(odb->repo, "maintenance.geometric-repack.splitFactor",
-				    &geometry.split_factor);
-
-		existing_packs.repo = odb->repo;
-		existing_packs_collect(&existing_packs, &kept_packs);
-		pack_geometry_init(&geometry, &existing_packs, &po_args);
-		pack_geometry_split(&geometry);
-
-		/*
-		 * When we'd merge at least two packs with one another we always
-		 * perform the repack.
-		 */
-		if (geometry.split) {
-			ret = true;
-			goto out;
-		}
-
-		/*
-		 * Otherwise, we estimate the number of loose objects to determine
-		 * whether we want to create a new packfile or not.
-		 */
-		if (too_many_loose_objects(files, auto_value)) {
-			ret = true;
-			goto out;
-		}
-
-		ret = false;
-
-	out:
-		existing_packs_release(&existing_packs);
-		pack_geometry_release(&geometry);
-		return ret;
-	}
-	default:
-		BUG("unknown maintenance strategy '%d'", opts->strategy);
-	}
-}
-
 /* return NULL on success, else hostname running the gc */
 static const char *lock_repo_for_gc(int force, pid_t* ret_pid)
 {
@@ -887,165 +558,6 @@ static int gc_foreground_tasks(struct maintenance_run_opts *opts,
 	return 0;
 }
 
-static int odb_optimize(struct object_database *odb,
-			const struct odb_optimize_options *opts)
-{
-	struct odb_source_files *files = odb_source_files_downcast(odb->sources);
-	struct child_process repack_cmd = CHILD_PROCESS_INIT;
-	unsigned long big_pack_threshold = 0;
-	int gc_auto_threshold = 6700;
-	int gc_auto_pack_limit = 50;
-	int ret;
-
-	repo_config_get_int(odb->repo, "gc.auto", &gc_auto_threshold);
-	repo_config_get_int(odb->repo, "gc.autopacklimit", &gc_auto_pack_limit);
-	repo_config_get_ulong(odb->repo, "gc.bigpackthreshold", &big_pack_threshold);
-
-	if (odb->repo->repository_format_precious_objects)
-		return 0;
-
-	repack_cmd.git_cmd = 1;
-	repack_cmd.odb_to_close = odb->repo->objects;
-
-	strvec_pushl(&repack_cmd.args, "repack", "-d", "-l", NULL);
-	if (opts->flags & ODB_OPTIMIZE_NO_REUSE_DELTAS)
-		strvec_push(&repack_cmd.args, "-f");
-	if (opts->depth > 0)
-		strvec_pushf(&repack_cmd.args, "--depth=%d", opts->depth);
-	if (opts->window > 0)
-		strvec_pushf(&repack_cmd.args, "--window=%d", opts->window);
-	if (!(opts->flags & ODB_OPTIMIZE_VERBOSE))
-		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.
-	 */
-	switch (opts->strategy) {
-	case ODB_OPTIMIZE_INCREMENTAL:
-		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(files, &keep_pack, 0);
-			} else if (big_pack_threshold) {
-				find_base_packs(files, &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(files, gc_auto_pack_limit)) {
-				struct string_list keep_pack = STRING_LIST_INIT_NODUP;
-
-				if (big_pack_threshold) {
-					find_base_packs(files, &keep_pack, big_pack_threshold);
-					if (keep_pack.nr >= (unsigned long) gc_auto_pack_limit) {
-						string_list_clear(&keep_pack, 0);
-						find_base_packs(files, &keep_pack, 0);
-					}
-				} else {
-					struct packed_git *p = find_base_packs(files, &keep_pack, 0);
-					uint64_t mem_have, mem_want;
-
-					mem_have = total_ram();
-					mem_want = estimate_repack_memory(files, 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(odb->repo, "maintenance.geometric-repack.splitFactor",
-				    &geometry.split_factor);
-
-		existing_packs.repo = odb->repo;
-		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 (odb->repo->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)) {
-		ret = error(FAILED_RUN, repack_cmd.args.v[0]);
-		goto out;
-	}
-
-	/* 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);
-		/* run `git prune` even if using cruft packs */
-		strvec_push(&prune_cmd.args, opts->prune_expire);
-		if (!(opts->flags & ODB_OPTIMIZE_VERBOSE))
-			strvec_push(&prune_cmd.args, "--no-progress");
-		if (repo_has_promisor_remote(odb->repo))
-			strvec_push(&prune_cmd.args,
-				    "--exclude-promisor-objects");
-		prune_cmd.git_cmd = 1;
-
-		if (run_command(&prune_cmd)) {
-			ret = error(FAILED_RUN, prune_cmd.args.v[0]);
-			goto out;
-		}
-	}
-
-	if (opts->flags & ODB_OPTIMIZE_AUTO && too_many_loose_objects(files, gc_auto_threshold))
-		warning(_("There are too many unreachable loose objects; "
-			"run 'git prune' to remove them."));
-
-	ret = 0;
-
-out:
-	return ret;
-}
-
 static int maintenance_task_odb(struct maintenance_run_opts *opts,
 				struct gc_config *cfg,
 				int keep_largest_pack,
diff --git a/odb.c b/odb.c
index 7d555be09f..89660981fe 100644
--- a/odb.c
+++ b/odb.c
@@ -1003,6 +1003,18 @@ int odb_write_object_stream(struct object_database *odb,
 	return odb_source_write_object_stream(odb->sources, stream, len, oid);
 }
 
+int odb_optimize(struct object_database *odb,
+		 const struct odb_optimize_options *opts)
+{
+	return odb_source_optimize(odb->sources, opts);
+}
+
+bool odb_optimize_required(struct object_database *odb,
+			   const struct odb_optimize_options *opts)
+{
+	return odb_source_optimize_required(odb->sources, opts);
+}
+
 struct object_database *odb_new(struct repository *repo,
 				const char *primary_source,
 				const char *secondary_sources)
diff --git a/odb.h b/odb.h
index 3834a0dcbf..7e1c85c22e 100644
--- a/odb.h
+++ b/odb.h
@@ -117,6 +117,51 @@ struct object_database *odb_new(struct repository *repo,
 /* Free the object database and release all resources. */
 void odb_free(struct object_database *o);
 
+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),
+
+	/* Perform auto-maintenance, only optimizing objects as required. */
+	ODB_OPTIMIZE_AUTO = (1 << 1),
+
+	/* Recompute existing deltas. */
+	ODB_OPTIMIZE_NO_REUSE_DELTAS = (1 << 2),
+};
+
+struct odb_optimize_options {
+	enum odb_optimize_strategy strategy;
+	enum odb_optimize_flags flags;
+	const char *prune_expire;
+	const char *expire_to;
+	int depth;
+	int window;
+
+	/* Backend-specific options. */
+	int keep_largest_pack;
+	int cruft_packs;
+	unsigned long max_cruft_size;
+};
+
+/*
+ * Optimize the object database. Returns 0 on success, a negative error code
+ * otherwise.
+ */
+int odb_optimize(struct object_database *odb,
+		 const struct odb_optimize_options *opts);
+
+/*
+ * Check whether optimization of the object database is required given the
+ * provided options. Returns true if optimization should be performed, false
+ * otherwise.
+ */
+bool odb_optimize_required(struct object_database *odb,
+			   const struct odb_optimize_options *opts);
+
 /*
  * Close the object database and all of its sources so that any held resources
  * will be released. The database can still be used after closing it, in which
diff --git a/odb/source-files.c b/odb/source-files.c
index bbd1784b33..82cf61da4a 100644
--- a/odb/source-files.c
+++ b/odb/source-files.c
@@ -1,6 +1,8 @@
 #include "git-compat-util.h"
 #include "abspath.h"
+#include "blob.h"
 #include "chdir-notify.h"
+#include "config.h"
 #include "gettext.h"
 #include "lockfile.h"
 #include "object-file.h"
@@ -8,8 +10,16 @@
 #include "odb/source.h"
 #include "odb/source-files.h"
 #include "odb/source-loose.h"
+#include "pack-objects.h"
 #include "packfile.h"
+#include "path.h"
+#include "promisor-remote.h"
+#include "repack.h"
+#include "run-command.h"
 #include "strbuf.h"
+#include "string-list.h"
+#include "strvec.h"
+#include "tree.h"
 #include "write-or-die.h"
 
 static void odb_source_files_reparent(const char *name UNUSED,
@@ -260,6 +270,464 @@ static int odb_source_files_write_alternate(struct odb_source *source,
 	return ret;
 }
 
+static int too_many_loose_objects(struct odb_source_files *files, int limit)
+{
+	unsigned long loose_count;
+
+	if (limit <= 0)
+		return 0;
+
+	if (odb_source_count_objects(&files->loose->base, ODB_COUNT_OBJECTS_APPROXIMATE,
+				     &loose_count) < 0)
+		return 0;
+
+	/*
+	 * This is weird, but stems from legacy behaviour: the GC auto
+	 * threshold was always essentially interpreted as if it was rounded up
+	 * to the next multiple 256 of, so we retain this behaviour for now.
+	 */
+	return loose_count > (DIV_ROUND_UP(((unsigned long) limit), 256) * 256);
+}
+
+static struct packed_git *find_base_packs(struct odb_source_files *files,
+					  struct string_list *packs,
+					  unsigned long limit)
+{
+	struct packfile_list_entry *e;
+	struct packed_git *base = NULL;
+
+	for (e = packfile_store_get_packs(files->packed); e; e = e->next) {
+		if (e->pack->is_cruft)
+			continue;
+		if (limit) {
+			if ((uintmax_t) e->pack->pack_size >= limit)
+				string_list_append(packs, e->pack->pack_name);
+		} else if (!base || base->pack_size < e->pack->pack_size) {
+			base = e->pack;
+		}
+	}
+
+	if (base)
+		string_list_append(packs, base->pack_name);
+
+	return base;
+}
+
+static int too_many_packs(struct odb_source_files *files, int gc_auto_pack_limit)
+{
+	struct packfile_list_entry *e;
+	int cnt = 0;
+
+	if (gc_auto_pack_limit <= 0)
+		return 0;
+
+	for (e = packfile_store_get_packs(files->packed); e; e = e->next) {
+		if (e->pack->pack_keep)
+			continue;
+		/*
+		 * Perhaps check the size of the pack and count only
+		 * very small ones here?
+		 */
+		cnt++;
+	}
+	return gc_auto_pack_limit < cnt;
+}
+
+static uint64_t total_ram(void)
+{
+#if defined(HAVE_SYSINFO)
+	struct sysinfo si;
+
+	if (!sysinfo(&si)) {
+		uint64_t total = si.totalram;
+
+		if (si.mem_unit > 1)
+			total *= (uint64_t)si.mem_unit;
+		return total;
+	}
+#elif defined(HAVE_BSD_SYSCTL) && (defined(HW_MEMSIZE) || defined(HW_PHYSMEM) || defined(HW_PHYSMEM64))
+	uint64_t physical_memory;
+	int mib[2];
+	size_t length;
+
+	mib[0] = CTL_HW;
+# if defined(HW_MEMSIZE)
+	mib[1] = HW_MEMSIZE;
+# elif defined(HW_PHYSMEM64)
+	mib[1] = HW_PHYSMEM64;
+# else
+	mib[1] = HW_PHYSMEM;
+# endif
+	length = sizeof(physical_memory);
+	if (!sysctl(mib, 2, &physical_memory, &length, NULL, 0)) {
+		if (length == 4) {
+			uint32_t mem;
+
+			if (!sysctl(mib, 2, &mem, &length, NULL, 0))
+				physical_memory = mem;
+		}
+		return physical_memory;
+	}
+#elif defined(GIT_WINDOWS_NATIVE)
+	MEMORYSTATUSEX memInfo;
+
+	memInfo.dwLength = sizeof(MEMORYSTATUSEX);
+	if (GlobalMemoryStatusEx(&memInfo))
+		return memInfo.ullTotalPhys;
+#endif
+	return 0;
+}
+
+static uint64_t estimate_repack_memory(struct odb_source_files *files,
+				       struct packed_git *pack)
+{
+	unsigned long max_delta_cache_size = DEFAULT_DELTA_CACHE_SIZE;
+	unsigned long delta_base_cache_limit = DEFAULT_DELTA_BASE_CACHE_LIMIT;
+	unsigned long nr_objects;
+	size_t os_cache, heap;
+
+	if (odb_source_count_objects(&files->base, ODB_COUNT_OBJECTS_APPROXIMATE,
+				     &nr_objects) < 0)
+		return 0;
+
+	if (!pack || !nr_objects)
+		return 0;
+
+	repo_config_get_ulong(files->base.odb->repo, "pack.deltacachesize",
+			      &max_delta_cache_size);
+	repo_config_get_ulong(files->base.odb->repo, "core.deltabasecachelimit",
+			      &delta_base_cache_limit);
+
+	/*
+	 * First we have to scan through at least one pack.
+	 * Assume enough room in OS file cache to keep the entire pack
+	 * or we may accidentally evict data of other processes from
+	 * the cache.
+	 */
+	os_cache = pack->pack_size + pack->index_size;
+	/* then pack-objects needs lots more for book keeping */
+	heap = sizeof(struct object_entry) * nr_objects;
+	/*
+	 * internal rev-list --all --objects takes up some memory too,
+	 * let's say half of it is for blobs
+	 */
+	heap += sizeof(struct blob) * nr_objects / 2;
+	/*
+	 * and the other half is for trees (commits and tags are
+	 * usually insignificant)
+	 */
+	heap += sizeof(struct tree) * nr_objects / 2;
+	/* and then obj_hash[], underestimated in fact */
+	heap += sizeof(struct object *) * nr_objects;
+	/* revindex is used also */
+	heap += (sizeof(off_t) + sizeof(uint32_t)) * nr_objects;
+	/*
+	 * read_sha1_file() (either at delta calculation phase, or
+	 * writing phase) also fills up the delta base cache
+	 */
+	heap += delta_base_cache_limit;
+	/* and of course pack-objects has its own delta cache */
+	heap += max_delta_cache_size;
+
+	return os_cache + heap;
+}
+
+static int keep_one_pack(struct string_list_item *item, void *data)
+{
+	struct strvec *args = data;
+	strvec_pushf(args, "--keep-pack=%s", basename(item->string));
+	return 0;
+}
+
+static void add_repack_all_option(struct repository *repo,
+				  const struct odb_optimize_options *opts,
+				  struct string_list *keep_pack,
+				  struct strvec *args)
+{
+	char *repack_filter = NULL;
+	char *repack_filter_to = NULL;
+
+	repo_config_get_string(repo, "gc.repackfilter", &repack_filter);
+	repo_config_get_string(repo, "gc.repackfilterto", &repack_filter_to);
+
+	if (opts->prune_expire && !strcmp(opts->prune_expire, "now") &&
+	    !(opts->cruft_packs && opts->expire_to))
+		strvec_push(args, "-a");
+	else if (opts->cruft_packs) {
+		strvec_push(args, "--cruft");
+		if (opts->prune_expire)
+			strvec_pushf(args, "--cruft-expiration=%s", opts->prune_expire);
+		if (opts->max_cruft_size)
+			strvec_pushf(args, "--max-cruft-size=%lu",
+				     opts->max_cruft_size);
+		if (opts->expire_to)
+			strvec_pushf(args, "--expire-to=%s", opts->expire_to);
+	} else {
+		strvec_push(args, "-A");
+		if (opts->prune_expire)
+			strvec_pushf(args, "--unpack-unreachable=%s", opts->prune_expire);
+	}
+
+	if (keep_pack)
+		for_each_string_list(keep_pack, keep_one_pack, args);
+
+	if (repack_filter && *repack_filter)
+		strvec_pushf(args, "--filter=%s", repack_filter);
+	if (repack_filter_to && *repack_filter_to)
+		strvec_pushf(args, "--filter-to=%s", repack_filter_to);
+
+	free(repack_filter);
+	free(repack_filter_to);
+}
+
+static void add_repack_incremental_option(struct strvec *args)
+{
+	strvec_push(args, "--no-write-bitmap-index");
+}
+
+bool odb_source_files_optimize_required(struct odb_source *source,
+					const struct odb_optimize_options *opts)
+{
+	struct odb_source_files *files = odb_source_files_downcast(source);
+	struct repository *repo = source->odb->repo;
+
+	switch (opts->strategy) {
+	case ODB_OPTIMIZE_INCREMENTAL: {
+		int gc_auto_threshold = 6700;
+		int gc_auto_pack_limit = 50;
+
+		repo_config_get_int(repo, "gc.auto", &gc_auto_threshold);
+		repo_config_get_int(repo, "gc.autopacklimit", &gc_auto_pack_limit);
+
+		/*
+		 * Setting gc.auto to 0 or negative can disable the
+		 * automatic gc.
+		 */
+		if (gc_auto_threshold <= 0)
+			return false;
+		if (!too_many_packs(files, gc_auto_pack_limit) &&
+		    !too_many_loose_objects(files, gc_auto_threshold))
+			return false;
+
+		return true;
+	}
+	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;
+		int auto_value = 100;
+		bool ret;
+
+		repo_config_get_int(repo, "maintenance.geometric-repack.auto",
+				    &auto_value);
+		if (!auto_value)
+			return false;
+		if (auto_value < 0)
+			return true;
+
+		repo_config_get_int(repo, "maintenance.geometric-repack.splitFactor",
+				    &geometry.split_factor);
+
+		existing_packs.repo = repo;
+		existing_packs_collect(&existing_packs, &kept_packs);
+		pack_geometry_init(&geometry, &existing_packs, &po_args);
+		pack_geometry_split(&geometry);
+
+		/*
+		 * When we'd merge at least two packs with one another we always
+		 * perform the repack.
+		 */
+		if (geometry.split) {
+			ret = true;
+			goto out;
+		}
+
+		/*
+		 * Otherwise, we estimate the number of loose objects to determine
+		 * whether we want to create a new packfile or not.
+		 */
+		if (too_many_loose_objects(files, auto_value)) {
+			ret = true;
+			goto out;
+		}
+
+		ret = false;
+
+	out:
+		existing_packs_release(&existing_packs);
+		pack_geometry_release(&geometry);
+		return ret;
+	}
+	default:
+		BUG("unknown maintenance strategy '%d'", opts->strategy);
+	}
+}
+
+int odb_source_files_optimize(struct odb_source *source,
+			      const struct odb_optimize_options *opts)
+{
+	struct odb_source_files *files = odb_source_files_downcast(source);
+	struct repository *repo = source->odb->repo;
+	struct child_process repack_cmd = CHILD_PROCESS_INIT;
+	unsigned long big_pack_threshold = 0;
+	int gc_auto_threshold = 6700;
+	int gc_auto_pack_limit = 50;
+	int ret;
+
+	repo_config_get_int(repo, "gc.auto", &gc_auto_threshold);
+	repo_config_get_int(repo, "gc.autopacklimit", &gc_auto_pack_limit);
+	repo_config_get_ulong(repo, "gc.bigpackthreshold", &big_pack_threshold);
+
+	if (repo->repository_format_precious_objects)
+		return 0;
+
+	repack_cmd.git_cmd = 1;
+	repack_cmd.odb_to_close = repo->objects;
+
+	strvec_pushl(&repack_cmd.args, "repack", "-d", "-l", NULL);
+	if (opts->flags & ODB_OPTIMIZE_NO_REUSE_DELTAS)
+		strvec_push(&repack_cmd.args, "-f");
+	if (opts->depth > 0)
+		strvec_pushf(&repack_cmd.args, "--depth=%d", opts->depth);
+	if (opts->window > 0)
+		strvec_pushf(&repack_cmd.args, "--window=%d", opts->window);
+	if (!(opts->flags & ODB_OPTIMIZE_VERBOSE))
+		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.
+	 */
+	switch (opts->strategy) {
+	case ODB_OPTIMIZE_INCREMENTAL:
+		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(files, &keep_pack, 0);
+			} else if (big_pack_threshold) {
+				find_base_packs(files, &keep_pack, big_pack_threshold);
+			}
+
+			add_repack_all_option(repo, opts, &keep_pack, &repack_cmd.args);
+			string_list_clear(&keep_pack, 0);
+		} else {
+			if (too_many_packs(files, gc_auto_pack_limit)) {
+				struct string_list keep_pack = STRING_LIST_INIT_NODUP;
+
+				if (big_pack_threshold) {
+					find_base_packs(files, &keep_pack, big_pack_threshold);
+					if (keep_pack.nr >= (unsigned long) gc_auto_pack_limit) {
+						string_list_clear(&keep_pack, 0);
+						find_base_packs(files, &keep_pack, 0);
+					}
+				} else {
+					struct packed_git *p = find_base_packs(files, &keep_pack, 0);
+					uint64_t mem_have, mem_want;
+
+					mem_have = total_ram();
+					mem_want = estimate_repack_memory(files, 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(repo, 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(repo, "maintenance.geometric-repack.splitFactor",
+				    &geometry.split_factor);
+
+		existing_packs.repo = repo;
+		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(repo, opts, NULL, &repack_cmd.args);
+		}
+		if (repo->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)) {
+		ret = error("failed to run %s", repack_cmd.args.v[0]);
+		goto out;
+	}
+
+	/* 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);
+		/* run `git prune` even if using cruft packs */
+		strvec_push(&prune_cmd.args, opts->prune_expire);
+		if (!(opts->flags & ODB_OPTIMIZE_VERBOSE))
+			strvec_push(&prune_cmd.args, "--no-progress");
+		if (repo_has_promisor_remote(repo))
+			strvec_push(&prune_cmd.args,
+				    "--exclude-promisor-objects");
+		prune_cmd.git_cmd = 1;
+
+		if (run_command(&prune_cmd)) {
+			ret = error("failed to run %s", prune_cmd.args.v[0]);
+			goto out;
+		}
+	}
+
+	if (opts->flags & ODB_OPTIMIZE_AUTO && too_many_loose_objects(files, gc_auto_threshold))
+		warning(_("There are too many unreachable loose objects; "
+			"run 'git prune' to remove them."));
+
+	ret = 0;
+
+out:
+	return ret;
+}
+
 struct odb_source_files *odb_source_files_new(struct object_database *odb,
 					      const char *path,
 					      bool local)
@@ -285,6 +753,8 @@ struct odb_source_files *odb_source_files_new(struct object_database *odb,
 	files->base.begin_transaction = odb_source_files_begin_transaction;
 	files->base.read_alternates = odb_source_files_read_alternates;
 	files->base.write_alternate = odb_source_files_write_alternate;
+	files->base.optimize = odb_source_files_optimize;
+	files->base.optimize_required = odb_source_files_optimize_required;
 
 	/*
 	 * Ideally, we would only ever store absolute paths in the source. This
diff --git a/odb/source-files.h b/odb/source-files.h
index d7ac3c1c81..044242bc36 100644
--- a/odb/source-files.h
+++ b/odb/source-files.h
@@ -21,6 +21,21 @@ struct odb_source_files *odb_source_files_new(struct object_database *odb,
 					      const char *path,
 					      bool local);
 
+/*
+ * Optimize the files object database source by repacking loose objects and
+ * packfiles as needed. Returns 0 on success, a negative error code otherwise.
+ */
+int odb_source_files_optimize(struct odb_source *source,
+			      const struct odb_optimize_options *opts);
+
+/*
+ * Check whether optimization of the files object database source is required
+ * given the provided options. Returns true if optimization should be
+ * performed, false otherwise.
+ */
+bool odb_source_files_optimize_required(struct odb_source *source,
+					const struct odb_optimize_options *opts);
+
 /*
  * Cast the given object database source to the files backend. This will cause
  * a BUG in case the source doesn't use this backend.
diff --git a/odb/source.h b/odb/source.h
index 8767708c9c..88a48ba3c3 100644
--- a/odb/source.h
+++ b/odb/source.h
@@ -258,6 +258,21 @@ struct odb_source {
 	 */
 	int (*write_alternate)(struct odb_source *source,
 			       const char *alternate);
+
+	/*
+	 * This callback is expected to optimize the object database source.
+	 * Returns 0 on success, a negative error code otherwise.
+	 */
+	int (*optimize)(struct odb_source *source,
+			const struct odb_optimize_options *opts);
+
+	/*
+	 * This callback is expected to check whether optimization of the
+	 * object database source is required given the provided options.
+	 * Returns true if optimization should be performed, false otherwise.
+	 */
+	bool (*optimize_required)(struct odb_source *source,
+				  const struct odb_optimize_options *opts);
 };
 
 /*
@@ -475,4 +490,25 @@ static inline int odb_source_begin_transaction(struct odb_source *source,
 	return source->begin_transaction(source, out);
 }
 
+/*
+ * Optimize the object database source. Returns 0 on success, a negative error
+ * code otherwise.
+ */
+static inline int odb_source_optimize(struct odb_source *source,
+				      const struct odb_optimize_options *opts)
+{
+	return source->optimize(source, opts);
+}
+
+/*
+ * Check whether optimization of the object database source is required given
+ * the provided options. Returns true if optimization should be performed,
+ * false otherwise.
+ */
+static inline bool odb_source_optimize_required(struct odb_source *source,
+						const struct odb_optimize_options *opts)
+{
+	return source->optimize_required(source, opts);
+}
+
 #endif

-- 
2.55.0.313.g8d093f411d.dirty

Re: [PATCH v2 12/12] odb: make optimizations pluggable

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> Move `odb_optimize()` and `odb_optimize_required()` from "builtin/gc.c"
> into the "files" source and wire them up via newly introduced vtable
> pointers for the object database sources. This makes the logic pluggable
> and thus allows other backends to have their own, custom implementation.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  builtin/gc.c       | 490 +----------------------------------------------------
>  odb.c              |  12 ++
>  odb.h              |  45 +++++
>  odb/source-files.c | 470 ++++++++++++++++++++++++++++++++++++++++++++++++++

Reviewing this commit with "git show --color-moved" is an excellent
way to verify that this is 90% pure code motion.  Only minimal
adjustments were required, such as replacing a direct reference to a
struct member in the original with a variable that was previously
assigned the equivalent value.

For example, this original

> -	switch (opts->strategy) {
> -	case ODB_OPTIMIZE_INCREMENTAL: {
> -		int gc_auto_threshold = 6700;
> -		int gc_auto_pack_limit = 50;
> -
> -		repo_config_get_int(odb->repo, "gc.auto", &gc_auto_threshold);
> -		repo_config_get_int(odb->repo, "gc.autopacklimit", &gc_auto_pack_limit);
> -
> -		/*
> -		 * Setting gc.auto to 0 or negative can disable the
> -		 * automatic gc.
> -		 */

turns into

> +	switch (opts->strategy) {
> +	case ODB_OPTIMIZE_INCREMENTAL: {
> +		int gc_auto_threshold = 6700;
> +		int gc_auto_pack_limit = 50;
> +
> +		repo_config_get_int(repo, "gc.auto", &gc_auto_threshold);
> +		repo_config_get_int(repo, "gc.autopacklimit", &gc_auto_pack_limit);
> +
> +		/*
> +		 * Setting gc.auto to 0 or negative can disable the
> +		 * automatic gc.
> +		 */

And the change from odb->repo vs repo is such an adjustment.

Looking good.