Subject: Re: [PATCH 4/4] gc docs: downplay the usefulness of --aggressive

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Mon, 18 Mar 2019 23:13:57 +0100

On Mon, Mar 18 2019, Jeff King wrote:

> On Mon, Mar 18, 2019 at 01:28:24PM -0700, Jonathan Nieder wrote:
>
>> > +Using this option may optimize for disk space at the expense of
>> > +runtime performance. See the `--depth` and `--window` documentation in
>> > +linkgit:git-repack[1]. It is not recommended that this option be used
>> > +to improve performance for a given repository without running tailored
>> > +performance benchmarks on it. It may make things better, or worse. Not
>> > +using this at all is the right trade-off for most users and their
>> > +repositories.
>>
>> This part kind of feels like giving up.  Can we make --aggressive have
>> good runtime read performance so we don't have to hedge this way?
>> E.g. is this patch papering over a poor choice of --depth setting in
>> --aggressive?
>
> I thought we already did that, in 07e7dbf0db (gc: default aggressive
> depth to 50, 2016-08-11). As far as I know, "--aggressive" produces
> packs with similar runtime performance.

What happened here is that I'd entirely forgotten about your 07e7dbf0db
and in skimming while writing this throught we were still picking larger
depth values, which we aren't.

I'll fix that, and see that gc.aggressiveDepth also needs to be changed
to note that the depth it's now using as "aggressive" is just the
default of 50 you'd get without --aggressive.

> It is possible, if it finds more deltas due to the larger window, that
> we'd spend more time accessing those deltas. But if the chains aren't
> long, the base cache tends to perform well, and delta reconstruction is
> about the same cost as zlib inflating. And we have a smaller disk cache
> footprint.

I haven't tested that but suspect it won't matter. We do spend a *lot*
more time though, so that still needs to be noted...

On the topic of other things I may have screwed up, is this:

    +The effects of this option are persistent to the extent that
    +`gc.autoPackLimit` and friends don't cause a consolidation of existing
    +pack(s) generated with this option.

Actually wrong since we don't pass -f usually, and thus a one-off
--aggressive would live forever for the objects involved in that run no
matter if we later consolidate?

>From the docs it seems so, but I'd like to confirm...