Merge pull request #5834 from guybe7/trim_module_sds

Trim SDS free space of retained module strings

src/module.c

@@ -521,6 +521,17 @@ void RedisModuleCommandDispatcher(client *c) {
     cp->func(&ctx,(void**)c->argv,c->argc);
     moduleHandlePropagationAfterCommandCallback(&ctx);
     moduleFreeContext(&ctx);
+
+    /* In some cases processMultibulkBuffer uses sdsMakeRoomFor to
+     * expand the querybuf, but later in some cases it uses that query
+     * buffer as is for an argv element (see "Optimization"), which means
+     * that the sds in argv may have a lot of wasted space, and then in case
+     * modules keep that argv RedisString inside their data structure, this
+     * space waste will remain for long (until restarted from rdb). */
+    for (int i = 0; i < c->argc; i++) {
+        if (c->argv[i]->refcount > 1)
+            trimStringObjectIfNeeded(c->argv[i]);
+    }
 }
 
 /* This function returns the list of keys, with the same interface as the

src/object.c

@@ -415,6 +415,17 @@ int isObjectRepresentableAsLongLong(robj *o, long long *llval) {
     }
 }
 
+void trimStringObjectIfNeeded(robj *o) {
+    /* Optimize the SDS string inside the string object to require
+     * little space, in case there is more than 10% of free space
+     * at the end of the SDS string. */
+    if (o->encoding == OBJ_ENCODING_RAW &&
+        sdsavail(o->ptr) > sdslen(o->ptr)/10)
+    {
+        o->ptr = sdsRemoveFreeSpace(o->ptr);
+    }
+}
+
 /* Try to encode a string object in order to save space */
 robj *tryObjectEncoding(robj *o) {
     long value;
@@ -484,11 +495,7 @@ robj *tryObjectEncoding(robj *o) {
      * We do that only for relatively large strings as this branch
      * is only entered if the length of the string is greater than
      * OBJ_ENCODING_EMBSTR_SIZE_LIMIT. */
-    if (o->encoding == OBJ_ENCODING_RAW &&
-        sdsavail(s) > len/10)
-    {
-        o->ptr = sdsRemoveFreeSpace(o->ptr);
-    }
+    trimStringObjectIfNeeded(o);
 
     /* Return the original object. */
     return o;

src/sds.c

@@ -257,8 +257,12 @@ sds sdsRemoveFreeSpace(sds s) {
     char type, oldtype = s[-1] & SDS_TYPE_MASK;
     int hdrlen, oldhdrlen = sdsHdrSize(oldtype);
     size_t len = sdslen(s);
+    size_t avail = sdsavail(s);
     sh = (char*)s-oldhdrlen;
 
+    /* Return ASAP if there is no space left. */
+    if (avail == 0) return s;
+
     /* Check what would be the minimum SDS header that is just good enough to
      * fit this string. */
     type = sdsReqType(len);

src/server.h

@@ -1662,6 +1662,7 @@ int compareStringObjects(robj *a, robj *b);
 int collateStringObjects(robj *a, robj *b);
 int equalStringObjects(robj *a, robj *b);
 unsigned long long estimateObjectIdleTime(robj *o);
+void trimStringObjectIfNeeded(robj *o);
 #define sdsEncodedObject(objptr) (objptr->encoding == OBJ_ENCODING_RAW || objptr->encoding == OBJ_ENCODING_EMBSTR)
 
 /* Synchronous I/O with timeout */