| Message ID | 20190318161502.7979-4-avarab@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | gc docs: modernize and fix the documentation | expand |
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
On Mon, Mar 18 2019, Jeff King wrote: > 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. Yeah I can just drop it while I'm at it. Was just losslessly trying to port the existing docs. >> 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. Mistake, will fix. >> 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. Or not mention it either? >> @@ -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? That by default we don't use gc.bigPackThreshold, unless we find that you're under memory pressure. I.e. "it's off by default, unless your system has too little memory". > 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. Will try to make it suck less. >> --- 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/ *Nod*. Thanks.
On Mon, Mar 18, 2019 at 11:48:46PM +0100, Ævar Arnfjörð Bjarmason wrote: > > 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. > > Yeah I can just drop it while I'm at it. Was just losslessly trying to > port the existing docs. Yeah, I'm sympathetic to that (if you did drop it, you might have gotten the opposite review). ;) I think it would be OK to just mention it in the commit message, but I'd also be OK dropping it in a preliminary patch. > >> 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. > > Or not mention it either? Yes. :) > > 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? > > That by default we don't use gc.bigPackThreshold, unless we find that > you're under memory pressure. I.e. "it's off by default, unless your > system has too little memory". OK, I see. It might make sense to write that out more explicitly. -Peff
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`. gc.autoPackLimit:: + When there are more than this many packs that are not 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`. ++ +See the `gc.bigPackThreshold` configuration variable below. When in +use it'll effect how the auto pack limit works. gc.autoDetach:: Make `git gc --auto` return immediately and run in background @@ -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. + Note that if the number of kept packs is more than gc.autoPackLimit, this configuration variable is ignored, all packs except the base pack will be repacked. After this the number of packs should go below gc.autoPackLimit and gc.bigPackThreshold should be respected again. ++ +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`). gc.writeCommitGraph:: If true, then gc will rewrite the commit-graph file when diff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt index 9edf4e465b4..154c7c5e652 100644 --- a/Documentation/git-gc.txt +++ b/Documentation/git-gc.txt @@ -49,29 +49,12 @@ OPTIONS --auto:: With this option, 'git gc' checks whether any housekeeping is required; if not, it exits without performing any work. - Some git commands run `git gc --auto` after performing - operations that could create many loose objects. Housekeeping - is required if there are too many loose objects or too many - packs in the repository. + -If the number of loose objects exceeds the value of the `gc.auto` -configuration variable, then all loose objects are combined into a -single pack using `git repack -d -l`. Setting the value of `gc.auto` -to 0 disables automatic packing of loose objects. +See the `gc.auto' option in the "CONFIGURATION" below for how this +heuristic works. + -If the number of packs exceeds the value of `gc.autoPackLimit`, -then existing packs (except those marked with a `.keep` file -or over `gc.bigPackThreshold` limit) -are consolidated into a single pack by using the `-A` option of -'git repack'. -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 (this is the equivalent of running `git gc` -with `--keep-base-pack`). -Setting `gc.autoPackLimit` to 0 disables automatic consolidation of -packs. -+ -If houskeeping is required due to many loose objects or packs, all +Once housekeeping is triggered by exceeding the limits of +configurations options such as `gc.auto` and `gc.autoPackLimit`, all other housekeeping tasks (e.g. rerere, working trees, reflog...) will be performed as well.
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. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- Documentation/config/gc.txt | 27 +++++++++++++++++++++++---- Documentation/git-gc.txt | 25 ++++--------------------- 2 files changed, 27 insertions(+), 25 deletions(-)