From 0143b7c9dd7ec38fec5469a1055d96e8f4f0984c Mon Sep 17 00:00:00 2001 From: poiuj <1099644+poiuj@users.noreply.github.com> Date: Thu, 20 Jun 2024 02:13:55 +0300 Subject: [PATCH] Add zfree_with_size to optimize sdsfree since we can get zmalloc_size from the header (#453) ### Description ### zfree updates memory statistics. It gets the size of the buffer from jemalloc by calling zmalloc_size. This operation is costly. We can avoid it if we know the buffer size. For example, we can calculate size of sds from the data we have in its header. This commit introduces zfree_with_size function that accepts both pointer to a buffer, and its size. zfree is refactored to call zfree_with_size. sdsfree uses the new interface for all but SDS_TYPE_5. ### Benchmark ### Dataset is 3 million strings. Each benchmark run uses its own value size (8192, 512, and 120). The benchmark is 100% write load for 5 minutes. ``` value size new tps old tps % new us/call old us/call % 8k 272088.53 269971.75 0.78 1.83 1.92 -4.69 512 356881.91 352856.72 1.14 1.27 1.35 -5.93 120 377523.81 368774.78 2.37 1.14 1.19 -4.20 ``` --------- Signed-off-by: Vadym Khoptynets Signed-off-by: Madelyn Olson Co-authored-by: Madelyn Olson --- src/sds.c | 13 +++++++++---- src/sdsalloc.h | 1 + src/zmalloc.c | 49 +++++++++++++++++++++++++++++++++++++------------ src/zmalloc.h | 1 + 4 files changed, 48 insertions(+), 16 deletions(-) diff --git a/src/sds.c b/src/sds.c index c47901d73a..c52c14759b 100644 --- a/src/sds.c +++ b/src/sds.c @@ -195,7 +195,7 @@ sds sdsdup(const sds s) { /* Free an sds string. No operation is performed if 's' is NULL. */ void sdsfree(sds s) { if (s == NULL) return; - s_free((char *)s - sdsHdrSize(s[-1])); + s_free_with_size(sdsAllocPtr(s), sdsAllocSize(s)); } /* Set the sds string length to the length as obtained with strlen(), so @@ -369,7 +369,7 @@ sds sdsResize(sds s, size_t size, int would_regrow) { * We aim to avoid calling realloc() when using Jemalloc if there is no * change in the allocation size, as it incurs a cost even if the * allocation size stays the same. */ - bufsize = zmalloc_size(sh); + bufsize = sdsAllocSize(s); alloc_already_optimal = (je_nallocx(newlen, 0) == bufsize); #endif if (!alloc_already_optimal) { @@ -412,8 +412,13 @@ sds sdsResize(sds s, size_t size, int would_regrow) { * 4) The implicit null term. */ size_t sdsAllocSize(sds s) { - size_t alloc = sdsalloc(s); - return sdsHdrSize(s[-1]) + alloc + 1; + char type = s[-1] & SDS_TYPE_MASK; + /* SDS_TYPE_5 header doesn't contain the size of the allocation */ + if (type == SDS_TYPE_5) { + return s_malloc_size(sdsAllocPtr(s)); + } else { + return sdsHdrSize(type) + sdsalloc(s) + 1; + } } /* Return the pointer of the actual SDS allocation (normally SDS strings diff --git a/src/sdsalloc.h b/src/sdsalloc.h index 6644eb3c83..6fd076d9f0 100644 --- a/src/sdsalloc.h +++ b/src/sdsalloc.h @@ -45,6 +45,7 @@ #define s_trymalloc ztrymalloc #define s_tryrealloc ztryrealloc #define s_free zfree +#define s_free_with_size zfree_with_size #define s_malloc_usable zmalloc_usable #define s_realloc_usable zrealloc_usable #define s_trymalloc_usable ztrymalloc_usable diff --git a/src/zmalloc.c b/src/zmalloc.c index 0117d8d91a..3ab646dd71 100644 --- a/src/zmalloc.c +++ b/src/zmalloc.c @@ -31,6 +31,7 @@ #include "fmacros.h" #include "config.h" #include "solarisfixes.h" +#include "serverassert.h" #include #include @@ -361,22 +362,48 @@ size_t zmalloc_usable_size(void *ptr) { } #endif -void zfree(void *ptr) { -#ifndef HAVE_MALLOC_SIZE - void *realptr; - size_t oldsize; +/* Frees the memory buffer pointed to by ptr and updates statistics. When using + * jemalloc it uses the fast track by specifying the buffer size. + * + * ptr must have been returned by a previous call to the system allocator which + * returned the usable size, such as zmalloc_usable. ptr must not be NULL. The + * caller is responsible to provide the actual allocation size, which may be + * different from the requested size. */ +static inline void zfree_internal(void *ptr, size_t size) { + assert(ptr != NULL); + update_zmalloc_stat_free(size); + +#ifdef USE_JEMALLOC + je_sdallocx(ptr, size, 0); +#else + free(ptr); #endif +} +void zfree(void *ptr) { if (ptr == NULL) return; + #ifdef HAVE_MALLOC_SIZE - update_zmalloc_stat_free(zmalloc_size(ptr)); - free(ptr); + size_t size = zmalloc_size(ptr); #else - realptr = (char *)ptr - PREFIX_SIZE; - oldsize = *((size_t *)realptr); - update_zmalloc_stat_free(oldsize + PREFIX_SIZE); - free(realptr); + ptr = (char *)ptr - PREFIX_SIZE; + size_t data_size = *((size_t *)ptr); + size_t size = data_size + PREFIX_SIZE; +#endif + + zfree_internal(ptr, size); +} + +/* Like zfree(), but doesn't call zmalloc_size(). */ +void zfree_with_size(void *ptr, size_t size) { + if (ptr == NULL) return; + +#ifndef HAVE_MALLOC_SIZE + ptr = (char *)ptr - PREFIX_SIZE; + size += PREFIX_SIZE; #endif + + zfree_internal(ptr, size); } char *zstrdup(const char *s) { @@ -604,8 +631,6 @@ size_t zmalloc_get_rss(void) { #if defined(USE_JEMALLOC) -#include "serverassert.h" - #define STRINGIFY_(x) #x #define STRINGIFY(x) STRINGIFY_(x) diff --git a/src/zmalloc.h b/src/zmalloc.h index a909366c13..421cb9bcaa 100644 --- a/src/zmalloc.h +++ b/src/zmalloc.h @@ -118,6 +118,7 @@ __attribute__((malloc, alloc_size(1), noinline)) void *ztrymalloc(size_t size); __attribute__((malloc, alloc_size(1), noinline)) void *ztrycalloc(size_t size); __attribute__((alloc_size(2), noinline)) void *ztryrealloc(void *ptr, size_t size); void zfree(void *ptr); +void zfree_with_size(void *ptr, size_t size); void *zmalloc_usable(size_t size, size_t *usable); void *zcalloc_usable(size_t size, size_t *usable); void *zrealloc_usable(void *ptr, size_t size, size_t *usable);