From d71478a889935382f3bf8b9a39ab073eba0a856a Mon Sep 17 00:00:00 2001 From: judeng Date: Wed, 24 May 2023 14:40:11 +0800 Subject: [PATCH] postpone the initialization of oject's lru&lfu until it is added to the db as a value object (#11626) This pr can get two performance benefits: 1. Stop redundant initialization when most robj objects are created 2. LRU_CLOCK will no longer be called in io threads, so we can avoid the `atomicGet` Another code optimization: deleted the redundant judgment in dbSetValue, no matter in LFU or LRU, the lru field inold robj is always the freshest (it is always updated in lookupkey), so we don't need to judge if in LFU --- src/db.c | 8 +++++--- src/evict.c | 2 +- src/object.c | 16 +++++++++------- src/server.c | 11 ++++------- src/server.h | 3 ++- tests/unit/maxmemory.tcl | 17 +++++++++++++++++ 6 files changed, 38 insertions(+), 19 deletions(-) diff --git a/src/db.c b/src/db.c index 38881a7f8..9c8342fc7 100644 --- a/src/db.c +++ b/src/db.c @@ -192,6 +192,7 @@ void dbAdd(redisDb *db, robj *key, robj *val) { sds copy = sdsdup(key->ptr); dictEntry *de = dictAddRaw(db->dict, copy, NULL); serverAssertWithInfo(NULL, key, de != NULL); + initObjectLRUOrLFU(val); dictSetVal(db->dict, de, val); signalKeyAsReady(db, key, val->type); if (server.cluster_enabled) slotToKeyAddEntry(de, db); @@ -212,6 +213,7 @@ void dbAdd(redisDb *db, robj *key, robj *val) { int dbAddRDBLoad(redisDb *db, sds key, robj *val) { dictEntry *de = dictAddRaw(db->dict, key, NULL); if (de == NULL) return 0; + initObjectLRUOrLFU(val); dictSetVal(db->dict, de, val); if (server.cluster_enabled) slotToKeyAddEntry(de, db); return 1; @@ -232,9 +234,9 @@ static void dbSetValue(redisDb *db, robj *key, robj *val, int overwrite) { serverAssertWithInfo(NULL,key,de != NULL); robj *old = dictGetVal(de); - if (server.maxmemory_policy & MAXMEMORY_FLAG_LFU) { - val->lru = old->lru; - } + + val->lru = old->lru; + if (overwrite) { /* RM_StringDMA may call dbUnshareStringValue which may free val, so we * need to incr to retain old */ diff --git a/src/evict.c b/src/evict.c index 4ca9f62ed..909714b43 100644 --- a/src/evict.c +++ b/src/evict.c @@ -80,7 +80,7 @@ unsigned int getLRUClock(void) { unsigned int LRU_CLOCK(void) { unsigned int lruclock; if (1000/server.hz <= LRU_CLOCK_RESOLUTION) { - atomicGet(server.lruclock,lruclock); + lruclock = server.lruclock; } else { lruclock = getLRUClock(); } diff --git a/src/object.c b/src/object.c index e089fde87..8a911d334 100644 --- a/src/object.c +++ b/src/object.c @@ -46,15 +46,21 @@ robj *createObject(int type, void *ptr) { o->encoding = OBJ_ENCODING_RAW; o->ptr = ptr; o->refcount = 1; + o->lru = 0; + return o; +} +void initObjectLRUOrLFU(robj *o) { + if (o->refcount == OBJ_SHARED_REFCOUNT) + return; /* Set the LRU to the current lruclock (minutes resolution), or * alternatively the LFU counter. */ if (server.maxmemory_policy & MAXMEMORY_FLAG_LFU) { - o->lru = (LFUGetTimeInMinutes()<<8) | LFU_INIT_VAL; + o->lru = (LFUGetTimeInMinutes() << 8) | LFU_INIT_VAL; } else { o->lru = LRU_CLOCK(); } - return o; + return; } /* Set a special refcount in the object to make it "shared": @@ -91,11 +97,7 @@ robj *createEmbeddedStringObject(const char *ptr, size_t len) { o->encoding = OBJ_ENCODING_EMBSTR; o->ptr = sh+1; o->refcount = 1; - if (server.maxmemory_policy & MAXMEMORY_FLAG_LFU) { - o->lru = (LFUGetTimeInMinutes()<<8) | LFU_INIT_VAL; - } else { - o->lru = LRU_CLOCK(); - } + o->lru = 0; sh->len = len; sh->alloc = len; diff --git a/src/server.c b/src/server.c index 9389e707f..dba3c6262 100644 --- a/src/server.c +++ b/src/server.c @@ -1324,8 +1324,7 @@ int serverCron(struct aeEventLoop *eventLoop, long long id, void *clientData) { * * Note that you can change the resolution altering the * LRU_CLOCK_RESOLUTION define. */ - unsigned int lruclock = getLRUClock(); - atomicSet(server.lruclock,lruclock); + server.lruclock = getLRUClock(); cronUpdateMemoryStats(); @@ -1985,6 +1984,7 @@ void createSharedObjects(void) { for (j = 0; j < OBJ_SHARED_INTEGERS; j++) { shared.integers[j] = makeObjectShared(createObject(OBJ_STRING,(void*)(long)j)); + initObjectLRUOrLFU(shared.integers[j]); shared.integers[j]->encoding = OBJ_ENCODING_INT; } for (j = 0; j < OBJ_SHARED_BULKHDR_LEN; j++) { @@ -2089,8 +2089,7 @@ void initServerConfig(void) { server.latency_tracking_info_percentiles[1] = 99.0; /* p99 */ server.latency_tracking_info_percentiles[2] = 99.9; /* p999 */ - unsigned int lruclock = getLRUClock(); - atomicSet(server.lruclock,lruclock); + server.lruclock = getLRUClock(); resetServerSaveParams(); appendServerSaveParams(60*60,1); /* save after 1 hour and 1 change */ @@ -5496,8 +5495,6 @@ sds genRedisInfoString(dict *section_dict, int all_sections, int everything) { call_uname = 0; } - unsigned int lruclock; - atomicGet(server.lruclock,lruclock); info = sdscatfmt(info, "# Server\r\n" "redis_version:%s\r\n" @@ -5548,7 +5545,7 @@ sds genRedisInfoString(dict *section_dict, int all_sections, int everything) { (int64_t)(uptime/(3600*24)), server.hz, server.config_hz, - lruclock, + server.lruclock, server.executable ? server.executable : "", server.configfile ? server.configfile : "", server.io_threads_active); diff --git a/src/server.h b/src/server.h index c9a97966c..7bf099499 100644 --- a/src/server.h +++ b/src/server.h @@ -1549,7 +1549,7 @@ struct redisServer { dict *orig_commands; /* Command table before command renaming. */ aeEventLoop *el; rax *errors; /* Errors table */ - redisAtomic unsigned int lruclock; /* Clock for LRU eviction */ + unsigned int lruclock; /* Clock for LRU eviction */ volatile sig_atomic_t shutdown_asap; /* Shutdown ordered by signal handler. */ mstime_t shutdown_mstime; /* Timestamp to limit graceful shutdown. */ int last_sig_received; /* Indicates the last SIGNAL received, if any (e.g., SIGINT or SIGTERM). */ @@ -2728,6 +2728,7 @@ void freeZsetObject(robj *o); void freeHashObject(robj *o); void dismissObject(robj *o, size_t dump_size); robj *createObject(int type, void *ptr); +void initObjectLRUOrLFU(robj *o); robj *createStringObject(const char *ptr, size_t len); robj *createRawStringObject(const char *ptr, size_t len); robj *createEmbeddedStringObject(const char *ptr, size_t len); diff --git a/tests/unit/maxmemory.tcl b/tests/unit/maxmemory.tcl index 54aba6715..89eaf9bba 100644 --- a/tests/unit/maxmemory.tcl +++ b/tests/unit/maxmemory.tcl @@ -571,3 +571,20 @@ start_server {tags {"maxmemory" "external:skip"}} { r config set maxmemory-policy noeviction } } + +start_server {tags {"maxmemory" "external:skip"}} { + test {lru/lfu value of the key just added} { + r config set maxmemory-policy allkeys-lru + r set foo a + assert {[r object idletime foo] <= 2} + r del foo + r set foo 1 + r get foo + assert {[r object idletime foo] <= 2} + + r config set maxmemory-policy allkeys-lfu + r del foo + r set foo a + assert {[r object freq foo] == 5} + } +}