From 3945a321779a0be52e556aa11babb3fe15883a78 Mon Sep 17 00:00:00 2001 From: Oran Agra Date: Fri, 2 Oct 2020 08:19:44 +0300 Subject: [PATCH] performance and memory reporting improvement - sds take control of it's internal frag (#7875) This commit has two aspects: 1) improve memory reporting for all the places that use sdsAllocSize to compute memory used by a string, in this case it'll include the internal fragmentation. 2) reduce the need for realloc calls by making the sds implicitly take over the internal fragmentation of the block it allocated. --- src/networking.c | 6 ++-- src/sds.c | 38 ++++++++++++++++----- src/sdsalloc.h | 3 ++ src/zmalloc.c | 86 +++++++++++++++++++++++++++++++++++++++++++++++- src/zmalloc.h | 8 +++-- 5 files changed, 127 insertions(+), 14 deletions(-) diff --git a/src/networking.c b/src/networking.c index 6d9bf960f..bfb65f5a1 100644 --- a/src/networking.c +++ b/src/networking.c @@ -293,7 +293,7 @@ void _addReplyProtoToList(client *c, const char *s, size_t len) { size_t size = len < PROTO_REPLY_CHUNK_BYTES? PROTO_REPLY_CHUNK_BYTES: len; tail = zmalloc(size + sizeof(clientReplyBlock)); /* take over the allocation's internal fragmentation */ - tail->size = zmalloc_usable(tail) - sizeof(clientReplyBlock); + tail->size = zmalloc_usable_size(tail) - sizeof(clientReplyBlock); tail->used = len; memcpy(tail->buf, s, len); listAddNodeTail(c->reply, tail); @@ -489,7 +489,7 @@ void trimReplyUnusedTailSpace(client *c) { tail = zrealloc(tail, tail->used + sizeof(clientReplyBlock)); /* take over the allocation's internal fragmentation (at least for * memory usage tracking) */ - tail->size = zmalloc_usable(tail) - sizeof(clientReplyBlock); + tail->size = zmalloc_usable_size(tail) - sizeof(clientReplyBlock); c->reply_bytes = c->reply_bytes + tail->size - old_size; listNodeValue(ln) = tail; } @@ -541,7 +541,7 @@ void setDeferredAggregateLen(client *c, void *node, long length, char prefix) { /* Create a new node */ clientReplyBlock *buf = zmalloc(lenstr_len + sizeof(clientReplyBlock)); /* Take over the allocation's internal fragmentation */ - buf->size = zmalloc_usable(buf) - sizeof(clientReplyBlock); + buf->size = zmalloc_usable_size(buf) - sizeof(clientReplyBlock); buf->used = lenstr_len; memcpy(buf->buf, lenstr, lenstr_len); listNodeValue(ln) = buf; diff --git a/src/sds.c b/src/sds.c index e30b33b6c..4cbbabfbb 100644 --- a/src/sds.c +++ b/src/sds.c @@ -73,6 +73,20 @@ static inline char sdsReqType(size_t string_size) { #endif } +static inline size_t sdsTypeMaxSize(char type) { + if (type == SDS_TYPE_5) + return (1<<5) - 1; + if (type == SDS_TYPE_8) + return (1<<8) - 1; + if (type == SDS_TYPE_16) + return (1<<16) - 1; +#if (LONG_MAX == LLONG_MAX) + if (type == SDS_TYPE_32) + return (1ll<<32) - 1; +#endif + return -1; /* this is equivalent to the max SDS_TYPE_64 or SDS_TYPE_32 */ +} + /* Create a new sds string with the content specified by the 'init' pointer * and 'initlen'. * If NULL is used for 'init' the string is initialized with zero bytes. @@ -95,8 +109,9 @@ sds sdsnewlen(const void *init, size_t initlen) { if (type == SDS_TYPE_5 && initlen == 0) type = SDS_TYPE_8; int hdrlen = sdsHdrSize(type); unsigned char *fp; /* flags pointer. */ + size_t usable; - sh = s_malloc(hdrlen+initlen+1); + sh = s_malloc_usable(hdrlen+initlen+1, &usable); if (sh == NULL) return NULL; if (init==SDS_NOINIT) init = NULL; @@ -104,6 +119,9 @@ sds sdsnewlen(const void *init, size_t initlen) { memset(sh, 0, hdrlen+initlen+1); s = (char*)sh+hdrlen; fp = ((unsigned char*)s)-1; + usable = usable-hdrlen-1; + if (usable > sdsTypeMaxSize(type)) + usable = sdsTypeMaxSize(type); switch(type) { case SDS_TYPE_5: { *fp = type | (initlen << SDS_TYPE_BITS); @@ -112,28 +130,28 @@ sds sdsnewlen(const void *init, size_t initlen) { case SDS_TYPE_8: { SDS_HDR_VAR(8,s); sh->len = initlen; - sh->alloc = initlen; + sh->alloc = usable; *fp = type; break; } case SDS_TYPE_16: { SDS_HDR_VAR(16,s); sh->len = initlen; - sh->alloc = initlen; + sh->alloc = usable; *fp = type; break; } case SDS_TYPE_32: { SDS_HDR_VAR(32,s); sh->len = initlen; - sh->alloc = initlen; + sh->alloc = usable; *fp = type; break; } case SDS_TYPE_64: { SDS_HDR_VAR(64,s); sh->len = initlen; - sh->alloc = initlen; + sh->alloc = usable; *fp = type; break; } @@ -207,6 +225,7 @@ sds sdsMakeRoomFor(sds s, size_t addlen) { size_t len, newlen; char type, oldtype = s[-1] & SDS_TYPE_MASK; int hdrlen; + size_t usable; /* Return ASAP if there is enough space left. */ if (avail >= addlen) return s; @@ -228,13 +247,13 @@ sds sdsMakeRoomFor(sds s, size_t addlen) { hdrlen = sdsHdrSize(type); if (oldtype==type) { - newsh = s_realloc(sh, hdrlen+newlen+1); + newsh = s_realloc_usable(sh, hdrlen+newlen+1, &usable); if (newsh == NULL) return NULL; s = (char*)newsh+hdrlen; } else { /* Since the header size changes, need to move the string forward, * and can't use realloc */ - newsh = s_malloc(hdrlen+newlen+1); + newsh = s_malloc_usable(hdrlen+newlen+1, &usable); if (newsh == NULL) return NULL; memcpy((char*)newsh+hdrlen, s, len+1); s_free(sh); @@ -242,7 +261,10 @@ sds sdsMakeRoomFor(sds s, size_t addlen) { s[-1] = type; sdssetlen(s, len); } - sdssetalloc(s, newlen); + usable = usable-hdrlen-1; + if (usable > sdsTypeMaxSize(type)) + usable = sdsTypeMaxSize(type); + sdssetalloc(s, usable); return s; } diff --git a/src/sdsalloc.h b/src/sdsalloc.h index c04ff2a0a..725ad79e3 100644 --- a/src/sdsalloc.h +++ b/src/sdsalloc.h @@ -43,5 +43,8 @@ #define s_malloc zmalloc #define s_realloc zrealloc #define s_free zfree +#define s_malloc_usable zmalloc_usable +#define s_realloc_usable zrealloc_usable +#define s_free_usable zfree_usable #endif diff --git a/src/zmalloc.c b/src/zmalloc.c index 01cb0f917..4a7f2028e 100644 --- a/src/zmalloc.c +++ b/src/zmalloc.c @@ -99,6 +99,21 @@ void *zmalloc(size_t size) { #endif } +/* Similar to zmalloc, '*usable' is set to the usable size. */ +void *zmalloc_usable(size_t size, size_t *usable) { + void *ptr = malloc(size+PREFIX_SIZE); + + if (!ptr) zmalloc_oom_handler(size); +#ifdef HAVE_MALLOC_SIZE + update_zmalloc_stat_alloc(*usable = zmalloc_size(ptr)); + return ptr; +#else + *((size_t*)ptr) = *usable = size; + update_zmalloc_stat_alloc(size+PREFIX_SIZE); + return (char*)ptr+PREFIX_SIZE; +#endif +} + /* Allocation and free functions that bypass the thread cache * and go straight to the allocator arena bins. * Currently implemented only for jemalloc. Used for online defragmentation. */ @@ -131,6 +146,21 @@ void *zcalloc(size_t size) { #endif } +/* Similar to zcalloc, '*usable' is set to the usable size. */ +void *zcalloc_usable(size_t size, size_t *usable) { + void *ptr = calloc(1, size+PREFIX_SIZE); + + if (!ptr) zmalloc_oom_handler(size); +#ifdef HAVE_MALLOC_SIZE + update_zmalloc_stat_alloc(*usable = zmalloc_size(ptr)); + return ptr; +#else + *((size_t*)ptr) = *usable = size; + update_zmalloc_stat_alloc(size+PREFIX_SIZE); + return (char*)ptr+PREFIX_SIZE; +#endif +} + void *zrealloc(void *ptr, size_t size) { #ifndef HAVE_MALLOC_SIZE void *realptr; @@ -164,6 +194,41 @@ void *zrealloc(void *ptr, size_t size) { #endif } +/* Similar to zrealloc, '*usable' is set to the new usable size. */ +void *zrealloc_usable(void *ptr, size_t size, size_t *usable) { +#ifndef HAVE_MALLOC_SIZE + void *realptr; +#endif + size_t oldsize; + void *newptr; + + if (size == 0 && ptr != NULL) { + zfree(ptr); + *usable = 0; + return NULL; + } + if (ptr == NULL) return zmalloc_usable(size, usable); +#ifdef HAVE_MALLOC_SIZE + oldsize = zmalloc_size(ptr); + newptr = realloc(ptr,size); + if (!newptr) zmalloc_oom_handler(size); + + update_zmalloc_stat_free(oldsize); + update_zmalloc_stat_alloc(*usable = zmalloc_size(newptr)); + return newptr; +#else + realptr = (char*)ptr-PREFIX_SIZE; + oldsize = *((size_t*)realptr); + newptr = realloc(realptr,size+PREFIX_SIZE); + if (!newptr) zmalloc_oom_handler(size); + + *((size_t*)newptr) = *usable = size; + update_zmalloc_stat_free(oldsize); + update_zmalloc_stat_alloc(size); + return (char*)newptr+PREFIX_SIZE; +#endif +} + /* Provide zmalloc_size() for systems where this function is not provided by * malloc itself, given that in that case we store a header with this * information as the first bytes of every allocation. */ @@ -176,7 +241,7 @@ size_t zmalloc_size(void *ptr) { if (size&(sizeof(long)-1)) size += sizeof(long)-(size&(sizeof(long)-1)); return size+PREFIX_SIZE; } -size_t zmalloc_usable(void *ptr) { +size_t zmalloc_usable_size(void *ptr) { return zmalloc_size(ptr)-PREFIX_SIZE; } #endif @@ -199,6 +264,25 @@ void zfree(void *ptr) { #endif } +/* Similar to zfree, '*usable' is set to the usable size being freed. */ +void zfree_usable(void *ptr, size_t *usable) { +#ifndef HAVE_MALLOC_SIZE + void *realptr; + size_t oldsize; +#endif + + if (ptr == NULL) return; +#ifdef HAVE_MALLOC_SIZE + update_zmalloc_stat_free(*usable = zmalloc_size(ptr)); + free(ptr); +#else + realptr = (char*)ptr-PREFIX_SIZE; + *usable = oldsize = *((size_t*)realptr); + update_zmalloc_stat_free(oldsize+PREFIX_SIZE); + free(realptr); +#endif +} + char *zstrdup(const char *s) { size_t l = strlen(s)+1; char *p = zmalloc(l); diff --git a/src/zmalloc.h b/src/zmalloc.h index b136a910d..df63253ef 100644 --- a/src/zmalloc.h +++ b/src/zmalloc.h @@ -81,6 +81,10 @@ void *zmalloc(size_t size); void *zcalloc(size_t size); void *zrealloc(void *ptr, size_t size); void zfree(void *ptr); +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); +void zfree_usable(void *ptr, size_t *usable); char *zstrdup(const char *s); size_t zmalloc_used_memory(void); void zmalloc_set_oom_handler(void (*oom_handler)(size_t)); @@ -100,9 +104,9 @@ void *zmalloc_no_tcache(size_t size); #ifndef HAVE_MALLOC_SIZE size_t zmalloc_size(void *ptr); -size_t zmalloc_usable(void *ptr); +size_t zmalloc_usable_size(void *ptr); #else -#define zmalloc_usable(p) zmalloc_size(p) +#define zmalloc_usable_size(p) zmalloc_size(p) #endif #ifdef REDIS_TEST