From e9280d891b944f6a28ad64796204a1e29523e77b Mon Sep 17 00:00:00 2001 From: Rain Valentine Date: Tue, 22 Oct 2024 21:53:02 +0000 Subject: [PATCH] PR feedback: don't include server.h in hashset Signed-off-by: Rain Valentine --- src/hashset.c | 5 ++--- src/object.c | 6 +++--- src/server.c | 24 ++---------------------- src/server.h | 3 ++- src/zmalloc.c | 19 +++++++++++++++---- src/zmalloc.h | 2 +- 6 files changed, 25 insertions(+), 34 deletions(-) diff --git a/src/hashset.c b/src/hashset.c index 1e6bfc6b164..145b76932ff 100644 --- a/src/hashset.c +++ b/src/hashset.c @@ -49,7 +49,6 @@ * addressing scheme, including the use of linear probing by scan cursor * increment, by Viktor Söderqvist. */ #include "hashset.h" -#include "server.h" #include "serverassert.h" #include "zmalloc.h" #include "mt19937-64.h" @@ -961,10 +960,10 @@ hashset *hashsetDefragInternals(hashset *s, void *(*defragfn)(void *)) { /* Used to release memory to OS to avoid unnecessary CoW. * Called when we've forked and memory won't be used again. - * See dismissObject() */ + * See zmadvise_dontneed() */ void dismissHashset(hashset *t) { for (int i = 0; i < 2; i++) { - dismissMemory(t->tables[i], numBuckets(t->bucketExp[i]) * sizeof(bucket *)); + zmadvise_dontneed(t->tables[i], numBuckets(t->bucketExp[i]) * sizeof(bucket *)); } } diff --git a/src/object.c b/src/object.c index 539403dd2a3..53795560fae 100644 --- a/src/object.c +++ b/src/object.c @@ -542,7 +542,7 @@ void dismissStreamObject(robj *o, size_t size_hint) { * modifies any keys due to write traffic, it'll cause CoW which consume * physical memory. In the child process, after serializing the key and value, * the data is definitely not accessed again, so to avoid unnecessary CoW, we - * try to release their memory back to OS. see dismissMemory(). + * try to release their memory back to OS. see zmadvise_dontneed(). * * Because of the cost of iterating all node/field/member/entry of complex data * types, we iterate and dismiss them only when approximate average we estimate @@ -993,11 +993,11 @@ size_t objectComputeSize(robj *key, robj *o, size_t sample_size, int dbid) { if (o->encoding == OBJ_ENCODING_HASHSET) { hashset *set = o->ptr; asize = sizeof(*o) + hashsetMemUsage(set); - + hashsetIterator iter; hashsetInitIterator(&iter, set); sds element; - while (hashsetNext(&iter, (void**)&element) && samples < sample_size) { + while (hashsetNext(&iter, (void **)&element) && samples < sample_size) { elesize += sdsAllocSize(element); samples++; } diff --git a/src/server.c b/src/server.c index 96683ca5d07..875480aff87 100644 --- a/src/server.c +++ b/src/server.c @@ -6409,27 +6409,7 @@ void sendChildInfo(childInfoType info_type, size_t keys, char *pname) { sendChildInfoGeneric(info_type, keys, -1, pname); } -/* Try to release pages back to the OS directly (bypassing the allocator), - * in an effort to decrease CoW during fork. For small allocations, we can't - * release any full page, so in an effort to avoid getting the size of the - * allocation from the allocator (malloc_size) when we already know it's small, - * we check the size_hint. If the size is not already known, passing a size_hint - * of 0 will lead the checking the real size of the allocation. - * Also please note that the size may be not accurate, so in order to make this - * solution effective, the judgement for releasing memory pages should not be - * too strict. */ -void dismissMemory(void *ptr, size_t size_hint) { - if (ptr == NULL) return; - - /* madvise(MADV_DONTNEED) can not release pages if the size of memory - * is too small, we try to release only for the memory which the size - * is more than half of page size. */ - if (size_hint && size_hint <= server.page_size / 2) return; - - zmadvise_dontneed(ptr); -} - -/* Dismiss big chunks of memory inside a client structure, see dismissMemory() */ +/* Dismiss big chunks of memory inside a client structure, see zmadvise_dontneed() */ void dismissClientMemory(client *c) { /* Dismiss client query buffer and static reply buffer. */ dismissMemory(c->buf, c->buf_usable_size); @@ -6460,7 +6440,7 @@ void dismissClientMemory(client *c) { /* In the child process, we don't need some buffers anymore, and these are * likely to change in the parent when there's heavy write traffic. * We dismiss them right away, to avoid CoW. - * see dismissMemory(). */ + * see zmadvise_dontneed(). */ void dismissMemoryInChild(void) { /* madvise(MADV_DONTNEED) may not work if Transparent Huge Pages is enabled. */ if (server.thp_enabled) return; diff --git a/src/server.h b/src/server.h index 4aab07ea93b..e0ae3679f49 100644 --- a/src/server.h +++ b/src/server.h @@ -82,6 +82,8 @@ typedef long long ustime_t; /* microsecond time type. */ #include "connection.h" /* Connection abstraction */ #include "memory_prefetch.h" +#define dismissMemory zmadvise_dontneed + #define VALKEYMODULE_CORE 1 typedef struct serverObject robj; #include "valkeymodule.h" /* Modules API defines. */ @@ -3329,7 +3331,6 @@ void rejectCommandFormat(client *c, const char *fmt, ...); void *activeDefragAlloc(void *ptr); robj *activeDefragStringOb(robj *ob); void dismissSds(sds s); -void dismissMemory(void *ptr, size_t size_hint); void dismissMemoryInChild(void); #define RESTART_SERVER_NONE 0 diff --git a/src/zmalloc.c b/src/zmalloc.c index e18fa8bac24..4dfc50efccd 100644 --- a/src/zmalloc.c +++ b/src/zmalloc.c @@ -472,15 +472,25 @@ void zmalloc_set_oom_handler(void (*oom_handler)(size_t)) { zmalloc_oom_handler = oom_handler; } -/* Use 'MADV_DONTNEED' to release memory to operating system quickly. - * We do that in a fork child process to avoid CoW when the parent modifies - * these shared pages. */ -void zmadvise_dontneed(void *ptr) { +/* Try to release pages back to the OS directly using 'MADV_DONTNEED' (bypassing + * the allocator) in a fork child process to avoid CoW when the parent modifies + * those shared pages. For small allocations, we can't release any full page, + * so in an effort to avoid getting the size of the allocation from the + * allocator (malloc_size) when we already know it's small, we check the + * size_hint. If the size is not already known, passing a size_hint of 0 will + * lead the checking the real size of the allocation. + * Also please note that the size may be not accurate, so in order to make this + * solution effective, the judgement for releasing memory pages should not be + * too strict. */ +void zmadvise_dontneed(void *ptr, size_t size_hint) { #if defined(USE_JEMALLOC) && defined(__linux__) + if (ptr == NULL) return; + static size_t page_size = 0; if (page_size == 0) page_size = sysconf(_SC_PAGESIZE); size_t page_size_mask = page_size - 1; + if (size_hint && size_hint / 2 < page_size) return; size_t real_size = zmalloc_size(ptr); if (real_size < page_size) return; @@ -494,6 +504,7 @@ void zmadvise_dontneed(void *ptr) { } #else (void)(ptr); + (void)(size_hint); #endif } diff --git a/src/zmalloc.h b/src/zmalloc.h index 9b51f4c8665..97254af2556 100644 --- a/src/zmalloc.h +++ b/src/zmalloc.h @@ -151,7 +151,7 @@ size_t zmalloc_get_smap_bytes_by_field(char *field, long pid); size_t zmalloc_get_memory_size(void); void zlibc_free(void *ptr); void zlibc_trim(void); -void zmadvise_dontneed(void *ptr); +void zmadvise_dontneed(void *ptr, size_t size_hint); #ifdef HAVE_DEFRAG void zfree_no_tcache(void *ptr);