From 1848809f669ea538eb283f496ad6d8d7983d503f Mon Sep 17 00:00:00 2001
From: "Filipe Oliveira (Redis)" <filipe@redis.com>
Date: Mon, 24 Feb 2025 14:27:06 +0000
Subject: [PATCH] Optimize dictFind by leveraging key length functions to avoid
 redundant computations. (#13792)

This PR enhances dictFind by introducing support for key length
functions, allowing the use of keyCompareWithLen when available. This
avoids redundant key length computations, improving efficiency,
especially when the dictionary is rehashing or there are a significant
number of hash collisions.

Additionally, it maintains backward compatibility and optimizes key
lookups without altering existing behavior.

Performance improvement on 100% GETs use-case

benchmark command used
```
taskset -c 1-11 memtier_benchmark --ratio 0:1 --key-maximum 1000000 --key-minimum 1 -c 1 -t 5 --pipeline 100 --key-pattern P:P --test-time 30 --hide-histogram -d 1024 -S /tmp/1.socket  -x 3
```

In unstable dictFindByHash takes 29% (and sdslen within it takes 8.9%)
of CPU cycles for a high-pipeline 100% gets use-case.
After this change dictFindByHash takes 27.8% (and sdslen within it takes
7.7%)

---------

Co-authored-by: debing.sun <debing.sun@redis.com>
Co-authored-by: Yuan Wang <wangyuancode@163.com>
---
 src/dict.c   | 12 ++++++++++--
 src/dict.h   |  6 ++++++
 src/server.c | 15 +++++++++++++++
 src/server.h |  2 ++
 4 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/src/dict.c b/src/dict.c
index bf183422ae..b74bed2d4f 100644
--- a/src/dict.c
+++ b/src/dict.c
@@ -778,6 +778,11 @@ dictEntry *dictFindByHash(dict *d, const void *key, const uint64_t hash) {
     /* Rehash the hash table if needed */
     _dictRehashStepIfNeeded(d,idx);
 
+    /* Check if we can use the compare function with length to avoid recomputing length of key always */
+    keyCmpFuncWithLen cmpFuncWithLen = d->type->keyCompareWithLen;
+    keyLenFunc keyLenFunc = d->type->keyLen;
+    const int has_len_fn = (keyLenFunc != NULL && cmpFuncWithLen != NULL);
+    const size_t key_len = has_len_fn ? keyLenFunc(d,key) : 0;
     for (table = 0; table <= 1; table++) {
         if (table == 0 && (long)idx < d->rehashidx) continue;
         idx = hash & DICTHT_SIZE_MASK(d->ht_size_exp[table]);
@@ -791,9 +796,12 @@ dictEntry *dictFindByHash(dict *d, const void *key, const uint64_t hash) {
 
             /* Prefetch the next entry to improve cache efficiency */
             redis_prefetch_read(dictGetNext(he));
-
-            if (key == he_key || cmpFunc(d, key, he_key))
+            if (key == he_key || (has_len_fn ?
+                cmpFuncWithLen(d, key, key_len, he_key, keyLenFunc(d,he_key)) :
+                cmpFunc(d, key, he_key)))
+            {
                 return he;
+            }
             he = dictGetNext(he);
         }
         /* Use unlikely to optimize branch prediction for the common case */
diff --git a/src/dict.h b/src/dict.h
index fc4554ae23..4fe28d8c76 100644
--- a/src/dict.h
+++ b/src/dict.h
@@ -28,6 +28,8 @@
 
 typedef struct dictEntry dictEntry; /* opaque */
 typedef struct dict dict;
+typedef size_t (*keyLenFunc)(dict *d, const void *key1);
+typedef int (*keyCmpFuncWithLen)(dict *d, const void *key1, const size_t key1_len, const void *key2, const size_t key2_len);
 
 typedef struct dictType {
     /* Callbacks */
@@ -92,6 +94,10 @@ typedef struct dictType {
 
     /* Optional callback called when the dict is destroyed. */
     void (*onDictRelease)(dict *d);
+
+    /* Optional keylen to avoid duplication computation of key lengths. */
+    keyLenFunc keyLen;
+    keyCmpFuncWithLen keyCompareWithLen;
 } dictType;
 
 #define DICTHT_SIZE(exp) ((exp) == -1 ? 0 : (unsigned long)1<<(exp))
diff --git a/src/server.c b/src/server.c
index 3fb17da597..e4030be47e 100644
--- a/src/server.c
+++ b/src/server.c
@@ -289,6 +289,19 @@ void dictDictDestructor(dict *d, void *val)
     dictRelease((dict*)val);
 }
 
+size_t dictSdsKeyLen(dict *d, const void *key) {
+    UNUSED(d);
+    return sdslen((sds)key);
+}
+
+int dictSdsKeyCompareWithLen(dict *d, const void *key1, const size_t l1,
+                                      const void *key2, const size_t l2)
+{
+    UNUSED(d);
+    if (l1 != l2) return 0;
+    return memcmp(key1, key2, l1) == 0;
+}
+
 int dictSdsKeyCompare(dict *d, const void *key1,
         const void *key2)
 {
@@ -513,6 +526,8 @@ dictType dbDictType = {
     dictSdsDestructor,          /* key destructor */
     dictObjectDestructor,       /* val destructor */
     dictResizeAllowed,          /* allow to resize */
+    .keyLen = dictSdsKeyLen,    /* key length */
+    .keyCompareWithLen = dictSdsKeyCompareWithLen /* key compare with length */
 };
 
 /* Db->expires */
diff --git a/src/server.h b/src/server.h
index d133907220..585a654a83 100644
--- a/src/server.h
+++ b/src/server.h
@@ -3735,7 +3735,9 @@ void startEvictionTimeProc(void);
 uint64_t dictSdsHash(const void *key);
 uint64_t dictPtrHash(const void *key);
 uint64_t dictSdsCaseHash(const void *key);
+size_t dictSdsKeyLen(dict *d, const void *key);
 int dictSdsKeyCompare(dict *d, const void *key1, const void *key2);
+int dictSdsKeyCompareWithLen(dict *d, const void *key1, const size_t l1,const void *key2, const size_t l2);
 int dictSdsMstrKeyCompare(dict *d, const void *sdsLookup, const void *mstrStored);
 int dictSdsKeyCaseCompare(dict *d, const void *key1, const void *key2);
 void dictSdsDestructor(dict *d, void *val);