Thread (1 message) 1 message, 1 author, 2016-08-11

Re: git-diff opens too many files?

From: Junio C Hamano <hidden>
Date: 2016-08-11 20:36:20
Subsystem: the rest · Maintainer: Linus Torvalds

Possibly related (same subject, not in this thread)

Linus Torvalds [off-list ref] 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 <redacted>
---
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];
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help