Add comments, minor fixes

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

src/defrag.c

@@ -480,6 +480,7 @@ static void scanLaterSet(robj *ob, unsigned long *cursor) {
     *cursor = hashtableScanDefrag(ht, *cursor, activeDefragSdsHashtableCallback, NULL, activeDefragAlloc, HASHTABLE_SCAN_EMIT_REF);
 }
 
+/* Hashtable scan callback for hash datatype */
 static void activeDefragHashTypeEntry(void *privdata, void *element_ref) {
     UNUSED(privdata);
     hashTypeEntry **entry_ref = (hashTypeEntry **)element_ref;

src/t_hash.c

@@ -30,14 +30,18 @@
 #include "server.h"
 #include <math.h>
 
+/*-----------------------------------------------------------------------------
+ * Hash Entry API
+ *----------------------------------------------------------------------------*/
+
 struct hashTypeEntry {
     sds value;
     unsigned char field_offset;
     unsigned char field_data[];
 };
 
 /* takes ownership of value, does not take ownership of field */
-hashTypeEntry *hashTypeCreateEntry(const sds field, sds value) {
+hashTypeEntry *hashTypeCreateEntry(sds field, sds value) {
     size_t field_size = sdscopytobuffer(NULL, 0, field, NULL);
 
     size_t total_size = sizeof(hashTypeEntry) + field_size;
@@ -63,22 +67,33 @@ static void hashTypeEntryReplaceValue(hashTypeEntry *entry, sds value) {
     entry->value = value;
 }
 
+/* Returns allocation size of hashTypeEntry and data owned by hashTypeEntry,
+ * even if not embedded in the same allocation. */
 size_t hashTypeEntryAllocSize(hashTypeEntry *entry) {
     size_t size = zmalloc_usable_size(entry);
     size += sdsAllocSize(entry->value);
     return size;
 }
 
+/* Defragments a hashtable entry (field-value pair) if needed, using the
+ * provided defrag functions. The defrag functions return NULL if the allocation
+ * was not moved, otherwise they return a pointer to the new memory location.
+ * A separate sds defrag function is needed because of the unique memory layout
+ * of sds strings.
+ * If the location of the hashTypeEntry changed we return the new location,
+ * otherwise we return NULL. */
 hashTypeEntry *hashTypeEntryDefrag(hashTypeEntry *entry, void *(*defragfn)(void *), sds (*sdsdefragfn)(sds)) {
     hashTypeEntry *new_entry = defragfn(entry);
     if (new_entry) entry = new_entry;
 
     sds new_value = sdsdefragfn(entry->value);
     if (new_value) entry->value = new_value;
 
-    return entry;
+    return new_entry;
 }
 
+/* Used for releasing memory to OS to avoid unnecessary CoW. Called when we've
+ * forked and memory won't be used again. See zmadvise_dontneed() */
 void dismissHashTypeEntry(hashTypeEntry *entry) {
     /* Only dismiss values memory since the field size usually is small. */
     dismissSds(entry->value);
@@ -603,8 +618,7 @@ static void hashTypeRandomElement(robj *hashobj, unsigned long hashsize, listpac
         field->sval = (unsigned char *)sds_field;
         field->slen = sdslen(sds_field);
         if (val) {
-            hashTypeEntry *hash_entry = entry;
-            sds sds_val = hashTypeEntryGetValue(hash_entry);
+            sds sds_val = hashTypeEntryGetValue(entry);
             val->sval = (unsigned char *)sds_val;
             val->slen = sdslen(sds_val);
         }