From: antirez <antirez@gmail.com>
Date: Thu, 16 Sep 2010 09:38:40 +0000 (+0200)
Subject: Merge remote branch 'pietern/networking-perf'
X-Git-Url: https://git.saurik.com/redis.git/commitdiff_plain/89f9f83769a2ad0b4aeb8907b82b453ace6ebe9d?hp=-c

Merge remote branch 'pietern/networking-perf'
---

89f9f83769a2ad0b4aeb8907b82b453ace6ebe9d
diff --combined src/aof.c
index 25febb91,b639eb52..16713481
--- a/src/aof.c
+++ b/src/aof.c
@@@ -189,6 -189,7 +189,7 @@@ struct redisClient *createFakeClient(vo
      c->querybuf = sdsempty();
      c->argc = 0;
      c->argv = NULL;
+     c->bufpos = 0;
      c->flags = 0;
      /* We set the fake client as a slave waiting for the synchronization
       * so that Redis will not try to send replies to this client. */
@@@ -272,12 -273,14 +273,14 @@@ int loadAppendOnlyFile(char *filename) 
          fakeClient->argc = argc;
          fakeClient->argv = argv;
          cmd->proc(fakeClient);
-         /* Discard the reply objects list from the fake client */
-         while(listLength(fakeClient->reply))
-             listDelNode(fakeClient->reply,listFirst(fakeClient->reply));
+ 
+         /* The fake client should not have a reply */
+         redisAssert(fakeClient->bufpos == 0 && listLength(fakeClient->reply) == 0);
+ 
          /* Clean up, ready for the next command */
          for (j = 0; j < argc; j++) decrRefCount(argv[j]);
          zfree(argv);
+ 
          /* Handle swapping while loading big datasets when VM is on */
          force_swapout = 0;
          if ((zmalloc_used_memory() - server.vm_max_memory) > 1024*1024*32)
@@@ -307,7 -310,7 +310,7 @@@ readerr
      }
      exit(1);
  fmterr:
 -    redisLog(REDIS_WARNING,"Bad file format reading the append only file");
 +    redisLog(REDIS_WARNING,"Bad file format reading the append only file: make a backup of your AOF file, then use ./redis-check-aof --fix <filename>");
      exit(1);
  }
  
