{"thread":{"id":"7995","subject":"[PATCH] diff: release blobs after generating textual diff.","startedAt":"2007-05-06T08:47:25Z","lastAt":"2007-05-08T20:58:48Z","messageCount":6,"participants":["Junio C Hamano","Nicolas Pitre","Robin Rosenberg"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"41192","messageId":"7vr6pucp9e.fsf@assigned-by-dhcp.cox.net","threadId":"7995","inReplyTo":null,"subject":"[PATCH] diff: release blobs after generating textual diff.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-06T08:47:25Z","receivedAt":"2007-05-06T08:47:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This reduces the memory pressure when dealing with many paths.\n\nAn unscientific test of running \"diff-tree --stat --summary -M\"\nbetween v2.6.19 and v2.6.20-rc1 in the linux kernel repository\nindicates that the number of minor faults are reduced by 2/3\n(153k vs 49k).\n\nSigned-off-by: Junio C Hamano \n---\n\n * This is still a WIP, not in the sense that it breaks anything\n (it doesn't seem to), but in the sense that it is not known\n if it is useful in general and would make that much of a\n difference with a project much larger than the kernel.\n\ndiff --git a/diff.c b/diff.c\nindex 7bbe759..9de053c 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1236,6 +1236,8 @@ static void builtin_diff(const char *name_a,\n \t}\n \n free_ab_and_return:\n+\tdiff_free_filespec_data(one);\n+\tdiff_free_filespec_data(two);\n \tfree(a_one);\n \tfree(b_two);\n \treturn;\n@@ -1262,7 +1264,7 @@ static void builtin_diffstat(const char *name_a, const char *name_b,\n \t\tdiff_populate_filespec(two, 0);\n \t\tdata->deleted = count_lines(one->data, one->size);\n \t\tdata->added = count_lines(two->data, two->size);\n-\t\treturn;\n+\t\tgoto free_and_return;\n \t}\n \tif (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)\n \t\tdie(\"unable to read files to diff\");\n@@ -1284,6 +1286,10 @@ static void builtin_diffstat(const char *name_a, const char *name_b,\n \t\tecb.priv = diffstat;\n \t\txdl_diff(&mf1, &mf2, &xpp, &xecfg, &ecb);\n \t}\n+\n+ free_and_return:\n+\tdiff_free_filespec_data(one);\n+\tdiff_free_filespec_data(two);\n }\n \n static void builtin_checkdiff(const char *name_a, const char *name_b,\n@@ -1306,7 +1312,7 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,\n \t\tdie(\"unable to read files to diff\");\n \n \tif (file_is_binary(two))\n-\t\treturn;\n+\t\tgoto free_and_return;\n \telse {\n \t\t/* Crazy xdl interfaces.. */\n \t\txpparam_t xpp;\n@@ -1320,6 +1326,9 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,\n \t\tecb.priv = &data;\n \t\txdl_diff(&mf1, &mf2, &xpp, &xecfg, &ecb);\n \t}\n+ free_and_return:\n+\tdiff_free_filespec_data(one);\n+\tdiff_free_filespec_data(two);\n }\n \n struct diff_filespec *alloc_filespec(const char *path)\n"},{"id":"41193","messageId":"7vlkg2cp5h.fsf@assigned-by-dhcp.cox.net","threadId":"7995","inReplyTo":"7vr6pucp9e.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH] diff.c: truly honor size_only request for populate_filespec.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-06T08:49:46Z","receivedAt":"2007-05-06T08:49:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"For some reason we did not honor size_only requests when \"size cache\"\nwas in use. They should be independent.\n\nSigned-off-by: Junio C Hamano \n---\n\n * I think this is a right fix. When used together with other\n patches, it seems to help reducing the memory footprint a\n bit, but by itself not that much and it seems to have\n slightly negative performance impact.\n\n diff.c | 3 ---\n 1 files changed, 0 insertions(+), 3 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 7bbe759..02c9467 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1503,9 +1503,6 @@ int diff_populate_filespec(struct diff_filespec *s, int size_only)\n \tif (S_ISDIR(s->mode))\n \t\treturn -1;\n \n-\tif (!use_size_cache)\n-\t\tsize_only = 0;\n-\n \tif (s->data)\n \t\treturn err;\n \n-- \n1.5.2.rc1.697.g5086\n"},{"id":"41194","messageId":"7vfy6acp32.fsf@assigned-by-dhcp.cox.net","threadId":"7995","inReplyTo":"7vr6pucp9e.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH] diff: use filespec as in-place size cache","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-06T08:51:13Z","receivedAt":"2007-05-06T08:51:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When we have a filespec primed with size, later size_only\nrequest should be fillable without recomputing it.\n\n---\ndiff --git a/diff.c b/diff.c\nindex 0f8c68f..fb3eba5 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1515,6 +1515,9 @@ int diff_populate_filespec(struct diff_filespec *s, int size_only)\n \tif (s->data)\n \t\treturn err;\n \n+\tif (size_only && 0 < s->size)\n+\t\treturn err;\n+\n \tif (S_ISDIRLNK(s->mode))\n \t\treturn diff_populate_gitlink(s, size_only);\n \n"},{"id":"41222","messageId":"alpine.LFD.0.99.0705061301190.24220@xanadu.home","threadId":"7995","inReplyTo":"7vr6pucp9e.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] diff: release blobs after generating textual diff.","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-05-06T17:03:25Z","receivedAt":"2007-05-06T17:03:25Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Sun, 6 May 2007, Junio C Hamano wrote:\n\n> This reduces the memory pressure when dealing with many paths.\n> \n> An unscientific test of running \"diff-tree --stat --summary -M\"\n> between v2.6.19 and v2.6.20-rc1 in the linux kernel repository\n> indicates that the number of minor faults are reduced by 2/3\n> (153k vs 49k).\n> \n> Signed-off-by: Junio C Hamano \n> ---\n> \n> * This is still a WIP, not in the sense that it breaks anything\n> (it doesn't seem to), but in the sense that it is not known\n> if it is useful in general and would make that much of a\n> difference with a project much larger than the kernel.\n\nThis can only be good. People are really starting to use Git with \ngigantic repos on limited memory hardware.\n\n\nNicolas\n"},{"id":"41254","messageId":"200705062336.28201.robin.rosenberg.lists@dewire.com","threadId":"7995","inReplyTo":"alpine.LFD.0.99.0705061301190.24220@xanadu.home","subject":"Re: [PATCH] diff: release blobs after generating textual diff.","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg.lists@dewire.com","sentAt":"2007-05-06T21:36:26Z","receivedAt":"2007-05-06T21:36:26Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"söndag 06 maj 2007 skrev Nicolas Pitre:\n> On Sun, 6 May 2007, Junio C Hamano wrote:\n> \n> > This reduces the memory pressure when dealing with many paths.\n> > \n> > An unscientific test of running \"diff-tree --stat --summary -M\"\n> > between v2.6.19 and v2.6.20-rc1 in the linux kernel repository\n> > indicates that the number of minor faults are reduced by 2/3\n> > (153k vs 49k).\n> > \n> > Signed-off-by: Junio C Hamano \n> > ---\n> > \n> > * This is still a WIP, not in the sense that it breaks anything\n> > (it doesn't seem to), but in the sense that it is not known\n> > if it is useful in general and would make that much of a\n> > difference with a project much larger than the kernel.\n> \n> This can only be good. People are really starting to use Git with \n> gigantic repos on limited memory hardware.\n\nThis did wonders on the usually unreasonable diffs on huge repos. The openoffice\ndiff mentioned in the openoffice thread went from 6 to ~3 minutes, and most importantly\nthe computer was perfectly usable meanwhile. Git memory usage dropped from 1,7GB to\n400MB.\n\nA more reasonable test diffing against a recent branch , master vs v33M4-patches, in \nthe eclipse repo some of you have didn't gain much in performance, but memory usage\ndropped from 700MB to a peak just under 400MB, which makes a huge difference in\nresponsiveness for the other applications that I have, since they were not swapped out\nduring the diff.\n\n-- robin\n"},{"id":"41498","messageId":"7v7irj2fsn.fsf@assigned-by-dhcp.cox.net","threadId":"7995","inReplyTo":"200705062336.28201.robin.rosenberg.lists@dewire.com","subject":"Re: [PATCH] diff: release blobs after generating textual diff.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-08T20:58:48Z","receivedAt":"2007-05-08T20:58:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Robin Rosenberg writes:\n\n> söndag 06 maj 2007 skrev Nicolas Pitre:\n>> On Sun, 6 May 2007, Junio C Hamano wrote:\n>> \n>> > This reduces the memory pressure when dealing with many paths.\n>> > \n>> > An unscientific test of running \"diff-tree --stat --summary -M\"\n>> > between v2.6.19 and v2.6.20-rc1 in the linux kernel repository\n>> > indicates that the number of minor faults are reduced by 2/3\n>> > (153k vs 49k).\n>> > \n>> > Signed-off-by: Junio C Hamano \n>> > ---\n>> > \n>> > * This is still a WIP, not in the sense that it breaks anything\n>> > (it doesn't seem to), but in the sense that it is not known\n>> > if it is useful in general and would make that much of a\n>> > difference with a project much larger than the kernel.\n>> \n>> This can only be good. People are really starting to use Git with \n>> gigantic repos on limited memory hardware.\n>\n> This did wonders on the usually unreasonable diffs on huge\n> repos. The openoffice diff mentioned in the openoffice thread\n> went from 6 to ~3 minutes, and most importantly the computer\n> was perfectly usable meanwhile. Git memory usage dropped from\n> 1,7GB to 400MB.\n\nI've parked a cleaned-up version in 'next'. Hopefully we can\nmerge it to 'master' before 1.5.2, as it is not about a new\nfeature but about fixing a performance 'bug'.\n"}]}