switch hashtable type only when object has volatile items

Signed-off-by: Ran Shidlansik <ranshid@amazon.com>

Author: ranshid

src/hashtable.c

@@ -1139,6 +1139,13 @@ hashtableType *hashtableGetType(hashtable *ht) {
     return ht->type;
 }
 
+/* Set the hashtable type and returns the old type of the hashtable. */
+hashtableType *hashtableSetType(hashtable *ht, hashtableType *type) {
+    hashtableType *oldtype = ht->type;
+    ht->type = type;
+    return oldtype;
+}
+
 /* Returns a pointer to the table's metadata (userdata) section. */
 void *hashtableMetadata(hashtable *ht) {
     return &ht->metadata;

src/hashtable.h

@@ -122,6 +122,7 @@ hashtable *hashtableCreate(hashtableType *type);
 void hashtableRelease(hashtable *ht);
 void hashtableEmpty(hashtable *ht, void(callback)(hashtable *));
 hashtableType *hashtableGetType(hashtable *ht);
+hashtableType *hashtableSetType(hashtable *ht, hashtableType *type);
 void *hashtableMetadata(hashtable *ht);
 size_t hashtableSize(const hashtable *ht);
 size_t hashtableBuckets(hashtable *ht);

src/server.c

@@ -688,6 +688,15 @@ hashtableType hashHashtableType = {
     .accessElement = hashHashtableTypeAccess,
 };
 
+hashtableType hashWithVolatileItemsHashtableType = {
+    .hashFunction = dictSdsHash,
+    .entryGetKey = hashHashtableTypeGetKey,
+    .keyCompare = hashtableSdsKeyCompare,
+    .entryDestructor = hashHashtableTypeDestructor,
+    .getMetadataSize = hashHashtableTypeMetadataSize,
+    .accessElement = hashHashtableTypeAccess,
+};
+
 /* Hashtable type without destructor */
 hashtableType sdsReplyHashtableType = {
     .hashFunction = dictSdsCaseHash,

src/server.h

@@ -2632,6 +2632,7 @@ extern hashtableType kvstoreKeysHashtableType;
 extern hashtableType kvstoreExpiresHashtableType;
 extern double R_Zero, R_PosInf, R_NegInf, R_Nan;
 extern hashtableType hashHashtableType;
+extern hashtableType hashWithVolatileItemsHashtableType;
 extern dictType stringSetDictType;
 extern dictType externalStringType;
 extern dictType sdsHashDictType;

src/t_hash.c

@@ -405,6 +405,8 @@ hashTypeGetOrcreateVolatileSet(robj *o) {
 void hashTypeTrackEntry(robj *o, void *entry) {
     volatile_set *set = hashTypeGetOrcreateVolatileSet(o);
     serverAssert(volatileSetAddEntry(set, entry, hashTypeEntryGetExpiry(entry)));
+    /* serves mainly for optimization. Use type which supports access function only when needed. */
+    hashtableSetType(o->ptr, &hashWithVolatileItemsHashtableType);
 }
 
 void hashTypeUntrackEntry(robj *o, void *entry) {
@@ -416,6 +418,8 @@ void hashTypeUntrackEntry(robj *o, void *entry) {
         freeVolatileSet(set);
         volatile_set **volatile_set_ref = hashtableMetadata(o->ptr);
         *volatile_set_ref = NULL;
+        /* serves mainly for optimization. by changing the hashtable type we can avoid extra function call in hashtable access */
+        hashtableSetType(o->ptr, &hashHashtableType);
     }
 }
 
@@ -1448,14 +1452,12 @@ void hsetexCommand(client *c) {
             break;
         }
     }
-    /* In case missing fields argument or bad number of fields provided, bail with syntax error */
-    if (num_fields <= 0) {
+    /* Check that the parsed fields number matches the real provided number of fields */
+    if (num_fields != (c->argc - fields_index) / 2) {
         addReplyErrorObject(c, shared.syntaxerr);
         return;
     }
 
-    if (num_fields > (c->argc - fields_index) / 2) num_fields = (c->argc - fields_index) / 2; // Potential user error, but we would like to make effort to comply with the request.
-
     o = lookupKeyWrite(c->db, c->argv[1]);
     if (checkType(c, o, OBJ_HASH))
         return;
@@ -1555,8 +1557,8 @@ void hgetexCommand(client *c) {
         }
     }
 
-    /* In case missing fields argument or bad number of fields provided, bail with syntax error */
-    if (num_fields <= 0) {
+    /* Check that the parsed fields number matches the real provided number of fields */
+    if (num_fields != (c->argc - fields_index)) {
         addReplyErrorObject(c, shared.syntaxerr);
         return;
     }
@@ -1749,8 +1751,8 @@ void hexpireGenericCommand(client *c, long long basetime, int unit) {
         }
     }
 
-    /* In case missing fields argument or bad number of fields provided, bail with syntax error */
-    if (num_fields <= 0) {
+    /* Check that the parsed fields number matches the real provided number of fields */
+    if (num_fields != (c->argc - fields_index)) {
         addReplyErrorObject(c, shared.syntaxerr);
         return;
     }