Re: [PATCH 8/8] builtin/maintenance: use "geometric" strategy by default

[Justin Tobler <jltobler@gmail.com>]

On 26/02/20 11:15AM, Patrick Steinhardt wrote:
> The git-gc(1) command has been introduced in the early days of Git in
> 30f610b7b0 (Create 'git gc' to perform common maintenance operations.,
> 2006-12-27) as the main repository maintenance utility. And while the
> tool has of course evolved since then to cover new parts, the basic
> strategy it uses has never really changed much.
> 
> It is safe to say that since 2006 the Git ecosystem has changed quite a
> bit. Repositories tend to be much larger nowadays than they have been
> almost 20 years ago, and large parts of the industry went crazy for
> monorepos (for various wildly different definitions of "monorepo"). So
> the maintenance strategy we used back then may not be the best fit
> nowadays anymore.
> 
> Arguably, most of the maintenance tasks that git-gc(1) does are still
> perfectly fine today: repacking references, expiring various data
> structures and things like tend to not cause huge problems. But the big
> exception is the way we repack objects.
> 
> git-gc(1) by default uses a split strategy: it performs incremental
> repacks by default, and then whenever we have too many packs we perform
> a large all-into-one repack. This all-into-one repack is what is causing
> problems nowadays, as it is an operation that is quite expensive. While
> it is wasteful in small- and medium-sized repositories, in large repos
> it may even be prohibitively expensive.
> 
> We have eventually introduced git-maintenance(1) that was slated as a
> replacement for git-gc(1). In contrast to git-gc(1), it was much more
> flexible as it is structured around configurable tasks and strategies.
> And while it knows about the "incremental" strategy that we may use for
> scheduled maintenance when configured via Scalar, its default still is
> to use git-gc(1) in the background.

I'm a tad bit confused here. git-gc(1) by default uses an
"incremental/all-into-one" strategy and it is my understanding that this
is what git-maintenance(1) is currently using. Is there also another
"incremental" strategy for git-maintenance(1)?

> The "incremental" strategy isn't really a full replacement for git-gc(1)
> though, as it doesn't know to expire unused data structures. In Git 2.52
> we have thus introduced a new "geometric" strategy that is a proper
> replacement for the old git-gc(1).
> 
> In contrast to the incremental/all-into-one split used by git-gc(1), the
> new "geometric" strategy maintains a geometric progression of packfiles,
> which significantly reduces the number of all-into-one repacks that we
> have to perform in large repositories. It is thus a much better fit for
> large repositories than git-gc(1).
> 
> Note that the "geometric" strategy isn't perfect though: while we
> perform way less all-into-one repacks compared to git-gc(1), we still
> have to perform them eventually. But for the largest repositories out
> there this may not be an option, as client machines might not be
> powerful enough to perform such a repack in the first place. These cases
> would thus still be covered by Scalar's "incremental" strategy.

So the "problem" is the "all-into-one" repack. This ultimately occurs
for both the "gc" and "geometric" strategies so changing the default
strategy shouldn't make anything worse. Geometric repacking should in
fact delay the costly "all-into-one" repacks which is good.

> Switch the default strategy away from "gc" to "geometric", but retain
> the "incremental" strategy configured by Scalar.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  builtin/gc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/builtin/gc.c b/builtin/gc.c
> index 4390eee6ec..fb329c2cff 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -1980,7 +1980,7 @@ static void initialize_task_config(struct maintenance_run_opts *opts,
>  		strategy = none_strategy;
>  		type = MAINTENANCE_TYPE_SCHEDULED;
>  	} else {
> -		strategy = gc_strategy;
> +		strategy = geometric_strategy;

Looks good.

-Justin