Re: People unaware of the importance of "git gc"?

Nicolas Pitre [off-list ref] writes:

Not only that.  Currently the "Counting objects" phase when running 
git-gc on the Linux repo takes a significant amount of time, even if 
there is little to repack.

If any kind of automatic repack is implemented, it should be an 
incremental repacking only, not the full thing, i.e. git-repack without 
-a, or git-pack-objects with --unpacked.  The idea is to be the least 
intrusive as possible.  Also, object walking should be limited to 
objects linked to a commit object which is itself unpacked in order to 
cut on the time required to fully enumerate all objects.

This way a semi-packed state will always be preserved and should be good 
enough.  The full repacking should probably be left to manual execution 
of git-gc.

Ok, how about doing something like this?

-- >8 -- snipsnap -- >8 -- clipcrap -- >8 --
Implement git gc --auto

This implements a new option "git gc --auto".  When gc.auto is
set to a positive value, and the object database has accumulated
roughly that many number of loose objects, this runs a
lightweight version of "git gc".  The primary difference from
the full "git gc" is that it does not pass "-a" option to "git
repack", which means we do not try to repack _everything_, but
only repack incrementally.  We still do "git prune-packed".  The
default threshold is arbitrarily set by yours truly to:

 - not trigger it for fully unpacked git v0.99 history;

 - do trigger it for fully unpacked git v1.0.0 history;

 - not trigger it for incremental update to git v1.0.0 starting
   from fully packed git v0.99 history.

This patch does not add invocation of the "auto repacking".  It
is left to key Porcelain commands that could produce tons of
loose objects to add a call to "git gc --auto" after they are
done their work.  Obvious candidates are:

	git add
	git fetch
        git merge
        git rebase        

Signed-off-by: Junio C Hamano <redacted>
---

 builtin-gc.c |   64 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 63 insertions(+), 1 deletions(-)

diff --git a/builtin-gc.c b/builtin-gc.c
index 9397482..093b3dd 100644
--- a/builtin-gc.c
+++ b/builtin-gc.c

@@ -20,6 +20,7 @@ static const char builtin_gc_usage[] = "git-gc [--prune] [--aggressive]";
 
 static int pack_refs = 1;
 static int aggressive_window = -1;
+static int gc_auto_threshold = 6700;
 
 #define MAX_ADD 10
 static const char *argv_pack_refs[] = {"pack-refs", "--all", "--prune", NULL};

@@ -28,6 +29,8 @@ static const char *argv_repack[MAX_ADD] = {"repack", "-a", "-d", "-l", NULL};
 static const char *argv_prune[] = {"prune", NULL};
 static const char *argv_rerere[] = {"rerere", "gc", NULL};
 
+static const char *argv_repack_auto[] = {"repack", "-d", "-l", NULL};
+
 static int gc_config(const char *var, const char *value)
 {
 	if (!strcmp(var, "gc.packrefs")) {

@@ -41,6 +44,10 @@ static int gc_config(const char *var, const char *value)
 		aggressive_window = git_config_int(var, value);
 		return 0;
 	}
+	if (!strcmp(var, "gc.auto")) {
+		gc_auto_threshold = git_config_int(var, value);
+		return 0;
+	}
 	return git_default_config(var, value);
 }
 

@@ -57,10 +64,49 @@ static void append_option(const char **cmd, const char *opt, int max_length)
 	cmd[i] = NULL;
 }
 
