Re: git-diff opens too many files?

[Junio C Hamano <junkio@cox.net>]

Linus Torvalds <torvalds@osdl.org> writes:

> Junio? This is your speciality, I'm not sure how painful it would be to 
> 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 
> mmaps active).
>
> One simple thing that might be worth it is to simply _not_ use mmap() at 
> all for small files. If a file is less than 1kB, it might be better to do 
> a malloc() and a read() - partly because it avoids having tons of file 
> descriptors, but partly because it's also more efficient from a virtual 
> memory usage perspective (not that you're probably very likely to ever 
> really hit that problem in practice).
>
> Nguyen - that "use malloc+read" thing might be a quick workaround, but 
> only if you have tons of _small_ files (and if you can't easily just 
> increase file-max). 

So here is a lunch-time hack to get Nguyen unstuck.

-- >8 --
[PATCH] diff.c: avoid mmap() of small files in populate_filespec()

This would hopefully behave better in VM usage.  It is not a
real fix if you are dealing with truly huge diff, in which case
we would have to LRU out the data in memory for filespecs that
are not used in immediate future.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
diff --git a/diff.c b/diff.c
index 3315378..af50b6f 100644
--- a/diff.c
+++ b/diff.c
@@ -1294,11 +1294,22 @@ int diff_populate_filespec(struct diff_f
 		fd = open(s->path, O_RDONLY);
 		if (fd < 0)
 			goto err_empty;
-		s->data = mmap(NULL, s->size, PROT_READ, MAP_PRIVATE, fd, 0);
+		if (s->size < MINIMUM_MMAP) {
+			s->data = xmalloc(s->size);
+			s->should_free = 1;
+			if (xread(fd, s->data, s->size) != s->size) {
+				free(s->data);
+				goto err_empty;
+			}
+		}
+		else {
+			s->data = mmap(NULL, s->size, PROT_READ, MAP_PRIVATE,
+				       fd, 0);
+			s->should_munmap = 1;
+		}
 		close(fd);
 		if (s->data == MAP_FAILED)
 			goto err_empty;
-		s->should_munmap = 1;
 	}
 	else {
 		char type[20];
diff --git a/diffcore.h b/diffcore.h
index 2249bc2..f129aa0 100644
--- a/diffcore.h
+++ b/diffcore.h
@@ -21,6 +21,7 @@
 #define DEFAULT_MERGE_SCORE  36000 /* maximum for break-merge to happen 60%) */
 
 #define MINIMUM_BREAK_SIZE     400 /* do not break a file smaller than this */
+#define MINIMUM_MMAP	      4096 /* do not mmap a file smaller than this */
 
 struct diff_filespec {
 	unsigned char sha1[20];