{"thread":{"id":"38156","subject":"Poor push performance with large number of refs","startedAt":"2014-12-10T00:37:36Z","lastAt":"2014-12-20T22:18:03Z","messageCount":14,"participants":["brian m. carlson","Shawn Pearce","Duy Nguyen","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"253471","messageId":"20141210003735.GA124293@vauxhall.crustytoothpaste.net","threadId":"38156","inReplyTo":null,"subject":"Poor push performance with large number of refs","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2014-12-10T00:37:36Z","receivedAt":"2014-12-10T00:37:36Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"I have a repository that's just under 2 GiB in size and contains over\n20000 refs, with a copy of it on a server. Both sides are using Git\n2.1.2. If I push a branch that contains a single commit, it takes about\n15 seconds to push. However, if everything is up-to-date, it completes\nwithin 2 seconds. Notably, HTTPS performs the same as SSH.\n\nMost of the time is spent between the \"Pushing to remote machine\" and\n\"Counting objects\", running git pack-objects:\n\n git pack-objects --all-progress-implied --revs --stdout --thin --delta-base-offset --progress\n\nUnfortunately, -vvv doesn't provide any helpful output. I have some\nsuspicions what's going on here, but no hard data. Where should I\nbe looking to determine the bottleneck?\n\n brianc no # time git push -vvv remote-machine:/tmp/testcase +HEAD\n Pushing to remote-machine:/tmp/testcase\n Counting objects: 1, done.\n Writing objects: 100% (1/1), 184 bytes | 0 bytes/s, done.\n Total 1 (delta 0), reused 0 (delta 0)\n To remote-machine:/tmp/testcase\n + 5059893...2db9804 HEAD -> test (forced update)\n git push -vvv +HEAD 12.35s user 1.61s system 91% cpu 15.337 total\n \n brianc ok # time git push -vvv remote-machine:/tmp/testcase +HEAD\n Pushing to remote-machine:/tmp/testcase\n To remote-machine:/tmp/testcase\n = [up to date] HEAD -> test\n Everything up-to-date\n git push -vvv +HEAD 0.50s user 0.22s system 51% cpu 1.408 total\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\n+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187\n"},{"id":"253477","messageId":"CAJo=hJvKBvQvN=EV4y=ACz5pou9A0tD+txAn_8VR9L3KKtQSiA@mail.gmail.com","threadId":"38156","inReplyTo":"20141210003735.GA124293@vauxhall.crustytoothpaste.net","subject":"Re: Poor push performance with large number of refs","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2014-12-10T05:41:28Z","receivedAt":"2014-12-10T05:41:28Z","isPatch":false,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"On Tue, Dec 9, 2014 at 4:37 PM, brian m. carlson\n wrote:\n> I have a repository that's just under 2 GiB in size and contains over\n> 20000 refs, with a copy of it on a server. Both sides are using Git\n> 2.1.2. If I push a branch that contains a single commit, it takes about\n> 15 seconds to push. However, if everything is up-to-date, it completes\n> within 2 seconds. Notably, HTTPS performs the same as SSH.\n>\n> Most of the time is spent between the \"Pushing to remote machine\" and\n> \"Counting objects\", running git pack-objects:\n>\n> git pack-objects --all-progress-implied --revs --stdout --thin --delta-base-offset --progress\n>\n> Unfortunately, -vvv doesn't provide any helpful output. I have some\n> suspicions what's going on here, but no hard data. Where should I\n> be looking to determine the bottleneck?\n\nMy guess is the revision queue is struggling to insert 20,000 commits\nthat the remote side \"has\", are uninteresting, and should not be\ntransmitted. This queue insertion usually requires parsing the commit\nobject out of the local object store to get the commit timestamp, then\nbubble sort inserting that commit into the queue.\n"},{"id":"253486","messageId":"CACsJy8CkydEE6Q+0PRQOwUkoo5rO9MhsZC=Ps1gWM9kf2QtS8g@mail.gmail.com","threadId":"38156","inReplyTo":"20141210003735.GA124293@vauxhall.crustytoothpaste.net","subject":"Re: Poor push performance with large number of refs","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-12-10T09:17:39Z","receivedAt":"2014-12-10T09:17:39Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Dec 10, 2014 at 7:37 AM, brian m. carlson\n wrote:\n> I have a repository that's just under 2 GiB in size and contains over\n> 20000 refs, with a copy of it on a server. Both sides are using Git\n> 2.1.2. If I push a branch that contains a single commit, it takes about\n> 15 seconds to push. However, if everything is up-to-date, it completes\n> within 2 seconds. Notably, HTTPS performs the same as SSH.\n>\n> Most of the time is spent between the \"Pushing to remote machine\" and\n> \"Counting objects\", running git pack-objects:\n>\n> git pack-objects --all-progress-implied --revs --stdout --thin --delta-base-offset --progress\n>\n> Unfortunately, -vvv doesn't provide any helpful output. I have some\n> suspicions what's going on here, but no hard data. Where should I\n> be looking to determine the bottleneck?\n\nStart with \"perf record\", if this is on linux?\n-- \nDuy\n"},{"id":"253545","messageId":"20141210233443.GA130424@vauxhall.crustytoothpaste.net","threadId":"38156","inReplyTo":"CAJo=hJvKBvQvN=EV4y=ACz5pou9A0tD+txAn_8VR9L3KKtQSiA@mail.gmail.com","subject":"Re: Poor push performance with large number of refs","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2014-12-10T23:34:43Z","receivedAt":"2014-12-10T23:34:43Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Tue, Dec 09, 2014 at 09:41:28PM -0800, Shawn Pearce wrote:\n> On Tue, Dec 9, 2014 at 4:37 PM, brian m. carlson\n> wrote:\n> > Most of the time is spent between the \"Pushing to remote machine\" and\n> > \"Counting objects\", running git pack-objects:\n> >\n> > git pack-objects --all-progress-implied --revs --stdout --thin --delta-base-offset --progress\n> >\n> > Unfortunately, -vvv doesn't provide any helpful output. I have some\n> > suspicions what's going on here, but no hard data. Where should I\n> > be looking to determine the bottleneck?\n> \n> My guess is the revision queue is struggling to insert 20,000 commits\n> that the remote side \"has\", are uninteresting, and should not be\n> transmitted. This queue insertion usually requires parsing the commit\n> object out of the local object store to get the commit timestamp, then\n> bubble sort inserting that commit into the queue.\n\nI looked at this more in depth today and I found that the bottleneck is\n--thin. I tried git send-pack, which does not use --thin by default,\nwhich led me to further testing. A particular push went from 24 seconds\nwith --thin to 4 seconds without.\n\nI agree that the large number of refs is at least part of the problem,\nbecause reducing the number of refs has a slight but noticeable impact.\nIt's also the factor I can least control.\n\nI have a patch which allows per-remote configuration of whether to use\nthin packs (which I will send shortly), but I'm wondering if we can do\nbetter, especially since --thin is the default. It looks like --thin\nforces pack-objects to do its own lookup (essentially a rev-list)\ninstead of using the values provided on stdin.\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\n+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187\n"},{"id":"253551","messageId":"1418255389-133205-1-git-send-email-sandals@crustytoothpaste.net","threadId":"38156","inReplyTo":"20141210233443.GA130424@vauxhall.crustytoothpaste.net","subject":"[PATCH] push: add remote.pushThin to control thin pack generation","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2014-12-10T23:49:49Z","receivedAt":"2014-12-10T23:49:49Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"From: \"brian m. carlson\" \n\nThin packs are enabled when pushing by default, but with a large number\nof refs and a fast network, git may spend more time trying to find a\ngood delta than it would spend simply sending data over the network.\nAdd a per-remote option, pushThin, that controls the default for pushes\non that remote.\n\nSigned-off-by: brian m. carlson \n---\n SOURCES/git/Documentation/config.txt | 6 ++++++\n SOURCES/git/builtin/push.c | 6 ++++--\n SOURCES/git/remote.c | 3 +++\n SOURCES/git/remote.h | 1 +\n SOURCES/git/transport.c | 2 ++\n 5 files changed, 16 insertions(+), 2 deletions(-)\n\ndiff --git a/SOURCES/git/Documentation/config.txt b/SOURCES/git/Documentation/config.txt\nindex 9220725..7fededd 100644\n--- a/SOURCES/git/Documentation/config.txt\n+++ b/SOURCES/git/Documentation/config.txt\n@@ -2178,6 +2178,12 @@ remote..push::\n \tThe default set of \"refspec\" for linkgit:git-push[1]. See\n \tlinkgit:git-push[1].\n \n+remote..pushThin::\n+\tIf true (the default), pass the \\--thin option to\n+\tlinkgit:git-pack-objects[1] during push. This results in a smaller\n+\tpack being sent and can improve push time over slow networks. Over\n+\tfast networks, setting this value to false may improve performance.\n+\n remote..mirror::\n \tIf true, pushing to this remote will automatically behave\n \tas if the `--mirror` option was given on the command line.\ndiff --git a/SOURCES/git/builtin/push.c b/SOURCES/git/builtin/push.c\nindex a076b19..ae39677 100644\n--- a/SOURCES/git/builtin/push.c\n+++ b/SOURCES/git/builtin/push.c\n@@ -15,7 +15,7 @@ static const char * const push_usage[] = {\n \tNULL,\n };\n \n-static int thin = 1;\n+static int thin = -1;\n static int deleterefs;\n static const char *receivepack;\n static int verbosity;\n@@ -347,7 +347,9 @@ static int push_with_options(struct transport *transport, int flags)\n \tif (receivepack)\n \t\ttransport_set_option(transport,\n \t\t\t\t TRANS_OPT_RECEIVEPACK, receivepack);\n-\ttransport_set_option(transport, TRANS_OPT_THIN, thin ? \"yes\" : NULL);\n+\n+\tif (thin != -1)\n+\t\ttransport_set_option(transport, TRANS_OPT_THIN, thin ? \"yes\" : NULL);\n \n \tif (!is_empty_cas(&cas)) {\n \t\tif (!transport->smart_options)\ndiff --git a/SOURCES/git/remote.c b/SOURCES/git/remote.c\nindex f624217..54777cb 100644\n--- a/SOURCES/git/remote.c\n+++ b/SOURCES/git/remote.c\n@@ -175,6 +175,7 @@ static struct remote *make_remote(const char *name, int len)\n \n \tret = xcalloc(1, sizeof(struct remote));\n \tret->prune = -1; /* unspecified */\n+\tret->push_thin = 1; /* default on */\n \tALLOC_GROW(remotes, remotes_nr + 1, remotes_alloc);\n \tremotes[remotes_nr++] = ret;\n \tret->name = xstrndup(name, len);\n@@ -445,6 +446,8 @@ static int handle_config(const char *key, const char *value, void *cb)\n \t\tif (git_config_string(&v, key, value))\n \t\t\treturn -1;\n \t\tadd_push_refspec(remote, v);\n+\t} else if (!strcmp(subkey, \".pushthin\")) {\n+\t\tremote->push_thin = git_config_bool(key, value);\n \t} else if (!strcmp(subkey, \".fetch\")) {\n \t\tconst char *v;\n \t\tif (git_config_string(&v, key, value))\ndiff --git a/SOURCES/git/remote.h b/SOURCES/git/remote.h\nindex 8b62efd..badf266 100644\n--- a/SOURCES/git/remote.h\n+++ b/SOURCES/git/remote.h\n@@ -46,6 +46,7 @@ struct remote {\n \tint skip_default_update;\n \tint mirror;\n \tint prune;\n+\tint push_thin;\n \n \tconst char *receivepack;\n \tconst char *uploadpack;\ndiff --git a/SOURCES/git/transport.c b/SOURCES/git/transport.c\nindex 70d38e4..2f495fa 100644\n--- a/SOURCES/git/transport.c\n+++ b/SOURCES/git/transport.c\n@@ -987,6 +987,8 @@ struct transport *transport_get(struct remote *remote, const char *url)\n \t\t\tret->smart_options->receivepack = remote->receivepack;\n \t}\n \n+\ttransport_set_option(ret, TRANS_OPT_THIN, remote->push_thin ? \"yes\" : NULL);\n+\n \treturn ret;\n }\n \n-- \n2.2.0\n"},{"id":"253555","messageId":"20141211004310.GB130424@vauxhall.crustytoothpaste.net","threadId":"38156","inReplyTo":"1418255389-133205-1-git-send-email-sandals@crustytoothpaste.net","subject":"Re: [PATCH] push: add remote.pushThin to control thin pack generation","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2014-12-11T00:43:10Z","receivedAt":"2014-12-11T00:43:10Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Wed, Dec 10, 2014 at 11:49:49PM +0000, brian m. carlson wrote:\n> From: \"brian m. carlson\" \n> \n> Thin packs are enabled when pushing by default, but with a large number\n> of refs and a fast network, git may spend more time trying to find a\n> good delta than it would spend simply sending data over the network.\n> Add a per-remote option, pushThin, that controls the default for pushes\n> on that remote.\n\nI just realized this patch doesn't apply outside of the particular repo\nI was using. Consider this more of an RFC, since I'd like to avoid\ngoing this route if possible, in favor of a more robust approach.\n\n> Signed-off-by: brian m. carlson \n> ---\n> SOURCES/git/Documentation/config.txt | 6 ++++++\n> SOURCES/git/builtin/push.c | 6 ++++--\n> SOURCES/git/remote.c | 3 +++\n> SOURCES/git/remote.h | 1 +\n> SOURCES/git/transport.c | 2 ++\n> 5 files changed, 16 insertions(+), 2 deletions(-)\n> \n> diff --git a/SOURCES/git/Documentation/config.txt b/SOURCES/git/Documentation/config.txt\n> index 9220725..7fededd 100644\n> --- a/SOURCES/git/Documentation/config.txt\n> +++ b/SOURCES/git/Documentation/config.txt\n> @@ -2178,6 +2178,12 @@ remote..push::\n> \tThe default set of \"refspec\" for linkgit:git-push[1]. See\n> \tlinkgit:git-push[1].\n> \n> +remote..pushThin::\n> +\tIf true (the default), pass the \\--thin option to\n> +\tlinkgit:git-pack-objects[1] during push. This results in a smaller\n> +\tpack being sent and can improve push time over slow networks. Over\n> +\tfast networks, setting this value to false may improve performance.\n> +\n> remote..mirror::\n> \tIf true, pushing to this remote will automatically behave\n> \tas if the `--mirror` option was given on the command line.\n> diff --git a/SOURCES/git/builtin/push.c b/SOURCES/git/builtin/push.c\n> index a076b19..ae39677 100644\n> --- a/SOURCES/git/builtin/push.c\n> +++ b/SOURCES/git/builtin/push.c\n> @@ -15,7 +15,7 @@ static const char * const push_usage[] = {\n> \tNULL,\n> };\n> \n> -static int thin = 1;\n> +static int thin = -1;\n> static int deleterefs;\n> static const char *receivepack;\n> static int verbosity;\n> @@ -347,7 +347,9 @@ static int push_with_options(struct transport *transport, int flags)\n> \tif (receivepack)\n> \t\ttransport_set_option(transport,\n> \t\t\t\t TRANS_OPT_RECEIVEPACK, receivepack);\n> -\ttransport_set_option(transport, TRANS_OPT_THIN, thin ? \"yes\" : NULL);\n> +\n> +\tif (thin != -1)\n> +\t\ttransport_set_option(transport, TRANS_OPT_THIN, thin ? \"yes\" : NULL);\n> \n> \tif (!is_empty_cas(&cas)) {\n> \t\tif (!transport->smart_options)\n> diff --git a/SOURCES/git/remote.c b/SOURCES/git/remote.c\n> index f624217..54777cb 100644\n> --- a/SOURCES/git/remote.c\n> +++ b/SOURCES/git/remote.c\n> @@ -175,6 +175,7 @@ static struct remote *make_remote(const char *name, int len)\n> \n> \tret = xcalloc(1, sizeof(struct remote));\n> \tret->prune = -1; /* unspecified */\n> +\tret->push_thin = 1; /* default on */\n> \tALLOC_GROW(remotes, remotes_nr + 1, remotes_alloc);\n> \tremotes[remotes_nr++] = ret;\n> \tret->name = xstrndup(name, len);\n> @@ -445,6 +446,8 @@ static int handle_config(const char *key, const char *value, void *cb)\n> \t\tif (git_config_string(&v, key, value))\n> \t\t\treturn -1;\n> \t\tadd_push_refspec(remote, v);\n> +\t} else if (!strcmp(subkey, \".pushthin\")) {\n> +\t\tremote->push_thin = git_config_bool(key, value);\n> \t} else if (!strcmp(subkey, \".fetch\")) {\n> \t\tconst char *v;\n> \t\tif (git_config_string(&v, key, value))\n> diff --git a/SOURCES/git/remote.h b/SOURCES/git/remote.h\n> index 8b62efd..badf266 100644\n> --- a/SOURCES/git/remote.h\n> +++ b/SOURCES/git/remote.h\n> @@ -46,6 +46,7 @@ struct remote {\n> \tint skip_default_update;\n> \tint mirror;\n> \tint prune;\n> +\tint push_thin;\n> \n> \tconst char *receivepack;\n> \tconst char *uploadpack;\n> diff --git a/SOURCES/git/transport.c b/SOURCES/git/transport.c\n> index 70d38e4..2f495fa 100644\n> --- a/SOURCES/git/transport.c\n> +++ b/SOURCES/git/transport.c\n> @@ -987,6 +987,8 @@ struct transport *transport_get(struct remote *remote, const char *url)\n> \t\t\tret->smart_options->receivepack = remote->receivepack;\n> \t}\n> \n> +\ttransport_set_option(ret, TRANS_OPT_THIN, remote->push_thin ? \"yes\" : NULL);\n> +\n> \treturn ret;\n> }\n> \n> -- \n> 2.2.0\n> \n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n> \n\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\n+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187\n"},{"id":"253556","messageId":"CACsJy8DxXdLsdgP7OKmv8bS1WVPSO_goBaNrttAqd0-ASBgbpQ@mail.gmail.com","threadId":"38156","inReplyTo":"20141210233443.GA130424@vauxhall.crustytoothpaste.net","subject":"Re: Poor push performance with large number of refs","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-12-11T01:41:07Z","receivedAt":"2014-12-11T01:41:07Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Dec 11, 2014 at 6:34 AM, brian m. carlson\n wrote:\n> I looked at this more in depth today and I found that the bottleneck is\n> --thin. I tried git send-pack, which does not use --thin by default,\n> which led me to further testing. A particular push went from 24 seconds\n> with --thin to 4 seconds without.\n>\n> I agree that the large number of refs is at least part of the problem,\n> because reducing the number of refs has a slight but noticeable impact.\n> It's also the factor I can least control.\n>\n> I have a patch which allows per-remote configuration of whether to use\n> thin packs (which I will send shortly), but I'm wondering if we can do\n> better, especially since --thin is the default. It looks like --thin\n> forces pack-objects to do its own lookup (essentially a rev-list)\n> instead of using the values provided on stdin.\n\nIt could be a regression by fbd4a70 (list-objects: mark more commits\nas edges in mark_edges_uninteresting - 2013-08-16). That commit makes\n--thin a lot more agressive (reading lots of trees). You can try to\nrevert that commit (or use a git version without that commit) and see\nif it improves performance. If so, we probably want to enable that\ncode for shallow repos only.\n-- \nDuy\n"},{"id":"253557","messageId":"20141211030948.GA137226@vauxhall.crustytoothpaste.net","threadId":"38156","inReplyTo":"CACsJy8DxXdLsdgP7OKmv8bS1WVPSO_goBaNrttAqd0-ASBgbpQ@mail.gmail.com","subject":"Re: Poor push performance with large number of refs","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2014-12-11T03:09:49Z","receivedAt":"2014-12-11T03:09:49Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Thu, Dec 11, 2014 at 08:41:07AM +0700, Duy Nguyen wrote:\n> It could be a regression by fbd4a70 (list-objects: mark more commits\n> as edges in mark_edges_uninteresting - 2013-08-16). That commit makes\n> --thin a lot more agressive (reading lots of trees). You can try to\n> revert that commit (or use a git version without that commit) and see\n> if it improves performance. If so, we probably want to enable that\n> code for shallow repos only.\n\nThat's exactly it. With Git 2.2.0, --no-thin was 2.295s, --thin was\n8.769s, and --thin with the patch reverted was 3.645s.\n\nI'll come up with a patch. Thanks for the suggestion.\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\n+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187\n"},{"id":"253558","messageId":"1418269615-139571-1-git-send-email-sandals@crustytoothpaste.net","threadId":"38156","inReplyTo":"20141211030948.GA137226@vauxhall.crustytoothpaste.net","subject":"[PATCH] list-objects: mark fewer commits as edges for non-shallow clones","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2014-12-11T03:46:55Z","receivedAt":"2014-12-11T03:46:55Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"In commit fbd4a70 (list-objects: mark more commits as edges in\nmark_edges_uninteresting - 2013-08-16), we made --thin much more\naggressive by reading lots more trees. This produces much smaller packs\nfor shallow clones; however, it causes a significant performance\nregression for non-shallow clones with lots of refs (23.322 seconds vs.\n4.785 seconds with 22400 refs). Limit this extra edge-marking to\nshallow clones to avoid poor performance.\n\nSuggested-by: Nguyễn Thái Ngọc Duy \nSigned-off-by: brian m. carlson \n---\n list-objects.c | 5 ++++-\n 1 file changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/list-objects.c b/list-objects.c\nindex 2910bec..274ebb4 100644\n--- a/list-objects.c\n+++ b/list-objects.c\n@@ -151,13 +151,14 @@ void mark_edges_uninteresting(struct rev_info *revs, show_edge_fn show_edge)\n {\n \tstruct commit_list *list;\n \tint i;\n+\tint shallow = is_repository_shallow();\n \n \tfor (list = revs->commits; list; list = list->next) {\n \t\tstruct commit *commit = list->item;\n \n \t\tif (commit->object.flags & UNINTERESTING) {\n \t\t\tmark_tree_uninteresting(commit->tree);\n-\t\t\tif (revs->edge_hint && !(commit->object.flags & SHOWN)) {\n+\t\t\tif (shallow && revs->edge_hint && !(commit->object.flags & SHOWN)) {\n \t\t\t\tcommit->object.flags |= SHOWN;\n \t\t\t\tshow_edge(commit);\n \t\t\t}\n@@ -165,6 +166,8 @@ void mark_edges_uninteresting(struct rev_info *revs, show_edge_fn show_edge)\n \t\t}\n \t\tmark_edge_parents_uninteresting(commit, revs, show_edge);\n \t}\n+\tif (!shallow)\n+\t\treturn;\n \tif (revs->edge_hint) {\n \t\tfor (i = 0; i < revs->cmdline.nr; i++) {\n \t\t\tstruct object *obj = revs->cmdline.rev[i].item;\n-- \n2.2.0.rc0.207.ga3a616c\n"},{"id":"253562","messageId":"CACsJy8DM17je-FJ7vq3SFZJWPL5KbZ=kE=iTFQ0X0bZUpKuNmg@mail.gmail.com","threadId":"38156","inReplyTo":"1418269615-139571-1-git-send-email-sandals@crustytoothpaste.net","subject":"Re: [PATCH] list-objects: mark fewer commits as edges for non-shallow clones","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-12-11T10:51:54Z","receivedAt":"2014-12-11T10:51:54Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Dec 11, 2014 at 10:46 AM, brian m. carlson\n wrote:\n> In commit fbd4a70 (list-objects: mark more commits as edges in\n> mark_edges_uninteresting - 2013-08-16), we made --thin much more\n> aggressive by reading lots more trees. This produces much smaller packs\n> for shallow clones; however, it causes a significant performance\n> regression for non-shallow clones with lots of refs (23.322 seconds vs.\n> 4.785 seconds with 22400 refs). Limit this extra edge-marking to\n> shallow clones to avoid poor performance.\n\nI'm glad it's now working better for you. Out of curiosity, have you\nenabled pack bitmap on this repo? I would expect it to reduce time\nsome (or a lot) more, or we would find something to optimize further.\n-- \nDuy\n"},{"id":"253574","messageId":"xmqqvblidmp1.fsf@gitster.dls.corp.google.com","threadId":"38156","inReplyTo":"1418269615-139571-1-git-send-email-sandals@crustytoothpaste.net","subject":"Re: [PATCH] list-objects: mark fewer commits as edges for non-shallow clones","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-12-11T19:13:30Z","receivedAt":"2014-12-11T19:13:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"brian m. carlson\" writes:\n\n> In commit fbd4a70 (list-objects: mark more commits as edges in\n> mark_edges_uninteresting - 2013-08-16), we made --thin much more\n> aggressive by reading lots more trees. This produces much smaller packs\n> for shallow clones; however, it causes a significant performance\n> regression for non-shallow clones with lots of refs (23.322 seconds vs.\n> 4.785 seconds with 22400 refs). Limit this extra edge-marking to\n> shallow clones to avoid poor performance.\n\nThis change affects non-clone/fetch uses of object listing depending\non the shallowness of the repository, and does not even care if it\nis driven as part of the pack-object codepath, if I am reading it\ncorrectly. It smells wrong.\n\nThe problematic fbd4a70 already had unintended fallout that needed\nto be corrected with 200abe74 (list-objects: only look at cmdline\ntrees with edge_hint, 2014-01-20). The current code with the fix,\nthe decision to use the more expensive marking is tied to\n\"edge_hint\". I notice that edge_hint is turned on only if the caller\nof rev-list passes the \"--objects-edge\" option, and currently that\nonly happens in the pack-objects codepath when \"thin\" is given.\nPerhaps that part should decide if it really wants to do edge_hint\ndepending on the shallowness of the repository perhaps?\n\nThat is, something like this instead?\n\n builtin/pack-objects.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 3f9f5c7..a9ebf56 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -2709,7 +2709,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \t\tusage_with_options(pack_usage, pack_objects_options);\n \n \targv_array_push(&rp, \"pack-objects\");\n-\tif (thin) {\n+\tif (thin && is_repository_shallow()) {\n \t\tuse_internal_rev_list = 1;\n \t\targv_array_push(&rp, \"--objects-edge\");\n \t} else\n"},{"id":"253575","messageId":"xmqqr3w6dm2w.fsf@gitster.dls.corp.google.com","threadId":"38156","inReplyTo":"xmqqvblidmp1.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] list-objects: mark fewer commits as edges for non-shallow clones","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-12-11T19:26:47Z","receivedAt":"2014-12-11T19:26:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> This change affects non-clone/fetch uses of object listing depending\n> on the shallowness of the repository, and does not even care if it\n> is driven as part of the pack-object codepath, if I am reading it\n> correctly. It smells wrong.\n>\n> The problematic fbd4a70 already had unintended fallout that needed\n> to be corrected with 200abe74 (list-objects: only look at cmdline\n> trees with edge_hint, 2014-01-20). The current code with the fix,\n> the decision to use the more expensive marking is tied to\n> \"edge_hint\". I notice that edge_hint is turned on only if the caller\n> of rev-list passes the \"--objects-edge\" option, and currently that\n> only happens in the pack-objects codepath when \"thin\" is given.\n> Perhaps that part should decide if it really wants to do edge_hint\n> depending on the shallowness of the repository perhaps?\n>\n> That is, something like this instead?\n\nEh, perhaps not like that, as that would disable milder use of\n\"thin\" when fetching into non-shallow repository.\n\nThe right approach would be more like allocating one more bit in\nstruct rev_info (call that edge_hint_aggressive), give a new option\n\"--objects-edge-aggressive\", and do something like\n\n\tif (thin) {\n \tuse_internal_rev_list = 1;\n\t\targv_array_push(&rp, is_repository_shallow()\n \t? \"--objects-edge-aggressive\"\n : \"--objects-edge\");\n\t}\n\nin this codepath? I'd actually suggest is_repository_shallow()\ndetection to happen one level even higher (i.e. make decision at the\ncaller of pack-objects) and decide to pass either \"--thin\" or\n\"--thin-aggressive\", so that we can make sure that the damage caused\nby fbd4a70 to be limited only to fetches into shallow repository\nwith stronger confidence.\n\n\n>\n> builtin/pack-objects.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\n> index 3f9f5c7..a9ebf56 100644\n> --- a/builtin/pack-objects.c\n> +++ b/builtin/pack-objects.c\n> @@ -2709,7 +2709,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n> \t\tusage_with_options(pack_usage, pack_objects_options);\n> \n> \targv_array_push(&rp, \"pack-objects\");\n> -\tif (thin) {\n> +\tif (thin && is_repository_shallow()) {\n> \t\tuse_internal_rev_list = 1;\n> \t\targv_array_push(&rp, \"--objects-edge\");\n> \t} else\n"},{"id":"253880","messageId":"20141220192820.GA137570@vauxhall.crustytoothpaste.net","threadId":"38156","inReplyTo":"xmqqr3w6dm2w.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] list-objects: mark fewer commits as edges for non-shallow clones","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2014-12-20T19:28:20Z","receivedAt":"2014-12-20T19:28:20Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Thu, Dec 11, 2014 at 11:26:47AM -0800, Junio C Hamano wrote:\n> The right approach would be more like allocating one more bit in\n> struct rev_info (call that edge_hint_aggressive), give a new option\n> \"--objects-edge-aggressive\", and do something like\n> \n> \tif (thin) {\n> \tuse_internal_rev_list = 1;\n> \t\targv_array_push(&rp, is_repository_shallow()\n> \t? \"--objects-edge-aggressive\"\n> : \"--objects-edge\");\n> \t}\n> \n> in this codepath? I'd actually suggest is_repository_shallow()\n> detection to happen one level even higher (i.e. make decision at the\n> caller of pack-objects) and decide to pass either \"--thin\" or\n> \"--thin-aggressive\", so that we can make sure that the damage caused\n> by fbd4a70 to be limited only to fetches into shallow repository\n> with stronger confidence.\n\nSorry it's taken me so long to get back to this. Real life keeps\ngetting in the way.\n\nI think adding --objects-edge-aggressive is the best way forward here\nand then applying the patch above. If we add --thin-aggressive, we push\nthe problem to a higher level, which will require more code changes and\nmake the performance regression continue to affect external remote\nhelpers, which we don't want. We know what the right decision is here,\nso let's just do it.\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\n+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187\n"},{"id":"253882","messageId":"20141220221803.GB137570@vauxhall.crustytoothpaste.net","threadId":"38156","inReplyTo":"CACsJy8DM17je-FJ7vq3SFZJWPL5KbZ=kE=iTFQ0X0bZUpKuNmg@mail.gmail.com","subject":"Re: [PATCH] list-objects: mark fewer commits as edges for non-shallow clones","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2014-12-20T22:18:03Z","receivedAt":"2014-12-20T22:18:03Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Thu, Dec 11, 2014 at 05:51:54PM +0700, Duy Nguyen wrote:\n> I'm glad it's now working better for you. Out of curiosity, have you\n> enabled pack bitmap on this repo? I would expect it to reduce time\n> some (or a lot) more, or we would find something to optimize further.\n\nThe client performs much, much worse with pack bitmaps. The server has\nbitmaps enabled, but it doesn't have any patches for this issue applied\nto it.\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\n+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187\n"}]}