unneeded mremap calls in realloc

unneeded mremap calls in realloc

[Szabolcs Nagy]

as discussed on irc, in realloc there is a mremap
where newlen is pagesize adjusted but oldlen is not
so oldlen==newlen almost always fails

run this simple test case with strace to see the issue:

#include <stdlib.h>
int main(){
	char *p = 0;
	int n;

	for (n = 0; n < 500000; n++)
		p = realloc(p, n);
	free(p);
	return 0;
}

the fix that significantly speeds up the above code:
(there might be better fix, eg why oldlen is not a
multiple of pagesize in the first place?)

diff --git a/src/malloc/malloc.c b/src/malloc/malloc.c
index abf3e8f..8e9b022 100644
--- a/src/malloc/malloc.c
+++ b/src/malloc/malloc.c
@@ -401,7 +401,7 @@ void *realloc(void *p, size_t n)
                        return new;
                }
                newlen = (newlen + PAGE_SIZE-1) & -PAGE_SIZE;
-               if (oldlen == newlen) return p;
+               if (((oldlen + PAGE_SIZE-1) & -PAGE_SIZE) == newlen) return p;
                base = __mremap(base, oldlen, newlen, MREMAP_MAYMOVE);
                if (base == (void *)-1)
                        return newlen < oldlen ? p : 0;

Re: unneeded mremap calls in realloc

[Rich Felker]

On Wed, Nov 16, 2011 at 01:45:37AM +0100, Szabolcs Nagy wrote:
> 
> as discussed on irc, in realloc there is a mremap
> where newlen is pagesize adjusted but oldlen is not
> so oldlen==newlen almost always fails
> 
> run this simple test case with strace to see the issue:
> 
> #include <stdlib.h>
> int main(){
> 	char *p = 0;
> 	int n;
> 
> 	for (n = 0; n < 500000; n++)
> 		p = realloc(p, n);
> 	free(p);
> 	return 0;
> }
> 
> the fix that significantly speeds up the above code:
> (there might be better fix, eg why oldlen is not a
> multiple of pagesize in the first place?)

oldlen should always be a multiple of page size.. Need to check out
why it's not...

Rich

Re: unneeded mremap calls in realloc

[Rich Felker]

On Wed, Nov 16, 2011 at 01:45:37AM +0100, Szabolcs Nagy wrote:
> (there might be better fix, eg why oldlen is not a
> multiple of pagesize in the first place?)

oldlen was computed in terms of CHUNK_SIZE macro, which was incorrect
for mmapped chunks (masking off too many bits). should be fixed now.

Rich

Re: unneeded mremap calls in realloc

[Szabolcs Nagy]

* Rich Felker <dalias@aerifal.cx> [2011-11-17 00:02:53 -0500]:

> On Wed, Nov 16, 2011 at 01:45:37AM +0100, Szabolcs Nagy wrote:
> > (there might be better fix, eg why oldlen is not a
> > multiple of pagesize in the first place?)
> 
> oldlen was computed in terms of CHUNK_SIZE macro, which was incorrect
> for mmapped chunks (masking off too many bits). should be fixed now.
> 
> Rich

btw in realloc there is this comment:

        /* FIXME: find what's wrong here and reenable it..? */
        if (0 && n > n1 && alloc_rev(self)) {
                self = PREV_CHUNK(self);
                n1 += CHUNK_SIZE(self);
        }

can it be that the CHUNK_SIZE fix solves this as well?