[PATCH] gc --auto: warn gc will soon run, give users a chance to run manually

[PATCH] gc --auto: warn gc will soon run, give users a chance to run manually

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 I hate it every single time git hangs because gc is activated.
 Opening another terminal is an option but I would lose all terminal
 settings I have on the current one (e.g. access to suspended vim
 sessions).

 I don't think gc_warn_* need their own config vars. Hopefully
 hardcoded offset is good enough.

 builtin/gc.c |   19 +++++++++++++++++--
 1 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index 0498094..1f4555e 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -25,7 +25,9 @@ static const char * const builtin_gc_usage[] = {
 static int pack_refs = 1;
 static int aggressive_window = 250;
 static int gc_auto_threshold = 6700;
+static int gc_warn_auto_threshold = 6600;
 static int gc_auto_pack_limit = 50;
+static int gc_warn_auto_pack_limit = 45;
 static const char *prune_expire = "2.weeks.ago";
 
 #define MAX_ADD 10
@@ -50,10 +52,12 @@ static int gc_config(const char *var, const char *value, void *cb)
 	}
 	if (!strcmp(var, "gc.auto")) {
 		gc_auto_threshold = git_config_int(var, value);
+		gc_warn_auto_threshold = gc_auto_threshold - 100;
 		return 0;
 	}
 	if (!strcmp(var, "gc.autopacklimit")) {
 		gc_auto_pack_limit = git_config_int(var, value);
+		gc_warn_auto_pack_limit = gc_auto_pack_limit - 5;
 		return 0;
 	}
 	if (!strcmp(var, "gc.pruneexpire")) {
@@ -118,7 +122,13 @@ static int too_many_loose_objects(void)
 		}
 	}
 	closedir(dir);
-	return needed;
+	if (needed)
+		return 1;
+
+	auto_threshold = (gc_warn_auto_threshold + 255) / 256;
+	if (num_loose > auto_threshold)
+		warning(_("Too many loose objects. \"git gc\" will soon run automatically"));
+	return 0;
 }
 
 static int too_many_packs(void)
@@ -141,7 +151,12 @@ static int too_many_packs(void)
 		 */
 		cnt++;
 	}
-	return gc_auto_pack_limit <= cnt;
+	if (gc_auto_pack_limit <= cnt)
+		return 1;
+
+	if (gc_warn_auto_pack_limit <= cnt)
+		warning(_("Too many packs, \"git gc\" will soon run automatically."));
+	return 0;
 }
 
 static int need_to_gc(void)
-- 
1.7.4.74.g639db

[PATCH] Introduce gc.autowarnonly config option

[Fernando Vezzosi]

When `git gc --auto` would detect need for garbage collection to run, it
would just run.  With this patch, enabling gc.autowarnonly will instead
make it just emit a warning.

Reviewed-by: Sverre Rabbelier <srabbelier@gmail.com>
Signed-off-by: Fernando Vezzosi <buccia@repnz.net>
---
 builtin/gc.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index 0498094..65b6616 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -26,6 +26,7 @@ static int pack_refs = 1;
 static int aggressive_window = 250;
 static int gc_auto_threshold = 6700;
 static int gc_auto_pack_limit = 50;
+static int gc_auto_warn_only = 0;
 static const char *prune_expire = "2.weeks.ago";
 
 #define MAX_ADD 10
@@ -64,6 +65,10 @@ static int gc_config(const char *var, const char *value, void *cb)
 		}
 		return git_config_string(&prune_expire, var, value);
 	}
+	if (!strcmp(var, "gc.autowarnonly")) {
+		gc_auto_warn_only = git_config_bool(var, value);
+		return 0;
+	}
 	return git_default_config(var, value, cb);
 }
 
@@ -219,6 +224,15 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
 		 */
 		if (!need_to_gc())
 			return 0;
