From add04d0da375e382e2195c6dd08abbdcb16fec1f Mon Sep 17 00:00:00 2001 From: Rain Valentine Date: Thu, 17 Oct 2024 21:59:08 +0000 Subject: [PATCH] Convert SET from dict -> hashset (squashed) Signed-off-by: Rain Valentine --- src/db.c | 64 +++++++++++----- src/debug.c | 29 ++++--- src/defrag.c | 42 ++++++---- src/hashset.c | 37 +++++---- src/hashset.h | 5 +- src/object.c | 52 +++++++------ src/rdb.c | 37 +++++---- src/server.c | 37 ++++++--- src/server.h | 6 +- src/t_set.c | 166 +++++++++++++++++++--------------------- src/t_zset.c | 22 ++---- tests/unit/type/set.tcl | 107 +------------------------- 12 files changed, 281 insertions(+), 323 deletions(-) diff --git a/src/db.c b/src/db.c index 00e6e7b2d6..5f9ffaa776 100644 --- a/src/db.c +++ b/src/db.c @@ -884,7 +884,7 @@ int objectTypeCompare(robj *o, long long target) { } /* This callback is used by scanGenericCommand in order to collect elements * returned by the dictionary iterator into a list. */ -void scanCallback(void *privdata, const dictEntry *de) { +void dictScanCallback(void *privdata, const dictEntry *de) { scanData *data = (scanData *)privdata; list *keys = data->keys; robj *o = data->o; @@ -911,8 +911,6 @@ void scanCallback(void *privdata, const dictEntry *de) { if (o == NULL) { key = keysds; - } else if (o->type == OBJ_SET) { - key = keysds; } else if (o->type == OBJ_HASH) { key = keysds; if (!data->only_keys) { @@ -926,13 +924,37 @@ void scanCallback(void *privdata, const dictEntry *de) { val = sdsnewlen(buf, len); } } else { - serverPanic("Type not handled in SCAN callback."); + serverPanic("Type not handled in dict SCAN callback."); } listAddNodeTail(keys, key); if (val) listAddNodeTail(keys, val); } +void hashsetScanCallback(void *privdata, void *voidElement) { + scanData *data = (scanData *)privdata; + sds key = (sds)voidElement; + + list *keys = data->keys; + robj *o = data->o; + data->sampled++; + + /* o and typename can not have values at the same time. */ + serverAssert(!((data->type != LLONG_MAX) && o)); + + // currently only implemented for SET scan + serverAssert(o && o->type == OBJ_SET && o->encoding == OBJ_ENCODING_HASHSET); + + /* Filter element if it does not match the pattern. */ + if (data->pattern) { + if (!stringmatchlen(data->pattern, sdslen(data->pattern), key, sdslen(key), 0)) { + return; + } + } + + listAddNodeTail(keys, key); +} + /* Try to parse a SCAN cursor stored at object 'o': * if the cursor is valid, store it as unsigned integer into *cursor and * returns C_OK. Otherwise return C_ERR and send an error to the @@ -996,7 +1018,6 @@ void scanGenericCommand(client *c, robj *o, unsigned long long cursor) { sds typename = NULL; long long type = LLONG_MAX; int patlen = 0, use_pattern = 0, only_keys = 0; - dict *ht; /* Object must be NULL (to iterate keys names), or the type of the object * must be Set, Sorted Set, or Hash. */ @@ -1065,34 +1086,37 @@ void scanGenericCommand(client *c, robj *o, unsigned long long cursor) { * just return everything inside the object in a single call, setting the * cursor to zero to signal the end of the iteration. */ - /* Handle the case of a hash table. */ - ht = NULL; + /* Handle the case of a dict or hashset. */ + dict *dictTable = NULL; + hashset *hashsetTable = NULL; if (o == NULL) { - ht = NULL; - } else if (o->type == OBJ_SET && o->encoding == OBJ_ENCODING_HT) { - ht = o->ptr; + dictTable = NULL; + } else if (o->type == OBJ_SET && o->encoding == OBJ_ENCODING_HASHSET) { + hashsetTable = o->ptr; } else if (o->type == OBJ_HASH && o->encoding == OBJ_ENCODING_HT) { - ht = o->ptr; + dictTable = o->ptr; } else if (o->type == OBJ_ZSET && o->encoding == OBJ_ENCODING_SKIPLIST) { zset *zs = o->ptr; - ht = zs->dict; + dictTable = zs->dict; } list *keys = listCreate(); /* Set a free callback for the contents of the collected keys list. - * For the main keyspace dict, and when we scan a key that's dict encoded - * (we have 'ht'), we don't need to define free method because the strings - * in the list are just a shallow copy from the pointer in the dictEntry. + * For the main keyspace dict, when we scan a key that's dict encoded + * (we have 'dictTable'), or when we scan a key that's hashset encoded + * (we have 'hashsetTable') we don't need to define free method because the + * strings in the list are just a shallow copy from the pointer in the + * dictEntry. * When scanning a key with other encodings (e.g. listpack), we need to * free the temporary strings we add to that list. * The exception to the above is ZSET, where we do allocate temporary * strings even when scanning a dict. */ - if (o && (!ht || o->type == OBJ_ZSET)) { + if (o && ((!dictTable && !hashsetTable) || o->type == OBJ_ZSET)) { listSetFreeMethod(keys, (void (*)(void *))sdsfree); } /* For main dictionary scan or data structure using hashtable. */ - if (!o || ht) { + if (!o || dictTable || hashsetTable) { /* We set the max number of iterations to ten times the specified * COUNT, so if the hash table is in a pathological state (very * sparsely populated) we avoid to block too much time at the cost @@ -1130,9 +1154,11 @@ void scanGenericCommand(client *c, robj *o, unsigned long long cursor) { /* In cluster mode there is a separate dictionary for each slot. * If cursor is empty, we should try exploring next non-empty slot. */ if (o == NULL) { - cursor = kvstoreScan(c->db->keys, cursor, onlydidx, scanCallback, NULL, &data); + cursor = kvstoreScan(c->db->keys, cursor, onlydidx, dictScanCallback, NULL, &data); + } else if (dictTable) { + cursor = dictScan(dictTable, cursor, dictScanCallback, &data); } else { - cursor = dictScan(ht, cursor, scanCallback, &data); + cursor = hashsetScan(hashsetTable, cursor, hashsetScanCallback, &data, 0); } } while (cursor && maxiterations-- && data.sampled < count); } else if (o->type == OBJ_SET) { diff --git a/src/debug.c b/src/debug.c index 98512fd436..5ce7ca5bb2 100644 --- a/src/debug.c +++ b/src/debug.c @@ -915,30 +915,35 @@ void debugCommand(client *c) { addReplyVerbatim(c, stats, sdslen(stats), "txt"); sdsfree(stats); } else if (!strcasecmp(c->argv[1]->ptr, "htstats-key") && c->argc >= 3) { - robj *o; - dict *ht = NULL; int full = 0; - if (c->argc >= 4 && !strcasecmp(c->argv[3]->ptr, "full")) full = 1; - if ((o = objectCommandLookupOrReply(c, c->argv[2], shared.nokeyerr)) == NULL) return; + robj *o = objectCommandLookupOrReply(c, c->argv[2], shared.nokeyerr); + if (o == NULL) return; - /* Get the hash table reference from the object, if possible. */ + /* Get the dict reference from the object, if possible. */ + dict *d = NULL; + hashset *hs = NULL; switch (o->encoding) { case OBJ_ENCODING_SKIPLIST: { zset *zs = o->ptr; - ht = zs->dict; + d = zs->dict; } break; - case OBJ_ENCODING_HT: ht = o->ptr; break; + case OBJ_ENCODING_HT: d = o->ptr; break; + case OBJ_ENCODING_HASHSET: hs = o->ptr; break; } - if (ht == NULL) { - addReplyError(c, "The value stored at the specified key is not " - "represented using an hash table"); - } else { + if (d != NULL) { char buf[4096]; - dictGetStats(buf, sizeof(buf), ht, full); + dictGetStats(buf, sizeof(buf), d, full); addReplyVerbatim(c, buf, strlen(buf), "txt"); + } else if (hs != NULL) { + char buf[4096]; + hashsetGetStats(buf, sizeof(buf), hs, full); + addReplyVerbatim(c, buf, strlen(buf), "txt"); + } else { + addReplyError(c, "The value stored at the specified key is not " + "represented using an hash table"); } } else if (!strcasecmp(c->argv[1]->ptr, "change-repl-id") && c->argc == 2) { serverLog(LL_NOTICE, "Changing replication IDs after receiving DEBUG change-repl-id"); diff --git a/src/defrag.c b/src/defrag.c index 4d34009f8b..37e163ce4f 100644 --- a/src/defrag.c +++ b/src/defrag.c @@ -34,6 +34,7 @@ */ #include "server.h" +#include "hashset.h" #include #ifdef HAVE_DEFRAG @@ -309,6 +310,20 @@ void activeDefragSdsDict(dict *d, int val_type) { } while (cursor != 0); } +void activeDefragSdsHashsetCallback(void *privdata, void *element_ref) { + UNUSED(privdata); + sds *sds_ref = (sds *)element_ref; + sds new_sds = activeDefragSds(*sds_ref); + if (new_sds != NULL) *sds_ref = new_sds; +} + +void activeDefragSdsHashset(hashset *hs) { + unsigned long cursor = 0; + do { + cursor = hashsetScan(hs, cursor, activeDefragSdsHashsetCallback, NULL, HASHSET_SCAN_EMIT_REF); + } while (cursor != 0); +} + /* Defrag a list of ptr, sds or robj string values */ void activeDefragList(list *l, int val_type) { listNode *ln, *newln; @@ -441,11 +456,9 @@ void scanCallbackCountScanned(void *privdata, const dictEntry *de) { } void scanLaterSet(robj *ob, unsigned long *cursor) { - if (ob->type != OBJ_SET || ob->encoding != OBJ_ENCODING_HT) return; - dict *d = ob->ptr; - dictDefragFunctions defragfns = {.defragAlloc = activeDefragAlloc, - .defragKey = (dictDefragAllocFunction *)activeDefragSds}; - *cursor = dictScanDefrag(d, *cursor, scanCallbackCountScanned, &defragfns, NULL); + if (ob->type != OBJ_SET || ob->encoding != OBJ_ENCODING_HASHSET) return; + hashset *hs = ob->ptr; + *cursor = hashsetScan(hs, *cursor, activeDefragSdsHashsetCallback, NULL, HASHSET_SCAN_EMIT_REF); } void scanLaterHash(robj *ob, unsigned long *cursor) { @@ -508,15 +521,16 @@ void defragHash(serverDb *db, dictEntry *kde) { void defragSet(serverDb *db, dictEntry *kde) { robj *ob = dictGetVal(kde); - dict *d, *newd; - serverAssert(ob->type == OBJ_SET && ob->encoding == OBJ_ENCODING_HT); - d = ob->ptr; - if (dictSize(d) > server.active_defrag_max_scan_fields) + serverAssert(ob->type == OBJ_SET && ob->encoding == OBJ_ENCODING_HASHSET); + hashset *hs = ob->ptr; + if (hashsetSize(hs) > server.active_defrag_max_scan_fields) { defragLater(db, kde); - else - activeDefragSdsDict(d, DEFRAG_SDS_DICT_NO_VAL); - /* defrag the dict struct and tables */ - if ((newd = dictDefragTables(ob->ptr))) ob->ptr = newd; + } else { + activeDefragSdsHashset(hs); + } + /* defrag the hashset struct and members */ + hashset *newHashset = hashsetDefragInternals(hs, activeDefragAlloc); + if (newHashset) ob->ptr = newHashset; } /* Defrag callback for radix tree iterator, called for each node, @@ -704,7 +718,7 @@ void defragKey(defragCtx *ctx, dictEntry *de) { serverPanic("Unknown list encoding"); } } else if (ob->type == OBJ_SET) { - if (ob->encoding == OBJ_ENCODING_HT) { + if (ob->encoding == OBJ_ENCODING_HASHSET) { defragSet(db, de); } else if (ob->encoding == OBJ_ENCODING_INTSET || ob->encoding == OBJ_ENCODING_LISTPACK) { void *newptr, *ptr = ob->ptr; diff --git a/src/hashset.c b/src/hashset.c index 80b58ff588..8af5460a18 100644 --- a/src/hashset.c +++ b/src/hashset.c @@ -71,6 +71,7 @@ * addressing scheme, including the use of linear probing by scan cursor * increment, by Viktor Söderqvist. */ #include "hashset.h" +#include "server.h" #include "serverassert.h" #include "zmalloc.h" #include "mt19937-64.h" @@ -814,12 +815,12 @@ void *hashsetMetadata(hashset *t) { } /* Returns the number of elements stored. */ -size_t hashsetSize(hashset *t) { +size_t hashsetSize(const hashset *t) { return t->used[0] + t->used[1]; } /* Returns the number of hash table buckets. */ -size_t hashsetBuckets(hashset *t) { +size_t hashsetBuckets(const hashset *t) { return numBuckets(t->bucketExp[0]) + numBuckets(t->bucketExp[1]); } @@ -962,6 +963,15 @@ hashset *hashsetDefragInternals(hashset *s, void *(*defragfn)(void *)) { return s1; } +/* Used to release memory to OS to avoid unnecessary CoW. + * Called when we've forked and memory won't be used again. + * See dismissObject() */ +void dismissHashset(hashset *t) { + for (int i = 0; i < 2; i++) { + dismissMemory(t->tables[i], numBuckets(t->bucketExp[i]) * sizeof(bucket *)); + } +} + /* Returns 1 if an element was found matching the key. Also points *found to it, * if found is provided. Returns 0 if no matching element was found. */ int hashsetFind(hashset *t, const void *key, void **found) { @@ -1311,16 +1321,14 @@ size_t hashsetScan(hashset *t, size_t cursor, hashsetScanFunction fn, void *priv /* Emit elements in the smaller table, if this bucket hasn't already * been rehashed. */ - if (table0 == 0 && !cursorIsLessThan(cursor, t->rehashIdx)) { - bucket *b = &t->tables[table0][cursor & mask0]; - for (int pos = 0; pos < ELEMENTS_PER_BUCKET; pos++) { - if (b->presence & (1 << pos)) { - void *emit = emit_ref ? &b->elements[pos] : b->elements[pos]; - fn(privdata, emit); - } + bucket *b = &t->tables[table0][cursor & mask0]; + for (int pos = 0; pos < ELEMENTS_PER_BUCKET; pos++) { + if (b->presence & (1 << pos)) { + void *emit = emit_ref ? &b->elements[pos] : b->elements[pos]; + fn(privdata, emit); } - in_probe_sequence |= b->everfull; } + in_probe_sequence |= b->everfull; /* Iterate over indices in larger table that are the expansion of * the index pointed to by the cursor in the smaller table. */ @@ -1431,6 +1439,10 @@ int hashsetNext(hashsetIterator *iter, void **elemptr) { } else { iter->fingerprint = hashsetFingerprint(iter->hashset); } + if (!iter->hashset->tables[0]) { + /* Empty table - we're done */ + break; + } iter->index = 0; /* skip the rehashed slots in table[0] */ if (hashsetIsRehashing(iter->hashset)) { @@ -1442,8 +1454,8 @@ int hashsetNext(hashsetIterator *iter, void **elemptr) { iter->posInBucket++; if (iter->posInBucket >= ELEMENTS_PER_BUCKET) { iter->posInBucket = 0; - iter->index++; - if (iter->index >= (long)numBuckets(iter->hashset->bucketExp[iter->table])) { + iter->index = nextCursor(iter->index, expToMask(iter->hashset->bucketExp[iter->table])); + if (iter->index == 0) { iter->index = 0; if (hashsetIsRehashing(iter->hashset) && iter->table == 0) { iter->table++; @@ -1511,7 +1523,6 @@ unsigned hashsetSampleElements(hashset *t, void **dst, unsigned count) { while (samples.count < count) { cursor = hashsetScan(t, cursor, sampleElementsScanFn, &samples, HASHSET_SCAN_SINGLE_STEP); } - rehashStepOnReadIfNeeded(t); return count; } diff --git a/src/hashset.h b/src/hashset.h index 68ab54db60..2a7838d70f 100644 --- a/src/hashset.h +++ b/src/hashset.h @@ -150,8 +150,8 @@ void hashsetRelease(hashset *t); void hashsetEmpty(hashset *t, void(callback)(hashset *)); hashsetType *hashsetGetType(hashset *t); void *hashsetMetadata(hashset *t); -size_t hashsetSize(hashset *t); -size_t hashsetBuckets(hashset *t); +size_t hashsetSize(const hashset *t); +size_t hashsetBuckets(const hashset *t); size_t hashsetProbeCounter(hashset *t, int table); size_t hashsetMemUsage(hashset *t); void hashsetPauseAutoShrink(hashset *t); @@ -165,6 +165,7 @@ int hashsetTryExpand(hashset *t, size_t size); int hashsetExpandIfNeeded(hashset *t); int hashsetShrinkIfNeeded(hashset *t); hashset *hashsetDefragInternals(hashset *t, void *(*defragfn)(void *)); +void dismissHashset(hashset *set); /* Elements */ int hashsetFind(hashset *t, const void *key, void **found); diff --git a/src/object.c b/src/object.c index 8c1cf64892..539403dd2a 100644 --- a/src/object.c +++ b/src/object.c @@ -251,9 +251,9 @@ robj *createListListpackObject(void) { } robj *createSetObject(void) { - dict *d = dictCreate(&setDictType); - robj *o = createObject(OBJ_SET, d); - o->encoding = OBJ_ENCODING_HT; + hashset *hs = hashsetCreate(&setHashsetType); + robj *o = createObject(OBJ_SET, hs); + o->encoding = OBJ_ENCODING_HASHSET; return o; } @@ -328,7 +328,7 @@ void freeListObject(robj *o) { void freeSetObject(robj *o) { switch (o->encoding) { - case OBJ_ENCODING_HT: dictRelease((dict *)o->ptr); break; + case OBJ_ENCODING_HASHSET: hashsetRelease((hashset *)o->ptr); break; case OBJ_ENCODING_INTSET: case OBJ_ENCODING_LISTPACK: zfree(o->ptr); break; default: serverPanic("Unknown set encoding type"); @@ -437,23 +437,22 @@ void dismissListObject(robj *o, size_t size_hint) { /* See dismissObject() */ void dismissSetObject(robj *o, size_t size_hint) { - if (o->encoding == OBJ_ENCODING_HT) { - dict *set = o->ptr; - serverAssert(dictSize(set) != 0); + if (o->encoding == OBJ_ENCODING_HASHSET) { + hashset *set = o->ptr; + serverAssert(hashsetSize(set) != 0); /* We iterate all nodes only when average member size is bigger than a * page size, and there's a high chance we'll actually dismiss something. */ - if (size_hint / dictSize(set) >= server.page_size) { - dictEntry *de; - dictIterator *di = dictGetIterator(set); - while ((de = dictNext(di)) != NULL) { - dismissSds(dictGetKey(de)); + if (size_hint / hashsetSize(set) >= server.page_size) { + hashsetIterator iter; + hashsetInitIterator(&iter, set); + sds item; + while (hashsetNext(&iter, (void **)(&item))) { + dismissSds(item); } - dictReleaseIterator(di); + hashsetResetIterator(&iter); } - /* Dismiss hash table memory. */ - dismissMemory(set->ht_table[0], DICTHT_SIZE(set->ht_size_exp[0]) * sizeof(dictEntry *)); - dismissMemory(set->ht_table[1], DICTHT_SIZE(set->ht_size_exp[1]) * sizeof(dictEntry *)); + dismissHashset(set); } else if (o->encoding == OBJ_ENCODING_INTSET) { dismissMemory(o->ptr, intsetBlobLen((intset *)o->ptr)); } else if (o->encoding == OBJ_ENCODING_LISTPACK) { @@ -939,6 +938,7 @@ char *strEncoding(int encoding) { case OBJ_ENCODING_RAW: return "raw"; case OBJ_ENCODING_INT: return "int"; case OBJ_ENCODING_HT: return "hashtable"; + case OBJ_ENCODING_HASHSET: return "hashtable"; case OBJ_ENCODING_QUICKLIST: return "quicklist"; case OBJ_ENCODING_LISTPACK: return "listpack"; case OBJ_ENCODING_INTSET: return "intset"; @@ -990,17 +990,19 @@ size_t objectComputeSize(robj *key, robj *o, size_t sample_size, int dbid) { serverPanic("Unknown list encoding"); } } else if (o->type == OBJ_SET) { - if (o->encoding == OBJ_ENCODING_HT) { - d = o->ptr; - di = dictGetIterator(d); - asize = sizeof(*o) + sizeof(dict) + (sizeof(struct dictEntry *) * dictBuckets(d)); - while ((de = dictNext(di)) != NULL && samples < sample_size) { - ele = dictGetKey(de); - elesize += dictEntryMemUsage(de) + sdsAllocSize(ele); + if (o->encoding == OBJ_ENCODING_HASHSET) { + hashset *set = o->ptr; + asize = sizeof(*o) + hashsetMemUsage(set); + + hashsetIterator iter; + hashsetInitIterator(&iter, set); + sds element; + while (hashsetNext(&iter, (void**)&element) && samples < sample_size) { + elesize += sdsAllocSize(element); samples++; } - dictReleaseIterator(di); - if (samples) asize += (double)elesize / samples * dictSize(d); + hashsetResetIterator(&iter); + if (samples) asize += (double)elesize / samples * hashsetSize(set); } else if (o->encoding == OBJ_ENCODING_INTSET) { asize = sizeof(*o) + zmalloc_size(o->ptr); } else if (o->encoding == OBJ_ENCODING_LISTPACK) { diff --git a/src/rdb.c b/src/rdb.c index bc2d03e86c..8d847b9d73 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -687,7 +687,7 @@ int rdbSaveObjectType(rio *rdb, robj *o) { case OBJ_SET: if (o->encoding == OBJ_ENCODING_INTSET) return rdbSaveType(rdb, RDB_TYPE_SET_INTSET); - else if (o->encoding == OBJ_ENCODING_HT) + else if (o->encoding == OBJ_ENCODING_HASHSET) return rdbSaveType(rdb, RDB_TYPE_SET); else if (o->encoding == OBJ_ENCODING_LISTPACK) return rdbSaveType(rdb, RDB_TYPE_SET_LISTPACK); @@ -871,26 +871,25 @@ ssize_t rdbSaveObject(rio *rdb, robj *o, robj *key, int dbid) { } } else if (o->type == OBJ_SET) { /* Save a set value */ - if (o->encoding == OBJ_ENCODING_HT) { - dict *set = o->ptr; - dictIterator *di = dictGetIterator(set); - dictEntry *de; + if (o->encoding == OBJ_ENCODING_HASHSET) { + hashset *set = o->ptr; - if ((n = rdbSaveLen(rdb, dictSize(set))) == -1) { - dictReleaseIterator(di); + if ((n = rdbSaveLen(rdb, hashsetSize(set))) == -1) { return -1; } nwritten += n; - while ((de = dictNext(di)) != NULL) { - sds ele = dictGetKey(de); + hashsetIterator iterator; + hashsetInitIterator(&iterator, set); + sds ele; + while (hashsetNext(&iterator, (void **)&ele)) { if ((n = rdbSaveRawString(rdb, (unsigned char *)ele, sdslen(ele))) == -1) { - dictReleaseIterator(di); + hashsetResetIterator(&iterator); return -1; } nwritten += n; } - dictReleaseIterator(di); + hashsetResetIterator(&iterator); } else if (o->encoding == OBJ_ENCODING_INTSET) { size_t l = intsetBlobLen((intset *)o->ptr); @@ -1902,8 +1901,8 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, int dbid, int *error) { o = createSetObject(); /* It's faster to expand the dict to the right size asap in order * to avoid rehashing */ - if (len > DICT_HT_INITIAL_SIZE && dictTryExpand(o->ptr, len) != DICT_OK) { - rdbReportCorruptRDB("OOM in dictTryExpand %llu", (unsigned long long)len); + if (!hashsetTryExpand(o->ptr, len)) { + rdbReportCorruptRDB("OOM in hashsetTryExpand %llu", (unsigned long long)len); decrRefCount(o); return NULL; } @@ -1942,8 +1941,8 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, int dbid, int *error) { * of many small ones. It's OK since lpSafeToAdd doesn't * care about individual elements, only the total size. */ setTypeConvert(o, OBJ_ENCODING_LISTPACK); - } else if (setTypeConvertAndExpand(o, OBJ_ENCODING_HT, len, 0) != C_OK) { - rdbReportCorruptRDB("OOM in dictTryExpand %llu", (unsigned long long)len); + } else if (setTypeConvertAndExpand(o, OBJ_ENCODING_HASHSET, len, 0) != C_OK) { + rdbReportCorruptRDB("OOM in hashsetTryExpand %llu", (unsigned long long)len); sdsfree(sdsele); decrRefCount(o); return NULL; @@ -1963,8 +1962,8 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, int dbid, int *error) { return NULL; } o->ptr = lpAppend(o->ptr, (unsigned char *)sdsele, elelen); - } else if (setTypeConvertAndExpand(o, OBJ_ENCODING_HT, len, 0) != C_OK) { - rdbReportCorruptRDB("OOM in dictTryExpand %llu", (unsigned long long)len); + } else if (setTypeConvertAndExpand(o, OBJ_ENCODING_HASHSET, len, 0) != C_OK) { + rdbReportCorruptRDB("OOM in hashsetTryExpand %llu", (unsigned long long)len); sdsfree(sdsele); decrRefCount(o); return NULL; @@ -1973,8 +1972,8 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, int dbid, int *error) { /* This will also be called when the set was just converted * to a regular hash table encoded set. */ - if (o->encoding == OBJ_ENCODING_HT) { - if (dictAdd((dict *)o->ptr, sdsele, NULL) != DICT_OK) { + if (o->encoding == OBJ_ENCODING_HASHSET) { + if (!hashsetAdd((hashset *)o->ptr, sdsele)) { rdbReportCorruptRDB("Duplicate set members detected"); decrRefCount(o); sdsfree(sdsele); diff --git a/src/server.c b/src/server.c index 6e2673a1fd..96683ca5d0 100644 --- a/src/server.c +++ b/src/server.c @@ -286,6 +286,7 @@ void dictDictDestructor(dict *d, void *val) { dictRelease((dict *)val); } +/* Returns 1 when keys match */ int dictSdsKeyCompare(dict *d, const void *key1, const void *key2) { int l1, l2; UNUSED(d); @@ -296,6 +297,16 @@ int dictSdsKeyCompare(dict *d, const void *key1, const void *key2) { return memcmp(key1, key2, l1) == 0; } +/* Returns 0 when keys match */ +int hashsetSdsKeyCompare(hashset *hs, const void *key1, const void *key2) { + UNUSED(hs); + + size_t l1 = sdslen((sds)key1); + size_t l2 = sdslen((sds)key2); + if (l1 != l2) return 1; + return memcmp(key1, key2, l1); +} + size_t dictSdsEmbedKey(unsigned char *buf, size_t buf_len, const void *key, uint8_t *key_offset) { return sdscopytobuffer(buf, buf_len, (sds)key, key_offset); } @@ -323,6 +334,11 @@ void dictSdsDestructor(dict *d, void *val) { sdsfree(val); } +void hashsetSdsDestructor(hashset *d, void *val) { + UNUSED(d); + sdsfree(val); +} + void *dictSdsDup(dict *d, const void *key) { UNUSED(d); return sdsdup((const sds)key); @@ -467,17 +483,11 @@ dictType objectKeyHeapPointerValueDictType = { NULL /* allow to expand */ }; -/* Set dictionary type. Keys are SDS strings, values are not used. */ -dictType setDictType = { - dictSdsHash, /* hash function */ - NULL, /* key dup */ - dictSdsKeyCompare, /* key compare */ - dictSdsDestructor, /* key destructor */ - NULL, /* val destructor */ - NULL, /* allow to expand */ - .no_value = 1, /* no values in this dict */ - .keys_are_odd = 1 /* an SDS string is always an odd pointer */ -}; +/* Set hashset type. Items are SDS strings */ +hashsetType setHashsetType = { + .hashFunction = dictSdsHash, + .keyCompare = hashsetSdsKeyCompare, + .elementDestructor = hashsetSdsDestructor}; /* Sorted sets hash (note: a skiplist is used in addition to the hash table) */ dictType zsetDictType = { @@ -549,6 +559,11 @@ dictType sdsReplyDictType = { NULL /* allow to expand */ }; +/* Hashset type without destructor */ +hashsetType sdsReplyHashsetType = { + .hashFunction = dictSdsCaseHash, + .keyCompare = hashsetSdsKeyCompare}; + /* Keylist hash table type has unencoded Objects as keys and * lists as values. It's used for blocking operations (BLPOP) and to * map swapped keys to a list of clients waiting for this keys to be loaded. */ diff --git a/src/server.h b/src/server.h index cd0eca3a51..4aab07ea93 100644 --- a/src/server.h +++ b/src/server.h @@ -866,6 +866,7 @@ struct ValkeyModuleDigest { #define OBJ_ENCODING_QUICKLIST 9 /* Encoded as linked list of listpacks */ #define OBJ_ENCODING_STREAM 10 /* Encoded as a radix tree of listpacks */ #define OBJ_ENCODING_LISTPACK 11 /* Encoded as a listpack */ +#define OBJ_ENCODING_HASHSET 12 /* Encoded as a hashset */ #define LRU_BITS 24 #define LRU_CLOCK_MAX ((1 << LRU_BITS) - 1) /* Max value of obj->lru */ @@ -2590,7 +2591,7 @@ typedef struct { robj *subject; int encoding; int ii; /* intset iterator */ - dictIterator *di; + hashsetIterator *hashset_iterator; unsigned char *lpi; /* listpack iterator */ } setTypeIterator; @@ -2621,7 +2622,7 @@ extern struct valkeyServer server; extern struct sharedObjectsStruct shared; extern dictType objectKeyPointerValueDictType; extern dictType objectKeyHeapPointerValueDictType; -extern dictType setDictType; +extern hashsetType setHashsetType; extern dictType BenchmarkDictType; extern dictType zsetDictType; extern dictType kvstoreKeysDictType; @@ -2636,6 +2637,7 @@ extern dictType objToDictDictType; extern dictType kvstoreChannelDictType; extern dictType modulesDictType; extern dictType sdsReplyDictType; +extern hashsetType sdsReplyHashsetType; extern dictType keylistDictType; extern dict *modules; diff --git a/src/t_set.c b/src/t_set.c index a540c3c49b..978bee232d 100644 --- a/src/t_set.c +++ b/src/t_set.c @@ -28,6 +28,7 @@ */ #include "server.h" +#include "hashset.h" #include "intset.h" /* Compact integer set structure */ /*----------------------------------------------------------------------------- @@ -50,7 +51,7 @@ robj *setTypeCreate(sds value, size_t size_hint) { /* We may oversize the set by using the hint if the hint is not accurate, * but we will assume this is acceptable to maximize performance. */ robj *o = createSetObject(); - dictExpand(o->ptr, size_hint); + hashsetExpand(o->ptr, size_hint); return o; } @@ -59,7 +60,7 @@ robj *setTypeCreate(sds value, size_t size_hint) { void setTypeMaybeConvert(robj *set, size_t size_hint) { if ((set->encoding == OBJ_ENCODING_LISTPACK && size_hint > server.set_max_listpack_entries) || (set->encoding == OBJ_ENCODING_INTSET && size_hint > server.set_max_intset_entries)) { - setTypeConvertAndExpand(set, OBJ_ENCODING_HT, size_hint, 1); + setTypeConvertAndExpand(set, OBJ_ENCODING_HASHSET, size_hint, 1); } } @@ -74,7 +75,7 @@ static size_t intsetMaxEntries(void) { /* Converts intset to HT if it contains too many entries. */ static void maybeConvertIntset(robj *subject) { serverAssert(subject->encoding == OBJ_ENCODING_INTSET); - if (intsetLen(subject->ptr) > intsetMaxEntries()) setTypeConvert(subject, OBJ_ENCODING_HT); + if (intsetLen(subject->ptr) > intsetMaxEntries()) setTypeConvert(subject, OBJ_ENCODING_HASHSET); } /* When you know all set elements are integers, call this to convert the set to @@ -91,7 +92,7 @@ static void maybeConvertToIntset(robj *set) { while (setTypeNext(si, &str, &len, &llval) != -1) { if (str) { /* If the element is returned as a string, we may be able to convert - * it to integer. This happens for OBJ_ENCODING_HT. */ + * it to integer. This happens for OBJ_ENCODING_HASHSET. */ serverAssert(string2ll(str, len, (long long *)&llval)); } uint8_t success = 0; @@ -134,15 +135,15 @@ int setTypeAddAux(robj *set, char *str, size_t len, int64_t llval, int str_is_sd } serverAssert(str); - if (set->encoding == OBJ_ENCODING_HT) { + if (set->encoding == OBJ_ENCODING_HASHSET) { /* Avoid duping the string if it is an sds string. */ sds sdsval = str_is_sds ? (sds)str : sdsnewlen(str, len); - dict *ht = set->ptr; - void *position = dictFindPositionForInsert(ht, sdsval, NULL); + hashset *hs = set->ptr; + void *position = hashsetFindPositionForInsert(hs, sdsval, NULL); if (position) { /* Key doesn't already exist in the set. Add it but dup the key. */ if (sdsval == str) sdsval = sdsdup(sdsval); - dictInsertAtPosition(ht, sdsval, position); + hashsetInsertAtPosition(hs, sdsval, position); } else if (sdsval != str) { /* String is already a member. Free our temporary sds copy. */ sdsfree(sdsval); @@ -165,9 +166,9 @@ int setTypeAddAux(robj *set, char *str, size_t len, int64_t llval, int str_is_sd } set->ptr = lp; } else { - /* Size limit is reached. Convert to hashtable and add. */ - setTypeConvertAndExpand(set, OBJ_ENCODING_HT, lpLength(lp) + 1, 1); - serverAssert(dictAdd(set->ptr, sdsnewlen(str, len), NULL) == DICT_OK); + /* Size limit is reached. Convert to hashset and add. */ + setTypeConvertAndExpand(set, OBJ_ENCODING_HASHSET, lpLength(lp) + 1, 1); + serverAssert(hashsetAdd(set->ptr, sdsnewlen(str, len))); } return 1; } @@ -204,10 +205,10 @@ int setTypeAddAux(robj *set, char *str, size_t len, int64_t llval, int str_is_sd set->ptr = lp; return 1; } else { - setTypeConvertAndExpand(set, OBJ_ENCODING_HT, intsetLen(set->ptr) + 1, 1); + setTypeConvertAndExpand(set, OBJ_ENCODING_HASHSET, intsetLen(set->ptr) + 1, 1); /* The set *was* an intset and this value is not integer - * encodable, so dictAdd should always work. */ - serverAssert(dictAdd(set->ptr, sdsnewlen(str, len), NULL) == DICT_OK); + * encodable, so hashsetAdd should always work. */ + serverAssert(hashsetAdd(set->ptr, sdsnewlen(str, len))); return 1; } } @@ -242,9 +243,9 @@ int setTypeRemoveAux(robj *setobj, char *str, size_t len, int64_t llval, int str str_is_sds = 0; } - if (setobj->encoding == OBJ_ENCODING_HT) { + if (setobj->encoding == OBJ_ENCODING_HASHSET) { sds sdsval = str_is_sds ? (sds)str : sdsnewlen(str, len); - int deleted = (dictDelete(setobj->ptr, sdsval) == DICT_OK); + int deleted = (hashsetDelete(setobj->ptr, sdsval)); if (sdsval != str) sdsfree(sdsval); /* free temp copy */ return deleted; } else if (setobj->encoding == OBJ_ENCODING_LISTPACK) { @@ -298,11 +299,11 @@ int setTypeIsMemberAux(robj *set, char *str, size_t len, int64_t llval, int str_ } else if (set->encoding == OBJ_ENCODING_INTSET) { long long llval; return string2ll(str, len, &llval) && intsetFind(set->ptr, llval); - } else if (set->encoding == OBJ_ENCODING_HT && str_is_sds) { - return dictFind(set->ptr, (sds)str) != NULL; - } else if (set->encoding == OBJ_ENCODING_HT) { + } else if (set->encoding == OBJ_ENCODING_HASHSET && str_is_sds) { + return hashsetFind(set->ptr, (sds)str, NULL); + } else if (set->encoding == OBJ_ENCODING_HASHSET) { sds sdsval = sdsnewlen(str, len); - int result = dictFind(set->ptr, sdsval) != NULL; + int result = hashsetFind(set->ptr, sdsval, NULL); sdsfree(sdsval); return result; } else { @@ -314,8 +315,8 @@ setTypeIterator *setTypeInitIterator(robj *subject) { setTypeIterator *si = zmalloc(sizeof(setTypeIterator)); si->subject = subject; si->encoding = subject->encoding; - if (si->encoding == OBJ_ENCODING_HT) { - si->di = dictGetIterator(subject->ptr); + if (si->encoding == OBJ_ENCODING_HASHSET) { + si->hashset_iterator = hashsetCreateIterator(subject->ptr); } else if (si->encoding == OBJ_ENCODING_INTSET) { si->ii = 0; } else if (si->encoding == OBJ_ENCODING_LISTPACK) { @@ -327,7 +328,7 @@ setTypeIterator *setTypeInitIterator(robj *subject) { } void setTypeReleaseIterator(setTypeIterator *si) { - if (si->encoding == OBJ_ENCODING_HT) dictReleaseIterator(si->di); + if (si->encoding == OBJ_ENCODING_HASHSET) hashsetReleaseIterator(si->hashset_iterator); zfree(si); } @@ -340,7 +341,7 @@ void setTypeReleaseIterator(setTypeIterator *si) { * (str and len) or (llele) depending on whether the value is stored as a string * or as an integer internally. * - * If OBJ_ENCODING_HT is returned, then str points to an sds string and can be + * If OBJ_ENCODING_HASHSET is returned, then str points to an sds string and can be * used as such. If OBJ_ENCODING_INTSET, then llele is populated and str is * pointed to NULL. If OBJ_ENCODING_LISTPACK is returned, the value can be * either a string or an integer. If *str is not NULL, then str and len are @@ -353,10 +354,8 @@ void setTypeReleaseIterator(setTypeIterator *si) { * * When there are no more elements -1 is returned. */ int setTypeNext(setTypeIterator *si, char **str, size_t *len, int64_t *llele) { - if (si->encoding == OBJ_ENCODING_HT) { - dictEntry *de = dictNext(si->di); - if (de == NULL) return -1; - *str = dictGetKey(de); + if (si->encoding == OBJ_ENCODING_HASHSET) { + if (!hashsetNext(si->hashset_iterator, (void **)str)) return -1; *len = sdslen(*str); *llele = -123456789; /* Not needed. Defensive. */ } else if (si->encoding == OBJ_ENCODING_INTSET) { @@ -406,15 +405,15 @@ sds setTypeNextObject(setTypeIterator *si) { * object. The return value of the function is the object->encoding * field of the object and can be used by the caller to check if the * int64_t pointer or the str and len pointers were populated, as for - * setTypeNext. If OBJ_ENCODING_HT is returned, str is pointed to a + * setTypeNext. If OBJ_ENCODING_HASHSET is returned, str is pointed to a * string which is actually an sds string and it can be used as such. * * Note that both the str, len and llele pointers should be passed and cannot * be NULL. If str is set to NULL, the value is an integer stored in llele. */ int setTypeRandomElement(robj *setobj, char **str, size_t *len, int64_t *llele) { - if (setobj->encoding == OBJ_ENCODING_HT) { - dictEntry *de = dictGetFairRandomKey(setobj->ptr); - *str = dictGetKey(de); + if (setobj->encoding == OBJ_ENCODING_HASHSET) { + *str = NULL; + hashsetFairRandomElement(setobj->ptr, (void **)str); *len = sdslen(*str); *llele = -123456789; /* Not needed. Defensive. */ } else if (setobj->encoding == OBJ_ENCODING_INTSET) { @@ -457,14 +456,14 @@ robj *setTypePopRandom(robj *set) { obj = createStringObject(str, len); else obj = createStringObjectFromLongLong(llele); - setTypeRemoveAux(set, str, len, llele, encoding == OBJ_ENCODING_HT); + setTypeRemoveAux(set, str, len, llele, encoding == OBJ_ENCODING_HASHSET); } return obj; } unsigned long setTypeSize(const robj *subject) { - if (subject->encoding == OBJ_ENCODING_HT) { - return dictSize((const dict *)subject->ptr); + if (subject->encoding == OBJ_ENCODING_HASHSET) { + return hashsetSize((const hashset *)subject->ptr); } else if (subject->encoding == OBJ_ENCODING_INTSET) { return intsetLen((const intset *)subject->ptr); } else if (subject->encoding == OBJ_ENCODING_LISTPACK) { @@ -474,7 +473,7 @@ unsigned long setTypeSize(const robj *subject) { } } -/* Convert the set to specified encoding. The resulting dict (when converting +/* Convert the set to specified encoding. The resulting hashset (when converting * to a hash table) is presized to hold the number of elements in the original * set. */ void setTypeConvert(robj *setobj, int enc) { @@ -489,28 +488,28 @@ int setTypeConvertAndExpand(robj *setobj, int enc, unsigned long cap, int panic) setTypeIterator *si; serverAssertWithInfo(NULL, setobj, setobj->type == OBJ_SET && setobj->encoding != enc); - if (enc == OBJ_ENCODING_HT) { - dict *d = dictCreate(&setDictType); + if (enc == OBJ_ENCODING_HASHSET) { + hashset *hs = hashsetCreate(&setHashsetType); sds element; - /* Presize the dict to avoid rehashing */ + /* Presize the hashset to avoid rehashing */ if (panic) { - dictExpand(d, cap); - } else if (dictTryExpand(d, cap) != DICT_OK) { - dictRelease(d); + hashsetExpand(hs, cap); + } else if (!hashsetTryExpand(hs, cap)) { + hashsetRelease(hs); return C_ERR; } /* To add the elements we extract integers and create Objects */ si = setTypeInitIterator(setobj); while ((element = setTypeNextObject(si)) != NULL) { - serverAssert(dictAdd(d, element, NULL) == DICT_OK); + serverAssert(hashsetAdd(hs, element)); } setTypeReleaseIterator(si); freeSetObject(setobj); /* frees the internals but not setobj itself */ - setobj->encoding = OBJ_ENCODING_HT; - setobj->ptr = d; + setobj->encoding = OBJ_ENCODING_HASHSET; + setobj->ptr = hs; } else if (enc == OBJ_ENCODING_LISTPACK) { /* Preallocate the minimum two bytes per element (enc/value + backlen) */ size_t estcap = cap * 2; @@ -568,10 +567,10 @@ robj *setTypeDup(robj *o) { memcpy(new_lp, lp, sz); set = createObject(OBJ_SET, new_lp); set->encoding = OBJ_ENCODING_LISTPACK; - } else if (o->encoding == OBJ_ENCODING_HT) { + } else if (o->encoding == OBJ_ENCODING_HASHSET) { set = createSetObject(); - dict *d = o->ptr; - dictExpand(set->ptr, dictSize(d)); + hashset *hs = o->ptr; + hashsetExpand(set->ptr, hashsetSize(hs)); si = setTypeInitIterator(o); char *str; size_t len; @@ -891,8 +890,8 @@ void spopWithCountCommand(client *c) { if (!newset) { newset = str ? createSetListpackObject() : createIntsetObject(); } - setTypeAddAux(newset, str, len, llele, encoding == OBJ_ENCODING_HT); - setTypeRemoveAux(set, str, len, llele, encoding == OBJ_ENCODING_HT); + setTypeAddAux(newset, str, len, llele, encoding == OBJ_ENCODING_HASHSET); + setTypeRemoveAux(set, str, len, llele, encoding == OBJ_ENCODING_HASHSET); } } @@ -1001,8 +1000,6 @@ void srandmemberWithCountCommand(client *c) { size_t len; int64_t llele; - dict *d; - if (getRangeLongFromObjectOrReply(c, c->argv[2], -LONG_MAX, LONG_MAX, &l, NULL) != C_OK) return; if (l >= 0) { count = (unsigned long)l; @@ -1111,8 +1108,8 @@ void srandmemberWithCountCommand(client *c) { return; } - /* For CASE 3 and CASE 4 we need an auxiliary dictionary. */ - d = dictCreate(&sdsReplyDictType); + /* For CASE 3 and CASE 4 we need an auxiliary hashset. */ + hashset *hs = hashsetCreate(&sdsReplyHashsetType); /* CASE 3: * The number of elements inside the set is not greater than @@ -1126,29 +1123,25 @@ void srandmemberWithCountCommand(client *c) { if (count * SRANDMEMBER_SUB_STRATEGY_MUL > size) { setTypeIterator *si; - /* Add all the elements into the temporary dictionary. */ + /* Add all the elements into the temporary hashset. */ si = setTypeInitIterator(set); - dictExpand(d, size); + hashsetExpand(hs, size); while (setTypeNext(si, &str, &len, &llele) != -1) { - int retval = DICT_ERR; - if (str == NULL) { - retval = dictAdd(d, sdsfromlonglong(llele), NULL); + serverAssert(hashsetAdd(hs, (void *)sdsfromlonglong(llele))); } else { - retval = dictAdd(d, sdsnewlen(str, len), NULL); + serverAssert(hashsetAdd(hs, (void *)sdsnewlen(str, len))); } - serverAssert(retval == DICT_OK); } setTypeReleaseIterator(si); - serverAssert(dictSize(d) == size); + serverAssert(hashsetSize(hs) == size); /* Remove random elements to reach the right count. */ while (size > count) { - dictEntry *de; - de = dictGetFairRandomKey(d); - dictUnlink(d, dictGetKey(de)); - sdsfree(dictGetKey(de)); - dictFreeUnlinkedEntry(d, de); + sds element; + hashsetFairRandomElement(hs, (void **)&element); + hashsetDelete(hs, (void *)element); + sdsfree(element); size--; } } @@ -1161,7 +1154,7 @@ void srandmemberWithCountCommand(client *c) { unsigned long added = 0; sds sdsele; - dictExpand(d, count); + hashsetExpand(hs, count); while (added < count) { setTypeRandomElement(set, &str, &len, &llele); if (str == NULL) { @@ -1172,7 +1165,7 @@ void srandmemberWithCountCommand(client *c) { /* Try to add the object to the dictionary. If it already exists * free it, otherwise increment the number of objects we have * in the result dictionary. */ - if (dictAdd(d, sdsele, NULL) == DICT_OK) + if (hashsetAdd(hs, sdsele)) added++; else sdsfree(sdsele); @@ -1181,14 +1174,15 @@ void srandmemberWithCountCommand(client *c) { /* CASE 3 & 4: send the result to the user. */ { - dictIterator *di; - dictEntry *de; + hashsetIterator iter; + hashsetInitIterator(&iter, hs); addReplyArrayLen(c, count); - di = dictGetIterator(d); - while ((de = dictNext(di)) != NULL) addReplyBulkSds(c, dictGetKey(de)); - dictReleaseIterator(di); - dictRelease(d); + serverAssert(count == hashsetSize(hs)); + sds element; + while (hashsetNext(&iter, (void **)&element)) addReplyBulkSds(c, element); + hashsetResetIterator(&iter); + hashsetRelease(hs); } } @@ -1336,7 +1330,7 @@ void sinterGenericCommand(client *c, while ((encoding = setTypeNext(si, &str, &len, &intobj)) != -1) { for (j = 1; j < setnum; j++) { if (sets[j] == sets[0]) continue; - if (!setTypeIsMemberAux(sets[j], str, len, intobj, encoding == OBJ_ENCODING_HT)) break; + if (!setTypeIsMemberAux(sets[j], str, len, intobj, encoding == OBJ_ENCODING_HASHSET)) break; } /* Only take action when all sets contain the member */ @@ -1355,7 +1349,7 @@ void sinterGenericCommand(client *c, } else { if (str && only_integers) { /* It may be an integer although we got it as a string. */ - if (encoding == OBJ_ENCODING_HT && string2ll(str, len, (long long *)&intobj)) { + if (encoding == OBJ_ENCODING_HASHSET && string2ll(str, len, (long long *)&intobj)) { if (dstset->encoding == OBJ_ENCODING_LISTPACK || dstset->encoding == OBJ_ENCODING_INTSET) { /* Adding it as an integer is more efficient. */ str = NULL; @@ -1365,7 +1359,7 @@ void sinterGenericCommand(client *c, only_integers = 0; } } - setTypeAddAux(dstset, str, len, intobj, encoding == OBJ_ENCODING_HT); + setTypeAddAux(dstset, str, len, intobj, encoding == OBJ_ENCODING_HASHSET); } } } @@ -1467,7 +1461,7 @@ void sunionDiffGenericCommand(client *c, robj **setkeys, int setnum, robj *dstke /* For a SET's encoding, according to the factory method setTypeCreate(), currently have 3 types: * 1. OBJ_ENCODING_INTSET * 2. OBJ_ENCODING_LISTPACK - * 3. OBJ_ENCODING_HT + * 3. OBJ_ENCODING_HASHSET * 'dstset_encoding' is used to determine which kind of encoding to use when initialize 'dstset'. * * If all sets are all OBJ_ENCODING_INTSET encoding or 'dstkey' is not null, keep 'dstset' @@ -1478,8 +1472,8 @@ void sunionDiffGenericCommand(client *c, robj **setkeys, int setnum, robj *dstke * the hashtable is more efficient when find and compare than the listpack. The corresponding * time complexity are O(1) vs O(n). */ if (!dstkey && dstset_encoding == OBJ_ENCODING_INTSET && - (setobj->encoding == OBJ_ENCODING_LISTPACK || setobj->encoding == OBJ_ENCODING_HT)) { - dstset_encoding = OBJ_ENCODING_HT; + (setobj->encoding == OBJ_ENCODING_LISTPACK || setobj->encoding == OBJ_ENCODING_HASHSET)) { + dstset_encoding = OBJ_ENCODING_HASHSET; } sets[j] = setobj; if (j > 0 && sets[0] == sets[j]) { @@ -1536,7 +1530,7 @@ void sunionDiffGenericCommand(client *c, robj **setkeys, int setnum, robj *dstke si = setTypeInitIterator(sets[j]); while ((encoding = setTypeNext(si, &str, &len, &llval)) != -1) { - cardinality += setTypeAddAux(dstset, str, len, llval, encoding == OBJ_ENCODING_HT); + cardinality += setTypeAddAux(dstset, str, len, llval, encoding == OBJ_ENCODING_HASHSET); } setTypeReleaseIterator(si); } @@ -1556,11 +1550,11 @@ void sunionDiffGenericCommand(client *c, robj **setkeys, int setnum, robj *dstke for (j = 1; j < setnum; j++) { if (!sets[j]) continue; /* no key is an empty set. */ if (sets[j] == sets[0]) break; /* same set! */ - if (setTypeIsMemberAux(sets[j], str, len, llval, encoding == OBJ_ENCODING_HT)) break; + if (setTypeIsMemberAux(sets[j], str, len, llval, encoding == OBJ_ENCODING_HASHSET)) break; } if (j == setnum) { /* There is no other set with this element. Add it. */ - cardinality += setTypeAddAux(dstset, str, len, llval, encoding == OBJ_ENCODING_HT); + cardinality += setTypeAddAux(dstset, str, len, llval, encoding == OBJ_ENCODING_HASHSET); } } setTypeReleaseIterator(si); @@ -1578,9 +1572,9 @@ void sunionDiffGenericCommand(client *c, robj **setkeys, int setnum, robj *dstke si = setTypeInitIterator(sets[j]); while ((encoding = setTypeNext(si, &str, &len, &llval)) != -1) { if (j == 0) { - cardinality += setTypeAddAux(dstset, str, len, llval, encoding == OBJ_ENCODING_HT); + cardinality += setTypeAddAux(dstset, str, len, llval, encoding == OBJ_ENCODING_HASHSET); } else { - cardinality -= setTypeRemoveAux(dstset, str, len, llval, encoding == OBJ_ENCODING_HT); + cardinality -= setTypeRemoveAux(dstset, str, len, llval, encoding == OBJ_ENCODING_HASHSET); } } setTypeReleaseIterator(si); diff --git a/src/t_zset.c b/src/t_zset.c index 069ab0924a..ea63641c80 100644 --- a/src/t_zset.c +++ b/src/t_zset.c @@ -2007,9 +2007,7 @@ typedef struct { int ii; } is; struct { - dict *dict; - dictIterator *di; - dictEntry *de; + hashsetIterator *iter; } ht; struct { unsigned char *lp; @@ -2064,10 +2062,8 @@ void zuiInitIterator(zsetopsrc *op) { if (op->encoding == OBJ_ENCODING_INTSET) { it->is.is = op->subject->ptr; it->is.ii = 0; - } else if (op->encoding == OBJ_ENCODING_HT) { - it->ht.dict = op->subject->ptr; - it->ht.di = dictGetIterator(op->subject->ptr); - it->ht.de = dictNext(it->ht.di); + } else if (op->encoding == OBJ_ENCODING_HASHSET) { + it->ht.iter = hashsetCreateIterator(op->subject->ptr); } else if (op->encoding == OBJ_ENCODING_LISTPACK) { it->lp.lp = op->subject->ptr; it->lp.p = lpFirst(it->lp.lp); @@ -2104,8 +2100,8 @@ void zuiClearIterator(zsetopsrc *op) { iterset *it = &op->iter.set; if (op->encoding == OBJ_ENCODING_INTSET) { UNUSED(it); /* skip */ - } else if (op->encoding == OBJ_ENCODING_HT) { - dictReleaseIterator(it->ht.di); + } else if (op->encoding == OBJ_ENCODING_HASHSET) { + hashsetReleaseIterator(it->ht.iter); } else if (op->encoding == OBJ_ENCODING_LISTPACK) { UNUSED(it); } else { @@ -2173,13 +2169,9 @@ int zuiNext(zsetopsrc *op, zsetopval *val) { /* Move to next element. */ it->is.ii++; - } else if (op->encoding == OBJ_ENCODING_HT) { - if (it->ht.de == NULL) return 0; - val->ele = dictGetKey(it->ht.de); + } else if (op->encoding == OBJ_ENCODING_HASHSET) { + if (!hashsetNext(it->ht.iter, (void **)&(val->ele))) return 0; val->score = 1.0; - - /* Move to next element. */ - it->ht.de = dictNext(it->ht.di); } else if (op->encoding == OBJ_ENCODING_LISTPACK) { if (it->lp.p == NULL) return 0; val->estr = lpGetValue(it->lp.p, &val->elen, &val->ell); diff --git a/tests/unit/type/set.tcl b/tests/unit/type/set.tcl index 944c3d3d98..1871ec9b4d 100644 --- a/tests/unit/type/set.tcl +++ b/tests/unit/type/set.tcl @@ -33,6 +33,7 @@ start_server { assert_equal {0 1} [r smismember myset bla foo] assert_equal {0} [r smismember myset bla] assert_equal "bar $initelems($type)" [lsort [r smembers myset]] + r memory usage myset } } @@ -51,6 +52,7 @@ start_server { assert_equal {0 1} [r smismember myset 18 16] assert_equal {0} [r smismember myset 18] assert_equal {16 17} [lsort [r smembers myset]] + r memory usage myset } test {SMISMEMBER SMEMBERS SCARD against non set} { @@ -1029,111 +1031,6 @@ foreach type {single multiple single_multiple} { r srem $myset {*}$members } - proc verify_rehashing_completed_key {myset table_size keys} { - set htstats [r debug HTSTATS-KEY $myset] - assert {![string match {*rehashing target*} $htstats]} - return {[string match {*table size: $table_size*number of elements: $keys*} $htstats]} - } - - test "SRANDMEMBER with a dict containing long chain" { - set origin_save [config_get_set save ""] - set origin_max_lp [config_get_set set-max-listpack-entries 0] - set origin_save_delay [config_get_set rdb-key-save-delay 2147483647] - - # 1) Create a hash set with 100000 members. - set members {} - for {set i 0} {$i < 100000} {incr i} { - lappend members [format "m:%d" $i] - } - create_set myset $members - - # 2) Wait for the hash set rehashing to finish. - while {[is_rehashing myset]} { - r srandmember myset 100 - } - - # 3) Turn off the rehashing of this set, and remove the members to 500. - r bgsave - rem_hash_set_top_N myset [expr {[r scard myset] - 500}] - assert_equal [r scard myset] 500 - - # 4) Kill RDB child process to restart rehashing. - set pid1 [get_child_pid 0] - catch {exec kill -9 $pid1} - waitForBgsave r - - # 5) Let the set hash to start rehashing - r spop myset 1 - assert [is_rehashing myset] - - # 6) Verify that when rdb saving is in progress, rehashing will still be performed (because - # the ratio is extreme) by waiting for it to finish during an active bgsave. - r bgsave - - while {[is_rehashing myset]} { - r srandmember myset 1 - } - if {$::verbose} { - puts [r debug HTSTATS-KEY myset full] - } - - set pid1 [get_child_pid 0] - catch {exec kill -9 $pid1} - waitForBgsave r - - # 7) Check that eventually, SRANDMEMBER returns all elements. - array set allmyset {} - foreach ele [r smembers myset] { - set allmyset($ele) 1 - } - unset -nocomplain auxset - set iterations 1000 - while {$iterations != 0} { - incr iterations -1 - set res [r srandmember myset -10] - foreach ele $res { - set auxset($ele) 1 - } - if {[lsort [array names allmyset]] eq - [lsort [array names auxset]]} { - break; - } - } - assert {$iterations != 0} - - # 8) Remove the members to 30 in order to calculate the value of Chi-Square Distribution, - # otherwise we would need more iterations. - rem_hash_set_top_N myset [expr {[r scard myset] - 30}] - assert_equal [r scard myset] 30 - - # Hash set rehashing would be completed while removing members from the `myset` - # We also check the size and members in the hash table. - verify_rehashing_completed_key myset 64 30 - - # Now that we have a hash set with only one long chain bucket. - set htstats [r debug HTSTATS-KEY myset full] - assert {[regexp {different slots: ([0-9]+)} $htstats - different_slots]} - assert {[regexp {max chain length: ([0-9]+)} $htstats - max_chain_length]} - assert {$different_slots == 1 && $max_chain_length == 30} - - # 9) Use positive count (PATH 4) to get 10 elements (out of 30) each time. - unset -nocomplain allkey - set iterations 1000 - while {$iterations != 0} { - incr iterations -1 - set res [r srandmember myset 10] - foreach ele $res { - lappend allkey $ele - } - } - # validate even distribution of random sampling (df = 29, 73 means 0.00001 probability) - assert_lessthan [chi_square_value $allkey] 73 - - r config set save $origin_save - r config set set-max-listpack-entries $origin_max_lp - r config set rdb-key-save-delay $origin_save_delay - } {OK} {needs:debug slow} - proc setup_move {} { r del myset3{t} myset4{t} create_set myset1{t} {1 a b}