{"thread":{"id":"1813","subject":"Avoid wasting memory in git-rev-list","startedAt":"2005-09-15T21:43:17Z","lastAt":"2005-09-15T22:38:58Z","messageCount":4,"participants":["Linus Torvalds","Junio C Hamano","Johannes Schindelin"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"8637","messageId":"Pine.LNX.4.58.0509151434470.26803@g5.osdl.org","threadId":"1813","inReplyTo":null,"subject":"Avoid wasting memory in git-rev-list","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-09-15T21:43:17Z","receivedAt":"2005-09-15T21:43:17Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nAs pointed out on the list, git-rev-list can use a lot of memory.\n\nOne low-hanging fruit is to free the commit buffer for commits that we\nparse. By default, parse_commit() will save away the buffer, since a lot\nof cases do want it, and re-reading it continually would be unnecessary. \nHowever, in many cases the buffer isn't actually necessary and saving it \njust wastes memory.\n\nWe could just free the buffer ourselves, but especially in git-rev-list, \nwe actually end up using the helper functions that automatically add \nparent commits to the commit lists, so we don't actually control the \ncommit parsing directly. \n\nInstead, just make this behaviour of \"parse_commit()\" a global flag.  \nMaybe this is a bit tasteless, but it's very simple, and it makes a\nnoticable difference in memory usage.\n\nBefore the change:\n\n\t[torvalds@g5 linux]$ /usr/bin/time git-rev-list v2.6.12..HEAD > /dev/null\n\t0.26user 0.02system 0:00.28elapsed 99%CPU (0avgtext+0avgdata 0maxresident)k\n\t0inputs+0outputs (0major+3714minor)pagefaults 0swaps\n\nafter the change:\n\n\t[torvalds@g5 linux]$ /usr/bin/time git-rev-list v2.6.12..HEAD > /dev/null\n\t0.26user 0.00system 0:00.27elapsed 100%CPU (0avgtext+0avgdata 0maxresident)k\n\t0inputs+0outputs (0major+2433minor)pagefaults 0swaps\n\nnote how the minor faults have decreased from 3714 pages to 2433 pages. \nThat's all due to the fewer anonymous pages allocated to hold the comment \nbuffers and their metadata.\n\nSigned-off-by: Linus Torvalds <torvalds@osdl.org>\n---\n\nHey, I'm not proud. If somebody wants to do a prettier patch, go wild.\n\nThis still doesn't attack the _big_ memory use by\n\n\t\"git-rev-list --objects\"\n\nwhere we just parse more stuff than we need to, but it's a first step.\n\ndiff --git a/commit.c b/commit.c\n--- a/commit.c\n+++ b/commit.c\n@@ -3,6 +3,8 @@\n #include \"commit.h\"\n #include \"cache.h\"\n \n+int save_commit_buffer = 1;\n+\n struct sort_node\n {\n \t/*\n@@ -264,7 +266,7 @@ int parse_commit(struct commit *item)\n \t\t\t     sha1_to_hex(item->object.sha1));\n \t}\n \tret = parse_commit_buffer(item, buffer, size);\n-\tif (!ret) {\n+\tif (save_commit_buffer && !ret) {\n \t\titem->buffer = buffer;\n \t\treturn 0;\n \t}\ndiff --git a/commit.h b/commit.h\n--- a/commit.h\n+++ b/commit.h\n@@ -17,6 +17,7 @@ struct commit {\n \tchar *buffer;\n };\n \n+extern int save_commit_buffer;\n extern const char *commit_type;\n \n struct commit *lookup_commit(const unsigned char *sha1);\ndiff --git a/rev-list.c b/rev-list.c\n--- a/rev-list.c\n+++ b/rev-list.c\n@@ -582,6 +582,8 @@ int main(int argc, char **argv)\n \t\thandle_one_commit(commit, &list);\n \t}\n \n+\tsave_commit_buffer = verbose_header;\n+\n \tif (!merge_order) {\t\t\n \t\tsort_by_date(&list);\n \t        if (limited)\n"},{"id":"8638","messageId":"7v7jdiyop9.fsf@assigned-by-dhcp.cox.net","threadId":"1813","inReplyTo":"Pine.LNX.4.58.0509151434470.26803@g5.osdl.org","subject":"Re: Avoid wasting memory in git-rev-list","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-09-15T22:02:26Z","receivedAt":"2005-09-15T22:02:26Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"So using this I could also avoid wasting memory by the \"are we\nalready walking the commit reachable from our existing refs\"\ncheck by doing something like this?\n\n------------\ngit diff\ndiff --git a/fetch.c b/fetch.c\n--- a/fetch.c\n+++ b/fetch.c\n@@ -207,6 +207,7 @@ int pull(char *target)\n \tunsigned char sha1[20];\n \tint fd = -1;\n \n+\tsave_commit_buffer = 0;\n \tif (write_ref && current_ref) {\n \t\tfd = lock_ref_sha1(write_ref, current_ref);\n \t\tif (fd < 0)\n"},{"id":"8639","messageId":"Pine.LNX.4.58.0509151507150.26803@g5.osdl.org","threadId":"1813","inReplyTo":"7v7jdiyop9.fsf@assigned-by-dhcp.cox.net","subject":"Re: Avoid wasting memory in git-rev-list","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-09-15T22:07:29Z","receivedAt":"2005-09-15T22:07:29Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Thu, 15 Sep 2005, Junio C Hamano wrote:\n>\n> So using this I could also avoid wasting memory by the \"are we\n> already walking the commit reachable from our existing refs\"\n> check by doing something like this?\n\nYes. It's still ugly, but it's _simple_.\n\n\t\tLinus\n"},{"id":"8643","messageId":"Pine.LNX.4.63.0509160034170.20360@wgmdd8.biozentrum.uni-wuerzburg.de","threadId":"1813","inReplyTo":"7v7jdiyop9.fsf@assigned-by-dhcp.cox.net","subject":"Re: Avoid wasting memory in git-rev-list","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2005-09-15T22:38:58Z","receivedAt":"2005-09-15T22:38:58Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Linus, hi Junio,\n\nOn Thu, 15 Sep 2005, Junio C Hamano wrote:\n\n> So using this I could also avoid wasting memory by the \"are we\n> already walking the commit reachable from our existing refs\"\n> check by doing something like this?\n> \n> ------------\n> git diff\n> diff --git a/fetch.c b/fetch.c\n> --- a/fetch.c\n> +++ b/fetch.c\n> @@ -207,6 +207,7 @@ int pull(char *target)\n>  \tunsigned char sha1[20];\n>  \tint fd = -1;\n>  \n> +\tsave_commit_buffer = 0;\n>  \tif (write_ref && current_ref) {\n>  \t\tfd = lock_ref_sha1(write_ref, current_ref);\n>  \t\tif (fd < 0)\n\nTogether with both of Linus' patches, this works wonders! It uses _lots_ \nless of memory. Actually, the performance impact on my tiny server was \nnegligible.\n\nCiao,\nDscho\n"}]}