@@@ -629,12 -632,11 +632,11 @@@ int rewriteAppendOnlyFileBackground(voi
  
  void bgrewriteaofCommand(redisClient *c) {
      if (server.bgrewritechildpid != -1) {
-         addReplySds(c,sdsnew("-ERR background append only file rewriting already in progress\r\n"));
+         addReplyError(c,"Background append only file rewriting already in progress");
          return;
      }
      if (rewriteAppendOnlyFileBackground() == REDIS_OK) {
-         char *status = "+Background append only file rewriting started\r\n";
-         addReplySds(c,sdsnew(status));
+         addReplyStatus(c,"Background append only file rewriting started");
      } else {
          addReply(c,shared.err);
      }
diff --combined src/db.c
index afca3cdd,fc059510..470310a3
--- a/src/db.c
+++ b/src/db.c
@@@ -123,11 -123,6 +123,11 @@@ robj *dbRandomKey(redisDb *db) 
  
  /* Delete a key, value, and associated expiration entry if any, from the DB */
  int dbDelete(redisDb *db, robj *key) {
 +    /* If VM is enabled make sure to awake waiting clients for this key:
 +     * deleting the key will kill the I/O thread bringing the key from swap
 +     * to memory, so the client will never be notified and unblocked if we
 +     * don't do it now. */
 +    if (server.vm_enabled) handleClientsBlockedOnSwappedKey(db,key);
      /* Deleting an entry from the expires dict will not free the sds of
       * the key, because it is shared with the main dictionary. */
      if (dictSize(db->expires) > 0) dictDelete(db->expires,key->ptr);
@@@ -204,7 -199,7 +204,7 @@@ void selectCommand(redisClient *c) 
      int id = atoi(c->argv[1]->ptr);
  
      if (selectDb(c,id) == REDIS_ERR) {
-         addReplySds(c,sdsnew("-ERR invalid DB index\r\n"));
+         addReplyError(c,"invalid DB index");
      } else {
          addReply(c,shared.ok);
      }
@@@ -228,11 -223,9 +228,9 @@@ void keysCommand(redisClient *c) 
      sds pattern = c->argv[1]->ptr;
      int plen = sdslen(pattern), allkeys;
      unsigned long numkeys = 0;
-     robj *lenobj = createObject(REDIS_STRING,NULL);
+     void *replylen = addDeferredMultiBulkLength(c);
  
      di = dictGetIterator(c->db->dict);
-     addReply(c,lenobj);
-     decrRefCount(lenobj);
      allkeys = (pattern[0] == '*' && pattern[1] == '\0');
      while((de = dictNext(di)) != NULL) {
          sds key = dictGetEntryKey(de);
@@@ -248,17 -241,15 +246,15 @@@
          }
      }
      dictReleaseIterator(di);
-     lenobj->ptr = sdscatprintf(sdsempty(),"*%lu\r\n",numkeys);
+     setDeferredMultiBulkLength(c,replylen,numkeys);
  }
  
  void dbsizeCommand(redisClient *c) {
-     addReplySds(c,
-         sdscatprintf(sdsempty(),":%lu\r\n",dictSize(c->db->dict)));
+     addReplyLongLong(c,dictSize(c->db->dict));
  }
  
  void lastsaveCommand(redisClient *c) {
-     addReplySds(c,
-         sdscatprintf(sdsempty(),":%lu\r\n",server.lastsave));
+     addReplyLongLong(c,server.lastsave);
  }
  
  void typeCommand(redisClient *c) {
@@@ -267,24 -258,23 +263,23 @@@
  
      o = lookupKeyRead(c->db,c->argv[1]);
      if (o == NULL) {
-         type = "+none";
+         type = "none";
      } else {
          switch(o->type) {
-         case REDIS_STRING: type = "+string"; break;
-         case REDIS_LIST: type = "+list"; break;
-         case REDIS_SET: type = "+set"; break;
-         case REDIS_ZSET: type = "+zset"; break;
-         case REDIS_HASH: type = "+hash"; break;
-         default: type = "+unknown"; break;
+         case REDIS_STRING: type = "string"; break;
+         case REDIS_LIST: type = "list"; break;
+         case REDIS_SET: type = "set"; break;
+         case REDIS_ZSET: type = "zset"; break;
+         case REDIS_HASH: type = "hash"; break;
+         default: type = "unknown"; break;
          }
      }
-     addReplySds(c,sdsnew(type));
-     addReply(c,shared.crlf);
+     addReplyStatus(c,type);
  }
  
  void saveCommand(redisClient *c) {
      if (server.bgsavechildpid != -1) {
-         addReplySds(c,sdsnew("-ERR background save in progress\r\n"));
+         addReplyError(c,"Background save already in progress");
          return;
      }
      if (rdbSave(server.dbfilename) == REDIS_OK) {
@@@ -296,12 -286,11 +291,11 @@@
  
  void bgsaveCommand(redisClient *c) {
      if (server.bgsavechildpid != -1) {
-         addReplySds(c,sdsnew("-ERR background save already in progress\r\n"));
+         addReplyError(c,"Background save already in progress");
          return;
      }
      if (rdbSaveBackground(server.dbfilename) == REDIS_OK) {
-         char *status = "+Background saving started\r\n";
-         addReplySds(c,sdsnew(status));
+         addReplyStatus(c,"Background saving started");
      } else {
          addReply(c,shared.err);
      }
@@@ -310,7 -299,7 +304,7 @@@
  void shutdownCommand(redisClient *c) {
      if (prepareForShutdown() == REDIS_OK)
          exit(0);
-     addReplySds(c, sdsnew("-ERR Errors trying to SHUTDOWN. Check logs.\r\n"));
+     addReplyError(c,"Errors trying to SHUTDOWN. Check logs.");
  }
  
  void renameGenericCommand(redisClient *c, int nx) {
diff --combined src/networking.c
index 104444f0,55b7475b..96ce5a99
--- a/src/networking.c
+++ b/src/networking.c
@@@ -1,5 -1,4 +1,4 @@@
  #include "redis.h"
- 
  #include <sys/uio.h>
  
  void *dupClientReplyValue(void *o) {
@@@ -12,19 -11,16 +11,24 @@@ int listMatchObjects(void *a, void *b) 
  }
  
  redisClient *createClient(int fd) {
-     redisClient *c = zmalloc(sizeof(*c));
+     redisClient *c;
+ 
+     /* Allocate more space to hold a static write buffer. */
+     c = zmalloc(sizeof(redisClient)+REDIS_REPLY_CHUNK_BYTES);
+     c->buflen = REDIS_REPLY_CHUNK_BYTES;
+     c->bufpos = 0;
  
      anetNonBlock(NULL,fd);
      anetTcpNoDelay(NULL,fd);
      if (!c) return NULL;
 +    if (aeCreateFileEvent(server.el,fd,AE_READABLE,
 +        readQueryFromClient, c) == AE_ERR)
 +    {
 +        close(fd);
 +        zfree(c);
 +        return NULL;
 +    }
 +
      selectDb(c,0);
      c->fd = fd;
      c->querybuf = sdsempty();
@@@ -51,75 -47,248 +55,243 @@@
      c->pubsub_patterns = listCreate();
      listSetFreeMethod(c->pubsub_patterns,decrRefCount);
      listSetMatchMethod(c->pubsub_patterns,listMatchObjects);
 -    if (aeCreateFileEvent(server.el, c->fd, AE_READABLE,
 -        readQueryFromClient, c) == AE_ERR) {
 -        freeClient(c);
 -        return NULL;
 -    }
      listAddNodeTail(server.clients,c);
      initClientMultiState(c);
      return c;
  }
  
- void addReply(redisClient *c, robj *obj) {
-     if (listLength(c->reply) == 0 &&
+ int _ensureFileEvent(redisClient *c) {
+     if (c->fd <= 0) return REDIS_ERR;
+     if (c->bufpos == 0 && listLength(c->reply) == 0 &&
          (c->replstate == REDIS_REPL_NONE ||
           c->replstate == REDIS_REPL_ONLINE) &&
          aeCreateFileEvent(server.el, c->fd, AE_WRITABLE,
-         sendReplyToClient, c) == AE_ERR) return;
+         sendReplyToClient, c) == AE_ERR) return REDIS_ERR;
+     return REDIS_OK;
+ }
+ 
+ /* Create a duplicate of the last object in the reply list when
+  * it is not exclusively owned by the reply list. */
+ robj *dupLastObjectIfNeeded(list *reply) {
+     robj *new, *cur;
+     listNode *ln;
+     redisAssert(listLength(reply) > 0);
+     ln = listLast(reply);
+     cur = listNodeValue(ln);
+     if (cur->refcount > 1) {
+         new = dupStringObject(cur);
+         decrRefCount(cur);
+         listNodeValue(ln) = new;
+     }
+     return listNodeValue(ln);
+ }
+ 
+ int _addReplyToBuffer(redisClient *c, char *s, size_t len) {
+     size_t available = c->buflen-c->bufpos;
+ 
+     /* If there already are entries in the reply list, we cannot
+      * add anything more to the static buffer. */
+     if (listLength(c->reply) > 0) return REDIS_ERR;
+ 
+     /* Check that the buffer has enough space available for this string. */
+     if (len > available) return REDIS_ERR;
  
+     memcpy(c->buf+c->bufpos,s,len);
+     c->bufpos+=len;
+     return REDIS_OK;
+ }
+ 
+ void _addReplyObjectToList(redisClient *c, robj *o) {
+     robj *tail;
+     if (listLength(c->reply) == 0) {
+         incrRefCount(o);
+         listAddNodeTail(c->reply,o);
+     } else {
+         tail = listNodeValue(listLast(c->reply));
+ 
+         /* Append to this object when possible. */
+         if (tail->ptr != NULL &&
+             sdslen(tail->ptr)+sdslen(o->ptr) <= REDIS_REPLY_CHUNK_BYTES)
+         {
+             tail = dupLastObjectIfNeeded(c->reply);
+             tail->ptr = sdscatlen(tail->ptr,o->ptr,sdslen(o->ptr));
+         } else {
+             incrRefCount(o);
+             listAddNodeTail(c->reply,o);
+         }
+     }
+ }
+ 
+ /* This method takes responsibility over the sds. When it is no longer
+  * needed it will be free'd, otherwise it ends up in a robj. */
+ void _addReplySdsToList(redisClient *c, sds s) {
+     robj *tail;
+     if (listLength(c->reply) == 0) {
+         listAddNodeTail(c->reply,createObject(REDIS_STRING,s));
+     } else {
+         tail = listNodeValue(listLast(c->reply));
+ 
+         /* Append to this object when possible. */
+         if (tail->ptr != NULL &&
+             sdslen(tail->ptr)+sdslen(s) <= REDIS_REPLY_CHUNK_BYTES)
+         {
+             tail = dupLastObjectIfNeeded(c->reply);
+             tail->ptr = sdscatlen(tail->ptr,s,sdslen(s));
+             sdsfree(s);
+         } else {
+             listAddNodeTail(c->reply,createObject(REDIS_STRING,s));
+         }
+     }
+ }
+ 
+ void _addReplyStringToList(redisClient *c, char *s, size_t len) {
+     robj *tail;
+     if (listLength(c->reply) == 0) {
+         listAddNodeTail(c->reply,createStringObject(s,len));
+     } else {
+         tail = listNodeValue(listLast(c->reply));
+ 
+         /* Append to this object when possible. */
+         if (tail->ptr != NULL &&
+             sdslen(tail->ptr)+len <= REDIS_REPLY_CHUNK_BYTES)
+         {
+             tail = dupLastObjectIfNeeded(c->reply);
+             tail->ptr = sdscatlen(tail->ptr,s,len);
+         } else {
+             listAddNodeTail(c->reply,createStringObject(s,len));
+         }
+     }
+ }
+ 
+ void addReply(redisClient *c, robj *obj) {
+     if (_ensureFileEvent(c) != REDIS_OK) return;
      if (server.vm_enabled && obj->storage != REDIS_VM_MEMORY) {
+         /* Returns a new object with refcount 1 */
          obj = dupStringObject(obj);
-         obj->refcount = 0; /* getDecodedObject() will increment the refcount */
+     } else {
+         /* This increments the refcount. */
+         obj = getDecodedObject(obj);
      }
-     listAddNodeTail(c->reply,getDecodedObject(obj));
+     if (_addReplyToBuffer(c,obj->ptr,sdslen(obj->ptr)) != REDIS_OK)
+         _addReplyObjectToList(c,obj);
+     decrRefCount(obj);
  }
  
  void addReplySds(redisClient *c, sds s) {
-     robj *o = createObject(REDIS_STRING,s);
-     addReply(c,o);
-     decrRefCount(o);
+     if (_ensureFileEvent(c) != REDIS_OK) {
+         /* The caller expects the sds to be free'd. */
+         sdsfree(s);
+         return;
+     }
+     if (_addReplyToBuffer(c,s,sdslen(s)) == REDIS_OK) {
+         sdsfree(s);
+     } else {
+         /* This method free's the sds when it is no longer needed. */
+         _addReplySdsToList(c,s);
+     }
  }
  
- void addReplyDouble(redisClient *c, double d) {
-     char buf[128];
+ void addReplyString(redisClient *c, char *s, size_t len) {
+     if (_ensureFileEvent(c) != REDIS_OK) return;
+     if (_addReplyToBuffer(c,s,len) != REDIS_OK)
+         _addReplyStringToList(c,s,len);
+ }
  
-     snprintf(buf,sizeof(buf),"%.17g",d);
-     addReplySds(c,sdscatprintf(sdsempty(),"$%lu\r\n%s\r\n",
-         (unsigned long) strlen(buf),buf));
+ void _addReplyError(redisClient *c, char *s, size_t len) {
+     addReplyString(c,"-ERR ",5);
+     addReplyString(c,s,len);
+     addReplyString(c,"\r\n",2);
  }
  
- void addReplyLongLong(redisClient *c, long long ll) {
-     char buf[128];
-     size_t len;
+ void addReplyError(redisClient *c, char *err) {
+     _addReplyError(c,err,strlen(err));
+ }
  
-     if (ll == 0) {
-         addReply(c,shared.czero);
-         return;
-     } else if (ll == 1) {
-         addReply(c,shared.cone);
-         return;
+ void addReplyErrorFormat(redisClient *c, const char *fmt, ...) {
+     va_list ap;
+     va_start(ap,fmt);
+     sds s = sdscatvprintf(sdsempty(),fmt,ap);
+     va_end(ap);
+     _addReplyError(c,s,sdslen(s));
+     sdsfree(s);
+ }
+ 
+ void _addReplyStatus(redisClient *c, char *s, size_t len) {
+     addReplyString(c,"+",1);
+     addReplyString(c,s,len);
+     addReplyString(c,"\r\n",2);
+ }
+ 
+ void addReplyStatus(redisClient *c, char *status) {
+     _addReplyStatus(c,status,strlen(status));
+ }
+ 
+ void addReplyStatusFormat(redisClient *c, const char *fmt, ...) {
+     va_list ap;
+     va_start(ap,fmt);
+     sds s = sdscatvprintf(sdsempty(),fmt,ap);
+     va_end(ap);
+     _addReplyStatus(c,s,sdslen(s));
+     sdsfree(s);
+ }
+ 
+ /* Adds an empty object to the reply list that will contain the multi bulk
+  * length, which is not known when this function is called. */
+ void *addDeferredMultiBulkLength(redisClient *c) {
+     if (_ensureFileEvent(c) != REDIS_OK) return NULL;
+     listAddNodeTail(c->reply,createObject(REDIS_STRING,NULL));
+     return listLast(c->reply);
+ }
+ 
+ /* Populate the length object and try glueing it to the next chunk. */
+ void setDeferredMultiBulkLength(redisClient *c, void *node, long length) {
+     listNode *ln = (listNode*)node;
+     robj *len, *next;
+ 
+     /* Abort when *node is NULL (see addDeferredMultiBulkLength). */
+     if (node == NULL) return;
+ 
+     len = listNodeValue(ln);
+     len->ptr = sdscatprintf(sdsempty(),"*%ld\r\n",length);
+     if (ln->next != NULL) {
+         next = listNodeValue(ln->next);
+ 
+         /* Only glue when the next node is non-NULL (an sds in this case) */
+         if (next->ptr != NULL) {
+             len->ptr = sdscatlen(len->ptr,next->ptr,sdslen(next->ptr));
+             listDelNode(c->reply,ln->next);
+         }
      }
-     buf[0] = ':';
+ }
+ 
+ void addReplyDouble(redisClient *c, double d) {
+     char dbuf[128], sbuf[128];
+     int dlen, slen;
+     dlen = snprintf(dbuf,sizeof(dbuf),"%.17g",d);
+     slen = snprintf(sbuf,sizeof(sbuf),"$%d\r\n%s\r\n",dlen,dbuf);
+     addReplyString(c,sbuf,slen);
+ }
+ 
+ void _addReplyLongLong(redisClient *c, long long ll, char prefix) {
+     char buf[128];
+     int len;
+     buf[0] = prefix;
      len = ll2string(buf+1,sizeof(buf)-1,ll);
      buf[len+1] = '\r';
      buf[len+2] = '\n';
-     addReplySds(c,sdsnewlen(buf,len+3));
+     addReplyString(c,buf,len+3);
  }
  
- void addReplyUlong(redisClient *c, unsigned long ul) {
-     char buf[128];
-     size_t len;
+ void addReplyLongLong(redisClient *c, long long ll) {
+     _addReplyLongLong(c,ll,':');
+ }
  
-     if (ul == 0) {
-         addReply(c,shared.czero);
-         return;
-     } else if (ul == 1) {
-         addReply(c,shared.cone);
-         return;
-     }
-     len = snprintf(buf,sizeof(buf),":%lu\r\n",ul);
-     addReplySds(c,sdsnewlen(buf,len));
+ void addReplyMultiBulkLen(redisClient *c, long length) {
+     _addReplyLongLong(c,length,'*');
  }
  
  void addReplyBulkLen(redisClient *c, robj *obj) {
-     size_t len, intlen;
-     char buf[128];
+     size_t len;
  
      if (obj->encoding == REDIS_ENCODING_RAW) {
          len = sdslen(obj->ptr);
@@@ -136,11 -305,7 +308,7 @@@
              len++;
          }
      }
-     buf[0] = '$';
-     intlen = ll2string(buf+1,sizeof(buf)-1,(long long)len);
-     buf[intlen+1] = '\r';
-     buf[intlen+2] = '\n';
-     addReplySds(c,sdsnewlen(buf,intlen+3));
+     _addReplyLongLong(c,len,'$');
  }
  
  void addReplyBulk(redisClient *c, robj *obj) {
@@@ -290,34 -455,6 +458,6 @@@ void freeClient(redisClient *c) 
      zfree(c);
  }
  
- #define GLUEREPLY_UP_TO (1024)
- static void glueReplyBuffersIfNeeded(redisClient *c) {
-     int copylen = 0;
-     char buf[GLUEREPLY_UP_TO];
-     listNode *ln;
-     listIter li;
-     robj *o;
- 
-     listRewind(c->reply,&li);
-     while((ln = listNext(&li))) {
-         int objlen;
- 
-         o = ln->value;
-         objlen = sdslen(o->ptr);
-         if (copylen + objlen <= GLUEREPLY_UP_TO) {
-             memcpy(buf+copylen,o->ptr,objlen);
-             copylen += objlen;
-             listDelNode(c->reply,ln);
-         } else {
-             if (copylen == 0) return;
-             break;
-         }
-     }
-     /* Now the output buffer is empty, add the new single element */
-     o = createObject(REDIS_STRING,sdsnewlen(buf,copylen));
-     listAddNodeHead(c->reply,o);
- }
- 
  void sendReplyToClient(aeEventLoop *el, int fd, void *privdata, int mask) {
      redisClient *c = privdata;
      int nwritten = 0, totwritten = 0, objlen;
@@@ -334,31 -471,48 +474,48 @@@
          return;
      }
  
-     while(listLength(c->reply)) {
-         if (server.glueoutputbuf && listLength(c->reply) > 1)
-             glueReplyBuffersIfNeeded(c);
+     while(c->bufpos > 0 || listLength(c->reply)) {
+         if (c->bufpos > 0) {
+             if (c->flags & REDIS_MASTER) {
+                 /* Don't reply to a master */
+                 nwritten = c->bufpos - c->sentlen;
+             } else {
+                 nwritten = write(fd,c->buf+c->sentlen,c->bufpos-c->sentlen);
+                 if (nwritten <= 0) break;
+             }
+             c->sentlen += nwritten;
+             totwritten += nwritten;
+ 
+             /* If the buffer was sent, set bufpos to zero to continue with
+              * the remainder of the reply. */
+             if (c->sentlen == c->bufpos) {
+                 c->bufpos = 0;
+                 c->sentlen = 0;
+             }
+         } else {
+             o = listNodeValue(listFirst(c->reply));
+             objlen = sdslen(o->ptr);
  
-         o = listNodeValue(listFirst(c->reply));
-         objlen = sdslen(o->ptr);
+             if (objlen == 0) {
+                 listDelNode(c->reply,listFirst(c->reply));
+                 continue;
+             }
  
-         if (objlen == 0) {
-             listDelNode(c->reply,listFirst(c->reply));
-             continue;
-         }
+             if (c->flags & REDIS_MASTER) {
+                 /* Don't reply to a master */
+                 nwritten = objlen - c->sentlen;
+             } else {
+                 nwritten = write(fd, ((char*)o->ptr)+c->sentlen,objlen-c->sentlen);
+                 if (nwritten <= 0) break;
+             }
+             c->sentlen += nwritten;
+             totwritten += nwritten;
  
-         if (c->flags & REDIS_MASTER) {
-             /* Don't reply to a master */
-             nwritten = objlen - c->sentlen;
-         } else {
-             nwritten = write(fd, ((char*)o->ptr)+c->sentlen, objlen - c->sentlen);
-             if (nwritten <= 0) break;
-         }
-         c->sentlen += nwritten;
-         totwritten += nwritten;
-         /* If we fully sent the object on head go to the next one */
-         if (c->sentlen == objlen) {
-             listDelNode(c->reply,listFirst(c->reply));
-             c->sentlen = 0;
+             /* If we fully sent the object on head go to the next one */
+             if (c->sentlen == objlen) {
+                 listDelNode(c->reply,listFirst(c->reply));
+                 c->sentlen = 0;
+             }
          }
          /* Note that we avoid to send more thank REDIS_MAX_WRITE_PER_EVENT
           * bytes, in a single threaded server it's a good idea to serve
diff --combined src/redis.c
index b6b42521,a3d02114..66e088b0
--- a/src/redis.c
+++ b/src/redis.c
@@@ -909,7 -909,7 +909,7 @@@ int processCommand(redisClient *c) 
      } else if (c->multibulk) {
          if (c->bulklen == -1) {
              if (((char*)c->argv[0]->ptr)[0] != '$') {
-                 addReplySds(c,sdsnew("-ERR multi bulk protocol error\r\n"));
+                 addReplyError(c,"multi bulk protocol error");
                  resetClient(c);
                  return 1;
              } else {
@@@ -922,7 -922,7 +922,7 @@@
                      bulklen < 0 || bulklen > 1024*1024*1024)
                  {
                      c->argc--;
-                     addReplySds(c,sdsnew("-ERR invalid bulk write count\r\n"));
+                     addReplyError(c,"invalid bulk write count");
                      resetClient(c);
                      return 1;
                  }
@@@ -975,17 -975,14 +975,14 @@@
       * such wrong arity, bad command name and so forth. */
      cmd = lookupCommand(c->argv[0]->ptr);
      if (!cmd) {
-         addReplySds(c,
-             sdscatprintf(sdsempty(), "-ERR unknown command '%s'\r\n",
-                 (char*)c->argv[0]->ptr));
+         addReplyErrorFormat(c,"unknown command '%s'",
+             (char*)c->argv[0]->ptr);
          resetClient(c);
          return 1;
      } else if ((cmd->arity > 0 && cmd->arity != c->argc) ||
                 (c->argc < -cmd->arity)) {
-         addReplySds(c,
-             sdscatprintf(sdsempty(),
-                 "-ERR wrong number of arguments for '%s' command\r\n",
-                 cmd->name));
+         addReplyErrorFormat(c,"wrong number of arguments for '%s' command",
+             cmd->name);
          resetClient(c);
          return 1;
      } else if (cmd->flags & REDIS_CMD_BULK && c->bulklen == -1) {
@@@ -999,7 -996,7 +996,7 @@@
              bulklen < 0 || bulklen > 1024*1024*1024)
          {
              c->argc--;
-             addReplySds(c,sdsnew("-ERR invalid bulk write count\r\n"));
+             addReplyError(c,"invalid bulk write count");
              resetClient(c);
              return 1;
          }
@@@ -1026,7 -1023,7 +1023,7 @@@
  
      /* Check if the user is authenticated */
      if (server.requirepass && !c->authenticated && cmd->proc != authCommand) {
-         addReplySds(c,sdsnew("-ERR operation not permitted\r\n"));
+         addReplyError(c,"operation not permitted");
          resetClient(c);
          return 1;
      }
@@@ -1035,7 -1032,7 +1032,7 @@@
      if (server.maxmemory && (cmd->flags & REDIS_CMD_DENYOOM) &&
          zmalloc_used_memory() > server.maxmemory)
      {
-         addReplySds(c,sdsnew("-ERR command not allowed when used memory > 'maxmemory'\r\n"));
+         addReplyError(c,"command not allowed when used memory > 'maxmemory'");
          resetClient(c);
          return 1;
      }
@@@ -1045,7 -1042,7 +1042,7 @@@
          &&
          cmd->proc != subscribeCommand && cmd->proc != unsubscribeCommand &&
          cmd->proc != psubscribeCommand && cmd->proc != punsubscribeCommand) {
-         addReplySds(c,sdsnew("-ERR only (P)SUBSCRIBE / (P)UNSUBSCRIBE / QUIT allowed in this context\r\n"));
+         addReplyError(c,"only (P)SUBSCRIBE / (P)UNSUBSCRIBE / QUIT allowed in this context");
          resetClient(c);
          return 1;
      }
@@@ -1109,7 -1106,7 +1106,7 @@@ void authCommand(redisClient *c) 
        addReply(c,shared.ok);
      } else {
        c->authenticated = 0;
-       addReplySds(c,sdscatprintf(sdsempty(),"-ERR invalid password\r\n"));
+       addReplyError(c,"invalid password");
      }
  }
  
@@@ -1343,7 -1340,6 +1340,7 @@@ void freeMemoryIfNeeded(void) 
                  }
                  keyobj = createStringObject(minkey,sdslen(minkey));
                  dbDelete(server.db+j,keyobj);
 +                server.stat_expiredkeys++;
                  decrRefCount(keyobj);
              }
          }