{"thread":{"id":"22711","subject":"[RFC/PATCH 0/3] git-gui: Don’t trigger garbage collection warning so easily","startedAt":"2010-02-18T14:34:32Z","lastAt":"2010-02-18T20:40:55Z","messageCount":6,"participants":["Jonathan Nieder","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"134968","messageId":"20100218143431.GA30486@progeny.tock","threadId":"22711","inReplyTo":null,"subject":"[RFC/PATCH 0/3] git-gui: Don’t trigger garbage collection warning so easily","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-02-18T14:34:32Z","receivedAt":"2010-02-18T14:34:32Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Using git gui from a repository tracking (for example) linux-next,\nthere are almost always enough loose objects to trigger the garbage\ncollection warning. The default threshold for this warning is much\nmore sensitive than the \"as many objects as were in v0.99\" rule that\nthe command-line tools use, and the result might be considered even\nmore invasive because the user has to hit yes before it starts\npacking, and it packs more thoroughly than the default gc --auto\nsettings would.\n\nMoreover, in repositories with many loose objects not referenced only\nby the reflog (like linux-next), the gc does not do any good. Taken\ntogether, these things mean that every time a person runs git gui,\nthey get a message asking them to forfeit a long period of time to\naccomplish nothing.\n\nThis problem was reported by Mark Brown at .\nMy first thought was to add a “don’t ask me again” check box to the\ndialog suggesting repacking, but my Tk fu is not up to it. :(\n\nThe next best thing would be to make ‘git gui’ use gc --auto itself.\nI wonder:\n\n - Could git gui use git gc? I understand from commit bc7452f that\n the output of git gc might not be stable enough for git gui to rely\n on, but why not add a --porcelain option whose output is?\n\n - Should git gui run git gc --auto without asking for permission?\n This is what the command line tools already do; with a GUI one\n could further provide a button to cancel the gc without closing\n the GUI.\n\nThis series does the simplest thing I could figure out how to\nimplement: it builds in the heuristic for counting loose objects from\nbuiltin-gc.c into git-gui (so it will respect the gc.auto\nconfiguration and by default will trigger much less often) and if that\nheuristic is satisfied, runs gc --auto.\n\nAdvice? Am I on the right track here? Any advice for specifying an\ninterface for a --porcelain option to do this the right way?\n\nJonathan Nieder (3):\n git-gui: factor out too_many_loose_objects routine from hint_gc\n git-gui: Do not hold the user hostage with a full gc at startup\n git-gui: Do not suggest a gc if gc --auto would not do it\n\n lib/database.tcl | 40 +++++++++++++++++++++++++++++++++++-----\n 1 files changed, 35 insertions(+), 5 deletions(-)\n"},{"id":"134969","messageId":"20100218143558.GA30520@progeny.tock","threadId":"22711","inReplyTo":"20100218143431.GA30486@progeny.tock","subject":"[PATCH 1/3] git-gui: factor out too_many_loose_objects routine from hint_gc","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-02-18T14:35:58Z","receivedAt":"2010-02-18T14:35:58Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Move the heuristic for deciding whether to try a gc into its own\nfunction, to make it easier to change. No change in behavior\nintended.\n\nSigned-off-by: Jonathan Nieder \n---\n lib/database.tcl | 21 +++++++++++++++++++--\n 1 files changed, 19 insertions(+), 2 deletions(-)\n\ndiff --git a/lib/database.tcl b/lib/database.tcl\nindex 1f187ed..25a0509 100644\n--- a/lib/database.tcl\n+++ b/lib/database.tcl\n@@ -88,7 +88,16 @@ proc do_fsck_objects {} {\n \tconsole::exec $w $cmd\n }\n \n-proc hint_gc {} {\n+proc too_many_loose_objects {} {\n+\t# Quickly check if a \"gc\" is needed, by estimating how\n+\t# many loose objects there are. Because SHA-1 is evenly\n+\t# distributed, we can check only one and get a reasonable\n+\t# estimate.\n+\t#\n+\t# Roughly based on the function of the same name in builtin-gc.c\n+\t#\n+\t# 'git gc' should learn a new --porcelain option\n+\t# so it can take care of this.\n \tset ndirs 1\n \tset limit 8\n \tif {[is_Windows]} {\n@@ -102,7 +111,15 @@ proc hint_gc {} {\n \t\t[gitdir objects 4\\[0-[expr {$ndirs-1}]\\]/*]]]\n \n \tif {$count >= $limit * $ndirs} {\n-\t\tset objects_current [expr {$count * 256/$ndirs}]\n+\t\treturn [expr {$count * 256/$ndirs}]\n+\t} else {\n+\t\treturn 0\n+\t}\n+}\n+\n+proc hint_gc {} {\n+\tset objects_current [too_many_loose_objects]\n+\tif {$objects_current != 0} {\n \t\tif {[ask_popup \\\n \t\t\t[mc \"This repository currently has approximately %i loose objects.\n \n-- \n1.7.0\n"},{"id":"134970","messageId":"20100218143903.GB30520@progeny.tock","threadId":"22711","inReplyTo":"20100218143431.GA30486@progeny.tock","subject":"[PATCH/RFC 2/3] git-gui: Do not hold the user hostage with a full gc at startup","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-02-18T14:39:03Z","receivedAt":"2010-02-18T14:39:03Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"The 'git gc --auto' command is meant to perform an incremental gc,\navoiding problems from an unpacked repository without forcing the user\nwait for intense I/O and computation before getting anything done.\n\nThe output is not really ideal for a GUI:\n\n Auto packing the repository for optimum performance. You may also\n run \"git gc\" manually. See \"git help gc\" for more information.\n Nothing new to pack.\n Removing duplicate objects: 100% (256/256), done.\n\ngit gc should pass the relevant information in machine-readable form\nto fix this.\n\nThe gui does not already use gc --auto because it was not implemented\nuntil git commit 2c3c439 (2007-09-05), a while after git gui gained\nits gc functionality. Moreover, the gui does not use git gc at all\never since commit bc7452f (git-gui: Use builtin version of 'git gc',\n2007-01-26) for reasons I don’t understand.\n\nSigned-off-by: Jonathan Nieder \n---\n lib/database.tcl | 8 +++++++-\n 1 files changed, 7 insertions(+), 1 deletions(-)\n\ndiff --git a/lib/database.tcl b/lib/database.tcl\nindex 25a0509..c8184de 100644\n--- a/lib/database.tcl\n+++ b/lib/database.tcl\n@@ -78,6 +78,12 @@ proc do_gc {} {\n \t}\n }\n \n+proc do_auto_gc {} {\n+\tset w [console::new {gc} [mc \"Compressing the object database\"]]\n+\tset cmd [list git gc --auto]\n+\tconsole::exec $w $cmd\n+}\n+\n proc do_fsck_objects {} {\n \tset w [console::new {fsck-objects} \\\n \t\t[mc \"Verifying the object database with fsck-objects\"]]\n@@ -126,7 +132,7 @@ proc hint_gc {} {\n To maintain optimal performance it is strongly recommended that you compress the database.\n \n Compress the database now?\" $objects_current]] eq yes} {\n-\t\t\tdo_gc\n+\t\t\tdo_auto_gc\n \t\t}\n \t}\n }\n-- \n1.7.0\n"},{"id":"134971","messageId":"20100218144122.GC30520@progeny.tock","threadId":"22711","inReplyTo":"20100218143431.GA30486@progeny.tock","subject":"[PATCH/RFC 3/3] git-gui: Do not suggest a gc if gc --auto would not do it","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-02-18T14:41:22Z","receivedAt":"2010-02-18T14:41:22Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"In particular, if the user has set gc.auto to 0 or some other value,\nrespect that setting.\n\nThe gui-specific heuristic of assuming Windows filesystems will\ntolerate fewer loose objects has been carried over.\n\nThis patch vastly increases the loose object count threshold for a\nsort of compatibility with the console-based git programs. The ideal\nthreshold may be different since the user can easily turn down an\noffer to gc. But for now, let us assume that a number of loose\nobjects that is tolerable when working from the command line will also\nbe tolerable in the gui.\n\nUnfortunately, this duplicates code from builtin-gc.c. A proper fix\nwould be involve teaching gc to cooperate with the gui more closely.\n\nSigned-off-by: Jonathan Nieder \n---\nThat’s the end of the series. Thoughts?\n\n lib/database.tcl | 13 ++++++++++---\n 1 files changed, 10 insertions(+), 3 deletions(-)\n\ndiff --git a/lib/database.tcl b/lib/database.tcl\nindex c8184de..4b07865 100644\n--- a/lib/database.tcl\n+++ b/lib/database.tcl\n@@ -100,15 +100,22 @@ proc too_many_loose_objects {} {\n \t# distributed, we can check only one and get a reasonable\n \t# estimate.\n \t#\n-\t# Roughly based on the function of the same name in builtin-gc.c\n+\t# Based on the function of the same name in builtin-gc.c\n \t#\n \t# 'git gc' should learn a new --porcelain option\n \t# so it can take care of this.\n+\n+\tset gc_auto_threshold [get_config gc.auto]\n+\tif {$gc_auto_threshold eq {}} {\n+\t\tset gc_auto_threshold 6700\n+\t}\n+\tif {$gc_auto_threshold <= 0} {\n+\t\treturn 0\n+\t}\n \tset ndirs 1\n-\tset limit 8\n+\tset limit [expr {($gc_auto_threshold + 255) / 256}]\n \tif {[is_Windows]} {\n \t\tset ndirs 4\n-\t\tset limit 1\n \t}\n \n \tset count [llength [glob \\\n-- \n1.7.0\n"},{"id":"134977","messageId":"20100218154911.GA32372@progeny.tock","threadId":"22711","inReplyTo":"20100218144122.GC30520@progeny.tock","subject":"Re: [PATCH/RFC 3/3] git-gui: Do not suggest a gc if gc --auto would not do it","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-02-18T15:49:12Z","receivedAt":"2010-02-18T15:49:12Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jonathan Nieder wrote:\n\n> The gui-specific heuristic of assuming Windows filesystems will\n> tolerate fewer loose objects has been carried over.\n\nEr, was carried over in a previous local patch but not this one.\nSorry for the confusion.\n\nJonathan\n\n> \tset ndirs 1\n> -\tset limit 8\n> +\tset limit [expr {($gc_auto_threshold + 255) / 256}]\n> \tif {[is_Windows]} {\n> \t\tset ndirs 4\n> -\t\tset limit 1\n> \t}\n"},{"id":"135007","messageId":"4B7DA5D7.20704@kdbg.org","threadId":"22711","inReplyTo":"20100218143431.GA30486@progeny.tock","subject":"Re: [RFC/PATCH 0/3] git-gui: Don’t trigger garbage collection warning so easily","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2010-02-18T20:40:55Z","receivedAt":"2010-02-18T20:40:55Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Jonathan Nieder schrieb:\n> This series does the simplest thing I could figure out how to\n> implement: it builds in the heuristic for counting loose objects from\n> builtin-gc.c into git-gui (so it will respect the gc.auto\n> configuration and by default will trigger much less often) and if that\n> heuristic is satisfied, runs gc --auto.\n\nAny improvement in this respect is appreciated.\n\nMinor nit: By all means while you are in this area, please do not carry \nover this silly \"your repository has *approximately* 576 objects\" message \n(highlight is mine). \"576\" is not approximate. \"500\" is approximate, \"750\" \nis, too, but \"1024\" etc. is not. ;-)\n\n-- Hannes\n"}]}