Skip to content

Commit

Permalink
chore: now it's not needed to allocate quicklistIter on heap (#3814)
Browse files Browse the repository at this point in the history
  • Loading branch information
romange authored Sep 29, 2024
1 parent 520dea0 commit ee2f00d
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 23 deletions.
5 changes: 5 additions & 0 deletions src/redis/quicklist.c
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
53 changes: 53 additions & 0 deletions src/redis/redis_aux.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

#include "crc64.h"
#include "endianconv.h"
#include "quicklist.h"
#include "zmalloc.h"

Server server;
Expand Down Expand Up @@ -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;
}
}
25 changes: 6 additions & 19 deletions src/redis/redis_aux.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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 */
9 changes: 5 additions & 4 deletions src/server/list_family.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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));
Expand Down

0 comments on commit ee2f00d

Please sign in to comment.