From c8144daafa9c4d55a9b14a8f9797010d7911470d Mon Sep 17 00:00:00 2001 From: Malavan Date: Tue, 4 May 2021 21:34:25 +0000 Subject: [PATCH 1/8] add lookup limit to active subkey expire Former-commit-id: b44f504ced6b8259fd5b6553cec932b512e5dfa2 --- src/expire.cpp | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/src/expire.cpp b/src/expire.cpp index 1d14f7152..7f7d4ddd6 100644 --- a/src/expire.cpp +++ b/src/expire.cpp @@ -68,11 +68,12 @@ void activeExpireCycleExpireFullKey(redisDb *db, const char *key) { *----------------------------------------------------------------------------*/ -void activeExpireCycleExpire(redisDb *db, expireEntry &e, long long now) { +int activeExpireCycleExpire(redisDb *db, expireEntry &e, long long now, int &tried) { if (!e.FFat()) { activeExpireCycleExpireFullKey(db, e.key()); - return; + ++tried; + return 1; } expireEntryFat *pfat = e.pfatentry(); @@ -86,6 +87,7 @@ void activeExpireCycleExpire(redisDb *db, expireEntry &e, long long now) { while (!pfat->FEmpty()) { + ++tried; if (pfat->nextExpireEntry().when > now) break; @@ -93,7 +95,7 @@ void activeExpireCycleExpire(redisDb *db, expireEntry &e, long long now) { if (pfat->nextExpireEntry().spsubkey == nullptr) { activeExpireCycleExpireFullKey(db, e.key()); - return; + return ++deleted; } switch (val->type) @@ -103,7 +105,7 @@ void activeExpireCycleExpire(redisDb *db, expireEntry &e, long long now) { deleted++; if (setTypeSize(val) == 0) { activeExpireCycleExpireFullKey(db, e.key()); - return; + return deleted; } } break; @@ -113,7 +115,7 @@ void activeExpireCycleExpire(redisDb *db, expireEntry &e, long long now) { deleted++; if (hashTypeLength(val) == 0) { activeExpireCycleExpireFullKey(db, e.key()); - return; + return deleted; } } break; @@ -123,7 +125,7 @@ void activeExpireCycleExpire(redisDb *db, expireEntry &e, long long now) { deleted++; if (zsetLength(val) == 0) { activeExpireCycleExpireFullKey(db, e.key()); - return; + return deleted; } } break; @@ -138,7 +140,7 @@ void activeExpireCycleExpire(redisDb *db, expireEntry &e, long long now) { decrRefCount(val); }, true /*fLock*/, true /*fForceQueue*/); } - return; + return deleted; case OBJ_LIST: default: @@ -151,6 +153,9 @@ void activeExpireCycleExpire(redisDb *db, expireEntry &e, long long now) { pfat->popfrontExpireEntry(); fTtlChanged = true; + if ((tried % ACTIVE_EXPIRE_CYCLE_LOOKUPS_PER_LOOP) == 0) { + return deleted; + } } if (!pfat->FEmpty() && fTtlChanged) @@ -177,6 +182,7 @@ void activeExpireCycleExpire(redisDb *db, expireEntry &e, long long now) { { removeExpire(db, &objKey); } + return deleted; } int parseUnitString(const char *sz) @@ -376,16 +382,14 @@ void activeExpireCycleCore(int type) { continue; } - size_t expired = 0; - size_t tried = 0; + int expired = 0; + int tried = 0; long long check = ACTIVE_EXPIRE_CYCLE_FAST_DURATION; // assume a check is roughly 1us. It isn't but good enough db->expireitr = db->setexpire->enumerate(db->expireitr, now, [&](expireEntry &e) __attribute__((always_inline)) { if (e.when() < now) { - activeExpireCycleExpire(db, e, now); - ++expired; + expired += activeExpireCycleExpire(db, e, now, tried); } - ++tried; if ((tried % ACTIVE_EXPIRE_CYCLE_LOOKUPS_PER_LOOP) == 0) { @@ -493,8 +497,8 @@ void expireSlaveKeys(void) { if (itr != db->setexpire->end()) { if (itr->when() < start) { - activeExpireCycleExpire(g_pserver->db+dbid,*itr,start); - expired = 1; + int tried = 0; + expired = activeExpireCycleExpire(g_pserver->db+dbid,*itr,start,tried); } } From 67f74ef465be5fc5118364164bfe5a9bdef334e5 Mon Sep 17 00:00:00 2001 From: Malavan Date: Tue, 4 May 2021 22:28:03 +0000 Subject: [PATCH 2/8] add lookup limit to active subkey expire Former-commit-id: 5d6da23108401b3b554eaec5d7c78aa0950a28f2 --- src/expire.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/expire.cpp b/src/expire.cpp index 7f7d4ddd6..4d7b4ce0a 100644 --- a/src/expire.cpp +++ b/src/expire.cpp @@ -68,7 +68,7 @@ void activeExpireCycleExpireFullKey(redisDb *db, const char *key) { *----------------------------------------------------------------------------*/ -int activeExpireCycleExpire(redisDb *db, expireEntry &e, long long now, int &tried) { +int activeExpireCycleExpire(redisDb *db, expireEntry &e, long long now, size_t &tried) { if (!e.FFat()) { activeExpireCycleExpireFullKey(db, e.key()); @@ -154,7 +154,7 @@ int activeExpireCycleExpire(redisDb *db, expireEntry &e, long long now, int &tri pfat->popfrontExpireEntry(); fTtlChanged = true; if ((tried % ACTIVE_EXPIRE_CYCLE_LOOKUPS_PER_LOOP) == 0) { - return deleted; + break; } } @@ -382,8 +382,8 @@ void activeExpireCycleCore(int type) { continue; } - int expired = 0; - int tried = 0; + size_t expired = 0; + size_t tried = 0; long long check = ACTIVE_EXPIRE_CYCLE_FAST_DURATION; // assume a check is roughly 1us. It isn't but good enough db->expireitr = db->setexpire->enumerate(db->expireitr, now, [&](expireEntry &e) __attribute__((always_inline)) { if (e.when() < now) @@ -497,7 +497,7 @@ void expireSlaveKeys(void) { if (itr != db->setexpire->end()) { if (itr->when() < start) { - int tried = 0; + size_t tried = 0; expired = activeExpireCycleExpire(g_pserver->db+dbid,*itr,start,tried); } } From d0de1741ed188be8a5c303f81ec98d10d5b9e494 Mon Sep 17 00:00:00 2001 From: Malavan Date: Fri, 7 May 2021 09:40:23 +0000 Subject: [PATCH 3/8] Speedup keyIsExpired by removing subkey search Former-commit-id: a01564158e40300ab4a0338c0a6e924972385407 --- src/db.cpp | 12 ++---------- src/expire.cpp | 2 +- src/expire.h | 32 +++++++++++++++++++++++++++----- src/server.h | 1 + 4 files changed, 31 insertions(+), 16 deletions(-) diff --git a/src/db.cpp b/src/db.cpp index de2ed2754..3935686c5 100644 --- a/src/db.cpp +++ b/src/db.cpp @@ -1626,17 +1626,9 @@ int keyIsExpired(redisDb *db, robj *key) { /* Don't expire anything while loading. It will be done later. */ if (g_pserver->loading) return 0; - long long when = -1; - for (auto &exp : *pexpire) - { - if (exp.subkey() == nullptr) - { - when = exp.when(); - break; - } - } + long long when = pexpire->whenFull(); - if (when == -1) + if (when == INVALID_EXPIRE) return 0; /* If we are in the context of a Lua script, we pretend that time is diff --git a/src/expire.cpp b/src/expire.cpp index 4d7b4ce0a..b6022c712 100644 --- a/src/expire.cpp +++ b/src/expire.cpp @@ -153,7 +153,7 @@ int activeExpireCycleExpire(redisDb *db, expireEntry &e, long long now, size_t & pfat->popfrontExpireEntry(); fTtlChanged = true; - if ((tried % ACTIVE_EXPIRE_CYCLE_LOOKUPS_PER_LOOP) == 0) { + if ((tried % ACTIVE_EXPIRE_CYCLE_SUBKEY_LOOKUPS_PER_LOOP) == 0) { break; } } diff --git a/src/expire.h b/src/expire.h index d002d6383..3600fcd97 100644 --- a/src/expire.h +++ b/src/expire.h @@ -1,5 +1,7 @@ #pragma once +#define INVALID_EXPIRE LLONG_MIN + class expireEntryFat { friend class expireEntry; @@ -31,6 +33,14 @@ public: ~expireEntryFat(); long long when() const noexcept { return m_vecexpireEntries.front().when; } + long long whenFull() const noexcept { + for (size_t i = 0; i < size(); ++i) { + if (m_vecexpireEntries[i].spsubkey == nullptr) { + return m_vecexpireEntries[i].when; + } + } + return INVALID_EXPIRE; + } const char *key() const noexcept { return m_keyPrimary; } bool operator<(long long when) const noexcept { return this->when() < when; } @@ -51,7 +61,7 @@ class expireEntry { expireEntryFat *m_pfatentry; } u; long long m_when; // LLONG_MIN means this is a fat entry and we should use the pointer - + long long m_whenFull; public: class iter { @@ -91,7 +101,8 @@ public: { if (subkey != nullptr) { - m_when = LLONG_MIN; + m_when = INVALID_EXPIRE; + m_whenFull = INVALID_EXPIRE; u.m_pfatentry = new (MALLOC_LOCAL) expireEntryFat(key); u.m_pfatentry->expireSubKey(subkey, when); } @@ -99,19 +110,22 @@ public: { u.m_key = key; m_when = when; + m_whenFull = when; } } expireEntry(expireEntryFat *pfatentry) { u.m_pfatentry = pfatentry; - m_when = LLONG_MIN; + m_when = INVALID_EXPIRE; + m_whenFull = pfatentry->whenFull(); } expireEntry(expireEntry &&e) { u.m_key = e.u.m_key; m_when = e.m_when; + m_whenFull = e.m_whenFull; e.u.m_key = (char*)key(); // we do this so it can still be found in the set e.m_when = 0; } @@ -130,7 +144,7 @@ public: u.m_key = key; } - inline bool FFat() const noexcept { return m_when == LLONG_MIN; } + inline bool FFat() const noexcept { return m_when == INVALID_EXPIRE; } expireEntryFat *pfatentry() { assert(FFat()); return u.m_pfatentry; } @@ -160,9 +174,17 @@ public: return u.m_pfatentry->when(); return m_when; } + long long whenFull() const noexcept + { + return m_whenFull; + } void update(const char *subkey, long long when) { + if (subkey == nullptr) + { + m_whenFull = when; + } if (!FFat()) { if (subkey == nullptr) @@ -175,7 +197,7 @@ public: // we have to upgrade to a fat entry long long whenT = m_when; sds keyPrimary = u.m_key; - m_when = LLONG_MIN; + m_when = INVALID_EXPIRE; u.m_pfatentry = new (MALLOC_LOCAL) expireEntryFat(keyPrimary); u.m_pfatentry->expireSubKey(nullptr, whenT); // at this point we're fat so fall through diff --git a/src/server.h b/src/server.h index 1eae8b0f2..c1bb7dd9f 100644 --- a/src/server.h +++ b/src/server.h @@ -303,6 +303,7 @@ inline bool operator!=(const void *p, const robj_sharedptr &rhs) #define CONFIG_DEFAULT_ENABLE_MULTIMASTER 0 #define ACTIVE_EXPIRE_CYCLE_LOOKUPS_PER_LOOP 64 /* Loopkups per loop. */ +#define ACTIVE_EXPIRE_CYCLE_SUBKEY_LOOKUPS_PER_LOOP 16384 /* Subkey loopkups per loop. */ #define ACTIVE_EXPIRE_CYCLE_FAST_DURATION 1000 /* Microseconds */ #define ACTIVE_EXPIRE_CYCLE_SLOW_TIME_PERC 25 /* CPU max % for keys collection */ #define ACTIVE_EXPIRE_CYCLE_SLOW 0 From fa533a5b62b725e9b6d57e57c843ab91d4b32aa6 Mon Sep 17 00:00:00 2001 From: Malavan Date: Sat, 15 May 2021 05:48:23 +0000 Subject: [PATCH 4/8] merge when and whenFull Former-commit-id: 693b209cb42ea357b3cbc4a757730fa2ac001729 --- src/expire.h | 37 +++++++++++++++---------------------- 1 file changed, 15 insertions(+), 22 deletions(-) diff --git a/src/expire.h b/src/expire.h index 3600fcd97..a2effca5c 100644 --- a/src/expire.h +++ b/src/expire.h @@ -61,7 +61,10 @@ class expireEntry { expireEntryFat *m_pfatentry; } u; long long m_when; // LLONG_MIN means this is a fat entry and we should use the pointer - long long m_whenFull; + + long long FFatMask() const noexcept { + return (1LL) << (sizeof(long long)*CHAR_BIT-1); + } public: class iter { @@ -101,8 +104,7 @@ public: { if (subkey != nullptr) { - m_when = INVALID_EXPIRE; - m_whenFull = INVALID_EXPIRE; + m_when = FFatMask() | INVALID_EXPIRE; u.m_pfatentry = new (MALLOC_LOCAL) expireEntryFat(key); u.m_pfatentry->expireSubKey(subkey, when); } @@ -110,22 +112,19 @@ public: { u.m_key = key; m_when = when; - m_whenFull = when; } } expireEntry(expireEntryFat *pfatentry) { u.m_pfatentry = pfatentry; - m_when = INVALID_EXPIRE; - m_whenFull = pfatentry->whenFull(); + m_when = FFatMask() | pfatentry->whenFull(); } expireEntry(expireEntry &&e) { u.m_key = e.u.m_key; m_when = e.m_when; - m_whenFull = e.m_whenFull; e.u.m_key = (char*)key(); // we do this so it can still be found in the set e.m_when = 0; } @@ -144,7 +143,7 @@ public: u.m_key = key; } - inline bool FFat() const noexcept { return m_when == INVALID_EXPIRE; } + inline bool FFat() const noexcept { return m_when & FFatMask(); } expireEntryFat *pfatentry() { assert(FFat()); return u.m_pfatentry; } @@ -172,19 +171,15 @@ public: { if (FFat()) return u.m_pfatentry->when(); - return m_when; + return whenFull(); } long long whenFull() const noexcept { - return m_whenFull; + return (m_when & (~FFatMask())) != 0; } void update(const char *subkey, long long when) { - if (subkey == nullptr) - { - m_whenFull = when; - } if (!FFat()) { if (subkey == nullptr) @@ -197,12 +192,14 @@ public: // we have to upgrade to a fat entry long long whenT = m_when; sds keyPrimary = u.m_key; - m_when = INVALID_EXPIRE; + m_when |= FFatMask(); u.m_pfatentry = new (MALLOC_LOCAL) expireEntryFat(keyPrimary); u.m_pfatentry->expireSubKey(nullptr, whenT); // at this point we're fat so fall through } } + if (subkey == nullptr) + m_when = when | FFatMask(); u.m_pfatentry->expireSubKey(subkey, when); } @@ -225,13 +222,9 @@ public: bool FGetPrimaryExpire(long long *pwhen) { *pwhen = -1; - for (auto itr : *this) - { - if (itr.subkey() == nullptr) - { - *pwhen = itr.when(); - return true; - } + if (this->whenFull() != INVALID_EXPIRE) { + *pwhen = this->whenFull(); + return true; } return false; } From 9b201b31c39c0184a3271bd3514e9c818890d061 Mon Sep 17 00:00:00 2001 From: Malavan Date: Mon, 17 May 2021 21:38:42 +0000 Subject: [PATCH 5/8] Fix issue with INVALID_EXPIRE overlapping with mask Former-commit-id: a6bbbeffcf7b5af5e67517aed01cabae51169c0a --- src/expire.h | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/expire.h b/src/expire.h index a2effca5c..2c9e94cf1 100644 --- a/src/expire.h +++ b/src/expire.h @@ -1,6 +1,7 @@ #pragma once -#define INVALID_EXPIRE LLONG_MIN +// eventually this number might actually end up being a valid expire time, this could cause bugs so at that time it might be a good idea to use a larger data type. +#define INVALID_EXPIRE (LLONG_MIN & ~((1LL) << (sizeof(long long)*CHAR_BIT - 1))) class expireEntryFat { @@ -60,11 +61,12 @@ class expireEntry { sds m_key; expireEntryFat *m_pfatentry; } u; - long long m_when; // LLONG_MIN means this is a fat entry and we should use the pointer + long long m_when; // bit wise and with FFatMask means this is a fat entry and we should use the pointer long long FFatMask() const noexcept { - return (1LL) << (sizeof(long long)*CHAR_BIT-1); + return (1LL) << (sizeof(long long)*CHAR_BIT - 1); } + public: class iter { @@ -175,7 +177,7 @@ public: } long long whenFull() const noexcept { - return (m_when & (~FFatMask())) != 0; + return m_when & (~FFatMask()); } void update(const char *subkey, long long when) From 3a379f09341d87d92b66c53a7cab636f77df2a35 Mon Sep 17 00:00:00 2001 From: malavan Date: Wed, 14 Jul 2021 22:13:29 +0000 Subject: [PATCH 6/8] use INVALID_EXPIRE instead of -1 Former-commit-id: 9e45984a97a293d87474f87612204a24c831a343 --- src/cluster.cpp | 4 ++-- src/db.cpp | 2 +- src/debug.cpp | 4 ++-- src/expire.cpp | 4 ++-- src/expire.h | 45 ++++++++++++++++++++++++++------------------- src/module.cpp | 4 ++-- src/rdb.cpp | 2 +- 7 files changed, 36 insertions(+), 29 deletions(-) diff --git a/src/cluster.cpp b/src/cluster.cpp index f51a62d42..e3fce7ec0 100644 --- a/src/cluster.cpp +++ b/src/cluster.cpp @@ -5425,11 +5425,11 @@ try_again: for (j = 0; j < num_keys; j++) { long long ttl = 0; expireEntry *pexpire = getExpire(c->db,kv[j]); - long long expireat = -1; + long long expireat = INVALID_EXPIRE; if (pexpire != nullptr) pexpire->FGetPrimaryExpire(&expireat); - if (expireat != -1) { + if (expireat != INVALID_EXPIRE) { ttl = expireat-mstime(); if (ttl < 0) { continue; diff --git a/src/db.cpp b/src/db.cpp index 7019aa29c..c7a27ea05 100644 --- a/src/db.cpp +++ b/src/db.cpp @@ -1626,7 +1626,7 @@ int keyIsExpired(redisDb *db, robj *key) { /* Don't expire anything while loading. It will be done later. */ if (g_pserver->loading) return 0; - long long when = pexpire->whenFull(); + long long when = pexpire->FGetPrimaryExpire(); if (when == INVALID_EXPIRE) return 0; diff --git a/src/debug.cpp b/src/debug.cpp index 53beaca4c..331f90d0e 100644 --- a/src/debug.cpp +++ b/src/debug.cpp @@ -128,7 +128,7 @@ void xorObjectDigest(redisDb *db, robj_roptr keyobj, unsigned char *digest, robj uint32_t aux = htonl(o->type); mixDigest(digest,&aux,sizeof(aux)); expireEntry *pexpire = getExpire(db,keyobj); - long long expiretime = -1; + long long expiretime = INVALID_EXPIRE; char buf[128]; if (pexpire != nullptr) @@ -260,7 +260,7 @@ void xorObjectDigest(redisDb *db, robj_roptr keyobj, unsigned char *digest, robj serverPanic("Unknown object type"); } /* If the key has an expire, add it to the mix */ - if (expiretime != -1) xorDigest(digest,"!!expire!!",10); + if (expiretime != INVALID_EXPIRE) xorDigest(digest,"!!expire!!",10); } /* Compute the dataset digest. Since keys, sets elements, hashes elements diff --git a/src/expire.cpp b/src/expire.cpp index b6022c712..662d3c71c 100644 --- a/src/expire.cpp +++ b/src/expire.cpp @@ -668,7 +668,7 @@ void pexpireatCommand(client *c) { /* Implements TTL and PTTL */ void ttlGenericCommand(client *c, int output_ms) { - long long expire = -1, ttl = -1; + long long expire = INVALID_EXPIRE, ttl = -1; /* If the key does not exist at all, return -2 */ if (lookupKeyReadWithFlags(c->db,c->argv[1],LOOKUP_NOTOUCH) == nullptr) { @@ -702,7 +702,7 @@ void ttlGenericCommand(client *c, int output_ms) { } - if (expire != -1) { + if (expire != INVALID_EXPIRE) { ttl = expire-mstime(); if (ttl < 0) ttl = 0; } diff --git a/src/expire.h b/src/expire.h index 2c9e94cf1..de55857f0 100644 --- a/src/expire.h +++ b/src/expire.h @@ -34,14 +34,7 @@ public: ~expireEntryFat(); long long when() const noexcept { return m_vecexpireEntries.front().when; } - long long whenFull() const noexcept { - for (size_t i = 0; i < size(); ++i) { - if (m_vecexpireEntries[i].spsubkey == nullptr) { - return m_vecexpireEntries[i].when; - } - } - return INVALID_EXPIRE; - } + const char *key() const noexcept { return m_keyPrimary; } bool operator<(long long when) const noexcept { return this->when() < when; } @@ -120,7 +113,10 @@ public: expireEntry(expireEntryFat *pfatentry) { u.m_pfatentry = pfatentry; - m_when = FFatMask() | pfatentry->whenFull(); + if (FGetPrimaryExpireSlow(&m_when)) + m_when = FFatMask() | m_when; + else + m_when = INVALID_EXPIRE; } expireEntry(expireEntry &&e) @@ -173,11 +169,7 @@ public: { if (FFat()) return u.m_pfatentry->when(); - return whenFull(); - } - long long whenFull() const noexcept - { - return m_when & (~FFatMask()); + return FGetPrimaryExpire(); } void update(const char *subkey, long long when) @@ -221,12 +213,27 @@ public: pfatentry()->m_vecexpireEntries.begin() + itr.m_idx); } - bool FGetPrimaryExpire(long long *pwhen) + long long FGetPrimaryExpire() const noexcept + { + return m_when & (~FFatMask()); + } + + bool FGetPrimaryExpire(long long *pwhen) const noexcept + { + *pwhen = FGetPrimaryExpire(); + return *pwhen != INVALID_EXPIRE; + } + + bool FGetPrimaryExpireSlow(long long *pwhen) { - *pwhen = -1; - if (this->whenFull() != INVALID_EXPIRE) { - *pwhen = this->whenFull(); - return true; + *pwhen = INVALID_EXPIRE; + for (auto itr : *this) + { + if (itr.subkey() == nullptr) + { + *pwhen = itr.when(); + return true; + } } return false; } diff --git a/src/module.cpp b/src/module.cpp index 183269c16..321ce1418 100644 --- a/src/module.cpp +++ b/src/module.cpp @@ -2264,11 +2264,11 @@ int RM_UnlinkKey(RedisModuleKey *key) { * REDISMODULE_NO_EXPIRE is returned. */ mstime_t RM_GetExpire(RedisModuleKey *key) { expireEntry *pexpire = getExpire(key->db,key->key); - mstime_t expire = -1; + mstime_t expire = INVALID_EXPIRE; if (pexpire != nullptr) pexpire->FGetPrimaryExpire(&expire); - if (expire == -1 || key->value == NULL) + if (expire == INVALID_EXPIRE || key->value == NULL) return REDISMODULE_NO_EXPIRE; expire -= mstime(); return expire >= 0 ? expire : 0; diff --git a/src/rdb.cpp b/src/rdb.cpp index 487992e57..c183bd34b 100644 --- a/src/rdb.cpp +++ b/src/rdb.cpp @@ -1101,7 +1101,7 @@ int rdbSaveKeyValuePair(rio *rdb, robj *key, robj *val, expireEntry *pexpire) { int savelfu = g_pserver->maxmemory_policy & MAXMEMORY_FLAG_LFU; /* Save the expire time */ - long long expiretime = -1; + long long expiretime = INVALID_EXPIRE; if (pexpire != nullptr && pexpire->FGetPrimaryExpire(&expiretime)) { if (rdbSaveType(rdb,RDB_OPCODE_EXPIRETIME_MS) == -1) return -1; if (rdbSaveMillisecondTime(rdb,expiretime) == -1) return -1; From 4291d2ddd183a0e2b7907ac82c342348986e8382 Mon Sep 17 00:00:00 2001 From: malavan Date: Wed, 21 Jul 2021 16:00:24 +0000 Subject: [PATCH 7/8] remove duplicate function Former-commit-id: 81cf9979ed1d86d0a690e5cd27ed018b125b1d58 --- src/expire.h | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/src/expire.h b/src/expire.h index de55857f0..0aeca1999 100644 --- a/src/expire.h +++ b/src/expire.h @@ -113,10 +113,15 @@ public: expireEntry(expireEntryFat *pfatentry) { u.m_pfatentry = pfatentry; - if (FGetPrimaryExpireSlow(&m_when)) - m_when = FFatMask() | m_when; - else - m_when = INVALID_EXPIRE; + m_when = FFatMask() | INVALID_EXPIRE; + for (auto itr : *this) + { + if (itr.subkey() == nullptr) + { + m_when = FFatMask() | itr.when(); + break; + } + } } expireEntry(expireEntry &&e) @@ -224,20 +229,6 @@ public: return *pwhen != INVALID_EXPIRE; } - bool FGetPrimaryExpireSlow(long long *pwhen) - { - *pwhen = INVALID_EXPIRE; - for (auto itr : *this) - { - if (itr.subkey() == nullptr) - { - *pwhen = itr.when(); - return true; - } - } - return false; - } - explicit operator const char*() const noexcept { return key(); } explicit operator long long() const noexcept { return when(); } }; From 778142bb15b3264908ddf4575d1e31d54299d6f2 Mon Sep 17 00:00:00 2001 From: malavan Date: Wed, 18 Aug 2021 20:59:01 +0000 Subject: [PATCH 8/8] should use LLONG_MAX for INVALID_EXPIRE not LLONG_MIN and add better comments Former-commit-id: 1ab19b445f00a9ccdab13bf6b96363d068e2fa02 --- src/expire.h | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/expire.h b/src/expire.h index 0aeca1999..86e284d95 100644 --- a/src/expire.h +++ b/src/expire.h @@ -1,7 +1,11 @@ #pragma once -// eventually this number might actually end up being a valid expire time, this could cause bugs so at that time it might be a good idea to use a larger data type. -#define INVALID_EXPIRE (LLONG_MIN & ~((1LL) << (sizeof(long long)*CHAR_BIT - 1))) +/* + * INVALID_EXPIRE is the value we set the expireEntry's m_when value to when the main key is not expired and the value we return when we try to get the expire time of a key or subkey that is not expired + * Want this value to be LLONG_MAX however we use the most significant bit of m_when as a flag to see if the expireEntry is Fat or not so we want to ensure that it is unset hence the (& ~((1LL) << (sizeof(long long)*CHAR_BIT - 1))) + * Eventually this number might actually end up being a valid expire time, this could cause bugs so at that time it might be a good idea to use a larger data type. + */ +#define INVALID_EXPIRE (LLONG_MAX & ~((1LL) << (sizeof(long long)*CHAR_BIT - 1))) class expireEntryFat { @@ -56,6 +60,7 @@ class expireEntry { } u; long long m_when; // bit wise and with FFatMask means this is a fat entry and we should use the pointer + /* Mask to check if an entry is Fat, most significant bit of m_when being set means it is Fat otherwise it is not */ long long FFatMask() const noexcept { return (1LL) << (sizeof(long long)*CHAR_BIT - 1); }