[RFC/PATCH 0/3] git-gui: Don’t trigger garbage collection warning so easily

[RFC/PATCH 0/3] git-gui: Don’t trigger garbage collection warning so easily

[Jonathan Nieder]

Using git gui from a repository tracking (for example) linux-next,
there are almost always enough loose objects to trigger the garbage
collection warning.  The default threshold for this warning is much
more sensitive than the "as many objects as were in v0.99" rule that
the command-line tools use, and the result might be considered even
more invasive because the user has to hit yes before it starts
packing, and it packs more thoroughly than the default gc --auto
settings would.

Moreover, in repositories with many loose objects not referenced only
by the reflog (like linux-next), the gc does not do any good.  Taken
together, these things mean that every time a person runs git gui,
they get a message asking them to forfeit a long period of time to
accomplish nothing.

This problem was reported by Mark Brown at <http://bugs.debian.org/497687>.
My first thought was to add a “don’t ask me again” check box to the
dialog suggesting repacking, but my Tk fu is not up to it. :(

The next best thing would be to make ‘git gui’ use gc --auto itself.
I wonder:

 - Could git gui use git gc?  I understand from commit bc7452f that
   the output of git gc might not be stable enough for git gui to rely
   on, but why not add a --porcelain option whose output is?

 - Should git gui run git gc --auto without asking for permission?
   This is what the command line tools already do; with a GUI one
   could further provide a button to cancel the gc without closing
   the GUI.

This series does the simplest thing I could figure out how to
implement: it builds in the heuristic for counting loose objects from
builtin-gc.c into git-gui (so it will respect the gc.auto
configuration and by default will trigger much less often) and if that
heuristic is satisfied, runs gc --auto.

Advice?  Am I on the right track here?  Any advice for specifying an
interface for a --porcelain option to do this the right way?

Jonathan Nieder (3):
  git-gui: factor out too_many_loose_objects routine from hint_gc
  git-gui: Do not hold the user hostage with a full gc at startup
  git-gui: Do not suggest a gc if gc --auto would not do it

 lib/database.tcl |   40 +++++++++++++++++++++++++++++++++++-----
 1 files changed, 35 insertions(+), 5 deletions(-)

[PATCH 1/3] git-gui: factor out too_many_loose_objects routine from hint_gc

[Jonathan Nieder]

Move the heuristic for deciding whether to try a gc into its own
function, to make it easier to change.  No change in behavior
intended.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 lib/database.tcl |   21 +++++++++++++++++++--
 1 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/lib/database.tcl b/lib/database.tcl
index 1f187ed..25a0509 100644
--- a/lib/database.tcl
+++ b/lib/database.tcl
@@ -88,7 +88,16 @@ proc do_fsck_objects {} {
 	console::exec $w $cmd
 }
 
-proc hint_gc {} {
+proc too_many_loose_objects {} {
+	# 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.
+	#
+	# Roughly based on the function of the same name in builtin-gc.c
+	#
+	# 'git gc' should learn a new --porcelain option
+	# so it can take care of this.
 	set ndirs 1
 	set limit 8
 	if {[is_Windows]} {
@@ -102,7 +111,15 @@ proc hint_gc {} {
 		[gitdir objects 4\[0-[expr {$ndirs-1}]\]/*]]]
 
 	if {$count >= $limit * $ndirs} {
-		set objects_current [expr {$count * 256/$ndirs}]
+		return [expr {$count * 256/$ndirs}]
+	} else {
+		return 0
+	}
+}
+
+proc hint_gc {} {
+	set objects_current [too_many_loose_objects]
+	if {$objects_current != 0} {
 		if {[ask_popup \
 			[mc "This repository currently has approximately %i loose objects.
 
-- 
1.7.0

[PATCH/RFC 2/3] git-gui: Do not hold the user hostage with a full gc at startup

[Jonathan Nieder]

The 'git gc --auto' command is meant to perform an incremental gc,
avoiding problems from an unpacked repository without forcing the user
wait for intense I/O and computation before getting anything done.

The output is not really ideal for a GUI:

 Auto packing the repository for optimum performance. You may also
 run "git gc" manually. See "git help gc" for more information.
 Nothing new to pack.
 Removing duplicate objects: 100% (256/256), done.

git gc should pass the relevant information in machine-readable form
to fix this.

The gui does not already use gc --auto because it was not implemented
until git commit 2c3c439 (2007-09-05), a while after git gui gained
its gc functionality.  Moreover, the gui does not use git gc at all
ever since commit bc7452f (git-gui: Use builtin version of 'git gc',
2007-01-26) for reasons I don’t understand.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 lib/database.tcl |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/lib/database.tcl b/lib/database.tcl
index 25a0509..c8184de 100644
--- a/lib/database.tcl
+++ b/lib/database.tcl
@@ -78,6 +78,12 @@ proc do_gc {} {
 	}
 }
 
+proc do_auto_gc {} {
+	set w [console::new {gc} [mc "Compressing the object database"]]
+	set cmd [list git gc --auto]
+	console::exec $w $cmd
+}
+
 proc do_fsck_objects {} {
 	set w [console::new {fsck-objects} \
 		[mc "Verifying the object database with fsck-objects"]]
@@ -126,7 +132,7 @@ proc hint_gc {} {
 To maintain optimal performance it is strongly recommended that you compress the database.
 
 Compress the database now?" $objects_current]] eq yes} {
-			do_gc
+			do_auto_gc
 		}
 	}
 }
-- 
1.7.0

[PATCH/RFC 3/3] git-gui: Do not suggest a gc if gc --auto would not do it

[Jonathan Nieder]

In particular, if the user has set gc.auto to 0 or some other value,
respect that setting.

The gui-specific heuristic of assuming Windows filesystems will
tolerate fewer loose objects has been carried over.

This patch vastly increases the loose object count threshold for a
sort of compatibility with the console-based git programs.  The ideal
threshold may be different since the user can easily turn down an
offer to gc.  But for now, let us assume that a number of loose
objects that is tolerable when working from the command line will also
be tolerable in the gui.

Unfortunately, this duplicates code from builtin-gc.c.  A proper fix
would be involve teaching gc to cooperate with the gui more closely.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
That’s the end of the series.  Thoughts?

 lib/database.tcl |   13 ++++++++++---
 1 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/lib/database.tcl b/lib/database.tcl
index c8184de..4b07865 100644
--- a/lib/database.tcl
+++ b/lib/database.tcl
@@ -100,15 +100,22 @@ proc too_many_loose_objects {} {
 	# distributed, we can check only one and get a reasonable
 	# estimate.
 	#
-	# Roughly based on the function of the same name in builtin-gc.c
+	# Based on the function of the same name in builtin-gc.c
 	#
 	# 'git gc' should learn a new --porcelain option
 	# so it can take care of this.
+
+	set gc_auto_threshold [get_config gc.auto]
+	if {$gc_auto_threshold eq {}} {
+		set gc_auto_threshold 6700
+	}
+	if {$gc_auto_threshold <= 0} {
+		return 0
+	}
 	set ndirs 1
-	set limit 8
+	set limit [expr {($gc_auto_threshold + 255) / 256}]
 	if {[is_Windows]} {
 		set ndirs 4
-		set limit 1
 	}
 
 	set count [llength [glob \
-- 
1.7.0

Re: [PATCH/RFC 3/3] git-gui: Do not suggest a gc if gc --auto would not do it

[Jonathan Nieder]

Jonathan Nieder wrote:

> The gui-specific heuristic of assuming Windows filesystems will
> tolerate fewer loose objects has been carried over.

Er, was carried over in a previous local patch but not this one.
Sorry for the confusion.

Jonathan

>  	set ndirs 1
> -	set limit 8
> +	set limit [expr {($gc_auto_threshold + 255) / 256}]
>  	if {[is_Windows]} {
>  		set ndirs 4
> -		set limit 1
>  	}

Re: [RFC/PATCH 0/3] git-gui: Don’t trigger garbage collection warning so easily

[Johannes Sixt]

Jonathan Nieder schrieb:
> This series does the simplest thing I could figure out how to
> implement: it builds in the heuristic for counting loose objects from
> builtin-gc.c into git-gui (so it will respect the gc.auto
> configuration and by default will trigger much less often) and if that
> heuristic is satisfied, runs gc --auto.

Any improvement in this respect is appreciated.

Minor nit: By all means while you are in this area, please do not carry 
over this silly "your repository has *approximately* 576 objects" message 
(highlight is mine). "576" is not approximate. "500" is approximate, "750" 
is, too, but "1024" etc. is not. ;-)

-- Hannes