{"thread":{"id":"46142","subject":"[PATCH] fast-import: Increase the default pack depth to 50","startedAt":"2017-06-08T05:54:31Z","lastAt":"2017-06-08T07:22:10Z","messageCount":4,"participants":["Mike Hommey","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"321732","messageId":"20170608053436.9121-1-mh@glandium.org","threadId":"46142","inReplyTo":null,"subject":"[PATCH] fast-import: Increase the default pack depth to 50","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2017-06-08T05:34:36Z","receivedAt":"2017-06-08T05:54:31Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"In 618e613a70, 10 years ago, the default for pack depth used for\ngit-pack-objects and git-repack was changed from 10 to 50, while\nleaving fast-import's default to 10.\n\nThere doesn't seem to be a reason besides oversight for the change not\nhaving happened in fast-import as well.\n\nInterestingly, fast-import uses pack.depth when it's set, and the\ngit-config manual says the default for pack.depth is 50. While the\ngit-fast-import manual does say the default depth is 10, the\ninconsistency is also confusing.\n\nSigned-off-by: Mike Hommey \n---\n Documentation/git-fast-import.txt | 2 +-\n fast-import.c | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt\nindex 2b762654bf..3d3d219e58 100644\n--- a/Documentation/git-fast-import.txt\n+++ b/Documentation/git-fast-import.txt\n@@ -121,7 +121,7 @@ Performance and Compression Tuning\n \n --depth=::\n \tMaximum delta depth, for blob and tree deltification.\n-\tDefault is 10.\n+\tDefault is 50.\n \n --export-pack-edges=::\n \tAfter creating a packfile, print a line of data to\ndiff --git a/fast-import.c b/fast-import.c\nindex e69d219682..05c73858b0 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -280,7 +280,7 @@ struct recent_command {\n };\n \n /* Configured limits on output */\n-static unsigned long max_depth = 10;\n+static unsigned long max_depth = 50;\n static off_t max_packsize;\n static int unpack_limit = 100;\n static int force_update;\n-- \n2.13.0\n\n"},{"id":"321735","messageId":"20170608070537.zgrcxhl57u4akzvq@sigill.intra.peff.net","threadId":"46142","inReplyTo":"20170608053436.9121-1-mh@glandium.org","subject":"Re: [PATCH] fast-import: Increase the default pack depth to 50","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-06-08T07:05:37Z","receivedAt":"2017-06-08T07:05:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 08, 2017 at 02:34:36PM +0900, Mike Hommey wrote:\n\n> In 618e613a70, 10 years ago, the default for pack depth used for\n> git-pack-objects and git-repack was changed from 10 to 50, while\n> leaving fast-import's default to 10.\n> \n> There doesn't seem to be a reason besides oversight for the change not\n> having happened in fast-import as well.\n> \n> Interestingly, fast-import uses pack.depth when it's set, and the\n> git-config manual says the default for pack.depth is 50. While the\n> git-fast-import manual does say the default depth is 10, the\n> inconsistency is also confusing.\n\nMakes sense. If anything, fast-import would want to allow a deeper depth\nthan normal, since (IIRC) its delta chains are always completely linear.\nWhereas in a real pack, if we decide not to make a delta off the 50th\nitem in a chain, we usually find the 48th or 49th, and end up with a\nbushier graph.\n\nIt probably doesn't matter that much, though, as you'd really want to\n`repack -f` afterwards if you care about getting good deltas. And one\nbase object every 50 versions is probably fine for keeping the initial\npack manageable.\n\n> ---\n> Documentation/git-fast-import.txt | 2 +-\n> fast-import.c | 2 +-\n> 2 files changed, 2 insertions(+), 2 deletions(-)\n\nPatch itself looks obviously correct.\n\n-Peff\n"},{"id":"321736","messageId":"20170608071152.bnjr5zopbb3aly62@glandium.org","threadId":"46142","inReplyTo":"20170608070537.zgrcxhl57u4akzvq@sigill.intra.peff.net","subject":"Re: [PATCH] fast-import: Increase the default pack depth to 50","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2017-06-08T07:11:52Z","receivedAt":"2017-06-08T07:12:03Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Thu, Jun 08, 2017 at 03:05:37AM -0400, Jeff King wrote:\n> On Thu, Jun 08, 2017 at 02:34:36PM +0900, Mike Hommey wrote:\n> \n> > In 618e613a70, 10 years ago, the default for pack depth used for\n> > git-pack-objects and git-repack was changed from 10 to 50, while\n> > leaving fast-import's default to 10.\n> > \n> > There doesn't seem to be a reason besides oversight for the change not\n> > having happened in fast-import as well.\n> > \n> > Interestingly, fast-import uses pack.depth when it's set, and the\n> > git-config manual says the default for pack.depth is 50. While the\n> > git-fast-import manual does say the default depth is 10, the\n> > inconsistency is also confusing.\n> \n> Makes sense. If anything, fast-import would want to allow a deeper depth\n> than normal, since (IIRC) its delta chains are always completely linear.\n> Whereas in a real pack, if we decide not to make a delta off the 50th\n> item in a chain, we usually find the 48th or 49th, and end up with a\n> bushier graph.\n> \n> It probably doesn't matter that much, though, as you'd really want to\n> `repack -f` afterwards if you care about getting good deltas. And one\n> base object every 50 versions is probably fine for keeping the initial\n> pack manageable.\n\nIt actually is possible to have non-linear delta chains with\nfast-import, because the cat-blob command resets the delta base when\nstoring a blob. See\nhttps://github.com/git/git/blob/v2.13.1/fast-import.c#L2984-L2987\n\nAs a side effect of that, git-cinnabar[1] ends up with decent-ish delta\nchains out of the box, without having to go with repack -f, when the\nmercurial server gives out changegroup version 2.\n\nMike\n\n1. https://github.com/glandium/git-cinnabar/\n"},{"id":"321737","messageId":"20170608072153.3fchkvjmudlhhstr@sigill.intra.peff.net","threadId":"46142","inReplyTo":"20170608071152.bnjr5zopbb3aly62@glandium.org","subject":"Re: [PATCH] fast-import: Increase the default pack depth to 50","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-06-08T07:21:53Z","receivedAt":"2017-06-08T07:22:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 08, 2017 at 04:11:52PM +0900, Mike Hommey wrote:\n\n> > It probably doesn't matter that much, though, as you'd really want to\n> > `repack -f` afterwards if you care about getting good deltas. And one\n> > base object every 50 versions is probably fine for keeping the initial\n> > pack manageable.\n> \n> It actually is possible to have non-linear delta chains with\n> fast-import, because the cat-blob command resets the delta base when\n> storing a blob. See\n> https://github.com/git/git/blob/v2.13.1/fast-import.c#L2984-L2987\n> \n> As a side effect of that, git-cinnabar[1] ends up with decent-ish delta\n> chains out of the box, without having to go with repack -f, when the\n> mercurial server gives out changegroup version 2.\n\nInteresting. Today I learned.\n\nI agree that bumping the \"10\" to \"50\" is doubly important then.\n\n-Peff\n"}]}