malloc: simplify "not my pool" lock dance

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

malloc: simplify "not my pool" lock dance

Otto Moerbeek
Hi,

This simpifies the lock dance when a free is done for a pointer not in
"my pool". Should reduce lock contention.

Please review & test, especially with multithread heavy apps.

        -Otto

Index: malloc.c
===================================================================
RCS file: /cvs/src/lib/libc/stdlib/malloc.c,v
retrieving revision 1.255
diff -u -p -r1.255 malloc.c
--- malloc.c 27 Nov 2018 17:29:55 -0000 1.255
+++ malloc.c 6 Dec 2018 10:26:56 -0000
@@ -1309,14 +1309,14 @@ findpool(void *p, struct dir_info *argpo
 }
 
 static void
-ofree(struct dir_info *argpool, void *p, int clear, int check, size_t argsz)
+ofree(struct dir_info **argpool, void *p, int clear, int check, size_t argsz)
 {
  struct region_info *r;
  struct dir_info *pool;
  char *saved_function;
  size_t sz;
 
- r = findpool(p, argpool, &pool, &saved_function);
+ r = findpool(p, *argpool, &pool, &saved_function);
 
  REALSIZE(sz, r);
  if (check) {
@@ -1405,12 +1405,9 @@ ofree(struct dir_info *argpool, void *p,
  }
  }
 
- if (argpool != pool) {
- pool->active--;
+ if (*argpool != pool) {
  pool->func = saved_function;
- _MALLOC_UNLOCK(pool->mutex);
- _MALLOC_LOCK(argpool->mutex);
- argpool->active++;
+ *argpool = pool;
  }
 }
 
@@ -1433,7 +1430,7 @@ free(void *ptr)
  malloc_recurse(d);
  return;
  }
- ofree(d, ptr, 0, 0, 0);
+ ofree(&d, ptr, 0, 0, 0);
  d->active--;
  _MALLOC_UNLOCK(d->mutex);
  errno = saved_errno;
@@ -1471,7 +1468,7 @@ freezero(void *ptr, size_t sz)
  malloc_recurse(d);
  return;
  }
- ofree(d, ptr, 1, 1, sz);
+ ofree(&d, ptr, 1, 1, sz);
  d->active--;
  _MALLOC_UNLOCK(d->mutex);
  errno = saved_errno;
@@ -1479,7 +1476,7 @@ freezero(void *ptr, size_t sz)
 DEF_WEAK(freezero);
 
 static void *
-orealloc(struct dir_info *argpool, void *p, size_t newsz, void *f)
+orealloc(struct dir_info **argpool, void *p, size_t newsz, void *f)
 {
  struct region_info *r;
  struct dir_info *pool;
@@ -1490,14 +1487,14 @@ orealloc(struct dir_info *argpool, void
  uint32_t chunknum;
 
  if (p == NULL)
- return omalloc(argpool, newsz, 0, f);
+ return omalloc(*argpool, newsz, 0, f);
 
  if (newsz >= SIZE_MAX - mopts.malloc_guard - MALLOC_PAGESIZE) {
  errno = ENOMEM;
  return  NULL;
  }
 
- r = findpool(p, argpool, &pool, &saved_function);
+ r = findpool(p, *argpool, &pool, &saved_function);
 
  REALSIZE(oldsz, r);
  if (mopts.chunk_canaries && oldsz <= MALLOC_MAXCHUNK) {
@@ -1631,7 +1628,7 @@ gotit:
  }
  if (newsz != 0 && oldsz != 0)
  memcpy(q, p, oldsz < newsz ? oldsz : newsz);
- ofree(pool, p, 0, 0, 0);
+ ofree(&pool, p, 0, 0, 0);
  ret = q;
  } else {
  /* oldsz == newsz */
@@ -1641,12 +1638,9 @@ gotit:
  ret = p;
  }
 done:
- if (argpool != pool) {
- pool->active--;
+ if (*argpool != pool) {
  pool->func = saved_function;
- _MALLOC_UNLOCK(pool->mutex);
- _MALLOC_LOCK(argpool->mutex);
- argpool->active++;
+ *argpool = pool;
  }
  return ret;
 }
@@ -1669,7 +1663,7 @@ realloc(void *ptr, size_t size)
  malloc_recurse(d);
  return NULL;
  }
