From: Koji Nakamaru Date: Wed, 22 Oct 2025 01:22:12 GMT Subject: Re: [PATCH] macOS: queue for munmap operations Message-ID: In-Reply-To: <20251021062621.GA370@tb-raspi4> 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 wrote: > > 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 > > > > 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. > > 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 > > --- > > 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 > > +#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. > > + > > +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. > > + 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 > > > > +#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) > > [snip] [1] https://stackoverflow.com/questions/9081479/is-there-a-good-reason-for-always-enclosing-a-define-in-parentheses-in-c