From ee2f00de2754f7b4604e3cb13a941d88e6304fd6 Mon Sep 17 00:00:00 2001
From: Roman Gershman <roman@dragonflydb.io>
Date: Sun, 29 Sep 2024 12:46:57 +0300
Subject: [PATCH] chore: now it's not needed to allocate quicklistIter on heap
 (#3814)

---
 src/redis/quicklist.c     |  5 ++++
 src/redis/redis_aux.c     | 53 +++++++++++++++++++++++++++++++++++++++
 src/redis/redis_aux.h     | 25 +++++-------------
 src/server/list_family.cc |  9 ++++---
 4 files changed, 69 insertions(+), 23 deletions(-)

diff --git a/src/redis/quicklist.c b/src/redis/quicklist.c
index 26dd4ee4d..aa860f245 100644
--- a/src/redis/quicklist.c
+++ b/src/redis/quicklist.c
@@ -1276,6 +1276,11 @@ void quicklistReleaseIterator(quicklistIter *iter) {
     zfree(iter);
 }
 
+// Based on quicklistReleaseIterator
+void quicklistCompressIterator(quicklistIter* iter) {
+  if (iter->current) quicklistCompress(iter->quicklist, iter->current);
+}
+
 /* Get next element in iterator.
  *
  * Note: You must NOT insert into the list while iterating over it.
diff --git a/src/redis/redis_aux.c b/src/redis/redis_aux.c
index 79fcbe837..bad6f9532 100644
--- a/src/redis/redis_aux.c
+++ b/src/redis/redis_aux.c
@@ -5,6 +5,7 @@
 
 #include "crc64.h"
 #include "endianconv.h"
+#include "quicklist.h"
 #include "zmalloc.h"
 
 Server server;
@@ -67,4 +68,56 @@ void memrev64(void* p) {
 uint64_t intrev64(uint64_t v) {
   memrev64(&v);
   return v;
+}
+
+// Based on quicklistGetIteratorAtIdx but without allocations
+void quicklistInitIterator(quicklistIter* iter, quicklist *quicklist, int direction,
+                           const long long idx) {
+    quicklistNode *n = NULL;
+    unsigned long long accum = 0;
+    int forward = idx < 0 ? 0 : 1; /* < 0 -> reverse, 0+ -> forward */
+    unsigned long long index = forward ? idx : (-idx) - 1;
+
+    iter->direction = direction;
+    iter->quicklist = quicklist;
+    iter->current = NULL;
+    iter->zi = NULL;
+
+    if (index >= quicklist->count) return;
+
+    /* Seek in the other direction if that way is shorter. */
+    int seek_forward = forward;
+    unsigned long long seek_index = index;
+    if (index > (quicklist->count - 1) / 2) {
+        seek_forward = !forward;
+        seek_index = quicklist->count - 1 - index;
+    }
+
+    n = seek_forward ? quicklist->head : quicklist->tail;
+    while (likely(n)) {
+        if ((accum + n->count) > seek_index) {
+            break;
+        } else {
+            accum += n->count;
+            n = seek_forward ? n->next : n->prev;
+        }
+    }
+
+    if (!n)
+      return;
+
+    iter->current = n;
+
+    /* Fix accum so it looks like we seeked in the other direction. */
+    if (seek_forward != forward)
+      accum = quicklist->count - n->count - accum;
+
+    if (forward) {
+        /* forward = normal head-to-tail offset. */
+        iter->offset = index - accum;
+    } else {
+        /* reverse = need negative offset for tail-to-head, so undo
+         * the result of the original index = (-idx) - 1 above. */
+        iter->offset = (-index) - 1 + accum;
+    }
 }
\ No newline at end of file
diff --git a/src/redis/redis_aux.h b/src/redis/redis_aux.h
index c536c32ff..80e3cf8ac 100644
--- a/src/redis/redis_aux.h
+++ b/src/redis/redis_aux.h
@@ -14,25 +14,6 @@
 
 #define CONFIG_RUN_ID_SIZE 40U
 
-/* To improve the quality of the LRU approximation we take a set of keys
- * that are good candidate for eviction across performEvictions() calls.
- *
- * Entries inside the eviction pool are taken ordered by idle time, putting
- * greater idle times to the right (ascending order).
- *
- * When an LFU policy is used instead, a reverse frequency indication is used
- * instead of the idle time, so that we still evict by larger value (larger
- * inverse frequency means to evict keys with the least frequent accesses).
- *
- * Empty entries have the key pointer set to NULL. */
-
-typedef struct dict dict;
-
-uint64_t dictSdsHash(const void* key);
-int dictSdsKeyCompare(dict* privdata, const void* key1, const void* key2);
-void dictSdsDestructor(dict* privdata, void* val);
-size_t sdsZmallocSize(sds s);
-
 typedef struct ServerStub {
   size_t max_map_field_len, max_listpack_map_bytes;
 
@@ -76,4 +57,10 @@ const char *strEncoding(int encoding);
 #define OBJ_ENCODING_LISTPACK 11 /* Encoded as a listpack */
 #define OBJ_ENCODING_COMPRESS_INTERNAL 15U  /* Kept as lzf compressed, to pass compressed blob to another thread */
 
+typedef struct quicklistIter quicklistIter;
+typedef struct quicklist quicklist;
+
+void quicklistInitIterator(quicklistIter* iter, quicklist *quicklist, int direction, const long long idx);
+void quicklistCompressIterator(quicklistIter* iter);
+
 #endif /* __REDIS_AUX_H */
diff --git a/src/server/list_family.cc b/src/server/list_family.cc
index fdc7a3ee4..7fddc7062 100644
--- a/src/server/list_family.cc
+++ b/src/server/list_family.cc
@@ -543,14 +543,15 @@ OpResult<uint32_t> OpRem(const OpArgs& op_args, string_view key, string_view ele
     index = -1;
   }
 
-  quicklistIter* qiter = quicklistGetIteratorAtIdx(ql, iter_direction, index);
+  quicklistIter qiter;
+  quicklistInitIterator(&qiter, ql, iter_direction, index);
   quicklistEntry entry;
   unsigned removed = 0;
   const uint8_t* elem_ptr = reinterpret_cast<const uint8_t*>(elem.data());
 
-  while (quicklistNext(qiter, &entry)) {
+  while (quicklistNext(&qiter, &entry)) {
     if (quicklistCompare(&entry, elem_ptr, elem.size())) {
-      quicklistDelEntry(qiter, &entry);
+      quicklistDelEntry(&qiter, &entry);
       removed++;
       if (count && removed == count)
         break;
@@ -559,7 +560,7 @@ OpResult<uint32_t> OpRem(const OpArgs& op_args, string_view key, string_view ele
 
   it_res->post_updater.Run();
 
-  quicklistReleaseIterator(qiter);
+  quicklistCompressIterator(&qiter);
 
   if (quicklistCount(ql) == 0) {
     CHECK(db_slice.Del(op_args.db_cntx, it));