Skip to content

Commit

Permalink
Add zfree_with_size to optimize sdsfree since we can get zmalloc_size…
Browse files Browse the repository at this point in the history
… 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 <[email protected]>
Signed-off-by: Madelyn Olson <[email protected]>
Co-authored-by: Madelyn Olson <[email protected]>
  • Loading branch information
poiuj and madolson authored Jun 19, 2024
1 parent 5d2348c commit 0143b7c
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 16 deletions.
13 changes: 9 additions & 4 deletions src/sds.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions src/sdsalloc.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
49 changes: 37 additions & 12 deletions src/zmalloc.c
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#include "fmacros.h"
#include "config.h"
#include "solarisfixes.h"
#include "serverassert.h"

#include <stdio.h>
#include <stdlib.h>
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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)

Expand Down
1 change: 1 addition & 0 deletions src/zmalloc.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 0143b7c

Please sign in to comment.