Skip to content

Commit f2a5613

Browse files
authored
chore: get rid of quicklist encoding (#5282)
c-based quicklist code was being sunset since 01/25. This PR removes all the references to the quicklist encoding, greatly simplifying list related logic. Signed-off-by: Roman Gershman <[email protected]>
1 parent ae72499 commit f2a5613

File tree

13 files changed

+116
-541
lines changed

13 files changed

+116
-541
lines changed

.github/workflows/ci.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ jobs:
146146
echo Run ctest -V -L DFLY
147147
148148
GLOG_alsologtostderr=1 GLOG_vmodule=rdb_load=1,rdb_save=1,snapshot=1,op_manager=1,op_manager_test=1 \
149-
FLAGS_fiber_safety_margin=4096 FLAGS_list_experimental_v2=true timeout 20m ctest -V -L DFLY -E allocation_tracker_test
149+
FLAGS_fiber_safety_margin=4096 timeout 20m ctest -V -L DFLY -E allocation_tracker_test
150150
151151
# Run allocation tracker test separately without alsologtostderr because it generates a TON of logs.
152152
FLAGS_fiber_safety_margin=4096 timeout 5m ./allocation_tracker_test

src/core/compact_object.cc

Lines changed: 2 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
extern "C" {
1111
#include "redis/intset.h"
1212
#include "redis/listpack.h"
13-
#include "redis/quicklist.h"
1413
#include "redis/redis_aux.h"
1514
#include "redis/stream.h"
1615
#include "redis/util.h"
@@ -43,18 +42,6 @@ namespace {
4342
constexpr XXH64_hash_t kHashSeed = 24061983;
4443
constexpr size_t kAlignSize = 8u;
4544

46-
// Approximation since does not account for listpacks.
47-
size_t QlMAllocSize(quicklist* ql, bool slow) {
48-
size_t node_size = ql->len * sizeof(quicklistNode) + znallocx(sizeof(quicklist));
49-
if (slow) {
50-
for (quicklistNode* node = ql->head; node; node = node->next) {
51-
node_size += zmalloc_usable_size(node->entry);
52-
}
53-
return node_size;
54-
}
55-
return node_size + ql->count * 16; // we account for each member 16 bytes.
56-
}
57-
5845
size_t UpdateSize(size_t size, int64_t update) {
5946
int64_t result = static_cast<int64_t>(size) + update;
6047
if (result < 0) {
@@ -80,16 +67,8 @@ inline void FreeObjSet(unsigned encoding, void* ptr, MemoryResource* mr) {
8067
}
8168

8269
void FreeList(unsigned encoding, void* ptr, MemoryResource* mr) {
83-
switch (encoding) {
84-
case OBJ_ENCODING_QUICKLIST:
85-
quicklistRelease((quicklist*)ptr);
86-
break;
87-
case kEncodingQL2:
88-
CompactObj::DeleteMR<QList>(ptr);
89-
break;
90-
default:
91-
LOG(FATAL) << "Unknown list encoding type";
92-
}
70+
CHECK_EQ(encoding, kEncodingQL2);
71+
CompactObj::DeleteMR<QList>(ptr);
9372
}
9473

9574
size_t MallocUsedSet(unsigned encoding, void* ptr) {
@@ -409,8 +388,6 @@ size_t RobjWrapper::MallocUsed(bool slow) const {
409388
CHECK_EQ(OBJ_ENCODING_RAW, encoding_);
410389
return InnerObjMallocUsed();
411390
case OBJ_LIST:
412-
if (encoding_ == OBJ_ENCODING_QUICKLIST)
413-
return QlMAllocSize((quicklist*)inner_obj_, slow);
414391
return ((QList*)inner_obj_)->MallocUsed(slow);
415392
case OBJ_SET:
416393
return MallocUsedSet(encoding_, inner_obj_);
@@ -434,8 +411,6 @@ size_t RobjWrapper::Size() const {
434411
DCHECK_EQ(OBJ_ENCODING_RAW, encoding_);
435412
return sz_;
436413
case OBJ_LIST:
437-
if (encoding_ == OBJ_ENCODING_QUICKLIST)
438-
return quicklistCount((quicklist*)inner_obj_);
439414
return ((QList*)inner_obj_)->Size();
440415
case OBJ_ZSET: {
441416
switch (encoding_) {

src/core/qlist.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ class QList {
3030
* attempted_compress: 1 bit, boolean, used for verifying during testing.
3131
* dont_compress: 1 bit, boolean, used for preventing compression of entry.
3232
* extra: 9 bits, free for future use; pads out the remainder of 32 bits
33-
* NOTE: do not change the ABI of this struct as long as we support --list_experimental_v2=false
3433
* */
3534

3635
typedef struct Node {

src/redis/redis_aux.c

Lines changed: 0 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55

66
#include "crc64.h"
77
#include "endianconv.h"
8-
#include "quicklist.h"
98
#include "zmalloc.h"
109

1110
Server server;
@@ -49,55 +48,3 @@ uint64_t intrev64(uint64_t v) {
4948
memrev64(&v);
5049
return v;
5150
}
52-
53-
// Based on quicklistGetIteratorAtIdx but without allocations
54-
void quicklistInitIterator(quicklistIter* iter, quicklist *quicklist, int direction,
55-
const long long idx) {
56-
quicklistNode *n = NULL;
57-
unsigned long long accum = 0;
58-
int forward = idx < 0 ? 0 : 1; /* < 0 -> reverse, 0+ -> forward */
59-
unsigned long long index = forward ? idx : (-idx) - 1;
60-
61-
iter->direction = direction;
62-
iter->quicklist = quicklist;
63-
iter->current = NULL;
64-
iter->zi = NULL;
65-
66-
if (index >= quicklist->count) return;
67-
68-
/* Seek in the other direction if that way is shorter. */
69-
int seek_forward = forward;
70-
unsigned long long seek_index = index;
71-
if (index > (quicklist->count - 1) / 2) {
72-
seek_forward = !forward;
73-
seek_index = quicklist->count - 1 - index;
74-
}
75-
76-
n = seek_forward ? quicklist->head : quicklist->tail;
77-
while (likely(n)) {
78-
if ((accum + n->count) > seek_index) {
79-
break;
80-
} else {
81-
accum += n->count;
82-
n = seek_forward ? n->next : n->prev;
83-
}
84-
}
85-
86-
if (!n)
87-
return;
88-
89-
iter->current = n;
90-
91-
/* Fix accum so it looks like we seeked in the other direction. */
92-
if (seek_forward != forward)
93-
accum = quicklist->count - n->count - accum;
94-
95-
if (forward) {
96-
/* forward = normal head-to-tail offset. */
97-
iter->offset = index - accum;
98-
} else {
99-
/* reverse = need negative offset for tail-to-head, so undo
100-
* the result of the original index = (-idx) - 1 above. */
101-
iter->offset = (-index) - 1 + accum;
102-
}
103-
}

src/redis/redis_aux.h

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -49,15 +49,10 @@ void InitRedisTables();
4949
#define OBJ_ENCODING_INTSET 6U /* Encoded as intset */
5050
#define OBJ_ENCODING_SKIPLIST 7U /* Encoded as skiplist */
5151
#define OBJ_ENCODING_EMBSTR 8U /* Embedded sds string encoding */
52-
#define OBJ_ENCODING_QUICKLIST 9U /* Encoded as linked list of ziplists */
52+
// #define OBJ_ENCODING_QUICKLIST 9U /* Encoded as linked list of ziplists */
5353
#define OBJ_ENCODING_STREAM 10U /* Encoded as a radix tree of listpacks */
5454
#define OBJ_ENCODING_LISTPACK 11 /* Encoded as a listpack */
5555
#define OBJ_ENCODING_COMPRESS_INTERNAL 15U /* Kept as lzf compressed, to pass compressed blob to another thread */
5656

57-
typedef struct quicklistIter quicklistIter;
58-
typedef struct quicklist quicklist;
59-
60-
void quicklistInitIterator(quicklistIter* iter, quicklist *quicklist, int direction, const long long idx);
61-
void quicklistCompressIterator(quicklistIter* iter);
6257

6358
#endif /* __REDIS_AUX_H */

src/server/container_utils.cc

Lines changed: 0 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -139,40 +139,9 @@ OpResult<ShardFFResult> FindFirstNonEmpty(Transaction* trans, int req_obj_type)
139139

140140
using namespace std;
141141

142-
quicklistEntry QLEntry() {
143-
quicklistEntry res{.quicklist = NULL,
144-
.node = NULL,
145-
.zi = NULL,
146-
.value = NULL,
147-
.longval = 0,
148-
.sz = 0,
149-
.offset = 0};
150-
return res;
151-
}
152-
153142
bool IterateList(const PrimeValue& pv, const IterateFunc& func, long start, long end) {
154143
bool success = true;
155144

156-
if (pv.Encoding() == OBJ_ENCODING_QUICKLIST) {
157-
quicklist* ql = static_cast<quicklist*>(pv.RObjPtr());
158-
long llen = quicklistCount(ql);
159-
if (end < 0 || end >= llen)
160-
end = llen - 1;
161-
162-
quicklistIter* qiter = quicklistGetIteratorAtIdx(ql, AL_START_HEAD, start);
163-
quicklistEntry entry = QLEntry();
164-
long lrange = end - start + 1;
165-
166-
while (success && quicklistNext(qiter, &entry) && lrange-- > 0) {
167-
if (entry.value) {
168-
success = func(ContainerEntry{reinterpret_cast<char*>(entry.value), entry.sz});
169-
} else {
170-
success = func(ContainerEntry{entry.longval});
171-
}
172-
}
173-
quicklistReleaseIterator(qiter);
174-
return success;
175-
}
176145
DCHECK_EQ(pv.Encoding(), kEncodingQL2);
177146
QList* ql = static_cast<QList*>(pv.RObjPtr());
178147

src/server/container_utils.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99

1010
extern "C" {
1111
#include "redis/listpack.h"
12-
#include "redis/quicklist.h"
1312
}
1413

1514
#include <functional>
@@ -26,9 +25,6 @@ inline bool IsContainer(const PrimeValue& pv) {
2625
return (type == OBJ_LIST || type == OBJ_SET || type == OBJ_ZSET);
2726
}
2827

29-
// Create empty quicklistEntry
30-
quicklistEntry QLEntry();
31-
3228
// Stores either:
3329
// - A single long long value (longval) when value = nullptr
3430
// - A single char* (value) when value != nullptr

src/server/debugcmd.cc

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -451,11 +451,7 @@ const char* EncodingName(unsigned obj_type, unsigned encoding) {
451451
case OBJ_STRING:
452452
return "raw";
453453
case OBJ_LIST:
454-
switch (encoding) {
455-
case kEncodingQL2:
456-
case OBJ_ENCODING_QUICKLIST:
457-
return "quicklist";
458-
}
454+
return "quicklist";
459455
break;
460456
case OBJ_SET:
461457
ABSL_FALLTHROUGH_INTENDED;

0 commit comments

Comments
 (0)