{"thread":{"id":"6790","subject":"[PU PATCH] Fix git fetch for very large ref counts","startedAt":"2007-02-13T01:21:38Z","lastAt":"2007-02-13T17:58:45Z","messageCount":8,"participants":["Julian Phillips","Linus Torvalds","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"34355","messageId":"11713297014015-git-send-email-julian@quantumfyre.co.uk","threadId":"6790","inReplyTo":null,"subject":"[PU PATCH] Fix git fetch for very large ref counts","fromName":"Julian Phillips","fromEmail":"jp3@quantumfyre.co.uk","sentAt":"2007-02-13T01:21:38Z","receivedAt":"2007-02-13T01:21:38Z","isPatch":true,"sender":{"key":"julian@quantumfyre.co.uk","avatar":"https://avatars.githubusercontent.com/u/948888?v=4"},"body":"The updated git fetch in pu is vastly improved on repositories with very\nlarge numbers of refs.  The time taken for a no-op fetch over ~9000 refs\ndrops from ~48m to ~0.5m.\n\nHowever, before git fetch will actually run on a repository with ~9000\nrefs the calling interface between fetch and fetch--tool needs to be\nchanged.  The existing version passes the entire reflist on the command\nline, which means that it is subject to the maxiumum environment size\npassed to a child process by execve.\n\nThe following patches add a stdin based interface to fetch--tool allowing\nthe ~9000 refs to be passed without exceeding the environment limit.\n\n--\nJulian\n"},{"id":"34358","messageId":"11713297013179-git-send-email-julian@quantumfyre.co.uk","threadId":"6790","inReplyTo":"11713297014015-git-send-email-julian@quantumfyre.co.uk","subject":"[PATCH] Allow fetch--tool to read from stdin","fromName":"Julian Phillips","fromEmail":"jp3@quantumfyre.co.uk","sentAt":"2007-02-13T01:21:39Z","receivedAt":"2007-02-13T01:21:39Z","isPatch":true,"sender":{"key":"julian@quantumfyre.co.uk","avatar":"https://avatars.githubusercontent.com/u/948888?v=4"},"body":"If the reflist is \"-\" then read the reflist data from stdin instead,\nthis will allow the passing of more than 128K of reflist data - which\nwon't fit in the environment passed by execve.\n\nSigned-off-by: Julian Phillips <julian@quantumfyre.co.uk>\n---\n builtin-fetch--tool.c |   24 ++++++++++++++++++++++--\n 1 files changed, 22 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-fetch--tool.c b/builtin-fetch--tool.c\nindex 3090ffe..619ceb0 100644\n--- a/builtin-fetch--tool.c\n+++ b/builtin-fetch--tool.c\n@@ -2,6 +2,20 @@\n #include \"refs.h\"\n #include \"commit.h\"\n \n+static char *get_stdin(void)\n+{\n+#define CHUNK_SIZE (1048576)\n+\tchar *data = xmalloc(CHUNK_SIZE);\n+\tint offset = 0, read = 0;\n+\tread = xread(0, data, CHUNK_SIZE);\n+\twhile (read == CHUNK_SIZE) {\n+\t\toffset += CHUNK_SIZE;\n+\t\tdata = xrealloc(data, offset + CHUNK_SIZE);\n+\t\tread = xread(0, data + offset, CHUNK_SIZE);\n+\t}\n+\treturn data;\n+}\n+\n static void show_new(char *type, unsigned char *sha1_new)\n {\n \tfprintf(stderr, \"  %s: %s\\n\", type,\n@@ -463,12 +477,18 @@ int cmd_fetch__tool(int argc, const char **argv, const char *prefix)\n \tif (!strcmp(\"parse-reflist\", argv[1])) {\n \t\tif (argc != 3)\n \t\t\treturn error(\"parse-reflist takes 1 arg\");\n-\t\treturn parse_reflist(argv[2]);\n+\t\tconst char *reflist = argv[2];\n+\t\tif (!strcmp(reflist, \"-\"))\n+\t\t\treflist = get_stdin();\n+\t\treturn parse_reflist(reflist);\n \t}\n \tif (!strcmp(\"expand-refs-wildcard\", argv[1])) {\n \t\tif (argc < 4)\n \t\t\treturn error(\"expand-refs-wildcard takes at least 2 args\");\n-\t\treturn expand_refs_wildcard(argv[2], argc - 3, argv + 3);\n+\t\tconst char *reflist = argv[2];\n+\t\tif (!strcmp(reflist, \"-\"))\n+\t\t\treflist = get_stdin();\n+\t\treturn expand_refs_wildcard(reflist, argc - 3, argv + 3);\n \t}\n \n \treturn error(\"Unknown subcommand: %s\", argv[1]);\n-- \n1.4.4.4\n"},{"id":"34356","messageId":"1171329702101-git-send-email-julian@quantumfyre.co.uk","threadId":"6790","inReplyTo":"11713297013179-git-send-email-julian@quantumfyre.co.uk","subject":"[PATCH] Use stdin reflist passing in parse-remote","fromName":"Julian Phillips","fromEmail":"jp3@quantumfyre.co.uk","sentAt":"2007-02-13T01:21:40Z","receivedAt":"2007-02-13T01:21:40Z","isPatch":true,"sender":{"key":"julian@quantumfyre.co.uk","avatar":"https://avatars.githubusercontent.com/u/948888?v=4"},"body":"Use the new stdin reflist passing mechanism for the call to\nfetch--tool expand-refs-wildcard, allowing passing of more\nthan ~128K of reflist data.\n\nSigned-off-by: Julian Phillips <julian@quantumfyre.co.uk>\n---\n git-parse-remote.sh |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/git-parse-remote.sh b/git-parse-remote.sh\nindex 9b19a21..185eb54 100755\n--- a/git-parse-remote.sh\n+++ b/git-parse-remote.sh\n@@ -81,7 +81,7 @@ get_remote_default_refs_for_push () {\n # is to help prevent randomly \"globbed\" ref from being chosen as\n # a merge candidate\n expand_refs_wildcard () {\n-\tgit fetch--tool expand-refs-wildcard \"$ls_remote_result\" \"$@\"\n+\techo \"$ls_remote_result\" | git fetch--tool expand-refs-wildcard \"-\" \"$@\"\n }\n \n # Subroutine to canonicalize remote:local notation.\n-- \n1.4.4.4\n"},{"id":"34357","messageId":"11713297023551-git-send-email-julian@quantumfyre.co.uk","threadId":"6790","inReplyTo":"1171329702101-git-send-email-julian@quantumfyre.co.uk","subject":"[PATCH] Use stdin reflist passing in git-fetch.sh","fromName":"Julian Phillips","fromEmail":"jp3@quantumfyre.co.uk","sentAt":"2007-02-13T01:21:41Z","receivedAt":"2007-02-13T01:21:41Z","isPatch":true,"sender":{"key":"julian@quantumfyre.co.uk","avatar":"https://avatars.githubusercontent.com/u/948888?v=4"},"body":"Use the new stdin reflist passing mechanism for the call to\nfetch--tool parse-reflist, allowing passing of more than ~128K\nof reflist data.\n\nSigned-off-by: Julian Phillips <julian@quantumfyre.co.uk>\n---\n git-fetch.sh |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/git-fetch.sh b/git-fetch.sh\nindex 6a8b196..5a1b4e7 100755\n--- a/git-fetch.sh\n+++ b/git-fetch.sh\n@@ -169,7 +169,7 @@ fi\n \n fetch_native () {\n \n-  eval=$(git-fetch--tool parse-reflist \"$1\")\n+  eval=$(echo \"$1\" | git-fetch--tool parse-reflist \"-\")\n   eval \"$eval\"\n \n     ( : subshell because we muck with IFS\n-- \n1.4.4.4\n"},{"id":"34362","messageId":"Pine.LNX.4.64.0702121830270.8424@woody.linux-foundation.org","threadId":"6790","inReplyTo":"11713297023551-git-send-email-julian@quantumfyre.co.uk","subject":"Re: [PATCH] Use stdin reflist passing in git-fetch.sh","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-02-13T02:31:47Z","receivedAt":"2007-02-13T02:31:47Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 13 Feb 2007, Julian Phillips wrote:\n>\n> Use the new stdin reflist passing mechanism for the call to\n> fetch--tool parse-reflist, allowing passing of more than ~128K\n> of reflist data.\n\nThis pretty much seems to assume that \"echo\" is a shell built-in.\n\nOtherwise, the\n\n\techo \"$1\"\n\npart will still fall afoul of any exec limits.\n\nMaybe that's aperfectly fine assumption. It's certainly true for bash (and \nprobably _any_ shells that do any built-ins at all - echo is pretty damn \nbasic ;)\n\n\t\tLinus\n"},{"id":"34365","messageId":"7vd54e4ufk.fsf@assigned-by-dhcp.cox.net","threadId":"6790","inReplyTo":"11713297014015-git-send-email-julian@quantumfyre.co.uk","subject":"Re: [PU PATCH] Fix git fetch for very large ref counts","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-02-13T03:18:07Z","receivedAt":"2007-02-13T03:18:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Julian Phillips\" <jp3@quantumfyre.co.uk> writes:\n\n> The updated git fetch in pu is vastly improved on repositories with very\n> large numbers of refs.  The time taken for a no-op fetch over ~9000 refs\n> drops from ~48m to ~0.5m.\n>\n> However, before git fetch will actually run on a repository with ~9000\n> refs the calling interface between fetch and fetch--tool needs to be\n> changed.  The existing version passes the entire reflist on the command\n> line, which means that it is subject to the maxiumum environment size\n> passed to a child process by execve.\n>\n> The following patches add a stdin based interface to fetch--tool allowing\n> the ~9000 refs to be passed without exceeding the environment limit.\n\nThanks.\n\nBut the ones in 'pu' were done primarily as demonstration of\nwhere the bottlenecks are, and not meant for real-world\nconsumption.  I think the final shaving of 0.5m down to a few\nseconds needs to move the ls_remote_result string currently kept\nas a shell variable to a list of strings represented in a\ngit-fetch largely rewritten in C, and at that point the\ninterface from outside fetch--tool to throw 9000 refs at it\nwould be an internal function call and the code you fixed along\nwith new function you introduced would probably need to be\ndiscarded.\n"},{"id":"34393","messageId":"Pine.LNX.4.64.0702131031180.6435@reaper.quantumfyre.co.uk","threadId":"6790","inReplyTo":"7vd54e4ufk.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PU PATCH] Fix git fetch for very large ref counts","fromName":"Julian Phillips","fromEmail":"julian@quantumfyre.co.uk","sentAt":"2007-02-13T10:39:41Z","receivedAt":"2007-02-13T10:39:41Z","isPatch":true,"sender":{"key":"julian@quantumfyre.co.uk","avatar":"https://avatars.githubusercontent.com/u/948888?v=4"},"body":"On Mon, 12 Feb 2007, Junio C Hamano wrote:\n\n> \"Julian Phillips\" <jp3@quantumfyre.co.uk> writes:\n>\n>> The updated git fetch in pu is vastly improved on repositories with very\n>> large numbers of refs.  The time taken for a no-op fetch over ~9000 refs\n>> drops from ~48m to ~0.5m.\n>>\n>> However, before git fetch will actually run on a repository with ~9000\n>> refs the calling interface between fetch and fetch--tool needs to be\n>> changed.  The existing version passes the entire reflist on the command\n>> line, which means that it is subject to the maxiumum environment size\n>> passed to a child process by execve.\n>>\n>> The following patches add a stdin based interface to fetch--tool allowing\n>> the ~9000 refs to be passed without exceeding the environment limit.\n>\n> Thanks.\n>\n> But the ones in 'pu' were done primarily as demonstration of\n> where the bottlenecks are, and not meant for real-world\n> consumption.  I think the final shaving of 0.5m down to a few\n> seconds needs to move the ls_remote_result string currently kept\n> as a shell variable to a list of strings represented in a\n> git-fetch largely rewritten in C, and at that point the\n> interface from outside fetch--tool to throw 9000 refs at it\n> would be an internal function call and the code you fixed along\n> with new function you introduced would probably need to be\n> discarded.\n\nAnd there I was thinking 0.5m was fast ... given how long I've been \nreading this list I really should have know better. ;)\n\nI only really made the changes so I could try your improvements to fetch \nout - if they aren't necessary because you're making it even faster then I \nreally don't have much cause to complain.\n\n-- \nJulian\n\n  ---\nTo be a kind of moral Unix, he touched the hem of Nature's shift.\n \t\t-- Shelley\n"},{"id":"34439","messageId":"7vfy9ax7l6.fsf@assigned-by-dhcp.cox.net","threadId":"6790","inReplyTo":"Pine.LNX.4.64.0702131031180.6435@reaper.quantumfyre.co.uk","subject":"Re: [PU PATCH] Fix git fetch for very large ref counts","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-02-13T17:58:45Z","receivedAt":"2007-02-13T17:58:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Julian Phillips <julian@quantumfyre.co.uk> writes:\n\n> And there I was thinking 0.5m was fast ... given how long I've been\n> reading this list I really should have know better. ;)\n\nWell, the only thing we need to do in fetching between two\nrepositories that are already identical should be to compare\nls-remote output from both ends and immediately declare victory,\nand it is unacceptable for such an obvious no-op to take 30\nseconds.  At this point it really is the shell loop that is\nkilling us.\n\n> I only really made the changes so I could try your improvements to\n> fetch out - if they aren't necessary because you're making it even\n> faster then I really don't have much cause to complain.\n\nI've applied your fixes to jc/fetch topic and merged it back to\n'pu'.  Thanks.\n"}]}