Skip to content

Commit

Permalink
PR feedback: don't include server.h in hashset
Browse files Browse the repository at this point in the history
Signed-off-by: Rain Valentine <[email protected]>
  • Loading branch information
SoftlyRaining committed Oct 22, 2024
1 parent b8ffef2 commit e9280d8
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 34 deletions.
5 changes: 2 additions & 3 deletions src/hashset.c
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 *));
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/object.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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++;
}
Expand Down
24 changes: 2 additions & 22 deletions src/server.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion src/server.h
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down Expand Up @@ -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
Expand Down
19 changes: 15 additions & 4 deletions src/zmalloc.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -494,6 +504,7 @@ void zmadvise_dontneed(void *ptr) {
}
#else
(void)(ptr);
(void)(size_hint);
#endif
}

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

0 comments on commit e9280d8

Please sign in to comment.