git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: git@vger.kernel.org, "Junio C Hamano" <gitster@pobox.com>,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
	"Michael Haggerty" <mhagger@alum.mit.edu>,
	"Stefan Beller" <stefanbeller@gmail.com>,
	"Jonathan Nieder" <jrnieder@gmail.com>,
	"Matt McCutchen" <matt@mattmccutchen.net>
Subject: Re: [PATCH 3/4] gc docs: de-duplicate "OPTIONS" and "CONFIGURATION"
Date: Mon, 18 Mar 2019 17:49:05 -0400	[thread overview]
Message-ID: <20190318214905.GG29661@sigill.intra.peff.net> (raw)
In-Reply-To: <20190318161502.7979-4-avarab@gmail.com>

On Mon, Mar 18, 2019 at 05:15:01PM +0100, Ævar Arnfjörð Bjarmason wrote:

> In an earlier commit I started including the "gc.*" documentation from
> git-config(1) in the git-gc(1) documentation. That still left us in a
> state where the "--auto" option and "gc.auto" were redundantly
> discussing the same thing.
> 
> Fix that by briefly discussing how the option itself works for
> "--auto", and for the rest referring to the configuration
> documentation.
> 
> This revealed existing blind spots in the configuration documentation,
> move over the documentation and reword as appropriate.

Nice improvement. A few comments:

> diff --git a/Documentation/config/gc.txt b/Documentation/config/gc.txt
> index a834a801cd6..605e14bc80b 100644
> --- a/Documentation/config/gc.txt
> +++ b/Documentation/config/gc.txt
> @@ -19,13 +19,27 @@ gc.auto::
>  	objects in the repository, `git gc --auto` will pack them.
>  	Some Porcelain commands use this command to perform a
>  	light-weight garbage collection from time to time.  The
> -	default value is 6700.  Setting this to 0 disables it.
> +	default value is 6700.
> ++
> +Setting this to 0 disables not only automatic packing based on the
> +number of loose objects, but any other heuristic `git gc --auto` will
> +otherwise use to determine if there's work to do, such as
> +`gc.autoPackLimit`.
> ++
> +The repacking of loose objects will be performed with `git repack -d
> +-l`.

I know this last sentence came from the existing documentation, but I
wonder if we should be more vague here. We'd pack with "repack -dl" when
we have just loose objects, and "repack -Adl" when we have too many
packs. Or "repack -adl" if we're pruning now, and "--unpack-unreachable"
otherwise.

I think the point of git-gc is that you don't have to care about that
stuff. It works magically, and if you are implementing your own custom
gc scheme, then you are probably better off reading the output of
GIT_TRACE or looking at the source, rather than this documentation.

>  gc.autoPackLimit::
> +
>  	When there are more than this many packs that are not

What's this newline for? I'm not completely opposed if that's the style
we want, but it seems odd that just this one has a blank between the
variable name and the text.

>  	marked with `*.keep` file in the repository, `git gc
>  	--auto` consolidates them into one larger pack.  The
> -	default	value is 50.  Setting this to 0 disables it.
> +	default value is 50.  Setting this (or `gc.auto`) to 0
> +	disables it. Packs will be consolidated using the `-A` option
> +	of `git repack`.

If we do revise the "-d -l" bit for the loose limit, we'd probably want
to adjust this to match.

> @@ -35,13 +49,18 @@ gc.bigPackThreshold::
>  	If non-zero, all packs larger than this limit are kept when
>  	`git gc` is run. This is very similar to `--keep-base-pack`
>  	except that all packs that meet the threshold are kept, not
> -	just the base pack. Defaults to zero. Common unit suffixes of
> -	'k', 'm', or 'g' are supported.
> +	just the base pack. Defaults to zero or a memory heuristic.
> +	Common unit suffixes of 'k', 'm', or 'g' are supported.

I'm not sure how to read this "or". What's the difference between "0" or
the memory heuristic, and when is one used? Or is that what the "if the
number of kept packs is more than..." below is trying to say?

If so, I wonder if it would be simpler to say "defaults to a memory
heuristic", but with a note for "but under these conditions it is not
used".

Or am I totally misunderstanding how it actually works (which seems
likely to me)?

> +If the amount of memory is estimated not enough for `git repack` to
> +run smoothly and `gc.bigPackThreshold` is not set, the largest pack
> +will also be excluded (which is the equivalent of running `git gc`
> +with `--keep-base-pack`).

