threads / rfc / 22711

RFC patch, 3 partsgit-gui: Don’t trigger garbage collection warning so easily

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

## tl;dr

6 messages between Feb 18, 2010 and Feb 18, 2010. Diffs are folded; open one to read it.

replies: 5people: 2as markdown or json

Jonathan Nieder· Feb 18, 2010, 14:34 UTC · lore

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(-)
Jonathan Nieder· Feb 18, 2010, 14:35 UTC · re: Jonathan Nieder · lore

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

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(-)
Show changes to lib/database.tcl +19 −2
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
Jonathan Nieder· Feb 18, 2010, 14:39 UTC · re: Jonathan Nieder · lore

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

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(-)
Show changes to lib/database.tcl +7 −1
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
Jonathan Nieder· Feb 18, 2010, 14:41 UTC · re: Jonathan Nieder · lore

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

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(-)
Show changes to lib/database.tcl +10 −3
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
Jonathan Nieder· Feb 18, 2010, 15:49 UTC · re: Jonathan Nieder · lore

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

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
Show 7 quoted lines
>  	set ndirs 1
> -	set limit 8
> +	set limit [expr {($gc_auto_threshold + 255) / 256}]
>  	if {[is_Windows]} {
>  		set ndirs 4
> -		set limit 1
>  	}
Johannes Sixt· Feb 18, 2010, 20:40 UTC · re: Jonathan Nieder · lore

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

Jonathan Nieder schrieb:
Show 5 quoted lines
> 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

← back to recent threads