Remove internal server object pointer overhead in small strings (valkey-io#2516)

Achieved 20-30% reduction in overhead.

This was accomplished by reusing the 8B robj->ptr memory to store
embedded data. For 16B key and 16B value, this reduces the per-item
memory overhead by ~20%. Overall performance was not measurably changed.
Raising the threshold for embedding strings produces ~30% reduction in
per-item overhead for affected value sizes.

Here's what I did:
1. Modify all `robj->ptr` access to use get/set methods. Most of this
was done with a script (code listed below), with a few manual touches.
Manual and programmatic changes are in separate commits to make review
easier.
2. Next I changed `objectGetVal()` to calculate the location instead of
using `robj->ptr`, and modified the embedding code to start writing
embedded data 8B earlier, overwriting the ptr location. I changed
`objectSetVal()` to assert that no value is embedded - all of the code
that assigned `o->ptr` would have been incorrect for embedded strings
anyway. (Except for one instance in module.c which I made a separate
method for.)

---------

Signed-off-by: Rain Valentine <rsg000@gmail.com>

Author: rainsupreme

src/acl.c

@@ -1516,7 +1516,7 @@ void ACLInit(void) {
  *  ENOENT: if the specified user does not exist at all.
  */
 int ACLCheckUserCredentials(robj *username, robj *password) {
-    user *u = ACLGetUserByName(username->ptr, sdslen(username->ptr));
+    user *u = ACLGetUserByName(objectGetVal(username), sdslen(objectGetVal(username)));
     if (u == NULL) {
         errno = ENOENT;
         return C_ERR;
@@ -1536,7 +1536,7 @@ int ACLCheckUserCredentials(robj *username, robj *password) {
     listIter li;
     listNode *ln;
     listRewind(u->passwords, &li);
-    sds hashed = ACLHashPassword(password->ptr, sdslen(password->ptr));
+    sds hashed = ACLHashPassword(objectGetVal(password), sdslen(objectGetVal(password)));
     while ((ln = listNext(&li))) {
         sds thispass = listNodeValue(ln);
         if (!time_independent_strcmp(hashed, thispass, HASH_PASSWORD_LEN)) {
@@ -1564,7 +1564,7 @@ void addAuthErrReply(client *c, robj *err) {
         addReplyError(c, "-WRONGPASS invalid username-password pair or user is disabled.");
         return;
     }
-    addReplyError(c, err->ptr);
+    addReplyError(c, objectGetVal(err));
 }
 
 /* This is like ACLCheckUserCredentials(), however if the user/pass
@@ -1576,18 +1576,18 @@ static int checkPasswordBasedAuth(client *c, robj *username, robj *password) {
     int result;
 
     if (ACLCheckUserCredentials(username, password) == C_OK) {
-        user *user = ACLGetUserByName(username->ptr, sdslen(username->ptr));
+        user *user = ACLGetUserByName(objectGetVal(username), sdslen(objectGetVal(username)));
         clientSetUser(c, user, 1);
         moduleNotifyUserChanged(c);
         result = AUTH_OK;
     } else {
-        addACLLogEntry(c, ACL_DENIED_AUTH, (c->flag.multi) ? ACL_LOG_CTX_MULTI : ACL_LOG_CTX_TOPLEVEL, 0, username->ptr,
+        addACLLogEntry(c, ACL_DENIED_AUTH, (c->flag.multi) ? ACL_LOG_CTX_MULTI : ACL_LOG_CTX_TOPLEVEL, 0, objectGetVal(username),
                        NULL);
         result = AUTH_ERR;
     }
 
     moduleFireAuthenticationEvent(c->id,
-                                  username->ptr,
+                                  objectGetVal(username),
                                   NULL,
                                   result == AUTH_OK);
 
@@ -1861,7 +1861,7 @@ static int ACLSelectorCheckCmd(aclSelector *selector,
             while (1) {
                 if (selector->allowed_firstargs[id][subid] == NULL) return ACL_DENIED_CMD;
                 int idx = cmd->parent ? 2 : 1;
-                if (!strcasecmp(argv[idx]->ptr, selector->allowed_firstargs[id][subid]))
+                if (!strcasecmp(objectGetVal(argv[idx]), selector->allowed_firstargs[id][subid]))
                     break; /* First argument match found. Stop here. */
                 subid++;
             }
@@ -1880,7 +1880,7 @@ static int ACLSelectorCheckCmd(aclSelector *selector,
         keyReference *resultidx = result->keys;
         for (int j = 0; j < result->numkeys; j++) {
             int idx = resultidx[j].pos;
-            ret = ACLSelectorCheckKey(selector, argv[idx]->ptr, sdslen(argv[idx]->ptr), resultidx[j].flags, false);
+            ret = ACLSelectorCheckKey(selector, objectGetVal(argv[idx]), sdslen(objectGetVal(argv[idx])), resultidx[j].flags, false);
             if (ret != ACL_OK) {
                 if (keyidxptr) *keyidxptr = resultidx[j].pos;
                 return ret;
@@ -1901,7 +1901,7 @@ static int ACLSelectorCheckCmd(aclSelector *selector,
             if (!(channelref[j].flags & channel_flags)) continue;
             int is_pattern = channelref[j].flags & CMD_CHANNEL_PATTERN;
             int ret =
-                ACLCheckChannelAgainstList(selector->channels, argv[idx]->ptr, sdslen(argv[idx]->ptr), is_pattern);
+                ACLCheckChannelAgainstList(selector->channels, objectGetVal(argv[idx]), sdslen(objectGetVal(argv[idx])), is_pattern);
             if (ret != ACL_OK) {
                 if (keyidxptr) *keyidxptr = channelref[j].pos;
                 getKeysFreeResult(&channels);
@@ -2132,7 +2132,7 @@ static int ACLShouldKillPubsubClient(client *c, list *upcoming) {
         void *next;
         while (!kill && hashtableNext(&iter, &next)) {
             robj *o = next;
-            int res = ACLCheckChannelAgainstList(upcoming, o->ptr, sdslen(o->ptr), 1);
+            int res = ACLCheckChannelAgainstList(upcoming, objectGetVal(o), sdslen(objectGetVal(o)), 1);
             kill = (res == ACL_DENIED_CHANNEL);
         }
         hashtableCleanupIterator(&iter);
@@ -2145,7 +2145,7 @@ static int ACLShouldKillPubsubClient(client *c, list *upcoming) {
             void *next;
             while (!kill && hashtableNext(&iter, &next)) {
                 robj *o = next;
-                int res = ACLCheckChannelAgainstList(upcoming, o->ptr, sdslen(o->ptr), 0);
+                int res = ACLCheckChannelAgainstList(upcoming, objectGetVal(o), sdslen(objectGetVal(o)), 0);
                 kill = (res == ACL_DENIED_CHANNEL);
             }
             hashtableCleanupIterator(&iter);
@@ -2157,7 +2157,7 @@ static int ACLShouldKillPubsubClient(client *c, list *upcoming) {
             void *next;
             while (!kill && hashtableNext(&iter, &next)) {
                 robj *o = next;
-                int res = ACLCheckChannelAgainstList(upcoming, o->ptr, sdslen(o->ptr), 0);
+                int res = ACLCheckChannelAgainstList(upcoming, objectGetVal(o), sdslen(objectGetVal(o)), 0);
                 kill = (res == ACL_DENIED_CHANNEL);
             }
             hashtableCleanupIterator(&iter);
@@ -2659,7 +2659,7 @@ static int ACLSaveToFile(const char *filename) {
         user = sdscatsds(user, u->name);
         user = sdscatlen(user, " ", 1);
         robj *descr = ACLDescribeUser(u);
-        user = sdscatsds(user, descr->ptr);
+        user = sdscatsds(user, objectGetVal(descr));
         decrRefCount(descr);
         acl = sdscatsds(acl, user);
         acl = sdscatlen(acl, "\n", 1);
@@ -2855,10 +2855,10 @@ void addACLLogEntry(client *c, int reason, int context, int argpos, sds username
     } else {
         switch (reason) {
         case ACL_DENIED_CMD: le->object = sdsdup(c->cmd->fullname); break;
-        case ACL_DENIED_KEY: le->object = sdsdup(c->argv[argpos]->ptr); break;
-        case ACL_DENIED_CHANNEL: le->object = sdsdup(c->argv[argpos]->ptr); break;
-        case ACL_DENIED_DB: le->object = argpos ? sdsdup(c->argv[argpos]->ptr) : sdsdup(c->cmd->fullname); break;
-        case ACL_DENIED_AUTH: le->object = sdsdup(c->argv[0]->ptr); break;
+        case ACL_DENIED_KEY: le->object = sdsdup(objectGetVal(c->argv[argpos])); break;
+        case ACL_DENIED_CHANNEL: le->object = sdsdup(objectGetVal(c->argv[argpos])); break;
+        case ACL_DENIED_DB: le->object = argpos ? sdsdup(objectGetVal(c->argv[argpos])) : sdsdup(c->cmd->fullname); break;
+        case ACL_DENIED_AUTH: le->object = sdsdup(objectGetVal(c->argv[0])); break;
         default: le->object = sdsempty();
         }
     }
@@ -3061,15 +3061,15 @@ static int aclAddReplySelectorDescription(client *c, aclSelector *s) {
  * ACL LOG [<count> | RESET]
  */
 void aclCommand(client *c) {
-    char *sub = c->argv[1]->ptr;
+    char *sub = objectGetVal(c->argv[1]);
     if (!strcasecmp(sub, "setuser") && c->argc >= 3) {
         /* Initially redact all of the arguments to not leak any information
          * about the user. */
         for (int j = 2; j < c->argc; j++) {
             redactClientCommandArgument(c, j);
         }
 
-        sds username = c->argv[2]->ptr;
+        sds username = objectGetVal(c->argv[2]);
         /* Check username validity. */
         if (ACLStringHasSpaces(username, sdslen(username))) {
             addReplyError(c, "Usernames can't contain spaces or null characters");
@@ -3079,7 +3079,7 @@ void aclCommand(client *c) {
         user *u = ACLGetUserByName(username, sdslen(username));
 
         sds *temp_argv = zmalloc(c->argc * sizeof(sds));
-        for (int i = 3; i < c->argc; i++) temp_argv[i - 3] = c->argv[i]->ptr;
+        for (int i = 3; i < c->argc; i++) temp_argv[i - 3] = objectGetVal(c->argv[i]);
 
         sds error = ACLStringSetUser(u, username, temp_argv, c->argc - 3);
         zfree(temp_argv);
@@ -3096,15 +3096,15 @@ void aclCommand(client *c) {
 
         int deleted = 0;
         for (int j = 2; j < c->argc; j++) {
-            sds username = c->argv[j]->ptr;
+            sds username = objectGetVal(c->argv[j]);
             if (!strcmp(username, "default")) {
                 addReplyError(c, "The 'default' user cannot be removed");
                 return;
             }
         }
 
         for (int j = 2; j < c->argc; j++) {
-            sds username = c->argv[j]->ptr;
+            sds username = objectGetVal(c->argv[j]);
             user *u;
             if (raxRemove(Users, (unsigned char *)username, sdslen(username), (void **)&u)) {
                 ACLFreeUserAndKillClients(u);
@@ -3116,7 +3116,7 @@ void aclCommand(client *c) {
         /* Redact the username to not leak any information about the user. */
         redactClientCommandArgument(c, 2);
 
-        user *u = ACLGetUserByName(c->argv[2]->ptr, sdslen(c->argv[2]->ptr));
+        user *u = ACLGetUserByName(objectGetVal(c->argv[2]), sdslen(objectGetVal(c->argv[2])));
         if (u == NULL) {
             addReplyNull(c);
             return;
@@ -3177,7 +3177,7 @@ void aclCommand(client *c) {
                 config = sdscatsds(config, u->name);
                 config = sdscatlen(config, " ", 1);
                 robj *descr = ACLDescribeUser(u);
-                config = sdscatsds(config, descr->ptr);
+                config = sdscatsds(config, objectGetVal(descr));
                 decrRefCount(descr);
                 addReplyBulkSds(c, config);
             }
@@ -3216,9 +3216,9 @@ void aclCommand(client *c) {
         for (j = 0; ACLCommandCategories[j].flag != 0; j++) addReplyBulkCString(c, ACLCommandCategories[j].name);
         setDeferredArrayLen(c, dl, j);
     } else if (!strcasecmp(sub, "cat") && c->argc == 3) {
-        uint64_t cflag = ACLGetCommandCategoryFlagByName(c->argv[2]->ptr);
+        uint64_t cflag = ACLGetCommandCategoryFlagByName(objectGetVal(c->argv[2]));
         if (cflag == 0) {
-            addReplyErrorFormat(c, "Unknown category '%.128s'", (char *)c->argv[2]->ptr);
+            addReplyErrorFormat(c, "Unknown category '%.128s'", (char *)objectGetVal(c->argv[2]));
             return;
         }
         int arraylen = 0;
@@ -3251,7 +3251,7 @@ void aclCommand(client *c) {
          * the number of entries the user wants to display, or alternatively
          * the "RESET" command in order to flush the old entries. */
         if (c->argc == 3) {
-            if (!strcasecmp(c->argv[2]->ptr, "reset")) {
+            if (!strcasecmp(objectGetVal(c->argv[2]), "reset")) {
                 listSetFreeMethod(ACLLog, ACLFreeLogEntry);
                 listEmpty(ACLLog);
                 listSetFreeMethod(ACLLog, NULL);
@@ -3320,14 +3320,14 @@ void aclCommand(client *c) {
         }
     } else if (!strcasecmp(sub, "dryrun") && c->argc >= 4) {
         struct serverCommand *cmd;
-        user *u = ACLGetUserByName(c->argv[2]->ptr, sdslen(c->argv[2]->ptr));
+        user *u = ACLGetUserByName(objectGetVal(c->argv[2]), sdslen(objectGetVal(c->argv[2])));
         if (u == NULL) {
-            addReplyErrorFormat(c, "User '%s' not found", (char *)c->argv[2]->ptr);
+            addReplyErrorFormat(c, "User '%s' not found", (char *)objectGetVal(c->argv[2]));
             return;
         }
 
         if ((cmd = lookupCommand(c->argv + 3, c->argc - 3)) == NULL) {
-            addReplyErrorFormat(c, "Command '%s' not found", (char *)c->argv[3]->ptr);
+            addReplyErrorFormat(c, "Command '%s' not found", (char *)objectGetVal(c->argv[3]));
             return;
         }
 
@@ -3340,7 +3340,7 @@ void aclCommand(client *c) {
         int dbid = (c->flag.multi) ? c->mstate->transaction_db_id : c->db->id;
         int result = ACLCheckAllUserCommandPerm(u, cmd, c->argv + 3, c->argc - 3, dbid, &idx);
         if (result != ACL_OK) {
-            sds err = getAclErrorMessage(result, u, cmd, c->argv[idx + 3]->ptr, 1);
+            sds err = getAclErrorMessage(result, u, cmd, objectGetVal(c->argv[idx + 3]), 1);
             addReplyBulkSds(c, err);
             return;
         }

src/aof.c

@@ -1280,11 +1280,11 @@ sds catAppendOnlyGenericCommand(sds dst, int argc, robj **argv) {
     for (j = 0; j < argc; j++) {
         o = getDecodedObject(argv[j]);
         buf[0] = '$';
-        len = 1 + ll2string(buf + 1, sizeof(buf) - 1, sdslen(o->ptr));
+        len = 1 + ll2string(buf + 1, sizeof(buf) - 1, sdslen(objectGetVal(o)));
         buf[len++] = '\r';
         buf[len++] = '\n';
         dst = sdscatlen(dst, buf, len);
-        dst = sdscatlen(dst, o->ptr, sdslen(o->ptr));
+        dst = sdscatlen(dst, objectGetVal(o), sdslen(objectGetVal(o)));
         dst = sdscatlen(dst, "\r\n", 2);
         decrRefCount(o);
     }
@@ -1778,9 +1778,9 @@ int rioWriteBulkObject(rio *r, robj *obj) {
     /* Avoid using getDecodedObject to help copy-on-write (we are often
      * in a child process when this function is called). */
     if (obj->encoding == OBJ_ENCODING_INT) {
-        return rioWriteBulkLongLong(r, (long)obj->ptr);
+        return rioWriteBulkLongLong(r, (long)objectGetVal(obj));
     } else if (sdsEncodedObject(obj)) {
-        return rioWriteBulkString(r, obj->ptr, sdslen(obj->ptr));
+        return rioWriteBulkString(r, objectGetVal(obj), sdslen(objectGetVal(obj)));
     } else {
         serverPanic("Unknown string encoding");
     }
@@ -1860,7 +1860,7 @@ int rewriteSortedSetObject(rio *r, robj *key, robj *o) {
     long long count = 0, items = zsetLength(o);
 
     if (o->encoding == OBJ_ENCODING_LISTPACK) {
-        unsigned char *zl = o->ptr;
+        unsigned char *zl = objectGetVal(o);
         unsigned char *eptr, *sptr;
         unsigned char *vstr;
         unsigned int vlen;
@@ -1895,7 +1895,7 @@ int rewriteSortedSetObject(rio *r, robj *key, robj *o) {
             items--;
         }
     } else if (o->encoding == OBJ_ENCODING_SKIPLIST) {
-        zset *zs = o->ptr;
+        zset *zs = objectGetVal(o);
         hashtableIterator iter;
         hashtableInitIterator(&iter, zs->ht, 0);
         void *next;
@@ -2069,7 +2069,7 @@ int rioWriteStreamEmptyConsumer(rio *r,
 /* Emit the commands needed to rebuild a stream object.
  * The function returns 0 on error, 1 on success. */
 int rewriteStreamObject(rio *r, robj *key, robj *o) {
-    stream *s = o->ptr;
+    stream *s = objectGetVal(o);
     streamIterator si;
     streamIteratorStart(&si, s, NULL, NULL, 0);
     streamID id;
@@ -2190,7 +2190,7 @@ int rewriteStreamObject(rio *r, robj *key, robj *o) {
  * The function returns 0 on error, 1 on success. */
 int rewriteModuleObject(rio *r, robj *key, robj *o, int dbid) {
     ValkeyModuleIO io;
-    moduleValue *mv = o->ptr;
+    moduleValue *mv = objectGetVal(o);
     moduleType *mt = mv->type;
     moduleInitIOContext(&io, mt, r, key, dbid);
     mt->aof_rewrite(&io, key, mv->value);