+static int need_to_gc(void)
+{
+	/*
+	 * Quickly check if a "gc" is needed, by estimating how
+	 * many loose objects there are.  Because SHA-1 is evenly
+	 * distributed, we can check only one and get a reasonable
+	 * estimate.
+	 */
+	char path[PATH_MAX];
+	const char *objdir = get_object_directory();
+	DIR *dir;
+	struct dirent *ent;
+	int auto_threshold;
+	int num_loose = 0;
+	int needed = 0;
+
+	if (sizeof(path) <= snprintf(path, sizeof(path), "%s/17", objdir)) {
+		warning("insanely long object directory %.*s", 50, objdir);
+		return 0;
+	}
+	dir = opendir(path);
+	if (!dir)
+		return 0;
+
+	auto_threshold = (gc_auto_threshold + 255) / 256;
+	while ((ent = readdir(dir)) != NULL) {
+		if (strspn(ent->d_name, "0123456789abcdef") != 38 ||
+		    ent->d_name[38] != '\0')
+			continue;
+		if (++num_loose > auto_threshold) {
+			needed = 1;
+			break;
+		}
+	}
+	closedir(dir);
+	return needed;
+}
+
 int cmd_gc(int argc, const char **argv, const char *prefix)
 {
 	int i;
 	int prune = 0;
+	int auto_gc = 0;
 	char buf[80];
 
 	git_config(gc_config);

@@ -82,12 +128,28 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
 			}
 			continue;
 		}
-		/* perhaps other parameters later... */
+		if (!strcmp(arg, "--auto")) {
+			if (gc_auto_threshold <= 0)
+				return 0;
+			auto_gc = 1;
+			continue;
+		}
 		break;
 	}
 	if (i != argc)
 		usage(builtin_gc_usage);
 
+	if (auto_gc) {
+		/*
+		 * Auto-gc should be least intrusive as possible.
+		 */
+		prune = 0;
+		for (i = 0; i < ARRAY_SIZE(argv_repack_auto); i++)
+			argv_repack[i] = argv_repack_auto[i];
+		if (!need_to_gc())
+			return 0;
+	}
+
 	if (pack_refs && run_command_v_opt(argv_pack_refs, RUN_GIT_CMD))
 		return error(FAILED_RUN, argv_pack_refs[0]);
 

On Wed, 5 Sep 2007, Junio C Hamano wrote:

Implement git gc --auto

This implements a new option "git gc --auto".  When gc.auto is
set to a positive value, and the object database has accumulated
roughly that many number of loose objects, this runs a
lightweight version of "git gc".  The primary difference from
the full "git gc" is that it does not pass "-a" option to "git
repack", which means we do not try to repack _everything_, but
only repack incrementally.  We still do "git prune-packed".  

A big part of the repack cost is the counting of objects. I don't know 
if --unpacked to git-pack-objects skips walking trees of a packed commit 
object.  If no then it probably should to gain a significant speed up, 
or maybe a separate option should be created to actually imply this 
loosened semantic.

This patch does not add invocation of the "auto repacking".  It
is left to key Porcelain commands that could produce tons of
loose objects to add a call to "git gc --auto" after they are
done their work.  Obvious candidates are:

	git add

Nope!  'git add' creates loose objects which are not yet reachable from 
anywhere.  They won't get repacked until a commit is made.

	git fetch

I think that would be a much better idea to simply decrease the 
fetch.unpackLimit default value.

        git merge
        git rebase        

and git commit.  Which resumes it to commit creating operation.


Nicolas

Junio C Hamano, Wed, Sep 05, 2007 22:01:37 +0200:

+	/*
+	 * Quickly check if a "gc" is needed, by estimating how
+	 * many loose objects there are.  Because SHA-1 is evenly
+	 * distributed, we can check only one and get a reasonable
+	 * estimate.
+	 */

:))

