Re: [PATCH] macOS: queue for munmap operations

>
> Some comments inline, all up to improvements
>
> On Mon, Oct 20, 2025 at 10:35:02PM +0000, Koji Nakamaru via GitGitGadget wrote:
> > From: Koji Nakamaru <koji.nakamaru@gree.net>
> >
> > 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.

> > 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 <koji.nakamaru@gree.net>
> > ---
> >     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

> > +
> > +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.

> > +     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