Skip to contentSkip to conversation

MinGW: implement mmap

PATCH In review Johannes Sixt 2 participants 2 messages Jun 15, 2016 Kbuild

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 MinGW: implement mmap

From: Janos Laube <[email protected]>
Date: Fri, 13 Mar 2009 16:50:45 +0100

Add USE_WIN32_MMAP which triggers the use of windows' native file memory mapping functionality in git_mmap()/git_munmap() functions.

As git functions currently use mmap with MAP_PRIVATE set only, this implementation supports only that mode for now.

On Windows, offsets for memory mapped files need to match the allocation granularity. Take this into account when calculating the packed git- windowsize and file offsets. At the moment, the only function which makes use of offsets in conjunction with mmap is use_pack() in sha1-file.c.

Git fast-import's code path tries to map a portion of the temporary packfile that exceeds the current filesize, i.e. offset+length is greater than the filesize. The NO_MMAP code worked with that since pread() just reads the file content until EOF and returns gracefully, while MapViewOfFile() aborts the mapping and returns 'Access Denied'. Working around that by determining the filesize and adjusting the length parameter.

---
 Makefile           |    7 +++++-
 compat/mingw.h     |    5 ++++
 compat/win32mmap.c |   53 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 git-compat-util.h  |   12 ++++++++--
 4 files changed, 73 insertions(+), 4 deletions(-)
 create mode 100644 compat/win32mmap.c
Makefile +6 -1
index 1087884..38e75e5 100644
@@ -776,7 +776,6 @@ ifneq (,$(findstring CYGWIN,$(uname_S)))
 	COMPAT_OBJS += compat/cygwin.o
 endif
 ifneq (,$(findstring MINGW,$(uname_S)))
-	NO_MMAP = YesPlease
 	NO_PREAD = YesPlease
 	NO_OPENSSL = YesPlease
 	NO_CURL = YesPlease
@@ -799,6 +798,7 @@ ifneq (,$(findstring MINGW,$(uname_S)))
 	RUNTIME_PREFIX = YesPlease
 	NO_POSIX_ONLY_PROGRAMS = YesPlease
 	NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease
+	USE_WIN32_MMAP = YesPlease
 	COMPAT_CFLAGS += -D__USE_MINGW_ACCESS -DNOGDI -Icompat -Icompat/regex -Icompat/fnmatch
 	COMPAT_CFLAGS += -DSNPRINTF_SIZE_CORR=1
 	COMPAT_CFLAGS += -DSTRIP_EXTENSION=\".exe\"
@@ -967,6 +967,11 @@ endif
 ifdef NO_MMAP
 	COMPAT_CFLAGS += -DNO_MMAP
 	COMPAT_OBJS += compat/mmap.o
+else
+	ifdef USE_WIN32_MMAP
+		COMPAT_CFLAGS += -DUSE_WIN32_MMAP
+		COMPAT_OBJS += compat/win32mmap.o
+	endif
 endif
 ifdef NO_PREAD
 	COMPAT_CFLAGS += -DNO_PREAD
compat/mingw.h +5 -0
index 6e24686..f5da647 100644
@@ -160,6 +160,11 @@ int mingw_connect(int sockfd, struct sockaddr *sa, size_t sz);
 int mingw_rename(const char*, const char*);
 #define rename mingw_rename
 
+#ifdef USE_WIN32_MMAP
+int mingw_getpagesize(void);
+#define getpagesize mingw_getpagesize
+#endif
+
 /* Use mingw_lstat() instead of lstat()/stat() and
  * mingw_fstat() instead of fstat() on Windows.
  */
compat/win32mmap.c new file +53 -0

53 lines not on this page · raw message

git-compat-util.h +9 -3
index 878d83d..1eef4eb 100644
@@ -166,7 +166,7 @@ static inline const char *skip_prefix(const char *str, const char *prefix)
 	return strncmp(str, prefix, len) ? NULL : str + len;
 }
 
-#ifdef NO_MMAP
+#if defined(NO_MMAP) || defined(USE_WIN32_MMAP)
 
 #ifndef PROT_READ
 #define PROT_READ 1
@@ -180,13 +180,19 @@ static inline const char *skip_prefix(const char *str, const char *prefix)
 extern void *git_mmap(void *start, size_t length, int prot, int flags, int fd, off_t offset);
 extern int git_munmap(void *start, size_t length);
 
+#else /* NO_MMAP || USE_WIN32_MMAP */
+
+#include <sys/mman.h>
+
+#endif /* NO_MMAP || USE_WIN32_MMAP */
+
+#ifdef NO_MMAP
+
 /* This value must be multiple of (pagesize * 2) */
 #define DEFAULT_PACKED_GIT_WINDOW_SIZE (1 * 1024 * 1024)
 
 #else /* NO_MMAP */
 
-#include <sys/mman.h>
-
 /* This value must be multiple of (pagesize * 2) */
 #define DEFAULT_PACKED_GIT_WINDOW_SIZE \
 	(sizeof(void*) >= 8 \
Signature · 1 line
1.6.2.rc2.971.g14d5
#1 lore raw

Hi,

Quoted 22 lines · Johannes Sixt

From: Janos Laube <[email protected]>
Date: Fri, 13 Mar 2009 16:50:45 +0100

Add USE_WIN32_MMAP which triggers the use of windows' native file memory mapping functionality in git_mmap()/git_munmap() functions.

As git functions currently use mmap with MAP_PRIVATE set only, this implementation supports only that mode for now.

On Windows, offsets for memory mapped files need to match the allocation granularity. Take this into account when calculating the packed git- windowsize and file offsets. At the moment, the only function which makes use of offsets in conjunction with mmap is use_pack() in sha1-file.c.

Git fast-import's code path tries to map a portion of the temporary packfile that exceeds the current filesize, i.e. offset+length is greater than the filesize. The NO_MMAP code worked with that since pread() just reads the file content until EOF and returns gracefully, while MapViewOfFile() aborts the mapping and returns 'Access Denied'. Working around that by determining the filesize and adjusting the length parameter.

Quoted 3 lines

---

In case anybody wonders why this is not integrated into mingw.[ch]: I asked for it, as it is Windows-specific and could benefit Cygwin (or MSVC, should someone attempt that), too.

And I think I missed this style in the patch:

compat/win32mmap.c line 16

Quoted 3 lines
compat/win32mmap.c new file +3 -0
+void *git_mmap
+(void *start, size_t length, int prot, int flags, int fd, off_t offset)
+{

... which I would like to see fixed, of course... :-)

Unfortunately, there is also a long line, which is corrupted due to wrapping:

compat/win32mmap.c line 37

Quoted 3 lines
(quoted diff) +1 -0
+	hmap = CreateFileMapping((HANDLE)_get_osfhandle(fd), 0,

PAGE_WRITECOPY,

compat/win32mmap.c new file +1 -0
+		0, 0, 0);

It is easily fixed by moving the PAGE_WRITECOPY to the next line.

These are only style issues, though.

The more important part is: how much does this buy us (or is it more expensive, as we saw one MacOSX at one stage). Janos was nice enough to perform some benchmark tests, and saw a ~40% improvement on rev-list --objects --all on a sizeable project (GCC 'twas, IIRC). Funnily, the numbers got better when reducing the window, up until 1MB, and they got worse when enlarging the window.

So I guess there will be a follow-up patch at some stage which changes the default pack window size on Windows to something smaller than 64MB.

Ciao,
Dscho