- r = orealloc(d, ptr, size, CALLER);
+ r = orealloc(&d, ptr, size, CALLER);
 
  d->active--;
  _MALLOC_UNLOCK(d->mutex);
@@ -1730,7 +1724,7 @@ calloc(size_t nmemb, size_t size)
 /*DEF_STRONG(calloc);*/
 
 static void *
-orecallocarray(struct dir_info *argpool, void *p, size_t oldsize,
+orecallocarray(struct dir_info **argpool, void *p, size_t oldsize,
     size_t newsize, void *f)
 {
  struct region_info *r;
@@ -1740,12 +1734,12 @@ orecallocarray(struct dir_info *argpool,
  size_t sz;
 
  if (p == NULL)
- return omalloc(argpool, newsize, 1, f);
+ return omalloc(*argpool, newsize, 1, f);
 
  if (oldsize == newsize)
  return p;
 
- r = findpool(p, argpool, &pool, &saved_function);
+ r = findpool(p, *argpool, &pool, &saved_function);
 
  REALSIZE(sz, r);
  if (sz <= MALLOC_MAXCHUNK) {
@@ -1772,15 +1766,12 @@ orecallocarray(struct dir_info *argpool,
  } else
  memcpy(newptr, p, newsize);
 
- ofree(pool, p, 1, 0, oldsize);
+ ofree(&pool, p, 1, 0, oldsize);
 
 done:
- if (argpool != pool) {
- pool->active--;
+ if (*argpool != pool) {
  pool->func = saved_function;
- _MALLOC_UNLOCK(pool->mutex);
- _MALLOC_LOCK(argpool->mutex);
- argpool->active++;
+ *argpool = pool;
  }
 
  return newptr;
@@ -1883,7 +1874,7 @@ recallocarray(void *ptr, size_t oldnmemb
  return NULL;
  }
 
- r = orecallocarray(d, ptr, oldsize, newsize, CALLER);
+ r = orecallocarray(&d, ptr, oldsize, newsize, CALLER);
 
  d->active--;
  _MALLOC_UNLOCK(d->mutex);

Reply | Threaded
Open this post in threaded view
|

Re: malloc: simplify "not my pool" lock dance

Florian Obser-2
This looks correct, OK florian

btw. I had some trouble fining getpool(), OK for the style(9) fix?

diff --git malloc.c malloc.c
index d5a651c7dea..b30ee820cf1 100644
--- malloc.c
+++ malloc.c
@@ -260,8 +260,8 @@ hash(void *p)
  return sum;
 }
 
-static inline
-struct dir_info *getpool(void)
+static inline struct dir_info *
+getpool(void)
 {
  if (!mopts.malloc_mt)
  return mopts.malloc_pool[0];


On Thu, Dec 06, 2018 at 11:30:03AM +0100, Otto Moerbeek wrote:

> Hi,
>
> This simpifies the lock dance when a free is done for a pointer not in
> "my pool". Should reduce lock contention.
>
> Please review & test, especially with multithread heavy apps.
>
> -Otto
>
> Index: malloc.c
> ===================================================================
> RCS file: /cvs/src/lib/libc/stdlib/malloc.c,v
> retrieving revision 1.255
> diff -u -p -r1.255 malloc.c
> --- malloc.c 27 Nov 2018 17:29:55 -0000 1.255
> +++ malloc.c 6 Dec 2018 10:26:56 -0000
> @@ -1309,14 +1309,14 @@ findpool(void *p, struct dir_info *argpo
>  }
>  
>  static void
> -ofree(struct dir_info *argpool, void *p, int clear, int check, size_t argsz)
> +ofree(struct dir_info **argpool, void *p, int clear, int check, size_t argsz)
>  {
>   struct region_info *r;
>   struct dir_info *pool;
>   char *saved_function;
>   size_t sz;
>  
> - r = findpool(p, argpool, &pool, &saved_function);
> + r = findpool(p, *argpool, &pool, &saved_function);
>  
>   REALSIZE(sz, r);
>   if (check) {
> @@ -1405,12 +1405,9 @@ ofree(struct dir_info *argpool, void *p,
>   }
>   }
>  
> - if (argpool != pool) {
> - pool->active--;
> + if (*argpool != pool) {
>   pool->func = saved_function;
> - _MALLOC_UNLOCK(pool->mutex);
> - _MALLOC_LOCK(argpool->mutex);
> - argpool->active++;
> + *argpool = pool;
>   }
>  }
>  
> @@ -1433,7 +1430,7 @@ free(void *ptr)
>   malloc_recurse(d);
>   return;
>   }
> - ofree(d, ptr, 0, 0, 0);
> + ofree(&d, ptr, 0, 0, 0);
>   d->active--;
>   _MALLOC_UNLOCK(d->mutex);
>   errno = saved_errno;
> @@ -1471,7 +1468,7 @@ freezero(void *ptr, size_t sz)
>   malloc_recurse(d);
>   return;
>   }
> - ofree(d, ptr, 1, 1, sz);
> + ofree(&d, ptr, 1, 1, sz);
>   d->active--;
>   _MALLOC_UNLOCK(d->mutex);
>   errno = saved_errno;
> @@ -1479,7 +1476,7 @@ freezero(void *ptr, size_t sz)
>  DEF_WEAK(freezero);
>  
>  static void *
> -orealloc(struct dir_info *argpool, void *p, size_t newsz, void *f)
> +orealloc(struct dir_info **argpool, void *p, size_t newsz, void *f)
>  {
>   struct region_info *r;
>   struct dir_info *pool;
> @@ -1490,14 +1487,14 @@ orealloc(struct dir_info *argpool, void
>   uint32_t chunknum;
>  
>   if (p == NULL)
> - return omalloc(argpool, newsz, 0, f);
> + return omalloc(*argpool, newsz, 0, f);
>  
>   if (newsz >= SIZE_MAX - mopts.malloc_guard - MALLOC_PAGESIZE) {
>   errno = ENOMEM;
>   return  NULL;
>   }
>  
> - r = findpool(p, argpool, &pool, &saved_function);
> + r = findpool(p, *argpool, &pool, &saved_function);
>  
>   REALSIZE(oldsz, r);
>   if (mopts.chunk_canaries && oldsz <= MALLOC_MAXCHUNK) {
> @@ -1631,7 +1628,7 @@ gotit:
>   }
>   if (newsz != 0 && oldsz != 0)
>   memcpy(q, p, oldsz < newsz ? oldsz : newsz);
> - ofree(pool, p, 0, 0, 0);
> + ofree(&pool, p, 0, 0, 0);
>   ret = q;
>   } else {
>   /* oldsz == newsz */
> @@ -1641,12 +1638,9 @@ gotit:
>   ret = p;
>   }
>  done:
> - if (argpool != pool) {
> - pool->active--;
> + if (*argpool != pool) {
>   pool->func = saved_function;
> - _MALLOC_UNLOCK(pool->mutex);
> - _MALLOC_LOCK(argpool->mutex);
> - argpool->active++;
> + *argpool = pool;
>   }
>   return ret;
>  }
> @@ -1669,7 +1663,7 @@ realloc(void *ptr, size_t size)
>   malloc_recurse(d);
>   return NULL;
>   }
> - r = orealloc(d, ptr, size, CALLER);
> + r = orealloc(&d, ptr, size, CALLER);
>  
>   d->active--;
>   _MALLOC_UNLOCK(d->mutex);
> @@ -1730,7 +1724,7 @@ calloc(size_t nmemb, size_t size)
>  /*DEF_STRONG(calloc);*/
>  
>  static void *
> -orecallocarray(struct dir_info *argpool, void *p, size_t oldsize,
> +orecallocarray(struct dir_info **argpool, void *p, size_t oldsize,
>      size_t newsize, void *f)
>  {
>   struct region_info *r;
> @@ -1740,12 +1734,12 @@ orecallocarray(struct dir_info *argpool,
>   size_t sz;
>  
>   if (p == NULL)
> - return omalloc(argpool, newsize, 1, f);
> + return omalloc(*argpool, newsize, 1, f);
>  
>   if (oldsize == newsize)
>   return p;
>  
> - r = findpool(p, argpool, &pool, &saved_function);
> + r = findpool(p, *argpool, &pool, &saved_function);
>  
>   REALSIZE(sz, r);
>   if (sz <= MALLOC_MAXCHUNK) {
> @@ -1772,15 +1766,12 @@ orecallocarray(struct dir_info *argpool,
>   } else
>   memcpy(newptr, p, newsize);
>  
> - ofree(pool, p, 1, 0, oldsize);
> + ofree(&pool, p, 1, 0, oldsize);
>  
>  done:
> - if (argpool != pool) {
> - pool->active--;
> + if (*argpool != pool) {
>   pool->func = saved_function;
> - _MALLOC_UNLOCK(pool->mutex);
> - _MALLOC_LOCK(argpool->mutex);
> - argpool->active++;
> + *argpool = pool;
>   }
>  
>   return newptr;
> @@ -1883,7 +1874,7 @@ recallocarray(void *ptr, size_t oldnmemb
>   return NULL;
>   }
>  
> - r = orecallocarray(d, ptr, oldsize, newsize, CALLER);
> + r = orecallocarray(&d, ptr, oldsize, newsize, CALLER);
>  
>   d->active--;
>   _MALLOC_UNLOCK(d->mutex);
>


--
I'm not entirely sure you are real.

Reply | Threaded
Open this post in threaded view
|

Re: malloc: simplify "not my pool" lock dance

Otto Moerbeek
In reply to this post by Otto Moerbeek
On Thu, Dec 06, 2018 at 11:30:03AM +0100, Otto Moerbeek wrote:

> Hi,
>
> This simpifies the lock dance when a free is done for a pointer not in
> "my pool". Should reduce lock contention.
>
> Please review & test, especially with multithread heavy apps.

This is now committed. Thanks to all the testers. Running this should
give you a noticable improvement in speed for multi-threaded apps
doing lots f allocations and de-alloctions (i.e. your web browser).

        -Otto

>
> -Otto
>
> Index: malloc.c
> ===================================================================
> RCS file: /cvs/src/lib/libc/stdlib/malloc.c,v
> retrieving revision 1.255
> diff -u -p -r1.255 malloc.c
> --- malloc.c 27 Nov 2018 17:29:55 -0000 1.255
> +++ malloc.c 6 Dec 2018 10:26:56 -0000
> @@ -1309,14 +1309,14 @@ findpool(void *p, struct dir_info *argpo
>  }
>  
>  static void
> -ofree(struct dir_info *argpool, void *p, int clear, int check, size_t argsz)
> +ofree(struct dir_info **argpool, void *p, int clear, int check, size_t argsz)
>  {
>   struct region_info *r;
>   struct dir_info *pool;
>   char *saved_function;
>   size_t sz;
>  
> - r = findpool(p, argpool, &pool, &saved_function);
> + r = findpool(p, *argpool, &pool, &saved_function);
>  
>   REALSIZE(sz, r);
>   if (check) {
> @@ -1405,12 +1405,9 @@ ofree(struct dir_info *argpool, void *p,
>   }
>   }
>  
> - if (argpool != pool) {
> - pool->active--;
> + if (*argpool != pool) {
>   pool->func = saved_function;
> - _MALLOC_UNLOCK(pool->mutex);
> - _MALLOC_LOCK(argpool->mutex);
> - argpool->active++;
> + *argpool = pool;
>   }
>  }
>  
> @@ -1433,7 +1430,7 @@ free(void *ptr)
>   malloc_recurse(d);
>   return;
>   }
> - ofree(d, ptr, 0, 0, 0);
> + ofree(&d, ptr, 0, 0, 0);
>   d->active--;
>   _MALLOC_UNLOCK(d->mutex);
>   errno = saved_errno;
> @@ -1471,7 +1468,7 @@ freezero(void *ptr, size_t sz)
>   malloc_recurse(d);
>   return;
>   }
> - ofree(d, ptr, 1, 1, sz);
> + ofree(&d, ptr, 1, 1, sz);
>   d->active--;
>   _MALLOC_UNLOCK(d->mutex);
>   errno = saved_errno;
> @@ -1479,7 +1476,7 @@ freezero(void *ptr, size_t sz)
>  DEF_WEAK(freezero);
>  
>  static void *
> -orealloc(struct dir_info *argpool, void *p, size_t newsz, void *f)
> +orealloc(struct dir_info **argpool, void *p, size_t newsz, void *f)
>  {
>   struct region_info *r;
>   struct dir_info *pool;
> @@ -1490,14 +1487,14 @@ orealloc(struct dir_info *argpool, void
>   uint32_t chunknum;
>  
>   if (p == NULL)
> - return omalloc(argpool, newsz, 0, f);
> + return omalloc(*argpool, newsz, 0, f);
>  
>   if (newsz >= SIZE_MAX - mopts.malloc_guard - MALLOC_PAGESIZE) {
>   errno = ENOMEM;
>   return  NULL;
>   }
>  
> - r = findpool(p, argpool, &pool, &saved_function);
> + r = findpool(p, *argpool, &pool, &saved_function);
>  
>   REALSIZE(oldsz, r);
>   if (mopts.chunk_canaries && oldsz <= MALLOC_MAXCHUNK) {
> @@ -1631,7 +1628,7 @@ gotit:
>   }
>   if (newsz != 0 && oldsz != 0)
>   memcpy(q, p, oldsz < newsz ? oldsz : newsz);
> - ofree(pool, p, 0, 0, 0);
> + ofree(&pool, p, 0, 0, 0);
>   ret = q;
>   } else {
>   /* oldsz == newsz */
> @@ -1641,12 +1638,9 @@ gotit:
>   ret = p;
>   }
>  done:
> - if (argpool != pool) {
> - pool->active--;
> + if (*argpool != pool) {
>   pool->func = saved_function;
> - _MALLOC_UNLOCK(pool->mutex);
> - _MALLOC_LOCK(argpool->mutex);
> - argpool->active++;
> + *argpool = pool;
>   }
>   return ret;
>  }
> @@ -1669,7 +1663,7 @@ realloc(void *ptr, size_t size)
>   malloc_recurse(d);
>   return NULL;
>   }
> - r = orealloc(d, ptr, size, CALLER);
> + r = orealloc(&d, ptr, size, CALLER);
>  
>   d->active--;
>   _MALLOC_UNLOCK(d->mutex);
> @@ -1730,7 +1724,7 @@ calloc(size_t nmemb, size_t size)
>  /*DEF_STRONG(calloc);*/
>  
>  static void *
> -orecallocarray(struct dir_info *argpool, void *p, size_t oldsize,
> +orecallocarray(struct dir_info **argpool, void *p, size_t oldsize,
>      size_t newsize, void *f)
>  {
>   struct region_info *r;
> @@ -1740,12 +1734,12 @@ orecallocarray(struct dir_info *argpool,
>   size_t sz;
>  
>   if (p == NULL)
> - return omalloc(argpool, newsize, 1, f);
> + return omalloc(*argpool, newsize, 1, f);
>  
>   if (oldsize == newsize)
>   return p;
>  
> - r = findpool(p, argpool, &pool, &saved_function);
> + r = findpool(p, *argpool, &pool, &saved_function);
>  
>   REALSIZE(sz, r);
>   if (sz <= MALLOC_MAXCHUNK) {
> @@ -1772,15 +1766,12 @@ orecallocarray(struct dir_info *argpool,
>   } else
>   memcpy(newptr, p, newsize);
>  
> - ofree(pool, p, 1, 0, oldsize);
> + ofree(&pool, p, 1, 0, oldsize);
>  
>  done:
> - if (argpool != pool) {
> - pool->active--;
> + if (*argpool != pool) {
>   pool->func = saved_function;
> - _MALLOC_UNLOCK(pool->mutex);
> - _MALLOC_LOCK(argpool->mutex);
> - argpool->active++;
> + *argpool = pool;
>   }
>  
>   return newptr;
> @@ -1883,7 +1874,7 @@ recallocarray(void *ptr, size_t oldnmemb
>   return NULL;
>   }
>  
> - r = orecallocarray(d, ptr, oldsize, newsize, CALLER);
> + r = orecallocarray(&d, ptr, oldsize, newsize, CALLER);
>  
>   d->active--;
>   _MALLOC_UNLOCK(d->mutex);
>