Redis/redis
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Optimize quicklistCompare with optional string2ll caching. (#14131)

Browse Source 
### Summary

This pull request improves the performance of quicklistCompare and
lpCompare by avoiding repeated calls to string2ll when comparing many
quicklist/listpack entries against the same string value. The
optimization targets use cases like LREM, LPOS, LINSERT, and ZRANK where
comparisons are made repeatedly in a loop.

By caching the result of string2ll during a single command execution, we
avoid re-parsing the same input string thousands of times—resulting in
up to **30% higher throughput and up to 25% lower p50 latency** in LREM
LINSERT benchmarks, and **5% higher throughput** in ZRANK (listpack)
command.

### Changes

- Updated quicklistCompare and lpCompare to accept two optional
parameters:
  - `long long *cached_val`
  - `int *cached_valid`
- If caching parameters are provided, string2ll is invoked only once and
its result is reused across comparisons.
- listTypeEqual was updated to forward these parameters.
- Commands such as LREM, LPOS, LINSERT, and ZRANK now use this
optimization.
- All internal tests and usage of quicklistCompare/lpCompare were
updated accordingly.

### Behavior

- If cached_valid is NULL, quicklistCompare/lpCompare behaves as before
(no caching).
- If cached_valid is non-NULL:
  - 0 means uninitialized: string2ll is attempted.
  - 1 means valid: cached_val is used.
  - -1 means invalid: string2ll previously failed and is skipped.

---------

Co-authored-by: debing.sun <debing.sun@redis.com>
This commit is contained in:
Filipe Oliveira (Redis)
2025-06-21 10:28:51 +08:00
committed by GitHub
co-authored by debing.sun
parent 117424f85c
commit 2e1a17c26c
7 changed files with 288 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/listpack.c
+41 -13
View File
@@ -1718,7 +1718,8 @@ int lpValidateIntegrity(unsigned char *lp, size_t size, int deep,
/* Compare entry pointer to by 'p' with string 's' of length 'slen'.
* Return 1 if equal. */
unsigned int lpCompare(unsigned char *p, unsigned char *s, uint32_t slen) {
unsigned int lpCompare(unsigned char *p, unsigned char *s, uint32_t slen,
long long *cached_longval, int *cached_valid) {
unsigned char *value;
int64_t sz;
if (p[0] == LP_EOF) return 0;
@@ -1727,12 +1728,25 @@ unsigned int lpCompare(unsigned char *p, unsigned char *s, uint32_t slen) {
if (value) {
return (slen == sz) && memcmp(value,s,slen) == 0;
} else {
int64_t sval;
/* We use lpStringToInt64() to get an integer representation of the
* string 's' and compare it to 'sval', it's much faster than convert
* integer to string and comparing. */
int64_t sval;
if (lpStringToInt64((const char*)s, slen, &sval))
return sz == sval;
if (cached_valid != NULL) {
/* Use caching */
if (*cached_valid == 0) {
if (lpStringToInt64((const char*)s, slen, (int64_t*)cached_longval)) {
*cached_valid = 1;
} else {
*cached_valid = -1;
}
}
return (*cached_valid == 1 && sz == *cached_longval);
} else {
/* No caching - direct conversion */
if (lpStringToInt64((const char*)s, slen, &sval))
return sz == sval;
}
}
return 0;
@@ -2139,7 +2153,7 @@ static int randstring(char *target, unsigned int min, unsigned int max) {
}
static void verifyEntry(unsigned char *p, unsigned char *s, size_t slen) {
assert(lpCompare(p, s, slen));
assert(lpCompare(p, s, slen, NULL, NULL));
}
static int lpValidation(unsigned char *p, unsigned int head_count, void *userdata) {
@@ -2148,7 +2162,7 @@ static int lpValidation(unsigned char *p, unsigned int head_count, void *userdat
int ret;
long *count = userdata;
ret = lpCompare(p, (unsigned char *)mixlist[*count], strlen(mixlist[*count]));
ret = lpCompare(p, (unsigned char *)mixlist[*count], strlen(mixlist[*count]), NULL, NULL);
(*count)++;
return ret;
}
@@ -2539,7 +2553,7 @@ int listpackTest(int argc, char *argv[], int flags) {
lp = createList();
p = lpFirst(lp);
while (p) {
if (lpCompare(p, (unsigned char*)"foo", 3)) {
if (lpCompare(p, (unsigned char*)"foo", 3, NULL, NULL)) {
lp = lpDelete(lp, p, &p);
} else {
p = lpNext(lp, p);
@@ -2620,12 +2634,12 @@ int listpackTest(int argc, char *argv[], int flags) {
TEST("Compare strings with listpack entries") {
lp = createList();
p = lpSeek(lp,0);
assert(lpCompare(p,(unsigned char*)"hello",5));
assert(!lpCompare(p,(unsigned char*)"hella",5));
assert(lpCompare(p,(unsigned char*)"hello",5,NULL,NULL));
assert(!lpCompare(p,(unsigned char*)"hella",5,NULL,NULL));
p = lpSeek(lp,3);
assert(lpCompare(p,(unsigned char*)"1024",4));
assert(!lpCompare(p,(unsigned char*)"1025",4));
assert(lpCompare(p,(unsigned char*)"1024",4,NULL,NULL));
assert(!lpCompare(p,(unsigned char*)"1025",4,NULL,NULL));
lpFree(lp);
}
@@ -3252,7 +3266,7 @@ int listpackTest(int argc, char *argv[], int flags) {
for (int i = 0; i < 2000; i++) {
unsigned char *eptr = lpSeek(lp,0);
while (eptr != NULL) {
lpCompare(eptr,(unsigned char*)"nothing",7);
lpCompare(eptr,(unsigned char*)"nothing",7,NULL,NULL);
eptr = lpNext(lp,eptr);
}
}
@@ -3264,7 +3278,21 @@ int listpackTest(int argc, char *argv[], int flags) {
for (int i = 0; i < 2000; i++) {
unsigned char *eptr = lpSeek(lp,0);
while (eptr != NULL) {
lpCompare(lp, (unsigned char*)"99999", 5);
lpCompare(eptr, (unsigned char*)"99999", 5, NULL, NULL);
eptr = lpNext(lp,eptr);
}
}
printf("Done. usec=%lld\n", usec()-start);
}
TEST("Benchmark lpCompare with number and caching") {
unsigned long long start = usec();
for (int i = 0; i < 2000; i++) {
unsigned char *eptr = lpSeek(lp,0);
long long cached_val = 0;
int cached_valid = 0;
while (eptr != NULL) {
lpCompare(eptr, (unsigned char*)"99999", 5, &cached_val, &cached_valid);
eptr = lpNext(lp,eptr);
}
}
src/listpack.h
+1 -1
View File
@@ -78,7 +78,7 @@ int lpValidateIntegrity(unsigned char *lp, size_t size, int deep,
listpackValidateEntryCB entry_cb, void *cb_userdata);
unsigned char *lpValidateFirst(unsigned char *lp);
int lpValidateNext(unsigned char *lp, unsigned char **pp, size_t lpbytes);
unsigned int lpCompare(unsigned char *p, unsigned char *s, uint32_t slen);
unsigned int lpCompare(unsigned char *p, unsigned char *s, uint32_t slen, long long *cached_longval, int *cached_valid);
void lpRandomPair(unsigned char *lp, unsigned long total_count,
listpackEntry *key, listpackEntry *val, int tuple_len);
void lpRandomPairs(unsigned char *lp, unsigned int count,
src/quicklist.c
+216 -9
View File
@@ -1242,17 +1242,46 @@ int quicklistDelRange(quicklist *quicklist, const long start,
return 1;
}
/* compare between a two entries */
int quicklistCompare(quicklistEntry* entry, unsigned char *p2, const size_t p2_len) {
/* Compare a quicklistEntry with a raw value.
*
* If the entry stores a string (entry->value != NULL), perform a binary-safe
* comparison against p2.
*
* If the entry stores an integer (entry->value == NULL), lazily convert p2 to
* a long long using string2ll() once and cache the result using cached_longval
* and cached_valid.
*
* This optimization avoids repeatedly calling string2ll() in tight loops.
* - If cached_valid == NULL: skip caching
* - If cached_valid == 0: conversion attempted
* - If cached_valid == 1/-1: cached result reused
*
* Returns 1 if equal, 0 otherwise.
*/
int quicklistCompare(quicklistEntry *entry, unsigned char *p2, const size_t p2_len,
long long *cached_longval, int *cached_valid) {
if (entry->value) {
return ((entry->sz == p2_len) && (memcmp(entry->value, p2, p2_len) == 0));
} else {
/* We use string2ll() to get an integer representation of the
* string 'p2' and compare it to 'entry->longval', it's much
* faster than convert integer to string and comparing. */
long long sval;
if (string2ll((const char*)p2, p2_len, &sval))
return entry->longval == sval;
if (cached_valid != NULL) {
/* Use caching */
if (*cached_valid == 0) {
if (string2ll((const char *)p2, p2_len, cached_longval)) {
*cached_valid = 1;
} else {
*cached_valid = -1;
}
}
return (*cached_valid == 1 && entry->longval == *cached_longval);
} else {
/* No caching - direct conversion */
long long sval;
if (string2ll((const char *)p2, p2_len, &sval))
return entry->longval == sval;
}
}
return 0;
}
@@ -2889,7 +2918,8 @@ int quicklistTest(int argc, char *argv[], int flags) {
quicklistEntry entry;
int i = 0;
while (quicklistNext(iter, &entry)) {
if (quicklistCompare(&entry, (unsigned char *)"bar", 3)) {
if (quicklistCompare(&entry, (unsigned char *)"bar", 3,
NULL, NULL)) {
quicklistDelEntry(iter, &entry);
}
i++;
@@ -2918,7 +2948,8 @@ int quicklistTest(int argc, char *argv[], int flags) {
i = 0;
int del = 2;
while (quicklistNext(iter, &entry)) {
if (quicklistCompare(&entry, (unsigned char *)"foo", 3)) {
if (quicklistCompare(&entry, (unsigned char *)"foo", 3,
NULL, NULL)) {
quicklistDelEntry(iter, &entry);
del--;
}
@@ -2965,7 +2996,8 @@ int quicklistTest(int argc, char *argv[], int flags) {
quicklistIter *iter = quicklistGetIterator(ql, AL_START_TAIL);
int i = 0;
while (quicklistNext(iter, &entry)) {
if (quicklistCompare(&entry, (unsigned char *)"hij", 3)) {
if (quicklistCompare(&entry, (unsigned char *)"hij", 3,
NULL, NULL)) {
quicklistDelEntry(iter, &entry);
}
i++;
@@ -2981,7 +3013,7 @@ int quicklistTest(int argc, char *argv[], int flags) {
char *vals[] = {"abc", "def", "jkl", "oop"};
while (quicklistNext(iter, &entry)) {
if (!quicklistCompare(&entry, (unsigned char *)vals[i],
3)) {
3, NULL, NULL)) {
ERR("Value at %d didn't match %s\n", i, vals[i]);
}
i++;
@@ -3266,6 +3298,181 @@ int quicklistTest(int argc, char *argv[], int flags) {
quicklistRelease(ql);
}
TEST("quicklistCompare cached string2ll optimization") {
quicklist *ql = quicklistNew(-2, 0);
/* Create a list with mixed integer and string entries */
quicklistPushTail(ql, "123", 3); /* integer as string */
quicklistPushTail(ql, "456", 3); /* integer as string */
quicklistPushTail(ql, "hello", 5); /* non-numeric string */
quicklistPushTail(ql, "789", 3); /* integer as string */
quicklistPushTail(ql, "world", 5); /* non-numeric string */
quicklistEntry entry;
quicklistIter *iter;
/* Test 1: NULL parameters should work without crashing */
iter = quicklistGetIterator(ql, AL_START_HEAD);
assert(quicklistNext(iter, &entry));
assert(quicklistCompare(&entry, (unsigned char *)"123", 3, NULL, NULL) == 1);
assert(quicklistCompare(&entry, (unsigned char *)"456", 3, NULL, NULL) == 0);
ql_release_iterator(iter);
/* Test 2: Caching with numeric strings */
long long cached_val = 0;
int cached_valid = 0;
/* First comparison should cache the value */
iter = quicklistGetIterator(ql, AL_START_HEAD);
assert(quicklistNext(iter, &entry)); /* entry = "123" */
assert(quicklistCompare(&entry, (unsigned char *)"123", 3, &cached_val, &cached_valid) == 1);
assert(cached_valid == 1); /* Should be cached as valid */
assert(cached_val == 123); /* Should have cached value */
/* Second comparison with same search string should use cache */
assert(quicklistNext(iter, &entry)); /* entry = "456" */
assert(quicklistCompare(&entry, (unsigned char *)"123", 3, &cached_val, &cached_valid) == 0);
assert(cached_valid == 1); /* Cache should still be valid */
assert(cached_val == 123); /* Cache value should be unchanged */
/* Third comparison with same search string should use cache */
assert(quicklistNext(iter, &entry)); /* entry = "hello" (string) */
assert(quicklistCompare(&entry, (unsigned char *)"123", 3, &cached_val, &cached_valid) == 0);
assert(cached_valid == 1); /* Cache should still be valid */
ql_release_iterator(iter);
/* Test 3: Caching with non-numeric strings */
cached_val = 0;
cached_valid = 0;
iter = quicklistGetIterator(ql, AL_START_HEAD);
assert(quicklistNext(iter, &entry)); /* entry = "123" */
assert(quicklistCompare(&entry, (unsigned char *)"abc", 3, &cached_val, &cached_valid) == 0);
assert(cached_valid == -1); /* Should be cached as invalid */
/* Second comparison with same non-numeric string should use cache */
assert(quicklistNext(iter, &entry)); /* entry = "456" */
assert(quicklistCompare(&entry, (unsigned char *)"abc", 3, &cached_val, &cached_valid) == 0);
assert(cached_valid == -1); /* Cache should still be invalid */
ql_release_iterator(iter);
/* Test 4: String entries should work correctly with both NULL and caching */
iter = quicklistGetIterator(ql, AL_START_HEAD);
quicklistNext(iter, &entry); /* skip "123" */
quicklistNext(iter, &entry); /* skip "456" */
assert(quicklistNext(iter, &entry)); /* entry = "hello" */
/* String comparison with NULL parameters */
assert(quicklistCompare(&entry, (unsigned char *)"hello", 5, NULL, NULL) == 1);
assert(quicklistCompare(&entry, (unsigned char *)"world", 5, NULL, NULL) == 0);
/* String comparison with caching parameters (cache not used for strings) */
cached_val = 0;
cached_valid = 0;
assert(quicklistCompare(&entry, (unsigned char *)"hello", 5, &cached_val, &cached_valid) == 1);
assert(cached_valid == 0); /* Cache should not be used for string entries */
ql_release_iterator(iter);
/* Test 5: Performance verification - cache should reduce conversions */
/* This test demonstrates the optimization by showing cache reuse */
cached_val = 0;
cached_valid = 0;
int comparisons = 0;
/* Search for "456" across all integer entries */
iter = quicklistGetIterator(ql, AL_START_HEAD);
while (quicklistNext(iter, &entry)) {
if (entry.value == NULL) { /* Only test integer entries */
quicklistCompare(&entry, (unsigned char *)"456", 3, &cached_val, &cached_valid);
comparisons++;
}
}
ql_release_iterator(iter);
/* After first comparison, cache should be valid and reused for subsequent ones */
assert(cached_valid == 1);
assert(cached_val == 456);
assert(comparisons >= 2); /* Should have compared against multiple integer entries */
quicklistRelease(ql);
}
/* Benchmarks for quicklistCompare caching optimization */
{
printf("\n=== quicklistCompare Caching Benchmarks ===\n");
/* Create a quicklist with 10K integer elements */
quicklist *ql = quicklistNew(-2, 0);
char buf[16];
for (int i = 1; i <= 10000; i++) {
snprintf(buf, sizeof(buf), "%d", i);
quicklistPushTail(ql, buf, strlen(buf));
}
printf("Created quicklist with %lu integer elements\n", ql->count);
/* Search string that exists in the middle */
unsigned char *search_str = (unsigned char *)"5000";
size_t search_len = 4;
int iterations = accurate ? 50000 : 10000;
/* Benchmark 1: quicklistCompare WITHOUT caching (NULL parameters) */
TEST("Benchmark quicklistCompare without caching") {
long long start = ustime();
int matches = 0;
for (int iter = 0; iter < iterations; iter++) {
quicklistIter *iter_ptr = quicklistGetIterator(ql, AL_START_HEAD);
quicklistEntry entry;
while (quicklistNext(iter_ptr, &entry)) {
if (entry.value == NULL) { /* Only test integer entries */
if (quicklistCompare(&entry, search_str, search_len, NULL, NULL)) {
matches++;
}
}
}
ql_release_iterator(iter_ptr);
}
long long elapsed = ustime() - start;
printf("Found %d matches in %d iterations\n", matches, iterations);
printf("Without caching: %lld usec (%.2f usec per iteration)\n",
elapsed, (double)elapsed / iterations);
}
/* Benchmark 2: quicklistCompare WITH caching */
TEST("Benchmark quicklistCompare with caching") {
long long start = ustime();
int matches = 0;
for (int iter = 0; iter < iterations; iter++) {
/* Reset cache for each iteration to simulate real usage */
long long cached_val = 0;
int cached_valid = 0;
quicklistIter *iter_ptr = quicklistGetIterator(ql, AL_START_HEAD);
quicklistEntry entry;
while (quicklistNext(iter_ptr, &entry)) {
if (entry.value == NULL) { /* Only test integer entries */
if (quicklistCompare(&entry, search_str, search_len, &cached_val, &cached_valid)) {
matches++;
}
}
}
ql_release_iterator(iter_ptr);
}
long long elapsed = ustime() - start;
printf("Found %d matches in %d iterations\n", matches, iterations);
printf("With caching: %lld usec (%.2f usec per iteration)\n",
elapsed, (double)elapsed / iterations);
}
quicklistRelease(ql);
printf("=== End quicklistCompare Benchmarks ===\n\n");
}
if (flags & REDIS_TEST_LARGE_MEMORY) {
TEST("compress and decompress quicklist listpack node") {
quicklistNode *node = quicklistCreateNode();
src/quicklist.h
+2 -1
View File
@@ -191,7 +191,8 @@ int quicklistPopCustom(quicklist *quicklist, int where, unsigned char **data,
int quicklistPop(quicklist *quicklist, int where, unsigned char **data,
size_t *sz, long long *slong);
unsigned long quicklistCount(const quicklist *ql);
int quicklistCompare(quicklistEntry *entry, unsigned char *p2, const size_t p2_len);
int quicklistCompare(quicklistEntry *entry, unsigned char *p2, const size_t p2_len,
long long *cached_longval, int *cached_valid);
size_t quicklistGetLzf(const quicklistNode *node, void **data);
void quicklistNodeLimit(int fill, size_t *size, unsigned int *count);
int quicklistNodeExceedsLimit(int fill, size_t new_sz, unsigned int new_count);
src/server.h
+2 -1
View File
@@ -2981,7 +2981,8 @@ robj *listTypeGet(listTypeEntry *entry);
unsigned char *listTypeGetValue(listTypeEntry *entry, size_t *vlen, long long *lval);
void listTypeInsert(listTypeEntry *entry, robj *value, int where);
void listTypeReplace(listTypeEntry *entry, robj *value);
int listTypeEqual(listTypeEntry *entry, robj *o, size_t object_len);
int listTypeEqual(listTypeEntry *entry, robj *o, size_t object_len,
long long *cached_longval, int *cached_valid);
void listTypeDelete(listTypeIterator *iter, listTypeEntry *entry);
robj *listTypeDup(robj *o);
void listTypeDelRange(robj *o, long start, long stop);
src/t_list.c
+22 -7
View File
@@ -384,13 +384,22 @@ int listTypeReplaceAtIndex(robj *o, int index, robj *value) {
return replaced;
}
/* Compare the given object with the entry at the current position. */
int listTypeEqual(listTypeEntry *entry, robj *o, size_t object_len) {
/* Compare the given object with the entry at the current position.
*
* If the list encoding is quicklist, delegates to quicklistCompare(),
* passing along the cached integer conversion state.
*
* If the list encoding is listpack, uses lpCompare().
*
* Returns 1 if equal, 0 otherwise.
*/
int listTypeEqual(listTypeEntry *entry, robj *o, size_t object_len,
long long *cached_longval, int *cached_valid) {
serverAssertWithInfo(NULL,o,sdsEncodedObject(o));
if (entry->li->encoding == OBJ_ENCODING_QUICKLIST) {
return quicklistCompare(&entry->entry,o->ptr,object_len);
return quicklistCompare(&entry->entry,o->ptr,object_len,cached_longval,cached_valid);
} else if (entry->li->encoding == OBJ_ENCODING_LISTPACK) {
return lpCompare(entry->lpe,o->ptr,object_len);
return lpCompare(entry->lpe,o->ptr,object_len,cached_longval,cached_valid);
} else {
serverPanic("Unknown list encoding");
}
@@ -545,8 +554,10 @@ void linsertCommand(client *c) {
/* Seek pivot from head to tail */
iter = listTypeInitIterator(subject,0,LIST_TAIL);
const size_t object_len = sdslen(c->argv[3]->ptr);
long long cached_longval = 0;
int cached_valid = 0;
while (listTypeNext(iter,&entry)) {
if (listTypeEqual(&entry,c->argv[3],object_len)) {
if (listTypeEqual(&entry,c->argv[3],object_len,&cached_longval,&cached_valid)) {
listTypeInsert(&entry,c->argv[4],where);
inserted = 1;
break;
@@ -1011,8 +1022,10 @@ void lposCommand(client *c) {
long llen = listTypeLength(o);
long index = 0, matches = 0, matchindex = -1, arraylen = 0;
const size_t ele_len = sdslen(ele->ptr);
long long cached_longval = 0;
int cached_valid = 0;
while (listTypeNext(li,&entry) && (maxlen == 0 || index < maxlen)) {
if (listTypeEqual(&entry,ele,ele_len)) {
if (listTypeEqual(&entry,ele,ele_len,&cached_longval,&cached_valid)) {
matches++;
matchindex = (direction == LIST_TAIL) ? index : llen - index - 1;
if (matches >= rank) {
@@ -1065,8 +1078,10 @@ void lremCommand(client *c) {
listTypeEntry entry;
const size_t object_len = sdslen(c->argv[3]->ptr);
long long cached_longval = 0;
int cached_valid = 0;
while (listTypeNext(li,&entry)) {
if (listTypeEqual(&entry,obj,object_len)) {
if (listTypeEqual(&entry,obj,object_len,&cached_longval,&cached_valid)) {
listTypeDelete(li, &entry);
server.dirty++;
removed++;
src/t_zset.c
+4 -2
View File
@@ -1615,10 +1615,12 @@ long zsetRank(robj *zobj, sds ele, int reverse, double *output_score) {
serverAssert(eptr != NULL);
sptr = lpNext(zl,eptr);
serverAssert(sptr != NULL);
const size_t ele_len = sdslen(ele);
long long cached_val = 0;
int cached_valid = 0;
rank = 1;
while(eptr != NULL) {
if (lpCompare(eptr,(unsigned char*)ele,sdslen(ele)))
if (lpCompare(eptr,(unsigned char*)ele,ele_len,&cached_val,&cached_valid))
break;
rank++;
zzlNext(zl,&eptr,&sptr);