+
+		if (gc_auto_warn_only){
+			fprintf(stderr,
+					_("Pack the repository for optimum performance by running\n"
+					"\"git gc\" manually. See "
+					"\"git help gc\" for more information.\n"));
+			return 0;
+		}
+
 		if (quiet)
 			fprintf(stderr, _("Auto packing the repository for optimum performance.\n"));
 		else
-- 
1.7.5.3

[PATCH 2/1] gc --auto: warn gc will soon run, give users a chance to run manually

[Fernando Vezzosi]

Signed-off-by: Fernando Vezzosi <buccia@repnz.net>
---

Rebased Nguyễn's patch on top of mine.

 builtin/gc.c |   19 +++++++++++++++++--
 1 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index 65b6616..ca620e3 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -25,8 +25,10 @@ static const char * const builtin_gc_usage[] = {
 static int pack_refs = 1;
 static int aggressive_window = 250;
 static int gc_auto_threshold = 6700;
+static int gc_warn_auto_threshold = 6600;
 static int gc_auto_pack_limit = 50;
 static int gc_auto_warn_only = 0;
+static int gc_warn_auto_pack_limit = 45;
 static const char *prune_expire = "2.weeks.ago";
 
 #define MAX_ADD 10
@@ -51,10 +53,12 @@ static int gc_config(const char *var, const char *value, void *cb)
 	}
 	if (!strcmp(var, "gc.auto")) {
 		gc_auto_threshold = git_config_int(var, value);
+		gc_warn_auto_threshold = gc_auto_threshold - 100;
 		return 0;
 	}
 	if (!strcmp(var, "gc.autopacklimit")) {
 		gc_auto_pack_limit = git_config_int(var, value);
+		gc_warn_auto_pack_limit = gc_auto_pack_limit - 5;
 		return 0;
 	}
 	if (!strcmp(var, "gc.pruneexpire")) {
@@ -123,7 +127,13 @@ static int too_many_loose_objects(void)
 		}
 	}
 	closedir(dir);
-	return needed;
+	if (needed)
+		return 1;
+
+	auto_threshold = (gc_warn_auto_threshold + 255) / 256;
+	if (num_loose > auto_threshold)
+		warning(_("Too many loose objects. \"git gc\" will soon run automatically"));
+	return 0;
 }
 
 static int too_many_packs(void)
@@ -146,7 +156,12 @@ static int too_many_packs(void)
 		 */
 		cnt++;
 	}
-	return gc_auto_pack_limit <= cnt;
+	if (gc_auto_pack_limit <= cnt)
+		return 1;
+
+	if (gc_warn_auto_pack_limit <= cnt)
+		warning(_("Too many packs, \"git gc\" will soon run automatically."));
+	return 0;
 }
 
 static int need_to_gc(void)
-- 
1.7.5.3

Re: [PATCH 2/1] gc --auto: warn gc will soon run, give users a chance to run manually

[Nguyen Thai Ngoc Duy]

On Sat, Nov 5, 2011 at 5:33 PM, Fernando Vezzosi <buccia@repnz.net> wrote:
> Signed-off-by: Fernando Vezzosi <buccia@repnz.net>
> ---
>
> Rebased Nguyễn's patch on top of mine.

I think when gc.autowarnonly is true, my patch should be no-op because
you'll get warnings eventually when you hit the thresholds.
-- 
Duy

[PATCH 2/1] gc --auto: warn gc will soon run, give users a chance to run manually

[Fernando Vezzosi]

Signed-off-by: Fernando Vezzosi <buccia@repnz.net>
---