I had trouble parsing this first line. Maybe:

  If the amount of memory estimated for `git repack` to run smoothly is
  not available and ...

I guess a lot of this is just being copied from elsewhere, but it's
probably worth cleaning it up while we're here.

> --- a/Documentation/git-gc.txt
> +++ b/Documentation/git-gc.txt
> [...]
> +See the `gc.auto' option in the "CONFIGURATION" below for how this
> +heuristic works.

s/CONFIGURATION/& section/?

> +Once housekeeping is triggered by exceeding the limits of
> +configurations options such as `gc.auto` and `gc.autoPackLimit`, all

s/configurations/configuration/

-Peff

  reply	other threads:[~2019-03-18 21:49 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-18 16:14 [PATCH 0/4] gc docs: modernize and fix the documentation Ævar Arnfjörð Bjarmason
2019-03-18 16:14 ` [PATCH 1/4] gc docs: modernize the advice for manually running "gc" Ævar Arnfjörð Bjarmason
2019-03-18 21:27   ` Jeff King
2019-03-18 22:18     ` Ævar Arnfjörð Bjarmason
2019-03-18 16:15 ` [PATCH 2/4] gc docs: include the "gc.*" section from "config" in "gc" Ævar Arnfjörð Bjarmason
2019-03-18 21:31   ` Jeff King
2019-03-21 22:11     ` Andreas Heiduk
2019-03-19  2:08   ` Duy Nguyen
2019-03-18 16:15 ` [PATCH 3/4] gc docs: de-duplicate "OPTIONS" and "CONFIGURATION" Ævar Arnfjörð Bjarmason
2019-03-18 21:49   ` Jeff King [this message]
2019-03-18 22:48     ` Ævar Arnfjörð Bjarmason
2019-03-18 23:42       ` Jeff King
2019-03-18 16:15 ` [PATCH 4/4] gc docs: downplay the usefulness of --aggressive Ævar Arnfjörð Bjarmason
2019-03-18 20:28   ` Jonathan Nieder
2019-03-18 21:22     ` Jeff King
2019-03-18 22:13       ` Ævar Arnfjörð Bjarmason
2019-03-18 23:53         ` Jeff King
2019-03-19  6:54   ` Johannes Sixt
2019-03-19  9:28     ` Ævar Arnfjörð Bjarmason
2019-03-18 21:51 ` [PATCH 0/4] gc docs: modernize and fix the documentation Jeff King
2019-03-18 22:45   ` Ævar Arnfjörð Bjarmason
2019-03-19  0:18     ` Jeff King
2019-05-06  9:44       ` Ævar Arnfjörð Bjarmason
2019-05-07  7:51         ` Jeff King
2019-05-09 23:20           ` Ævar Arnfjörð Bjarmason
2019-07-31  4:26             ` Jeff King
2019-07-31 10:12               ` Ævar Arnfjörð Bjarmason
2019-03-21 20:50 ` [PATCH v2 00/10] " Ævar Arnfjörð Bjarmason
2019-03-22  9:32   ` [PATCH v3 00/11] gc docs: modernize the advice for manually running "gc" Ævar Arnfjörð Bjarmason
2019-03-22  9:32   ` [PATCH v3 01/11] " Ævar Arnfjörð Bjarmason
2019-03-22  9:32   ` [PATCH v3 02/11] gc docs: stop noting "repack" flags Ævar Arnfjörð Bjarmason
2019-03-22  9:32   ` [PATCH v3 03/11] gc docs: clean grammar for "gc.bigPackThreshold" Ævar Arnfjörð Bjarmason
2019-03-22  9:32   ` [PATCH v3 04/11] gc docs: include the "gc.*" section from "config" in "gc" Ævar Arnfjörð Bjarmason
2019-03-30 18:04     ` Todd Zullinger
2019-04-07 19:52     ` [PATCH v4 00/11] gc docs: modernize and fix the documentation Ævar Arnfjörð Bjarmason
2019-04-07 19:52     ` [PATCH v4 01/11] gc docs: modernize the advice for manually running "gc" Ævar Arnfjörð Bjarmason
2019-04-07 19:52     ` [PATCH v4 02/11] gc docs: stop noting "repack" flags Ævar Arnfjörð Bjarmason
2019-04-07 19:52     ` [PATCH v4 03/11] gc docs: clean grammar for "gc.bigPackThreshold" Ævar Arnfjörð Bjarmason
2019-04-07 19:52     ` [PATCH v4 04/11] gc docs: include the "gc.*" section from "config" in "gc" Ævar Arnfjörð Bjarmason
2019-04-07 19:52     ` [PATCH v4 05/11] gc docs: re-flow the "gc.*" section in "config" Ævar Arnfjörð Bjarmason
2019-04-07 19:52     ` [PATCH v4 06/11] gc docs: fix formatting for "gc.writeCommitGraph" Ævar Arnfjörð Bjarmason
2019-04-07 19:52     ` [PATCH v4 07/11] gc docs: note how --aggressive impacts --window & --depth Ævar Arnfjörð Bjarmason
2019-04-07 19:52     ` [PATCH v4 08/11] gc docs: downplay the usefulness of --aggressive Ævar Arnfjörð Bjarmason
2019-04-07 19:52     ` [PATCH v4 09/11] gc docs: note "gc --aggressive" in "fast-import" Ævar Arnfjörð Bjarmason
2019-04-07 19:52     ` [PATCH v4 10/11] gc docs: clarify that "gc" doesn't throw away referenced objects Ævar Arnfjörð Bjarmason
2019-04-07 19:52     ` [PATCH v4 11/11] gc docs: remove incorrect reference to gc.auto=0 Ævar Arnfjörð Bjarmason
2019-03-22  9:32   ` [PATCH v3 05/11] gc docs: re-flow the "gc.*" section in "config" Ævar Arnfjörð Bjarmason
2019-03-22  9:32   ` [PATCH v3 06/11] gc docs: fix formatting for "gc.writeCommitGraph" Ævar Arnfjörð Bjarmason
2019-03-22  9:32   ` [PATCH v3 07/11] gc docs: note how --aggressive impacts --window & --depth Ævar Arnfjörð Bjarmason
2019-03-22  9:32   ` [PATCH v3 08/11] gc docs: downplay the usefulness of --aggressive Ævar Arnfjörð Bjarmason
2019-03-22  9:32   ` [PATCH v3 09/11] gc docs: note "gc --aggressive" in "fast-import" Ævar Arnfjörð Bjarmason
2019-03-22  9:32   ` [PATCH v3 10/11] gc docs: clarify that "gc" doesn't throw away referenced objects Ævar Arnfjörð Bjarmason
2019-03-22  9:32   ` [PATCH v3 11/11] gc docs: remove incorrect reference to gc.auto=0 Ævar Arnfjörð Bjarmason
2019-03-21 20:50 ` [PATCH v2 01/10] gc docs: modernize the advice for manually running "gc" Ævar Arnfjörð Bjarmason
2019-03-22  6:01   ` Junio C Hamano
2019-03-21 20:50 ` [PATCH v2 02/10] gc docs: stop noting "repack" flags Ævar Arnfjörð Bjarmason
2019-03-21 20:50 ` [PATCH v2 03/10] gc docs: clean grammar for "gc.bigPackThreshold" Ævar Arnfjörð Bjarmason
2019-03-21 20:50 ` [PATCH v2 04/10] gc docs: include the "gc.*" section from "config" in "gc" Ævar Arnfjörð Bjarmason
2019-03-21 20:50 ` [PATCH v2 05/10] gc docs: re-flow the "gc.*" section in "config" Ævar Arnfjörð Bjarmason
2019-03-21 20:50 ` [PATCH v2 06/10] gc docs: note how --aggressive impacts --window & --depth Ævar Arnfjörð Bjarmason
2019-03-21 20:50 ` [PATCH v2 07/10] gc docs: downplay the usefulness of --aggressive Ævar Arnfjörð Bjarmason
2019-03-21 20:50 ` [PATCH v2 08/10] gc docs: note "gc --aggressive" in "fast-import" Ævar Arnfjörð Bjarmason
2019-03-21 20:50 ` [PATCH v2 09/10] gc docs: clarify that "gc" doesn't throw away referenced objects Ævar Arnfjörð Bjarmason
2019-03-21 20:50 ` [PATCH v2 10/10] gc docs: remove incorrect reference to gc.auto=0 Ævar Arnfjörð Bjarmason

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190318214905.GG29661@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    --cc=matt@mattmccutchen.net \
    --cc=mhagger@alum.mit.edu \
    --cc=pclouds@gmail.com \
    --cc=stefanbeller@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).