make hashTypeEntry opaque and consistent naming: hash keys contain fi…

…eld/value pairs

Signed-off-by: Rain Valentine <[email protected]>

src/aof.c

@@ -1921,7 +1921,7 @@ int rewriteSortedSetObject(rio *r, robj *key, robj *o) {
 /* Write either the key or the value of the currently selected item of a hash.
  * The 'hi' argument passes a valid hash iterator.
  * The 'what' filed specifies if to write a key or a value and can be
- * either OBJ_HASH_KEY or OBJ_HASH_VALUE.
+ * either OBJ_HASH_FIELD or OBJ_HASH_VALUE.
  *
  * The function returns 0 on error, non-zero on success. */
 static int rioWriteHashIteratorCursor(rio *r, hashTypeIterator *hi, int what) {
@@ -1962,7 +1962,7 @@ int rewriteHashObject(rio *r, robj *key, robj *o) {
             }
         }
 
-        if (!rioWriteHashIteratorCursor(r, &hi, OBJ_HASH_KEY) || !rioWriteHashIteratorCursor(r, &hi, OBJ_HASH_VALUE)) {
+        if (!rioWriteHashIteratorCursor(r, &hi, OBJ_HASH_FIELD) || !rioWriteHashIteratorCursor(r, &hi, OBJ_HASH_VALUE)) {
             hashTypeResetIterator(&hi);
             return 0;
         }

src/db.c

@@ -1000,10 +1000,9 @@ void hashtableScanCallback(void *privdata, void *entry) {
         key = node->ele;
         /* zset data is copied after filtering by key */
     } else if (o->type == OBJ_HASH) {
-        key = (sds)hashTypeEntryGetKey(entry);
+        key = hashTypeEntryGetField(entry);
         if (!data->only_keys) {
-            hashTypeEntry *hash_entry = entry;
-            val = hash_entry->value;
+            val = hashTypeEntryGetValue(entry);
         }
     } else {
         serverPanic("Type not handled in hashtable SCAN callback.");

src/debug.c

@@ -231,7 +231,7 @@ void xorObjectDigest(serverDb *db, robj *keyobj, unsigned char *digest, robj *o)
             sds sdsele;
 
             memset(eledigest, 0, 20);
-            sdsele = hashTypeCurrentObjectNewSds(&hi, OBJ_HASH_KEY);
+            sdsele = hashTypeCurrentObjectNewSds(&hi, OBJ_HASH_FIELD);
             mixDigest(eledigest, sdsele, sdslen(sdsele));
             sdsfree(sdsele);
             sdsele = hashTypeCurrentObjectNewSds(&hi, OBJ_HASH_VALUE);

src/defrag.c

@@ -480,23 +480,18 @@ static void scanLaterSet(robj *ob, unsigned long *cursor) {
     *cursor = hashtableScanDefrag(ht, *cursor, activeDefragSdsHashtableCallback, NULL, activeDefragAlloc, HASHTABLE_SCAN_EMIT_REF);
 }
 
-static void activeDefragHashField(void *privdata, void *element_ref) {
+static void activeDefragHashTypeEntry(void *privdata, void *element_ref) {
     UNUSED(privdata);
     hashTypeEntry **entry_ref = (hashTypeEntry **)element_ref;
 
-    /* defragment field */
-    hashTypeEntry *new_entry = activeDefragAlloc(*entry_ref);
+    hashTypeEntry *new_entry = hashTypeEntryDefrag(*entry_ref, activeDefragAlloc, activeDefragSds);
     if (new_entry) *entry_ref = new_entry;
-
-    /* defragment value */
-    sds new_value = activeDefragSds((*entry_ref)->value);
-    if (new_value) (*entry_ref)->value = new_value;
 }
 
 static void scanLaterHash(robj *ob, unsigned long *cursor) {
     if (ob->type != OBJ_HASH || ob->encoding != OBJ_ENCODING_HASHTABLE) return;
     hashtable *ht = ob->ptr;
-    *cursor = hashtableScanDefrag(ht, *cursor, activeDefragHashField, NULL, activeDefragAlloc, HASHTABLE_SCAN_EMIT_REF);
+    *cursor = hashtableScanDefrag(ht, *cursor, activeDefragHashTypeEntry, NULL, activeDefragAlloc, HASHTABLE_SCAN_EMIT_REF);
 }
 
 static void defragQuicklist(robj *ob) {
@@ -541,7 +536,7 @@ static void defragHash(robj *ob) {
     } else {
         unsigned long cursor = 0;
         do {
-            cursor = hashtableScanDefrag(ht, cursor, activeDefragHashField, NULL, activeDefragAlloc, HASHTABLE_SCAN_EMIT_REF);
+            cursor = hashtableScanDefrag(ht, cursor, activeDefragHashTypeEntry, NULL, activeDefragAlloc, HASHTABLE_SCAN_EMIT_REF);
         } while (cursor != 0);
     }
     /* defrag the hashtable struct and tables */

src/module.c

@@ -11088,9 +11088,8 @@ static void moduleScanKeyHashtableCallback(void *privdata, void *entry) {
         key = node->ele;
         value = createStringObjectFromLongDouble(node->score, 0);
     } else if (o->type == OBJ_HASH) {
-        key = (sds)hashTypeEntryGetKey(entry);
-        hashTypeEntry *hash_entry = entry;
-        sds val = hash_entry->value;
+        key = hashTypeEntryGetField(entry);
+        sds val = hashTypeEntryGetValue(entry);
         value = createStringObject(val, sdslen(val));
     } else {
         serverPanic("unexpected object type");

src/object.c

@@ -685,11 +685,7 @@ void dismissHashObject(robj *o, size_t size_hint) {
             hashtableInitIterator(&iter, ht);
             void *next;
             while (hashtableNext(&iter, &next)) {
-                /* Only dismiss values memory since the field size
-                 * usually is small. */
-                hashTypeEntry *entry = next;
-                UNUSED(entry);
-                dismissSds(entry->value);
+                dismissHashTypeEntry(next);
             }
             hashtableResetIterator(&iter);
         }
@@ -1206,9 +1202,7 @@ size_t objectComputeSize(robj *key, robj *o, size_t sample_size, int dbid) {
 
             asize = sizeof(*o) + hashtableMemUsage(ht);
             while (hashtableNext(&iter, &next) && samples < sample_size) {
-                elesize += zmalloc_usable_size(next);
-                hashTypeEntry *entry = next;
-                elesize += sdsAllocSize(entry->value);
+                elesize += hashTypeEntryAllocSize(next);
                 samples++;
             }
             hashtableResetIterator(&iter);

src/rdb.c

@@ -962,8 +962,8 @@ ssize_t rdbSaveObject(rio *rdb, robj *o, robj *key, int dbid) {
             hashtableInitIterator(&iter, ht);
             void *next;
             while (hashtableNext(&iter, &next)) {
-                sds field = (sds)hashTypeEntryGetKey(next);
-                sds value = ((hashTypeEntry *)next)->value;
+                sds field = hashTypeEntryGetField(next);
+                sds value = hashTypeEntryGetValue(next);
 
                 if ((n = rdbSaveRawString(rdb, (unsigned char *)field, sdslen(field))) == -1) {
                     hashtableResetIterator(&iter);
@@ -2168,7 +2168,7 @@ robj *rdbLoadObject(int rdbtype, rio *rdb, sds key, int dbid, int *error) {
             }
 
             /* Add pair to hash table */
-            hashTypeEntry *entry = hashTypeCreateEntry(field, value); // TODO rainval avoid extra allocation of field?
+            hashTypeEntry *entry = hashTypeCreateEntry(field, value);
             sdsfree(field);
             if (!hashtableAdd((hashtable *)o->ptr, entry)) {
                 rdbReportCorruptRDB("Duplicate hash fields detected");

src/server.c

@@ -624,37 +624,22 @@ hashtableType subcommandSetType = {.entryGetKey = hashtableSubcommandGetKey,
                                    .keyCompare = hashtableStringKeyCaseCompare,
                                    .instant_rehashing = 1};
 
-/* takes ownership of value, does not take ownership of field */
-hashTypeEntry *hashTypeCreateEntry(const sds field, sds value) {
-    size_t field_size = sdsAllocSize(field);
-    void *field_data = sdsAllocPtr(field);
-
-    size_t total_size = sizeof(hashTypeEntry) + field_size;
-    hashTypeEntry *hf = zmalloc(total_size);
-
-    hf->value = value;
-    hf->field_offset = field - (char *)field_data;
-    memcpy(hf->field_data, field_data, field_size);
-    return hf;
-}
-
-const void *hashTypeEntryGetKey(const void *entry) {
-    const hashTypeEntry *hf = entry;
-    return hf->field_data + hf->field_offset;
+/* Hash type hash table (note that small hashes are represented with listpacks) */
+const void *hashHashtableTypeGetKey(const void *entry) {
+    const hashTypeEntry *hash_entry = entry;
+    return (const void *)hashTypeEntryGetField(hash_entry);
 }
 
-void freeHashTypeEntry(void *entry) {
-    hashTypeEntry *hf = entry;
-    sdsfree(hf->value);
-    zfree(hf);
+void hashHashtableTypeDestructor(void *entry) {
+    hashTypeEntry *hash_entry = entry;
+    freeHashTypeEntry(hash_entry);
 }
 
-/* Hash type hash table (note that small hashes are represented with listpacks) */
 hashtableType hashHashtableType = {
     .hashFunction = dictSdsHash,
-    .entryGetKey = hashTypeEntryGetKey,
+    .entryGetKey = hashHashtableTypeGetKey,
     .keyCompare = hashtableSdsKeyCompare,
-    .entryDestructor = freeHashTypeEntry,
+    .entryDestructor = hashHashtableTypeDestructor,
 };
 
 /* Hash type hash table (note that small hashes are represented with listpacks) */

src/server.h

@@ -2511,7 +2511,7 @@ typedef struct {
 
 #include "stream.h" /* Stream data type header file. */
 
-#define OBJ_HASH_KEY 1
+#define OBJ_HASH_FIELD 1
 #define OBJ_HASH_VALUE 2
 
 /*-----------------------------------------------------------------------------
@@ -3217,14 +3217,14 @@ robj *setTypeDup(robj *o);
 #define HASH_SET_TAKE_VALUE (1 << 1)
 #define HASH_SET_COPY 0
 
-typedef struct {
-    sds value;
-    unsigned char field_offset;
-    char field_data[];
-} hashTypeEntry;
+typedef struct hashTypeEntry hashTypeEntry;
 hashTypeEntry *hashTypeCreateEntry(sds field, sds value);
-const void *hashTypeEntryGetKey(const void *entry);
-void freeHashTypeEntry(void *entry);
+sds hashTypeEntryGetField(const hashTypeEntry *entry);
+sds hashTypeEntryGetValue(const hashTypeEntry *entry);
+size_t hashTypeEntryAllocSize(hashTypeEntry *entry);
+hashTypeEntry *hashTypeEntryDefrag(hashTypeEntry *entry, void *(*defragfn)(void *), sds (*sdsdefragfn)(sds));
+void dismissHashTypeEntry(hashTypeEntry *entry);
+void freeHashTypeEntry(hashTypeEntry *entry);
 
 void hashTypeConvert(robj *o, int enc);
 void hashTypeTryConversion(robj *subject, robj **argv, int start, int end);