+	if (sizeof(path) <= snprintf(path, sizeof(path), "%s/17", objdir)) {
+		warning("insanely long object directory %.*s", 50, objdir);

or a non-POSIX snprintf returning "negative value" (Microsoft)

Ok, how about doing something like this?

git add? merge? rebase? No, I have a sneakier place to invoke gc.

Whenever $EDITOR gets invoked. Heck, whenever git is waiting for any user input,
do some gc in the background, it'd just have to be incremental so that we could
pick up where we left off.

Similarly, you could mix it in with git pull/push so that while we are waiting
on the network, we can do some packing.

Course, this wouldn't work for all repositories.

Junio C Hamano [off-list ref] wrote:

Implement git gc --auto

... 

Danger...  If the user sets `gc.auto` to a low enough value and
they are also unlucky enough to have a few truely unreachable (thus
pruneable) objects in .git/objects/17/ then this is going to run
a bunch of gc work on every commit they make.

I'm actually running into this problem in git-gui.  On Windows
it suggests a repack if there is one object in .git/objects/42/.
Some users have been unlucky enough to stage a file, have it
hash into that directory, then restage a different version of it.
The prior one is never considered reachable (it was never committed),
but will now *always* cause git-gui to suggest a repack on every
startup.  For all time.

Yea, I need to fix that.

But this suffers from the same fate if the user sets gc.auto too
small and doesn't realize that the reason Git is always repacking
is because over the last 6 months they have been unlucky enough to
stage the magic number of unreachable blobs into the 17 directory
and they have *never* run `git gc --prune` because the auto thing
is working just fine for them and they don't realize they need to
prune every once in a blue moon.

-- 
Shawn.

Shawn O. Pearce wrote:

But this suffers from the same fate if the user sets gc.auto too
small and doesn't realize that the reason Git is always repacking
is because over the last 6 months they have been unlucky enough to
stage the magic number of unreachable blobs into the 17 directory
and they have *never* run `git gc --prune` because the auto thing
is working just fine for them and they don't realize they need to
prune every once in a blue moon.
  

Check the modification times on those files and don't count ones that 
are older than the last git-gc run, maybe? That'd take care of the problem.

-Steve

Russ Dill [off-list ref] wrote:

quoted

Ok, how about doing something like this?

git add? merge? rebase? No, I have a sneakier place to invoke gc.

Whenever $EDITOR gets invoked. Heck, whenever git is waiting for any user input,
do some gc in the background, it'd just have to be incremental so that we could
pick up where we left off.

Heh.  That is a really good idea.  I've been thinking about doing
some automatic generational style GC type repacking controls in
git-gui, and doing them when git-gui is sitting idle and has not
been used in the past couple of minutes.

This is along the same vein of thought.  I like it.  Often it
takes me a while to come up with a good commit message even if
I am using command line commit.

But git-rebase/git-am can cause a huge number of objects to be
created, especially if you are pushing a large stack of patches
around.  So it may still be a good idea to trigger `gc --auto`
at the end of those operations.
 

Similarly, you could mix it in with git pull/push so that while we are waiting
on the network, we can do some packing.

Here's a better thought:

If we are pushing somewhere, and the push size is "large-ish" and
we aren't pushing a thin pack (its currently considered not nice
to the remote end so it doesn't happen by default) and the objects
we are packing are mostly all loose maybe we should also save a
copy of that packfile locally, then prune *only* those loose objects
back.

Not every git user pushes their work.  But many do.  And those
that push usually will do so in bursts, are already expecting to
wait for the network latency, and usually are pushing the majority
of the things that are loose.  Such users will probably never see
the `gc --auto` trip in places like commit/am/merge as they would
already be clearing their ODB with the push.

-- 
Shawn.

Steven Grimm [off-list ref] wrote:

Shawn O. Pearce wrote:

quoted

But this suffers from the same fate if the user sets gc.auto too
small and doesn't realize that the reason Git is always repacking
is because over the last 6 months they have been unlucky enough to
stage the magic number of unreachable blobs into the 17 directory
and they have *never* run `git gc --prune` because the auto thing
is working just fine for them and they don't realize they need to
prune every once in a blue moon.

Check the modification times on those files and don't count ones that 
are older than the last git-gc run, maybe? That'd take care of the problem.

Eh, that could mean a bunch of stat calls that it would be nice
to avoid.  The counter Junio (and git-gui) implements just does
a readdir().  Reasonably cheap.

Maybe just save a ".git/gc_last_auto" with the last object count
of .git/objects/17, after repacking.  If the count is over the
gc.auto limit *and* is still over the limit after subtracting the
".git/gc_last_auto" value then consider that auto is required.

This way the file is only consulted if we are really thinking
about running a repack, and its only written to if we actually do
the repack.  So we only take the extra penalty if we are going to
be taking a *really* big extra penalty by repacking.

-- 
Shawn.

Russ Dill wrote:

quoted

Ok, how about doing something like this?

git add? merge? rebase? No, I have a sneakier place to invoke gc.

Whenever $EDITOR gets invoked. Heck, whenever git is waiting for any user input,
do some gc in the background, it'd just have to be incremental so that we could
pick up where we left off.

I like it. Writing a commit-message takes anywhere from 30 seconds to 5 minutes
for me (sometimes having to check up bug id's, or verifying details in the code).
Sneaking in a repack here would be absolutely stellar :)

It's also nice in that it won't affect people who just follow a project's tip to
get the bleeding edge. For them it shouldn't matter much that they have multiple
small packs obtained while fetching, or if it's all bungled together in a big one.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Hi,

On Wed, 5 Sep 2007, Junio C Hamano wrote:

Please don't do that.

When you share objects with another git directory, git-gc --auto can get 
rid of the objects when some objects go away in the referenced repository.  

So we need _at least_ check gc.auto not being set in the repo when "git 
clone --share"ing it (and fail otherwise).

My preferred way would be to set it in "git init" so that existing setups 
are not affected, and put some big red message on top of the next release 
notes that people might want to set gc.auto in their existing setups.

Ciao,
Dscho

In 2007 Junio wrote
(https://public-inbox.org/git/7vr6lcj2zi.fsf@gitster.siamese.dyndns.org/):

    +static int need_to_gc(void)
    +{
    +	/*
    +	 * Quickly check if a "gc" is needed, by estimating how
    +	 * many loose objects there are.  Because SHA-1 is evenly
    +	 * distributed, we can check only one and get a reasonable
    +	 * estimate.
    +	 */
    +	char path[PATH_MAX];
    +	const char *objdir = get_object_directory();
    +	DIR *dir;
    +	struct dirent *ent;
    +	int auto_threshold;
    +	int num_loose = 0;
    +	int needed = 0;
    +
    +	if (sizeof(path) <= snprintf(path, sizeof(path), "%s/17", objdir)) {
    +		warning("insanely long object directory %.*s", 50, objdir);
    +		return 0;
    +	}
    +	dir = opendir(path);
    +	if (!dir)
    +		return 0;
    +
    +	auto_threshold = (gc_auto_threshold + 255) / 256;
    +	while ((ent = readdir(dir)) != NULL) {
    +		if (strspn(ent->d_name, "0123456789abcdef") != 38 ||
    +		    ent->d_name[38] != '\0')
    +			continue;
    +		if (++num_loose > auto_threshold) {
    +			needed = 1;
    +			break;
    +		}
    +	}

A couple of questions about this patch, which is in git.git as
2c3c439947 ("Implement git gc --auto", 2007-09-05)

1. We still have this check of objects/17/ in builtin/gc.c today. Why
   objects/17/ and not e.g. objects/00/ to go with other 000* magic such
   as the 0000000000000000000000000000000000000000 SHA-1?  Statistically
   it doesn't matter, but 17 seems like an odd thing to pick at random
   out of 00..ff, does it have any significance?

2. It seems overly paranoid to be checking that the files in
  .git/objects/17/ look like a SHA-1. If we have stuff not generated by
  git in .git/objects/??/ we probably have bigger problems than
  prematurely triggering auto gc, can this just be removed as
  redundant. Was this some check e.g. expecting that this would need to
  deal with tempfiles in these directories that we created at the time
  (but no longer do?)?

Am 07.10.18 um 20:28 schrieb Ævar Arnfjörð Bjarmason:

In 2007 Junio wrote
(https://public-inbox.org/git/7vr6lcj2zi.fsf@gitster.siamese.dyndns.org/):

     +static int need_to_gc(void)
     +{
     +	/*
     +	 * Quickly check if a "gc" is needed, by estimating how
     +	 * many loose objects there are.  Because SHA-1 is evenly
     +	 * distributed, we can check only one and get a reasonable
     +	 * estimate.
     +	 */

1. We still have this check of objects/17/ in builtin/gc.c today. Why
    objects/17/ and not e.g. objects/00/ to go with other 000* magic such
    as the 0000000000000000000000000000000000000000 SHA-1?  Statistically
    it doesn't matter, but 17 seems like an odd thing to pick at random
    out of 00..ff, does it have any significance?

The reason is explained in the comment. And, BTW, you do know about this 
one: https://xkcd.com/221/ don't you? (TLDR: the title is "Random Number")

2. It seems overly paranoid to be checking that the files in
   .git/objects/17/ look like a SHA-1. If we have stuff not generated by
   git in .git/objects/??/ we probably have bigger problems than
   prematurely triggering auto gc, can this just be removed as
   redundant. Was this some check e.g. expecting that this would need to
   deal with tempfiles in these directories that we created at the time
   (but no longer do?)?

It's not about that there are SHA-1s in there, it's about how many there 
are.

-- Hannes

On Sun, Oct 07 2018, Johannes Sixt wrote:

Am 07.10.18 um 20:28 schrieb Ævar Arnfjörð Bjarmason:

quoted

In 2007 Junio wrote
(https://public-inbox.org/git/7vr6lcj2zi.fsf@gitster.siamese.dyndns.org/):

     +static int need_to_gc(void)
     +{
     +	/*
     +	 * Quickly check if a "gc" is needed, by estimating how
     +	 * many loose objects there are.  Because SHA-1 is evenly
     +	 * distributed, we can check only one and get a reasonable
     +	 * estimate.
     +	 */

quoted

1. We still have this check of objects/17/ in builtin/gc.c today. Why
    objects/17/ and not e.g. objects/00/ to go with other 000* magic such
    as the 0000000000000000000000000000000000000000 SHA-1?  Statistically
    it doesn't matter, but 17 seems like an odd thing to pick at random
    out of 00..ff, does it have any significance?

The reason is explained in the comment. And, BTW, you do know about
this one: https://xkcd.com/221/ don't you? (TLDR: the title is "Random
Number")

Picking any one number is explained in the comment. I'm asking why 17 in
particular not for correctness reasons but as a bit of historical lore,
and because my ulterior is to improve the GC docs.

The number in that comic is 4 (and no datestamp on when it was
published). Are you saying Junio's patch is somehow a reference to that
xkcd in particular, or that it's just a funny reference in this context?

quoted

2. It seems overly paranoid to be checking that the files in
   .git/objects/17/ look like a SHA-1. If we have stuff not generated by
   git in .git/objects/??/ we probably have bigger problems than
   prematurely triggering auto gc, can this just be removed as
   redundant. Was this some check e.g. expecting that this would need to
   deal with tempfiles in these directories that we created at the time
   (but no longer do?)?

It's not about that there are SHA-1s in there, it's about how many
there are.

Right, I'm wondering if it couldn't be replaced by some general path.c
"number_of_files_in_dir" helper. I.e. why this code is being paranoid
about ignoring the likes of
.git/objects/17/{foo,bar,some-other-garbage}. A number_of_files_in_dir()
would obviously need to ignore "." and "..".

Am 07.10.18 um 21:06 schrieb Ævar Arnfjörð Bjarmason:

Picking any one number is explained in the comment. I'm asking why 17 in
particular not for correctness reasons but as a bit of historical lore,
and because my ulterior is to improve the GC docs.

The number in that comic is 4 (and no datestamp on when it was
published). Are you saying Junio's patch is somehow a reference to that
xkcd in particular, or that it's just a funny reference in this context?

No lore, AFAIR. It's just a random number, determined by a fair dice 
roll or something ;)

-- Hannes