Re: malloc cleanup and small optimization (step 3)
On Wed, Jan 03, 2018 at 01:56:43PM +0100, Otto Moerbeek wrote:
> Hi,
>
> this is mostly kshe's work, with an additional change on the cache
> maintenance by me.
>
> I did not take the ffs change yet, since I want to measure the impact
> separately.
no feedback :-(
I'm going to commit this soon to force testing for machines running
snaps or current.
-Otto
>
> Index: malloc.c
> ===
> RCS file: /cvs/src/lib/libc/stdlib/malloc.c,v
> retrieving revision 1.238
> diff -u -p -r1.238 malloc.c
> --- malloc.c 1 Jan 2018 12:41:48 - 1.238
> +++ malloc.c 3 Jan 2018 12:52:14 -
> @@ -376,12 +376,11 @@ omalloc_parseopt(char opt)
> case 'X':
> mopts.malloc_xmalloc = 1;
> break;
> - default: {
> + default:
> dprintf(STDERR_FILENO, "malloc() warning: "
> "unknown char in MALLOC_OPTIONS\n");
> break;
> }
> - }
> }
>
> static void
> @@ -448,9 +447,6 @@ omalloc_init(void)
> ;
> }
>
> -/*
> - * Initialize a dir_info, which should have been cleared by caller
> - */
> static void
> omalloc_poolinit(struct dir_info **dp)
> {
> @@ -502,7 +498,7 @@ omalloc_grow(struct dir_info *d)
> size_t i;
> struct region_info *p;
>
> - if (d->regions_total > SIZE_MAX / sizeof(struct region_info) / 2 )
> + if (d->regions_total > SIZE_MAX / sizeof(struct region_info) / 2)
> return 1;
>
> newtotal = d->regions_total * 2;
> @@ -641,9 +637,9 @@ static void
> unmap(struct dir_info *d, void *p, size_t sz, int clear)
> {
> size_t psz = sz >> MALLOC_PAGESHIFT;
> - size_t rsz, tounmap;
> + size_t rsz;
> struct region_info *r;
> - u_int i, offset;
> + u_int i, offset, mask;
>
> if (sz != PAGEROUND(sz))
> wrterror(d, "munmap round");
> @@ -662,30 +658,34 @@ unmap(struct dir_info *d, void *p, size_
> STATS_SUB(d->malloc_used, sz);
> return;
> }
> - tounmap = 0;
> - if (psz > rsz)
> - tounmap = psz - rsz;
> offset = getrbyte(d);
> - for (i = 0; tounmap > 0 && i < mopts.malloc_cache; i++) {
> - r = >free_regions[(i + offset) & (mopts.malloc_cache - 1)];
> - if (r->p != NULL) {
> - rsz = r->size << MALLOC_PAGESHIFT;
> - if (munmap(r->p, rsz))
> - wrterror(d, "munmap %p", r->p);
> - r->p = NULL;
> - if (tounmap > r->size)
> - tounmap -= r->size;
> - else
> - tounmap = 0;
> - d->free_regions_size -= r->size;
> - r->size = 0;
> - STATS_SUB(d->malloc_used, rsz);
> + mask = mopts.malloc_cache - 1;
> + if (psz > rsz) {
> + size_t tounmap = psz - rsz;
> + i = 0;
> + for (;;) {
> + r = >free_regions[(i + offset) & mask];
> + if (r->p != NULL) {
> + rsz = r->size << MALLOC_PAGESHIFT;
> + if (munmap(r->p, rsz))
> + wrterror(d, "munmap %p", r->p);
> + r->p = NULL;
> + if (tounmap > r->size)
> + tounmap -= r->size;
> + else
> + tounmap = 0;
> + d->free_regions_size -= r->size;
> + STATS_SUB(d->malloc_used, rsz);
> + if (tounmap == 0) {
> + offset = i;
> + break;
> + }
> + }
> + i++;
> }
> }
> - if (tounmap > 0)
> - wrterror(d, "malloc cache underflow");
> - for (i = 0; i < mopts.malloc_cache; i++) {
> - r = >free_regions[(i + offset) & (mopts.malloc_cache - 1)];
> + for (i = 0; ; i++) {
> + r = >free_regions[(i + offset) & mask];
> if (r->p == NULL) {
> if (clear)
> memset(p, 0, sz - mopts.malloc_guard);
> @@ -702,8 +702,6 @@ unmap(struct dir_info *d, void *p, size_
> break;
> }
> }
> - if (i == mopts.malloc_cache)
> - wrterror(d, "malloc free slot lost");
> if (d->free_regions_size > mopts.malloc_cache)
> wrterror(d, "malloc cache overflow");
> }
> @@ -723,7 +721,6 @@ zapcacheregion(struct dir_info *d, void
> wrterror(d, "munmap %p", r->p);
> r->p = NULL;
> d->free_regions_size -= r->size;
> -
malloc cleanup and small optimization (step 3)
Hi,
this is mostly kshe's work, with an additional change on the cache
maintenance by me.
I did not take the ffs change yet, since I want to measure the impact
separately.
-Otto
Index: malloc.c
===
RCS file: /cvs/src/lib/libc/stdlib/malloc.c,v
retrieving revision 1.238
diff -u -p -r1.238 malloc.c
--- malloc.c1 Jan 2018 12:41:48 - 1.238
+++ malloc.c3 Jan 2018 12:52:14 -
@@ -376,12 +376,11 @@ omalloc_parseopt(char opt)
case 'X':
mopts.malloc_xmalloc = 1;
break;
- default: {
+ default:
dprintf(STDERR_FILENO, "malloc() warning: "
"unknown char in MALLOC_OPTIONS\n");
break;
}
- }
}
static void
@@ -448,9 +447,6 @@ omalloc_init(void)
;
}
-/*
- * Initialize a dir_info, which should have been cleared by caller
- */
static void
omalloc_poolinit(struct dir_info **dp)
{
@@ -502,7 +498,7 @@ omalloc_grow(struct dir_info *d)
size_t i;
struct region_info *p;
- if (d->regions_total > SIZE_MAX / sizeof(struct region_info) / 2 )
+ if (d->regions_total > SIZE_MAX / sizeof(struct region_info) / 2)
return 1;
newtotal = d->regions_total * 2;
@@ -641,9 +637,9 @@ static void
unmap(struct dir_info *d, void *p, size_t sz, int clear)
{
size_t psz = sz >> MALLOC_PAGESHIFT;
- size_t rsz, tounmap;
+ size_t rsz;
struct region_info *r;
- u_int i, offset;
+ u_int i, offset, mask;
if (sz != PAGEROUND(sz))
wrterror(d, "munmap round");
@@ -662,30 +658,34 @@ unmap(struct dir_info *d, void *p, size_
STATS_SUB(d->malloc_used, sz);
return;
}
- tounmap = 0;
- if (psz > rsz)
- tounmap = psz - rsz;
offset = getrbyte(d);
- for (i = 0; tounmap > 0 && i < mopts.malloc_cache; i++) {
- r = >free_regions[(i + offset) & (mopts.malloc_cache - 1)];
- if (r->p != NULL) {
- rsz = r->size << MALLOC_PAGESHIFT;
- if (munmap(r->p, rsz))
- wrterror(d, "munmap %p", r->p);
- r->p = NULL;
- if (tounmap > r->size)
- tounmap -= r->size;
- else
- tounmap = 0;
- d->free_regions_size -= r->size;
- r->size = 0;
- STATS_SUB(d->malloc_used, rsz);
+ mask = mopts.malloc_cache - 1;
+ if (psz > rsz) {
+ size_t tounmap = psz - rsz;
+ i = 0;
+ for (;;) {
+ r = >free_regions[(i + offset) & mask];
+ if (r->p != NULL) {
+ rsz = r->size << MALLOC_PAGESHIFT;
+ if (munmap(r->p, rsz))
+ wrterror(d, "munmap %p", r->p);
+ r->p = NULL;
+ if (tounmap > r->size)
+ tounmap -= r->size;
+ else
+ tounmap = 0;
+ d->free_regions_size -= r->size;
+ STATS_SUB(d->malloc_used, rsz);
+ if (tounmap == 0) {
+ offset = i;
+ break;
+ }
+ }
+ i++;
}
}
- if (tounmap > 0)
- wrterror(d, "malloc cache underflow");
- for (i = 0; i < mopts.malloc_cache; i++) {
- r = >free_regions[(i + offset) & (mopts.malloc_cache - 1)];
+ for (i = 0; ; i++) {
+ r = >free_regions[(i + offset) & mask];
if (r->p == NULL) {
if (clear)
memset(p, 0, sz - mopts.malloc_guard);
@@ -702,8 +702,6 @@ unmap(struct dir_info *d, void *p, size_
break;
}
}
- if (i == mopts.malloc_cache)
- wrterror(d, "malloc free slot lost");
if (d->free_regions_size > mopts.malloc_cache)
wrterror(d, "malloc cache overflow");
}
@@ -723,7 +721,6 @@ zapcacheregion(struct dir_info *d, void
wrterror(d, "munmap %p", r->p);
r->p = NULL;
d->free_regions_size -= r->size;
- r->size = 0;
STATS_SUB(d->malloc_used, rsz);
}
}
@@ -760,7 +757,6 @@ map(struct dir_info *d, void *hint, size
if (r->size == psz) {
p =