Rebased Nguyễn's patch on top of mine.

 builtin/gc.c |   19 +++++++++++++++++--
 1 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index 65b6616..ca620e3 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -25,8 +25,10 @@ static const char * const builtin_gc_usage[] = {
 static int pack_refs = 1;
 static int aggressive_window = 250;
 static int gc_auto_threshold = 6700;
+static int gc_warn_auto_threshold = 6600;
 static int gc_auto_pack_limit = 50;
 static int gc_auto_warn_only = 0;
+static int gc_warn_auto_pack_limit = 45;
 static const char *prune_expire = "2.weeks.ago";
 
 #define MAX_ADD 10
@@ -51,10 +53,12 @@ static int gc_config(const char *var, const char *value, void *cb)
 	}
 	if (!strcmp(var, "gc.auto")) {
 		gc_auto_threshold = git_config_int(var, value);
+		gc_warn_auto_threshold = gc_auto_threshold - 100;
 		return 0;
 	}
 	if (!strcmp(var, "gc.autopacklimit")) {
 		gc_auto_pack_limit = git_config_int(var, value);
+		gc_warn_auto_pack_limit = gc_auto_pack_limit - 5;
 		return 0;
 	}
 	if (!strcmp(var, "gc.pruneexpire")) {
@@ -123,7 +127,13 @@ static int too_many_loose_objects(void)
 		}
 	}
 	closedir(dir);
-	return needed;
+	if (needed)
+		return 1;
+
+	auto_threshold = (gc_warn_auto_threshold + 255) / 256;
+	if (num_loose > auto_threshold)
+		warning(_("Too many loose objects. \"git gc\" will soon run automatically"));
+	return 0;
 }
 
 static int too_many_packs(void)
@@ -146,7 +156,12 @@ static int too_many_packs(void)
 		 */
 		cnt++;
 	}
-	return gc_auto_pack_limit <= cnt;
+	if (gc_auto_pack_limit <= cnt)
+		return 1;
+
+	if (gc_warn_auto_pack_limit <= cnt)
+		warning(_("Too many packs, \"git gc\" will soon run automatically."));
+	return 0;
 }
 
 static int need_to_gc(void)
-- 
1.7.5.3

Re: [PATCH 2/1] gc --auto: warn gc will soon run, give users a chance to run manually

[Junio C Hamano]

Fernando Vezzosi <buccia@repnz.net> writes:

> Signed-off-by: Fernando Vezzosi <buccia@repnz.net>
> ---
>
> Rebased Nguyễn's patch on top of mine.

You don't have to do this.

Re: [PATCH] Introduce gc.autowarnonly config option

[Sverre Rabbelier]

Heya,

On Sat, Nov 5, 2011 at 14:39, Fernando Vezzosi <buccia@repnz.net> wrote:
> When `git gc --auto` would detect need for garbage collection to run, it
> would just run.  With this patch, enabling gc.autowarnonly will instead
> make it just emit a warning.
>
> Reviewed-by: Sverre Rabbelier <srabbelier@gmail.com>
> Signed-off-by: Fernando Vezzosi <buccia@repnz.net>

Highly relevant considering recent (3 hours ago) patch that instead
adds a warning that gc will happen soon.

-- 
Cheers,

Sverre Rabbelier

Re: [PATCH] Introduce gc.autowarnonly config option

[Jay Soffian]

On Sat, Nov 5, 2011 at 9:39 AM, Fernando Vezzosi <buccia@repnz.net> wrote:
> When `git gc --auto` would detect need for garbage collection to run, it
> would just run.  With this patch, enabling gc.autowarnonly will instead
> make it just emit a warning.
>
> Reviewed-by: Sverre Rabbelier <srabbelier@gmail.com>
> Signed-off-by: Fernando Vezzosi <buccia@repnz.net>

This is much better than the solution I've been living with the last
year, which was to put a pre-auto-gc in my templates directory of:

#!/bin/sh
echo "time to run git gc"
exit 1

So, thank you.

j.

Re: [PATCH] Introduce gc.autowarnonly config option

[Sverre Rabbelier]

On Sat, Nov 5, 2011 at 08:39, Fernando Vezzosi <buccia@repnz.net> wrote:
> When `git gc --auto` would detect need for garbage collection to run, it
> would just run.  With this patch, enabling gc.autowarnonly will instead
> make it just emit a warning.

Does anyone know what happened to this patch?

-- 
Cheers,

Sverre Rabbelier

Re: [PATCH] Introduce gc.autowarnonly config option

[Junio C Hamano]

Sverre Rabbelier <srabbelier@gmail.com> writes:

> On Sat, Nov 5, 2011 at 08:39, Fernando Vezzosi <buccia@repnz.net> wrote:
>> When `git gc --auto` would detect need for garbage collection to run, it
>> would just run. With this patch, enabling gc.autowarnonly will instead
>> make it just emit a warning.
>
> Does anyone know what happened to this patch?

If I recall correctly, it was pointed out that people who would get
annoyed by unexpected triggering of an auto gc would get annoyed by
constant nagging by this warning until they reach the point that
they can stop and run gc themselves, and the patch was never updated
to take that issue into account.

Re: [PATCH] gc --auto: warn gc will soon run, give users a chance to run manually

[Matthieu Moy]

Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:

> +		warning(_("Too many loose objects. \"git gc\" will soon run automatically"));
[...]
> +		warning(_("Too many packs, \"git gc\" will soon run automatically."));

Nit: you have a period in the second message, but not for the first.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] gc --auto: warn gc will soon run, give users a chance to run manually

[Junio C Hamano]

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  I hate it every single time git hangs because gc is activated.
>  Opening another terminal is an option but I would lose all terminal
>  settings I have on the current one (e.g. access to suspended vim
>  sessions).
>
>  I don't think gc_warn_* need their own config vars. Hopefully
>  hardcoded offset is good enough.

Let's think about How the user chooses a custom value for the threshold
(or accepts that the default threshold value 6600 is good enough).

In order to minimize the time the user has to endure when the eventual
auto-gc kicks in, the value needs to be small enough, and to minimize the
frequency the auto-gc kicks in, the value needs to be large enough. It is
a balancing act between the two.

Slower users (those who do not create that many objects within a given
time period) can afford to have a small threashold and do not have to
suffer too frequent auto-gc. Busier ones would rather choose larger
threashold to avoid frequent auto-gc because they create many objects
within a given time period.

The reason why the user would want a warning is probably because the user
sometimes is too busy and cannot afford the disruption of auto-gc, but the
busy period luckily does not last indefinitely, e.g. if a warning is given
two days before the auto-gc actually kicks in, then within that two days
there is a window of time that the user can manually run gc while fetching
a cup of coffee. For different users, the "two days" in this example would
be different. Some people would need longer warning period. Some people
can live with shorter warning period. The number of objects before the
next auto-gc run, divided by the number of objects the user would create
on average in an hour, would give the length of the warning period.

Now, I suspect that busier users would need longer window than slower
ones, which leads me to suspect that a good default would not be a
hardcoded constant offset (e.g. once you see the new warning, the auto-gc
kicks in after creating 100 objects) but some ratio of the threshold
(e.g. you have 2% margin after getting a warning before the auto-gc kicks
in).

Re: [PATCH] gc --auto: warn gc will soon run, give users a chance to run manually

[Nguyen Thai Ngoc Duy]

2011/11/7 Junio C Hamano <gitster@pobox.com>:
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>> ---
>>  I hate it every single time git hangs because gc is activated.
>>  Opening another terminal is an option but I would lose all terminal
>>  settings I have on the current one (e.g. access to suspended vim
>>  sessions).
>>
>>  I don't think gc_warn_* need their own config vars. Hopefully
>>  hardcoded offset is good enough.
>
> Let's think about How the user chooses a custom value for the threshold
> (or accepts that the default threshold value 6600 is good enough).
>
> ...
>
> Now, I suspect that busier users would need longer window than slower
> ones, which leads me to suspect that a good default would not be a
> hardcoded constant offset (e.g. once you see the new warning, the auto-gc
> kicks in after creating 100 objects) but some ratio of the threshold
> (e.g. you have 2% margin after getting a warning before the auto-gc kicks
> in).

Speaking for myself, as soon as I see the warning, I would immediately
start a new shell for git-gc alone and get back to my work (hopefully
gc will not be activated again while another gc is running) so one or
maybe three warnings may be enough. But yes, some ratio may be better
for different users.
-- 
Duy