{"thread":{"id":"42708","subject":"[RFC] gc: correct gc.autoPackLimit documentation","startedAt":"2016-06-25T01:14:57Z","lastAt":"2016-06-27T19:39:09Z","messageCount":6,"participants":["Eric Wong","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"290091","messageId":"20160625011450.GA14293@dcvr.yhbt.net","threadId":"42708","inReplyTo":null,"subject":"[RFC] gc: correct gc.autoPackLimit documentation","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2016-06-25T01:14:50Z","receivedAt":"2016-06-25T01:14:57Z","isPatch":false,"sender":{"key":"e@80x24.org","avatar":null},"body":"I'm not sure if this is the best approach, or if changing\ntoo_many_packs can be done without causing problems for\nhosts of big repos.\n\n-------8<-----\nSubject: [PATCH] gc: correct gc.autoPackLimit documentation\n\nI want to ensure there is only one pack in my repo to take\nadvantage of pack bitmaps.  Based on my reading of the\ndocumentation, I configured gc.autoPackLimit=1 which led to\n\"gc --auto\" constantly trying to repack on every invocation.\n\nUpdate the documentation to reflect what is probably a\nlong-standing off-by-one bug in builtin/gc.c::too_many_packs:\n\n\t-\treturn gc_auto_pack_limit <= cnt;\n\t+\treturn gc_auto_pack_limit < cnt;\n\nHowever, changing gc itself at this time may cause problems\nfor people who are already using gc.autoPackLimit=2 and\nexpect bitmaps to work for them.\n\nSigned-off-by: Eric Wong <e@80x24.org>\n---\n Documentation/config.txt | 2 +-\n Documentation/git-gc.txt | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 2e1b2e4..b0de3f1 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1345,7 +1345,7 @@ gc.auto::\n \tdefault value is 6700.  Setting this to 0 disables it.\n \n gc.autoPackLimit::\n-\tWhen there are more than this many packs that are not\n+\tWhen there at least this many packs that are not\n \tmarked with `*.keep` file in the repository, `git gc\n \t--auto` consolidates them into one larger pack.  The\n \tdefault\tvalue is 50.  Setting this to 0 disables it.\ndiff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt\nindex fa15104..658612d 100644\n--- a/Documentation/git-gc.txt\n+++ b/Documentation/git-gc.txt\n@@ -54,7 +54,7 @@ all loose objects are combined into a single pack using\n `git repack -d -l`.  Setting the value of `gc.auto` to 0\n disables automatic packing of loose objects.\n +\n-If the number of packs exceeds the value of `gc.autoPackLimit`,\n+If the number of packs matches or exceeds the value of `gc.autoPackLimit`,\n then existing packs (except those marked with a `.keep` file)\n are consolidated into a single pack by using the `-A` option of\n 'git repack'. Setting `gc.autoPackLimit` to 0 disables\n-- \nEW\n"},{"id":"290092","messageId":"20160625020620.GA31290@sigill.intra.peff.net","threadId":"42708","inReplyTo":"20160625011450.GA14293@dcvr.yhbt.net","subject":"Re: [RFC] gc: correct gc.autoPackLimit documentation","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-25T02:06:20Z","receivedAt":"2016-06-25T02:06:29Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jun 25, 2016 at 01:14:50AM +0000, Eric Wong wrote:\n\n> I'm not sure if this is the best approach, or if changing\n> too_many_packs can be done without causing problems for\n> hosts of big repos.\n> \n> -------8<-----\n> Subject: [PATCH] gc: correct gc.autoPackLimit documentation\n> \n> I want to ensure there is only one pack in my repo to take\n> advantage of pack bitmaps.  Based on my reading of the\n> documentation, I configured gc.autoPackLimit=1 which led to\n> \"gc --auto\" constantly trying to repack on every invocation.\n\nI'm not sure if you might be misinterpreting earlier advice on bitmaps\nhere. At the time of packing, bitmaps need for all of the objects to go\nto a single pack (they cannot handle a case where one object in the pack\ncan reach another object that is not in the pack). But that is easily\ndone with \"git repack -adb\".\n\nAfter that packing, you can add new packs that do not have bitmaps, and\nthe bitmaps will gracefully degrade. E.g., imagine master was at tip X\nwhen you repacked with bitmaps, and now somebody has pushed to make it\ntip Y.  Somebody then clones, asking for Y. The bitmap code will start\nat Y and walk backwards. When it hits X, it stops walking as it can fill\nin the rest of the reachability from there.\n\nSo you do have to walk X..Y the old-fashioned way, but that's generally\nnot a big problem for a few pushes.\n\nIOW, I think trying to repack on every single push is probably overkill.\nYes, it will buy you a little savings on fetch requests, but whether it\nis worthwhile to pack depends on:\n\n - how big the push was (e.g., 2 commits versus thousands; the bigger\n   it is, the more you save per fetch\n\n - how big the repo is (the bigger it is, the more it costs to do the\n   repack; packing is linear-ish effort in the number of objects in the\n   repo)\n\n - how often you get fetches versus pushes (your cost is amortized\n   across all the fetches)\n\nThere are numbers where it can be worth it to pack really aggressively,\nbut I doubt it's common. At GitHub we use a combination of number of\npacks (and we try to keep it under 50) and size of objects not in the\n\"main\" pack (I did a bunch of fancy logging and analysis of object\ncounts, bytes in packs, etc, at one point, and we basically realized\nthat for the common cases, all of the interesting metrics are roughly\nproportional to the number of bytes that could be moved into the main\npack).\n\nThat's neither here nor there for the off-by-one in gc or its\ndocumentation, of course, but just FYI.\n\n-Peff\n"},{"id":"290093","messageId":"20160625025300.GA29053@dcvr.yhbt.net","threadId":"42708","inReplyTo":"20160625020620.GA31290@sigill.intra.peff.net","subject":"Re: [RFC] gc: correct gc.autoPackLimit documentation","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2016-06-25T02:53:00Z","receivedAt":"2016-06-25T02:53:07Z","isPatch":false,"sender":{"key":"e@80x24.org","avatar":null},"body":"Jeff King <peff@peff.net> wrote:\n> On Sat, Jun 25, 2016 at 01:14:50AM +0000, Eric Wong wrote:\n> \n> > I'm not sure if this is the best approach, or if changing\n> > too_many_packs can be done without causing problems for\n> > hosts of big repos.\n> > \n> > -------8<-----\n> > Subject: [PATCH] gc: correct gc.autoPackLimit documentation\n> > \n> > I want to ensure there is only one pack in my repo to take\n> > advantage of pack bitmaps.  Based on my reading of the\n> > documentation, I configured gc.autoPackLimit=1 which led to\n> > \"gc --auto\" constantly trying to repack on every invocation.\n> \n> I'm not sure if you might be misinterpreting earlier advice on bitmaps\n> here. At the time of packing, bitmaps need for all of the objects to go\n> to a single pack (they cannot handle a case where one object in the pack\n> can reach another object that is not in the pack). But that is easily\n> done with \"git repack -adb\".\n> \n> After that packing, you can add new packs that do not have bitmaps, and\n> the bitmaps will gracefully degrade. E.g., imagine master was at tip X\n> when you repacked with bitmaps, and now somebody has pushed to make it\n> tip Y.  Somebody then clones, asking for Y. The bitmap code will start\n> at Y and walk backwards. When it hits X, it stops walking as it can fill\n> in the rest of the reachability from there.\n\nAh, thanks, makes sense.  I was misinterpreting earlier advice\non bitmaps.\n\n> That's neither here nor there for the off-by-one in gc or its\n> documentation, of course, but just FYI.\n\nI'm now inclined to fix the problem in gc and leave the\ndocumentation as-is (unless it cause other problems...)\n"},{"id":"290108","messageId":"xmqqa8i9eqwn.fsf@gitster.mtv.corp.google.com","threadId":"42708","inReplyTo":"20160625025300.GA29053@dcvr.yhbt.net","subject":"Re: [RFC] gc: correct gc.autoPackLimit documentation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-25T06:14:32Z","receivedAt":"2016-06-25T06:14:40Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Wong <e@80x24.org> writes:\n\n>> That's neither here nor there for the off-by-one in gc or its\n>> documentation, of course, but just FYI.\n>\n> I'm now inclined to fix the problem in gc and leave the\n> documentation as-is (unless it cause other problems...)\n\nI think that is the best in the longer term.\n"},{"id":"290112","messageId":"20160625064647.GA20659@dcvr.yhbt.net","threadId":"42708","inReplyTo":"xmqqa8i9eqwn.fsf@gitster.mtv.corp.google.com","subject":"[PATCH] gc: fix off-by-one error with gc.autoPackLimit","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2016-06-25T06:46:47Z","receivedAt":"2016-06-25T06:47:23Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"This matches the documentation and allows gc.autoPackLimit=1\nto maintain a single pack without attempting a repack on every\n\"git gc --auto\" invocation.\n\nSigned-off-by: Eric Wong <e@80x24.org>\n---\n builtin/gc.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex c583aad..332bcf7 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -177,7 +177,7 @@ static int too_many_packs(void)\n \t\t */\n \t\tcnt++;\n \t}\n-\treturn gc_auto_pack_limit <= cnt;\n+\treturn gc_auto_pack_limit < cnt;\n }\n \n static void add_repack_all_option(void)\n-- \nEW\n"},{"id":"290320","messageId":"20160627193842.GD10877@sigill.intra.peff.net","threadId":"42708","inReplyTo":"20160625064647.GA20659@dcvr.yhbt.net","subject":"Re: [PATCH] gc: fix off-by-one error with gc.autoPackLimit","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-27T19:38:42Z","receivedAt":"2016-06-27T19:39:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jun 25, 2016 at 06:46:47AM +0000, Eric Wong wrote:\n\n> This matches the documentation and allows gc.autoPackLimit=1\n> to maintain a single pack without attempting a repack on every\n> \"git gc --auto\" invocation.\n> \n> Signed-off-by: Eric Wong <e@80x24.org>\n> ---\n>  builtin/gc.c | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/builtin/gc.c b/builtin/gc.c\n> index c583aad..332bcf7 100644\n> --- a/builtin/gc.c\n> +++ b/builtin/gc.c\n> @@ -177,7 +177,7 @@ static int too_many_packs(void)\n>  \t\t */\n>  \t\tcnt++;\n>  \t}\n> -\treturn gc_auto_pack_limit <= cnt;\n> +\treturn gc_auto_pack_limit < cnt;\n>  }\n\nLooks good, and I cannot think of any real downside. \"0\" is special for\n\"do not use this limit\", so you now have no way of asking to gc every\ntime. But why would you want to? Asking for 1 pack is effectively \"gc if\nsomething happened, otherwise do nothing\".\n\n-Peff\n"}]}