On Wed, 10 Sep 2025, Paul Eggert wrote:
On 2025-09-10 17:19, Harry Sintonen wrote:
pagealign code (lib/pagealign_alloc.c) has a mistake whereas USE_MMAP code
uses the linked list to maintain the memory allocations. This is unnessary
as mmap() always returns memory aligned to the page size. Thus the buffer
could be passed as-is, similar to what HAVE_POSIX_MEMALIGN code is doing
But then how would pagealign_free (PTR) know what size to pass when it calls
munmap (PTR, SIZE)?
Indeed it would not. This obviously was a classic case of me getting too
clever without actually checking that it'd actually work. Oopsie.
You've hit on a severe scaling problem with pagealign_alloc. Thanks for
reporting it. From the symptoms you described, I fixed the performance bug by
installing the attached patch, which simply prefers posix_memalign to mmap.
Although mmap can outperform posix_memalign, evidently that win is small
compared to the loss you mention. Plus, the posix_memalign solution is
thread-safe.
Although we could instead (or also) try to optimize the mmap (or even the
malloc) cases I wonder whether it's worth the hassle nowadays. More likely
there are platforms where using C11-style aligned_alloc would be a
performance win. That would be easy to add, if there's a need.
All fair points.
For my use case I ended up using two different solutions:
1. Make the CVS server use posix_memalign in a similar vein as your
official fix, but with a quick'n'dirty hack by just inserting a strategic
#undef HAVE_MMAP and rebuilding the deb package. It can stay running until
a better solution is eventually available from Debian.
2. Elsewhere (for client side) I ended up replacing use of pagealign_alloc
entirely. This client is running on a platform that doesn't really do
paging to begin with (no real mmap), so I just ended up calling malloc/
free there. Less memory overhead and much faster as there's no worry
about tracking the "aligned allocs". This of course just sidesteps the
issue, but was good enough solution in this particular happenstance. If
there's some new CVS release that addresses these issues then I can always
switch to that, later.
Regards,
--
l=2001;main(i){float o,O,_,I,D;for(;O=I=l/571.-1.75,l;)for(putchar(--l%80?
i:10),o=D=l%80*.05-2,i=31;_=O*O,O=2*o*O+I,o=o*o-_+D,o+_+_<4+D&i++<87;);puts
(" Harry 'Piru' Sintonen <[email protected]> https://www.iki.fi/sintonen";);}
