Re: [PATCH] macOS: queue for munmap operations

Thank you for detailed suggestions. As I discussed in another thread,
the root cause of many mmap/munmap calls was simply my ~/.gitconfig, so
I'm withdrawing this patch. I'll answer some of your comments below.

On Tue, Oct 21, 2025 at 3:26 PM Torsten Bögershausen [off-list ref] wrote:

Some comments inline, all up to improvements

On Mon, Oct 20, 2025 at 10:35:02PM +0000, Koji Nakamaru via GitGitGadget wrote:

quoted

From: Koji Nakamaru <redacted>

Executing many mmap/munmap calls alternately can cause a huge load on
macOS. In order to reduce it, we should temporarily store munmap
operations in a queue and process them all at once when the queue is
filled. When the program terminates, we can discard any remaining munmap
operations as corresponding mmaped regions are automatically reclaimed.

Add a queue for munmap operations to perform them all at once.

Suggestions for rewording:
In order to reduce the peak load store all munmap operations in a queue.
Process them all at once (and more efficient) when the queue is filled.
The queue may be ignored when the git process terminates. The operating
system will do all munmap() when the process exits.

Thank you, it is much clear.

quoted

Here are some example timings. On the Linux kernel repository that
requires about 1700 mmap/munmap calls:

  time git ls-tree -r -l --full-tree 211ddde > /dev/null

  Before:
        real    0m2.083s
        user    0m0.201s
        sys     0m1.873s

  After:
        real    0m0.243s
        user    0m0.179s
        sys     0m0.052s

On a private repository that requires about 943000 mmap/munmap calls:

  time git ls-tree -r -l --full-tree xxxxxxx > /dev/null

  Before:
        real    27m15.138s
        user    0m5.084s
        sys     27m9.636s

  After:
        real    0m24.209s
        user    0m3.055s
        sys     0m21.123s

Signed-off-by: Koji Nakamaru <redacted>
---
    macOS: queue for munmap operations

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1993%2FKojiNakamaru%2Ffeature%2Fosx-queued-munmap-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1993/KojiNakamaru/feature/osx-queued-munmap-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1993

 Makefile                            |  1 +
 compat/osxmmap.c                    | 49 +++++++++++++++++++++++++++++
 compat/posix.h                      |  7 +++++
 contrib/buildsystems/CMakeLists.txt |  4 +++
 meson.build                         |  2 ++
 5 files changed, 63 insertions(+)
 create mode 100644 compat/osxmmap.c

diff --git a/Makefile b/Makefile
index f79c905bdc..058bc83753 100644
--- a/Makefile
+++ b/Makefile

@@ -1654,6 +1654,7 @@ ifeq ($(uname_S),Darwin)
              COMPAT_CFLAGS += -DAPPLE_COMMON_CRYPTO
         endif
      PTHREAD_LIBS =
+     COMPAT_OBJS += compat/osxmmap.o
 endif

 ifdef NO_LIBGEN_H

diff --git a/compat/osxmmap.c b/compat/osxmmap.c
new file mode 100644
index 0000000000..5f9cf633ca
--- /dev/null
+++ b/compat/osxmmap.c

@@ -0,0 +1,49 @@
+#include <pthread.h>
+#include "../git-compat-util.h"
+/* We need original mmap/munmap here. */
+#undef mmap
+#undef munmap
+
+/*
+ * OSX doesn't have any specific setting like Linux's vm.max_map_count,
+ * so COUNT_MAX can be any large number. We here set it to the default
+ * value of Linux's vm.max_map_count.
+ */
+#define COUNT_MAX (65530)

Why the parantheses ?
And would a less generic name be better, like
MAX_UNMAP_COUNT

The parentheses are not required but I prefer them as discussed in [1].
I agree MAX_UNMAP_COUNT is more clear.

quoted

+
+struct munmap_queue {
+     void *start;
+     size_t length;
+};
+
+void *git_mmap(void *start, size_t length, int prot, int flags, int fd, off_t offset)
+{
+     /*
+      * We can simply discard munmap operations in the queue by
+      * restricting mmap arguments.
+      */

Should I read this as
The munmap queue is only ment to defere read-only mappings.
And that is what Git does at the moment.

Yes. This part is actually borrowed from compat/mmap.c and I've also
verified that the predicate is valid by searching all mmap calls.

quoted

+     if (start != NULL || flags != MAP_PRIVATE || prot != PROT_READ)
+             die("invalid usage of mmap");
+     return mmap(start, length, prot, flags, fd, offset);
+}
+
+int git_munmap(void *start, size_t length)
+{
+     static pthread_mutex_t mutex;
+     static struct munmap_queue *queue;
+     static int count;
+     int i;
+
+     pthread_mutex_lock(&mutex);
+     if (!queue)
+             queue = xmalloc(COUNT_MAX * sizeof(struct munmap_queue));
+     queue[count].start = start;
+     queue[count].length = length;
+     if (++count == COUNT_MAX) {
+             for (i = 0; i < COUNT_MAX; i++)
+                     munmap(queue[i].start, queue[i].length);
+             count = 0;
+     }
+     pthread_mutex_unlock(&mutex);
+     return 0;
+}

diff --git a/compat/posix.h b/compat/posix.h
index 067a00f33b..3fa1218289 100644
--- a/compat/posix.h
+++ b/compat/posix.h

@@ -278,6 +278,13 @@ int git_munmap(void *start, size_t length);

 #include <sys/mman.h>

+#if defined(__APPLE__)

I think it would be better to have a global Makefile knob here.
Which
a) allows to take out this patch once the MacOs kernel is improved
b) allows to hook in this code for other OS
Something like DEFER_MUNMAPS - better suggestions welcome

I followed your suggestion and adjusted code and Makefile, etc. (locally)

quoted

[snip]

[1] https://stackoverflow.com/questions/9081479/is-there-a-good-reason-for-always-enclosing-a-define-in-parentheses-in-c