{"thread":{"id":"43224","subject":"Re: git-diff opens too many files?","startedAt":"2006-11-20T10:12:13Z","lastAt":"2006-11-20T21:02:24Z","messageCount":7,"participants":["Nguyen Thai Ngoc Duy","Junio C Hamano","Johannes Schindelin","Linus Torvalds"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"294838","messageId":"fcaeb9bf0611200212s6ddb0518k24f85223acfed08b@mail.gmail.com","threadId":"43224","inReplyTo":null,"subject":"git-diff opens too many files?","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2006-11-20T10:12:13Z","receivedAt":"2006-11-20T10:12:13Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"I got this error in a quite big (in files) repository:\nerror: open(\"vnexpress.net/Suc-khoe/2001/04/3B9AF976\"): Too many open\nfiles in system\nfatal: cannot hash vnexpress.net/Suc-khoe/2001/04/3B9AF976\n\nThe repository contained about 67.000 files and probably all were modified.\ngit version 1.4.4.rc1.g2bba\n-- \n"},{"id":"298689","messageId":"Pine.LNX.4.63.0611201620070.13772@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"43224","inReplyTo":"fcaeb9bf0611200212s6ddb0518k24f85223acfed08b@mail.gmail.com","subject":"Re: git-diff opens too many files?","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-11-20T15:20:45Z","receivedAt":"2006-11-20T15:20:45Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 20 Nov 2006, Nguyen Thai Ngoc Duy wrote:\n\n> I got this error in a quite big (in files) repository:\n> error: open(\"vnexpress.net/Suc-khoe/2001/04/3B9AF976\"): Too many open\n> files in system\n> fatal: cannot hash vnexpress.net/Suc-khoe/2001/04/3B9AF976\n> \n> The repository contained about 67.000 files and probably all were modified.\n> git version 1.4.4.rc1.g2bba\n\nWhat was the command line? \"git diff\" really is a wrapper around different \nprograms...\n\nCiao,\nDscho\n"},{"id":"294071","messageId":"fcaeb9bf0611200732y777868c5lb9d9061a4522de97@mail.gmail.com","threadId":"43224","inReplyTo":"Pine.LNX.4.63.0611201620070.13772@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: git-diff opens too many files?","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2006-11-20T15:32:05Z","receivedAt":"2006-11-20T15:32:05Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On 11/20/06, Johannes Schindelin wrote:\n> Hi,\n>\n> On Mon, 20 Nov 2006, Nguyen Thai Ngoc Duy wrote:\n>\n> > I got this error in a quite big (in files) repository:\n> > error: open(\"vnexpress.net/Suc-khoe/2001/04/3B9AF976\"): Too many open\n> > files in system\n> > fatal: cannot hash vnexpress.net/Suc-khoe/2001/04/3B9AF976\n> >\n> > The repository contained about 67.000 files and probably all were modified.\n> > git version 1.4.4.rc1.g2bba\n>\n> What was the command line? \"git diff\" really is a wrapper around different\n> programs...\n\n\"git diff\" from working directory with no argument. The real command is\n\ngit diff |LANG=C grep '^-'|LANG=C grep -v '^--'|LANG=C sort|LANG=C uniq -c\n\n(I squeezed blank lines, so I wanted to check that it just did that,\nnothing else)\n\n> Ciao,\n> Dscho\n>\n>\n\n\n-- \n"},{"id":"297750","messageId":"Pine.LNX.4.63.0611201646480.13772@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"43224","inReplyTo":"fcaeb9bf0611200732y777868c5lb9d9061a4522de97@mail.gmail.com","subject":"Re: git-diff opens too many files?","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-11-20T15:48:24Z","receivedAt":"2006-11-20T15:48:24Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 20 Nov 2006, Nguyen Thai Ngoc Duy wrote:\n\n> git diff |LANG=C grep '^-'|LANG=C grep -v '^--'|LANG=C sort|LANG=C uniq -c\n\nI looked in the code, but nothing jumps into my eyes. There is a place \nwhere files are mmap()ed, but AFAICT they are unmmap()ed just after \ndiffing. Could you run it with strace and find unclosed open calls?\n\nCiao,\nDscho\n"},{"id":"298452","messageId":"Pine.LNX.4.64.0611200832450.3692@woody.osdl.org","threadId":"43224","inReplyTo":"fcaeb9bf0611200212s6ddb0518k24f85223acfed08b@mail.gmail.com","subject":"Re: git-diff opens too many files?","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-11-20T17:00:55Z","receivedAt":"2006-11-20T17:00:55Z","isPatch":false,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Mon, 20 Nov 2006, Nguyen Thai Ngoc Duy wrote:\n>\n> I got this error in a quite big (in files) repository:\n> error: open(\"vnexpress.net/Suc-khoe/2001/04/3B9AF976\"): Too many open\n> files in system\n\nOk, \"too many open files in system\" is ENFILE - you haven't run out of \nfile descriptors in _one_ process, but you've exceeded the total number of \nfile descriptors in the whole system.\n\nWhich is not because we forget to close() something, but because we're \nkeeping file descriptors busy another way.\n\n> fatal: cannot hash vnexpress.net/Suc-khoe/2001/04/3B9AF976\n\nHmm. We keep files mmap'ed in \"git diff\" for possibly too long. What \nhappens is that we mmap a file that we want to diff when we start the \nwhole thing, and keep it mapped over the whole diff session, because we're \npotentially going to need to compare it against other files (ie rename \ndetection etc). And then we unmap it only at the end (in \"diff_flush()\" -> \n\"diff_free_filepair()\" -> \"diff_free_filespec_data()\").\n\nAnd that's normally great, and means that we don't need to worry about the \nfile data (we map it once, and can keep it in memory), but yeah, if you \nhave thousands of files changed, you'll have thousands of mappings. And \neach one will have a pointer to a \"struct file\" inside the kernel. \n\nWhat OS/distro is this? Normally, you shouldn't have that low a limit on \nnumber of files open, but we do end up potentially opening thousands.\n\nFor example, under Linux, you can do this:\n\n\t# in one terminal window, do:\n\twhile : ; do cat /proc/sys/fs/file-nr ; sleep 1; done\n\n\t# in another one:\n\tcd linux-repo\n\tgit ls-files '*.c' | xargs touch\n\tgit diff\n\nand if it looks anything like mine, it could be:\n\n\t2464 0 349662\n\t2464 0 349662\n\t2464 0 349662\n*\t5920 0 349662\n**\t7616 0 349662\n***\t9024 0 349662\n****\t10944 0 349662\n\t2464 0 349662\n\t2464 0 349662\n\n\n(see how the numnber of active files grows by thousands).\n\nAnyway, there's two possible solutions:\n\n - simply make sure that you can have that many open files. \n\n If it's a Linux system, just increase the value of the file\n /proc/sys/fs/file-max, and you're done. Of course, if you're not the \n admin of the box, you may need to ask somebody else to do it for you..\n\n - we could try to make git not keep them mmap'ed for the whole time. \n\nJunio? This is your speciality, I'm not sure how painful it would be to \nunmap and remap on demand.. (or switch it to some kind of \"keep the last \n mmaps active\" kind of thing to avoid having thousands and thousands of \nmmaps active).\n\nOne simple thing that might be worth it is to simply _not_ use mmap() at \nall for small files. If a file is less than 1kB, it might be better to do \na malloc() and a read() - partly because it avoids having tons of file \ndescriptors, but partly because it's also more efficient from a virtual \nmemory usage perspective (not that you're probably very likely to ever \nreally hit that problem in practice).\n\nNguyen - that \"use malloc+read\" thing might be a quick workaround, but \nonly if you have tons of _small_ files (and if you can't easily just \nincrease file-max). \n\n"},{"id":"295750","messageId":"7vvel928xx.fsf@assigned-by-dhcp.cox.net","threadId":"43224","inReplyTo":"Pine.LNX.4.64.0611200832450.3692@woody.osdl.org","subject":"Re: git-diff opens too many files?","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-11-20T19:51:22Z","receivedAt":"2006-11-20T19:51:22Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> Anyway, there's two possible solutions:\n>\n> - simply make sure that you can have that many open files. \n>\n> If it's a Linux system, just increase the value of the file\n> /proc/sys/fs/file-max, and you're done. Of course, if you're not the \n> admin of the box, you may need to ask somebody else to do it for you..\n>\n> - we could try to make git not keep them mmap'ed for the whole time. \n>\n> Junio? This is your speciality, I'm not sure how painful it would be to \n> unmap and remap on demand.. (or switch it to some kind of \"keep the last \n> mmaps active\" kind of thing to avoid having thousands and thousands of \n> mmaps active).\n\n60,000 files 1kB each is 60MB which is a peanuts these days, but\n10kB each would be already nontrivial burden on 32-bit (20%\nunder 3+1 split), so even if we do the \"read in small files\ninstead of mapping\" we would need diff_unpopulate_filespec() calls.\n\nI think after diffcore_rename() runs, the data in filespec is\nused only once during final textual diff generation. We would\nuse once more before diff generation if diffcore_pickaxe() is in\nuse. These codepaths begin with diff_populate_filespec(), so if\nwe unpopulate them after diffcore_rename() runs nobody would\nnotice (other than performance degradation and strace showing us\nreading the same thing twice).\n\nThe diffcore_rename() matrix code expects all filespecs involved\ncan be populated at the same time, but it should not be too hard\nto change it to keep one dst and all src candidates populated\nbut others dropped if space gets tight. I need to look at the\ncode for the details.\n\nBut Nguyen's command line did not have -M; I think the filespecs\nare populated only during the text generation in that case, so\nthe above would not help him while it would be a worthwhile\nchange.\n\nBecause there is _no_ processing that comes after textual diff\ngeneration that looks at the data, I think diff_flush_patch()\nafter calling run_diff() can unpopulate the data from the\nfilepair *p before returning without harming performance.\n\nI think diff_flush_stat() and diff_flush_checkdiff() would have\nthe same issue, though. Ideally these should be able to do\ntheir processing while the main textual diff holds the data in\nmemory for its processing but that is currently not the way the\ncode is structured.\n\n\n\n\n"},{"id":"298122","messageId":"7vfycd25nj.fsf@assigned-by-dhcp.cox.net","threadId":"43224","inReplyTo":"Pine.LNX.4.64.0611200832450.3692@woody.osdl.org","subject":"Re: git-diff opens too many files?","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-11-20T21:02:24Z","receivedAt":"2006-11-20T21:02:24Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> Junio? This is your speciality, I'm not sure how painful it would be to \n> unmap and remap on demand.. (or switch it to some kind of \"keep the last \n> mmaps active\" kind of thing to avoid having thousands and thousands of \n> mmaps active).\n>\n> One simple thing that might be worth it is to simply _not_ use mmap() at \n> all for small files. If a file is less than 1kB, it might be better to do \n> a malloc() and a read() - partly because it avoids having tons of file \n> descriptors, but partly because it's also more efficient from a virtual \n> memory usage perspective (not that you're probably very likely to ever \n> really hit that problem in practice).\n>\n> Nguyen - that \"use malloc+read\" thing might be a quick workaround, but \n> only if you have tons of _small_ files (and if you can't easily just \n> increase file-max). \n\nSo here is a lunch-time hack to get Nguyen unstuck.\n\n-- >8 --\n[PATCH] diff.c: avoid mmap() of small files in populate_filespec()\n\nThis would hopefully behave better in VM usage. It is not a\nreal fix if you are dealing with truly huge diff, in which case\nwe would have to LRU out the data in memory for filespecs that\nare not used in immediate future.\n\nSigned-off-by: Junio C Hamano \n---\ndiff --git a/diff.c b/diff.c\nindex 3315378..af50b6f 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1294,11 +1294,22 @@ int diff_populate_filespec(struct diff_f\n \t\tfd = open(s->path, O_RDONLY);\n \t\tif (fd < 0)\n \t\t\tgoto err_empty;\n-\t\ts->data = mmap(NULL, s->size, PROT_READ, MAP_PRIVATE, fd, 0);\n+\t\tif (s->size < MINIMUM_MMAP) {\n+\t\t\ts->data = xmalloc(s->size);\n+\t\t\ts->should_free = 1;\n+\t\t\tif (xread(fd, s->data, s->size) != s->size) {\n+\t\t\t\tfree(s->data);\n+\t\t\t\tgoto err_empty;\n+\t\t\t}\n+\t\t}\n+\t\telse {\n+\t\t\ts->data = mmap(NULL, s->size, PROT_READ, MAP_PRIVATE,\n+\t\t\t\t fd, 0);\n+\t\t\ts->should_munmap = 1;\n+\t\t}\n \t\tclose(fd);\n \t\tif (s->data == MAP_FAILED)\n \t\t\tgoto err_empty;\n-\t\ts->should_munmap = 1;\n \t}\n \telse {\n \t\tchar type[20];\ndiff --git a/diffcore.h b/diffcore.h\nindex 2249bc2..f129aa0 100644\n--- a/diffcore.h\n+++ b/diffcore.h\n@@ -21,6 +21,7 @@\n #define DEFAULT_MERGE_SCORE 36000 /* maximum for break-merge to happen 60%) */\n \n #define MINIMUM_BREAK_SIZE 400 /* do not break a file smaller than this */\n+#define MINIMUM_MMAP\t 4096 /* do not mmap a file smaller than this */\n \n struct diff_filespec {\n \tunsigned char sha1[20];\n"}]}