diff --git a/src/listpack.c b/src/listpack.c index eef83092d..f76690ee2 100644 --- a/src/listpack.c +++ b/src/listpack.c @@ -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); } } diff --git a/src/listpack.h b/src/listpack.h index 2c3b78933..a89c2c711 100644 --- a/src/listpack.h +++ b/src/listpack.h @@ -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, diff --git a/src/quicklist.c b/src/quicklist.c index 81bc70646..ca122aebc 100644 --- a/src/quicklist.c +++ b/src/quicklist.c @@ -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(); diff --git a/src/quicklist.h b/src/quicklist.h index c4b07e0c0..75598c5ce 100644 --- a/src/quicklist.h +++ b/src/quicklist.h @@ -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); diff --git a/src/server.h b/src/server.h index 3e6524971..8942bdc7c 100644 --- a/src/server.h +++ b/src/server.h @@ -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); diff --git a/src/t_list.c b/src/t_list.c index e08611881..5d4a16f0f 100644 --- a/src/t_list.c +++ b/src/t_list.c @@ -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++; diff --git a/src/t_zset.c b/src/t_zset.c index 21f034d9f..fe26409c6 100644 --- a/src/t_zset.c +++ b/src/t_zset.c @@ -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);