From 35267245948fef5561a921943c1eb118cfb704b0 Mon Sep 17 00:00:00 2001 From: antirez Date: Wed, 2 Nov 2011 16:09:14 +0100 Subject: [PATCH 01/21] added a comment to sdsMakeRoomFor() to make it clear what the function actually does. --- src/sds.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/sds.c b/src/sds.c index 2104eb36..fc104a4a 100644 --- a/src/sds.c +++ b/src/sds.c @@ -101,6 +101,12 @@ void sdsclear(sds s) { sh->buf[0] = '\0'; } +/* Enlarge the free space at the end of the sds string so that the caller + * is sure that after calling this function can overwrite up to addlen + * bytes after the end of the string, plus one more byte for nul term. + * + * Note: this does not change the *size* of the sds string as returned + * by sdslen(), but only the free buffer space we have. */ static sds sdsMakeRoomFor(sds s, size_t addlen) { struct sdshdr *sh, *newsh; size_t free = sdsavail(s); From d0b2a9b2234afe82d6ed42a8c45f8221ede25d8f Mon Sep 17 00:00:00 2001 From: antirez Date: Wed, 2 Nov 2011 16:50:59 +0100 Subject: [PATCH 02/21] sdsMakeRoomFor() exposed as public API. sdsIncrLen() added. Both the changes make it possible to copy stuff from a system call to an sds buffer without the need of an additional buffer and copying overhead. --- src/sds.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++++- src/sds.h | 4 ++++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/src/sds.c b/src/sds.c index fc104a4a..c3a0ccb9 100644 --- a/src/sds.c +++ b/src/sds.c @@ -40,6 +40,7 @@ #include #include #include +#include #include "sds.h" #include "zmalloc.h" @@ -107,7 +108,7 @@ void sdsclear(sds s) { * * Note: this does not change the *size* of the sds string as returned * by sdslen(), but only the free buffer space we have. */ -static sds sdsMakeRoomFor(sds s, size_t addlen) { +sds sdsMakeRoomFor(sds s, size_t addlen) { struct sdshdr *sh, *newsh; size_t free = sdsavail(s); size_t len, newlen; @@ -127,6 +128,37 @@ static sds sdsMakeRoomFor(sds s, size_t addlen) { return newsh->buf; } +/* Increment the sds length and decrements the left free space at the + * end of the string accordingly to 'incr'. Also set the null term + * in the new end of the string. + * + * This function is used in order to fix the string length after the + * user calls sdsMakeRoomFor(), writes something after the end of + * the current string, and finally needs to set the new length. + * + * Note: it is possible to use a negative increment in order to + * right-trim the string. + * + * Using sdsIncrLen() and sdsMakeRoomFor() it is possible to mount the + * following schema to cat bytes coming from the kerenl to the end of an + * sds string new things without copying into an intermediate buffer: + * + * oldlen = sdslen(s); + * s = sdsMakeRoomFor(s, BUFFER_SIZE); + * nread = read(fd, s+oldlen, BUFFER_SIZE); + * ... check for nread <= 0 and handle it ... + * sdsIncrLen(s, nhread); + */ +void sdsIncrLen(sds s, int incr) { + struct sdshdr *sh = (void*) (s-(sizeof(struct sdshdr))); + + assert(sh->free >= incr); + sh->len += incr; + sh->free -= incr; + assert(sh->free >= 0); + s[sh->len] = '\0'; +} + /* Grow the sds to have the specified length. Bytes that were not part of * the original length of the sds will be set to zero. */ sds sdsgrowzero(sds s, size_t len) { @@ -615,6 +647,7 @@ sds sdsmapchars(sds s, char *from, char *to, size_t setlen) { int main(void) { { + struct sdshdr *sh; sds x = sdsnew("foo"), y; test_cond("Create a string and obtain the length", @@ -694,7 +727,26 @@ int main(void) { x = sdsnew("aar"); y = sdsnew("bar"); test_cond("sdscmp(bar,bar)", sdscmp(x,y) < 0) + + { + int oldfree; + + sdsfree(x); + x = sdsnew("0"); + sh = (void*) (x-(sizeof(struct sdshdr))); + test_cond("sdsnew() free/len buffers", sh->len == 1 && sh->free == 0); + x = sdsMakeRoomFor(x,1); + sh = (void*) (x-(sizeof(struct sdshdr))); + test_cond("sdsMakeRoomFor()", sh->len == 1 && sh->free > 0); + oldfree = sh->free; + x[1] = '1'; + sdsIncrLen(x,1); + test_cond("sdsIncrLen() -- content", x[0] == '0' && x[1] == '1'); + test_cond("sdsIncrLen() -- len", sh->len == 2); + test_cond("sdsIncrLen() -- free", sh->free == oldfree-1); + } } test_report() + return 0; } #endif diff --git a/src/sds.h b/src/sds.h index 6e5684ee..eff1b03e 100644 --- a/src/sds.h +++ b/src/sds.h @@ -88,4 +88,8 @@ sds *sdssplitargs(char *line, int *argc); void sdssplitargs_free(sds *argv, int argc); sds sdsmapchars(sds s, char *from, char *to, size_t setlen); +/* Low level functions exposed to the user API */ +sds sdsMakeRoomFor(sds s, size_t addlen); +void sdsIncrLen(sds s, int incr); + #endif From dd5fbedf7bb9ac02d14aa9ecaeafb47e48b9a587 Mon Sep 17 00:00:00 2001 From: antirez Date: Wed, 2 Nov 2011 16:51:33 +0100 Subject: [PATCH 03/21] I/O buffer length enlarged --- src/redis.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/redis.h b/src/redis.h index 883db1f5..b923f59e 100644 --- a/src/redis.h +++ b/src/redis.h @@ -40,7 +40,7 @@ /* Static server configuration */ #define REDIS_SERVERPORT 6379 /* TCP port */ #define REDIS_MAXIDLETIME (60*5) /* default client timeout */ -#define REDIS_IOBUF_LEN 1024 +#define REDIS_IOBUF_LEN (1024*16) #define REDIS_LOADBUF_LEN 1024 #define REDIS_DEFAULT_DBNUM 16 #define REDIS_CONFIGLINE_MAX 1024 From a54806ac6af77940070a90a44b603bec46cfe599 Mon Sep 17 00:00:00 2001 From: antirez Date: Wed, 2 Nov 2011 16:52:10 +0100 Subject: [PATCH 04/21] testhelp.h now exits with retcode 1 on failed tests. --- src/testhelp.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/testhelp.h b/src/testhelp.h index d699f2ae..807a86e9 100644 --- a/src/testhelp.h +++ b/src/testhelp.h @@ -48,6 +48,7 @@ int __test_num = 0; __test_num-__failed_tests, __failed_tests); \ if (__failed_tests) { \ printf("=== WARNING === We have failed tests here...\n"); \ + exit(1); \ } \ } while(0); From b8d743e1813abbf3d55e92e9945dc47da3ef7836 Mon Sep 17 00:00:00 2001 From: antirez Date: Wed, 2 Nov 2011 16:52:45 +0100 Subject: [PATCH 05/21] sdsIncrLen() / sdsMakeRoomFor() used to avoid copying to intermediate buffer while reading the client query. --- src/networking.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/networking.c b/src/networking.c index 862e69f4..f6139f57 100644 --- a/src/networking.c +++ b/src/networking.c @@ -833,12 +833,14 @@ void processInputBuffer(redisClient *c) { void readQueryFromClient(aeEventLoop *el, int fd, void *privdata, int mask) { redisClient *c = (redisClient*) privdata; - char buf[REDIS_IOBUF_LEN]; int nread; + size_t qblen; REDIS_NOTUSED(el); REDIS_NOTUSED(mask); - nread = read(fd, buf, REDIS_IOBUF_LEN); + qblen = sdslen(c->querybuf); + c->querybuf = sdsMakeRoomFor(c->querybuf, REDIS_IOBUF_LEN); + nread = read(fd, c->querybuf+qblen, REDIS_IOBUF_LEN); if (nread == -1) { if (errno == EAGAIN) { nread = 0; @@ -853,7 +855,7 @@ void readQueryFromClient(aeEventLoop *el, int fd, void *privdata, int mask) { return; } if (nread) { - c->querybuf = sdscatlen(c->querybuf,buf,nread); + sdsIncrLen(c->querybuf,nread); c->lastinteraction = time(NULL); } else { return; From 921709557253dae2db676c2feb933386e4066494 Mon Sep 17 00:00:00 2001 From: antirez Date: Wed, 2 Nov 2011 17:30:19 +0100 Subject: [PATCH 06/21] optimized object creation in multi-bulk protocol parsing --- src/networking.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/src/networking.c b/src/networking.c index f6139f57..a24c00e6 100644 --- a/src/networking.c +++ b/src/networking.c @@ -775,15 +775,32 @@ int processMultibulkBuffer(redisClient *c) { /* Not enough data (+2 == trailing \r\n) */ break; } else { - c->argv[c->argc++] = createStringObject(c->querybuf+pos,c->bulklen); - pos += c->bulklen+2; + /* Optimization: if the buffer contanins JUST our bulk element + * instead of creating a new object by *copying* the sds we + * just use the current sds string. */ + if (pos == 0 && + sdslen(c->querybuf) > 4096 && + (signed) sdslen(c->querybuf) == c->bulklen+2) + { + c->argv[c->argc++] = createObject(REDIS_STRING,c->querybuf); + sdsIncrLen(c->querybuf,-2); /* remove CRLF */ + c->querybuf = sdsempty(); + /* Assume that if we saw a fat argument we'll see another one + * likely... */ + c->querybuf = sdsMakeRoomFor(c->querybuf,c->bulklen+2); + pos = 0; + } else { + c->argv[c->argc++] = + createStringObject(c->querybuf+pos,c->bulklen); + pos += c->bulklen+2; + } c->bulklen = -1; c->multibulklen--; } } /* Trim to pos */ - c->querybuf = sdsrange(c->querybuf,pos,-1); + if (pos) c->querybuf = sdsrange(c->querybuf,pos,-1); /* We're done when c->multibulk == 0 */ if (c->multibulklen == 0) { From 826b5beb9c04fef0d9942b8846989732b0d03ead Mon Sep 17 00:00:00 2001 From: antirez Date: Thu, 3 Nov 2011 15:53:40 +0100 Subject: [PATCH 07/21] further optimizations for the multi bulk protocol parsing code when big objects are transmitted to Redis. --- src/networking.c | 40 ++++++++++++++++++++++++++++++++++++---- src/redis-benchmark.c | 4 +++- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/src/networking.c b/src/networking.c index a24c00e6..4367e30b 100644 --- a/src/networking.c +++ b/src/networking.c @@ -767,6 +767,19 @@ int processMultibulkBuffer(redisClient *c) { } pos += newline-(c->querybuf+pos)+2; + #if 1 + if (ll > 4096) { + /* If we are going to read a large object from network + * try to make it likely that it will start at c->querybuf + * boundary so that we can optimized object creation + * avoiding a large copy of data. */ + c->querybuf = sdsrange(c->querybuf,pos,-1); + pos = 0; + } + #endif + /* Hint the sds library about the amount of bytes this string is + * going to contain. */ + if (ll > 4096) c->querybuf = sdsMakeRoomFor(c->querybuf,ll+2); c->bulklen = ll; } @@ -779,9 +792,10 @@ int processMultibulkBuffer(redisClient *c) { * instead of creating a new object by *copying* the sds we * just use the current sds string. */ if (pos == 0 && - sdslen(c->querybuf) > 4096 && + /* sdslen(c->querybuf) > 4096 && */ (signed) sdslen(c->querybuf) == c->bulklen+2) { + // printf("HERE (arg %d)\n",c->argc); c->argv[c->argc++] = createObject(REDIS_STRING,c->querybuf); sdsIncrLen(c->querybuf,-2); /* remove CRLF */ c->querybuf = sdsempty(); @@ -790,6 +804,7 @@ int processMultibulkBuffer(redisClient *c) { c->querybuf = sdsMakeRoomFor(c->querybuf,c->bulklen+2); pos = 0; } else { + // printf("NOT HERE (arg %d) (pos %d)\n",c->argc, pos); c->argv[c->argc++] = createStringObject(c->querybuf+pos,c->bulklen); pos += c->bulklen+2; @@ -850,14 +865,31 @@ void processInputBuffer(redisClient *c) { void readQueryFromClient(aeEventLoop *el, int fd, void *privdata, int mask) { redisClient *c = (redisClient*) privdata; - int nread; + int nread, readlen; size_t qblen; REDIS_NOTUSED(el); REDIS_NOTUSED(mask); + readlen = REDIS_IOBUF_LEN; + /* If this is a multi bulk request, and we are processing a bulk reply + * that is large enough, try to maximize the probabilty that the query + * buffer contains excatly the SDS string representing the object, even + * at the risk of requring more read(2) calls. This way the function + * processMultiBulkBuffer() can avoid copying buffers to create the + * Redis Object representing the argument. */ + #if 1 + if (c->reqtype == REDIS_REQ_MULTIBULK && c->multibulklen && c->bulklen != -1 + && c->bulklen > 4096) + { + int remaining = (unsigned)(c->bulklen+2)-sdslen(c->querybuf); + + if (remaining < readlen) readlen = remaining; + } + #endif + qblen = sdslen(c->querybuf); - c->querybuf = sdsMakeRoomFor(c->querybuf, REDIS_IOBUF_LEN); - nread = read(fd, c->querybuf+qblen, REDIS_IOBUF_LEN); + c->querybuf = sdsMakeRoomFor(c->querybuf, readlen); + nread = read(fd, c->querybuf+qblen, readlen); if (nread == -1) { if (errno == EAGAIN) { nread = 0; diff --git a/src/redis-benchmark.c b/src/redis-benchmark.c index e4a40e13..3aa495bd 100644 --- a/src/redis-benchmark.c +++ b/src/redis-benchmark.c @@ -367,7 +367,7 @@ int parseOptions(int argc, const char **argv) { if (lastarg) goto invalid; config.datasize = atoi(argv[++i]); if (config.datasize < 1) config.datasize=1; - if (config.datasize > 1024*1024) config.datasize = 1024*1024; + if (config.datasize > 1024*1024*1024) config.datasize = 1024*1024*1024; } else if (!strcmp(argv[i],"-r")) { if (lastarg) goto invalid; config.randomkeys = 1; @@ -500,6 +500,7 @@ int main(int argc, const char **argv) { memset(data,'x',config.datasize); data[config.datasize] = '\0'; +#if 0 benchmark("PING (inline)","PING\r\n",6); len = redisFormatCommand(&cmd,"PING"); @@ -515,6 +516,7 @@ int main(int argc, const char **argv) { len = redisFormatCommandArgv(&cmd,21,argv,NULL); benchmark("MSET (10 keys)",cmd,len); free(cmd); +#endif len = redisFormatCommand(&cmd,"SET foo:rand:000000000000 %s",data); benchmark("SET",cmd,len); From 94d490b9f68a1972a3c89cf0ffc801b64ec2083e Mon Sep 17 00:00:00 2001 From: antirez Date: Fri, 4 Nov 2011 11:16:11 +0100 Subject: [PATCH 08/21] Added a define to set the size threshold to enable the multi bulk parsing big objects optimization. --- src/networking.c | 21 +++++++++++++-------- src/redis.h | 1 + 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/networking.c b/src/networking.c index 4367e30b..4ecf78e8 100644 --- a/src/networking.c +++ b/src/networking.c @@ -767,8 +767,8 @@ int processMultibulkBuffer(redisClient *c) { } pos += newline-(c->querybuf+pos)+2; - #if 1 - if (ll > 4096) { +#ifdef REDIS_MBULK_BIG_ARG + if (ll >= REDIS_MBULK_BIG_ARG) { /* If we are going to read a large object from network * try to make it likely that it will start at c->querybuf * boundary so that we can optimized object creation @@ -776,10 +776,11 @@ int processMultibulkBuffer(redisClient *c) { c->querybuf = sdsrange(c->querybuf,pos,-1); pos = 0; } - #endif /* Hint the sds library about the amount of bytes this string is * going to contain. */ - if (ll > 4096) c->querybuf = sdsMakeRoomFor(c->querybuf,ll+2); + if (ll >= REDIS_MBULK_BIG_ARG) + c->querybuf = sdsMakeRoomFor(c->querybuf,ll+2); +#endif c->bulklen = ll; } @@ -791,8 +792,9 @@ int processMultibulkBuffer(redisClient *c) { /* Optimization: if the buffer contanins JUST our bulk element * instead of creating a new object by *copying* the sds we * just use the current sds string. */ +#ifdef REDIS_MBULK_BIG_ARG if (pos == 0 && - /* sdslen(c->querybuf) > 4096 && */ + c->bulklen >= REDIS_MBULK_BIG_ARG && (signed) sdslen(c->querybuf) == c->bulklen+2) { // printf("HERE (arg %d)\n",c->argc); @@ -804,11 +806,14 @@ int processMultibulkBuffer(redisClient *c) { c->querybuf = sdsMakeRoomFor(c->querybuf,c->bulklen+2); pos = 0; } else { +#endif // printf("NOT HERE (arg %d) (pos %d)\n",c->argc, pos); c->argv[c->argc++] = createStringObject(c->querybuf+pos,c->bulklen); pos += c->bulklen+2; +#ifdef REDIS_MBULK_BIG_ARG } +#endif c->bulklen = -1; c->multibulklen--; } @@ -871,21 +876,21 @@ void readQueryFromClient(aeEventLoop *el, int fd, void *privdata, int mask) { REDIS_NOTUSED(mask); readlen = REDIS_IOBUF_LEN; +#if REDIS_MBULK_BIG_ARG /* If this is a multi bulk request, and we are processing a bulk reply * that is large enough, try to maximize the probabilty that the query * buffer contains excatly the SDS string representing the object, even * at the risk of requring more read(2) calls. This way the function * processMultiBulkBuffer() can avoid copying buffers to create the * Redis Object representing the argument. */ - #if 1 if (c->reqtype == REDIS_REQ_MULTIBULK && c->multibulklen && c->bulklen != -1 - && c->bulklen > 4096) + && c->bulklen >= REDIS_MBULK_BIG_ARG) { int remaining = (unsigned)(c->bulklen+2)-sdslen(c->querybuf); if (remaining < readlen) readlen = remaining; } - #endif +#endif qblen = sdslen(c->querybuf); c->querybuf = sdsMakeRoomFor(c->querybuf, readlen); diff --git a/src/redis.h b/src/redis.h index b923f59e..b5e954fd 100644 --- a/src/redis.h +++ b/src/redis.h @@ -59,6 +59,7 @@ #define REDIS_REPL_TIMEOUT 60 #define REDIS_REPL_PING_SLAVE_PERIOD 10 +#define REDIS_MBULK_BIG_ARG (1024*32) /* Hash table parameters */ #define REDIS_HT_MINFILL 10 /* Minimal hash table fill 10% */ From ca908473e8298e617d099a57ae1fb8db4bccdfa9 Mon Sep 17 00:00:00 2001 From: antirez Date: Fri, 4 Nov 2011 11:18:15 +0100 Subject: [PATCH 09/21] A comment moved a few lines for clarity. --- src/networking.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/networking.c b/src/networking.c index 4ecf78e8..51c0ac61 100644 --- a/src/networking.c +++ b/src/networking.c @@ -789,10 +789,10 @@ int processMultibulkBuffer(redisClient *c) { /* Not enough data (+2 == trailing \r\n) */ break; } else { +#ifdef REDIS_MBULK_BIG_ARG /* Optimization: if the buffer contanins JUST our bulk element * instead of creating a new object by *copying* the sds we * just use the current sds string. */ -#ifdef REDIS_MBULK_BIG_ARG if (pos == 0 && c->bulklen >= REDIS_MBULK_BIG_ARG && (signed) sdslen(c->querybuf) == c->bulklen+2) From 410dfe90b2cc62cbbeddb7f7617e9946dc1eab98 Mon Sep 17 00:00:00 2001 From: antirez Date: Fri, 4 Nov 2011 11:20:19 +0100 Subject: [PATCH 10/21] PING / MSET benchmarks enabled again. --- src/redis-benchmark.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/redis-benchmark.c b/src/redis-benchmark.c index 3aa495bd..b599ec32 100644 --- a/src/redis-benchmark.c +++ b/src/redis-benchmark.c @@ -500,7 +500,6 @@ int main(int argc, const char **argv) { memset(data,'x',config.datasize); data[config.datasize] = '\0'; -#if 0 benchmark("PING (inline)","PING\r\n",6); len = redisFormatCommand(&cmd,"PING"); @@ -516,7 +515,6 @@ int main(int argc, const char **argv) { len = redisFormatCommandArgv(&cmd,21,argv,NULL); benchmark("MSET (10 keys)",cmd,len); free(cmd); -#endif len = redisFormatCommand(&cmd,"SET foo:rand:000000000000 %s",data); benchmark("SET",cmd,len); From 7b86f5e6d5ecdafc3e47f640ac99614ee2295c06 Mon Sep 17 00:00:00 2001 From: antirez Date: Fri, 4 Nov 2011 14:49:24 +0100 Subject: [PATCH 11/21] csv output for redis-benchmark --- src/redis-benchmark.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/redis-benchmark.c b/src/redis-benchmark.c index b599ec32..49432030 100644 --- a/src/redis-benchmark.c +++ b/src/redis-benchmark.c @@ -68,6 +68,7 @@ static struct config { const char *title; list *clients; int quiet; + int csv; int loop; int idlemode; } config; @@ -295,7 +296,7 @@ static void showLatencyReport(void) { float perc, reqpersec; reqpersec = (float)config.requests_finished/((float)config.totlatency/1000); - if (!config.quiet) { + if (!config.quiet && !config.csv) { printf("====== %s ======\n", config.title); printf(" %d requests completed in %.2f seconds\n", config.requests_finished, (float)config.totlatency/1000); @@ -313,6 +314,8 @@ static void showLatencyReport(void) { } } printf("%.2f requests per second\n\n", reqpersec); + } else if (config.csv) { + printf("\"%s\",\"%.2f\"\n", config.title, reqpersec); } else { printf("%s: %.2f requests per second\n", config.title, reqpersec); } @@ -376,6 +379,8 @@ int parseOptions(int argc, const char **argv) { config.randomkeys_keyspacelen = 0; } else if (!strcmp(argv[i],"-q")) { config.quiet = 1; + } else if (!strcmp(argv[i],"--csv")) { + config.csv = 1; } else if (!strcmp(argv[i],"-l")) { config.loop = 1; } else if (!strcmp(argv[i],"-I")) { @@ -414,6 +419,7 @@ usage: printf(" if set to 10 only rand000000000000 - rand000000000009\n"); printf(" range will be allowed.\n"); printf(" -q Quiet. Just show query/sec values\n"); + printf(" --csv Output in CSV format\n"); printf(" -l Loop. Run the tests forever\n"); printf(" -I Idle mode. Just open N idle connections and wait.\n"); exit(exit_status); @@ -424,6 +430,7 @@ int showThroughput(struct aeEventLoop *eventLoop, long long id, void *clientData REDIS_NOTUSED(id); REDIS_NOTUSED(clientData); + if (config.csv) return 250; float dt = (float)(mstime()-config.start)/1000.0; float rps = (float)config.requests_finished/dt; printf("%s: %.2f\r", config.title, rps); @@ -451,6 +458,7 @@ int main(int argc, const char **argv) { config.randomkeys = 0; config.randomkeys_keyspacelen = 0; config.quiet = 0; + config.csv = 0; config.loop = 0; config.idlemode = 0; config.latency = NULL; @@ -564,7 +572,7 @@ int main(int argc, const char **argv) { benchmark("LRANGE (first 600 elements)",cmd,len); free(cmd); - printf("\n"); + if (!config.csv) printf("\n"); } while(config.loop); return 0; From 9f080a01fac2353a1c3a855729cd337ef4384c5a Mon Sep 17 00:00:00 2001 From: antirez Date: Fri, 4 Nov 2011 20:45:46 +0100 Subject: [PATCH 12/21] first version of the speed regression test --- utils/speed-regression.tcl | 85 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) create mode 100755 utils/speed-regression.tcl diff --git a/utils/speed-regression.tcl b/utils/speed-regression.tcl new file mode 100755 index 00000000..503a0799 --- /dev/null +++ b/utils/speed-regression.tcl @@ -0,0 +1,85 @@ +#!/usr/bin/env tclsh8.5 +# Copyright (C) 2011 Salvatore Sanfilippo +# Released under the BSD license like Redis itself + +proc run-tests branches { + set runs {} + set branch_id 0 + foreach b $branches { + cd ../src + puts "Benchmarking $b" + exec -ignorestderr git checkout $b 2> /dev/null + exec -ignorestderr make clean 2> /dev/null + puts " compiling..." + exec -ignorestderr make 2> /dev/null + + if {$branch_id == 0} { + puts " copy redis-benchmark from unstable to /tmp..." + exec -ignorestderr cp ./redis-benchmark /tmp + incr branch_id + continue + } + + # Start the Redis server + puts " starting the server... [exec ./redis-server -v]" + set pids [exec echo "port 12123\nloglevel warning\n" | ./redis-server - > /dev/null 2> /dev/null &] + after 1000 + puts " running the benchmark" + set output [exec /tmp/redis-benchmark -n 100000 --csv -p 12123] + lappend runs $b $output + puts " killing server..." + catch { + exec kill -9 [lindex $pids 0] + exec kill -9 [lindex $pids 1] + } + incr branch_id + } + return $runs +} + +proc get-result-with-name {output name} { + foreach line [split $output "\n"] { + lassign [split $line ","] key value + set key [string tolower [string range $key 1 end-1]] + set value [string range $value 1 end-1] + if {$key eq [string tolower $name]} { + return $value + } + } + return "n/a" +} + +proc combine-results {results} { + set tests { + ping set get incr lpush lpop sadd spop + "lrange (first 100 elements)" + "lrange (first 600 elements)" + "mset (10 keys)" + } + foreach test $tests { + puts $test + foreach {branch output} $results { + puts [format "%-20s %s" \ + $branch [get-result-with-name $output $test]] + } + puts {} + } +} + +proc main {} { + # Note: the first branch is only used in order to get the redis-benchmark + # executable. Tests are performed starting from the second branch. + set branches { + slowset 2.2.0 2.4.0 unstable slowset + } + set results [run-tests $branches] + puts [combine-results $results] +} + +# Force the user to run the script from the 'utils' directory. +if {![file exists speed-regression.tcl]} { + puts "Please make sure to run speed-regression.tcl while inside /utils." + puts "Example: cd utils; ./speed-regression.tcl" + exit 1 +} +main From d9747b496e06e6647cd9d4dbc4e8b89eda5f2ccc Mon Sep 17 00:00:00 2001 From: antirez Date: Mon, 7 Nov 2011 11:29:37 +0100 Subject: [PATCH 13/21] redis-benchmark: ability to run selected tests. Better help with examples. --- src/redis-benchmark.c | 211 ++++++++++++++++++++++++++++-------------- 1 file changed, 143 insertions(+), 68 deletions(-) diff --git a/src/redis-benchmark.c b/src/redis-benchmark.c index 49432030..b22322f4 100644 --- a/src/redis-benchmark.c +++ b/src/redis-benchmark.c @@ -71,6 +71,7 @@ static struct config { int csv; int loop; int idlemode; + char *tests; } config; typedef struct _client { @@ -385,6 +386,17 @@ int parseOptions(int argc, const char **argv) { config.loop = 1; } else if (!strcmp(argv[i],"-I")) { config.idlemode = 1; + } else if (!strcmp(argv[i],"-t")) { + if (lastarg) goto invalid; + /* We get the list of tests to run as a string in the form + * get,set,lrange,...,test_N. Then we add a comma before and + * after the string in order to make sure that searching + * for ",testname," will always get a match if the test is + * enabled. */ + config.tests = sdsnew(","); + config.tests = sdscat(config.tests,(char*)argv[++i]); + config.tests = sdscat(config.tests,","); + sdstolower(config.tests); } else if (!strcmp(argv[i],"--help")) { exit_status = 0; goto usage; @@ -403,25 +415,38 @@ invalid: printf("Invalid option \"%s\" or option argument missing\n\n",argv[i]); usage: - printf("Usage: redis-benchmark [-h ] [-p ] [-c ] [-n [-k ]\n\n"); - printf(" -h Server hostname (default 127.0.0.1)\n"); - printf(" -p Server port (default 6379)\n"); - printf(" -s Server socket (overrides host and port)\n"); - printf(" -c Number of parallel connections (default 50)\n"); - printf(" -n Total number of requests (default 10000)\n"); - printf(" -d Data size of SET/GET value in bytes (default 2)\n"); - printf(" -k 1=keep alive 0=reconnect (default 1)\n"); - printf(" -r Use random keys for SET/GET/INCR, random values for SADD\n"); - printf(" Using this option the benchmark will get/set keys\n"); - printf(" in the form mykey_rand000000012456 instead of constant\n"); - printf(" keys, the argument determines the max\n"); - printf(" number of values for the random number. For instance\n"); - printf(" if set to 10 only rand000000000000 - rand000000000009\n"); - printf(" range will be allowed.\n"); - printf(" -q Quiet. Just show query/sec values\n"); - printf(" --csv Output in CSV format\n"); - printf(" -l Loop. Run the tests forever\n"); - printf(" -I Idle mode. Just open N idle connections and wait.\n"); + printf( +"Usage: redis-benchmark [-h ] [-p ] [-c ] [-n [-k ]\n\n" +" -h Server hostname (default 127.0.0.1)\n" +" -p Server port (default 6379)\n" +" -s Server socket (overrides host and port)\n" +" -c Number of parallel connections (default 50)\n" +" -n Total number of requests (default 10000)\n" +" -d Data size of SET/GET value in bytes (default 2)\n" +" -k 1=keep alive 0=reconnect (default 1)\n" +" -r Use random keys for SET/GET/INCR, random values for SADD\n" +" Using this option the benchmark will get/set keys\n" +" in the form mykey_rand000000012456 instead of constant\n" +" keys, the argument determines the max\n" +" number of values for the random number. For instance\n" +" if set to 10 only rand000000000000 - rand000000000009\n" +" range will be allowed.\n" +" -q Quiet. Just show query/sec values\n" +" --csv Output in CSV format\n" +" -l Loop. Run the tests forever\n" +" -t Only run the comma separated list of tests. The test\n" +" names are the same as the ones produced as output.\n" +" -I Idle mode. Just open N idle connections and wait.\n\n" +"Examples:\n\n" +" Run the benchmark with the default configuration against 127.0.0.1:6379:\n" +" $ redis-benchmark\n\n" +" Use 20 parallel clients, for a total of 100k requests, against 192.168.1.1:\n" +" $ redis-benchmark -h 192.168.1.1 -p 6379 -n 100000 -c 20\n\n" +" Fill 127.0.0.1:6379 with about 1 million keys only using the SET test:\n" +" $ redis-benchmark -t set -n 1000000 -r 100000000\n\n" +" Benchmark 127.0.0.1:6379 for a few commands producing CSV output:\n" +" $ redis-benchmark -t ping,set,get -n 100000 --csv\n\n" + ); exit(exit_status); } @@ -438,6 +463,20 @@ int showThroughput(struct aeEventLoop *eventLoop, long long id, void *clientData return 250; /* every 250ms */ } +/* Return true if the named test was selected using the -t command line + * switch, or if all the tests are selected (no -t passed by user). */ +int test_is_selected(char *name) { + char buf[256]; + int l = strlen(name); + + if (config.tests == NULL) return 1; + buf[0] = ','; + memcpy(buf+1,name,l); + buf[l+1] = ','; + buf[l+2] = '\0'; + return strstr(config.tests,buf) != NULL; +} + int main(int argc, const char **argv) { int i; char *data, *cmd; @@ -466,6 +505,7 @@ int main(int argc, const char **argv) { config.hostip = "127.0.0.1"; config.hostport = 6379; config.hostsocket = NULL; + config.tests = NULL; i = parseOptions(argc,argv); argc -= i; @@ -508,69 +548,104 @@ int main(int argc, const char **argv) { memset(data,'x',config.datasize); data[config.datasize] = '\0'; - benchmark("PING (inline)","PING\r\n",6); + if (test_is_selected("ping_inline") || test_is_selected("ping")) + benchmark("PING_INLINE","PING\r\n",6); - len = redisFormatCommand(&cmd,"PING"); - benchmark("PING",cmd,len); - free(cmd); - - const char *argv[21]; - argv[0] = "MSET"; - for (i = 1; i < 21; i += 2) { - argv[i] = "foo:rand:000000000000"; - argv[i+1] = data; + if (test_is_selected("ping_mbulk") || test_is_selected("ping")) { + len = redisFormatCommand(&cmd,"PING"); + benchmark("PING_BULK",cmd,len); + free(cmd); } - len = redisFormatCommandArgv(&cmd,21,argv,NULL); - benchmark("MSET (10 keys)",cmd,len); - free(cmd); - len = redisFormatCommand(&cmd,"SET foo:rand:000000000000 %s",data); - benchmark("SET",cmd,len); - free(cmd); + if (test_is_selected("set")) { + len = redisFormatCommand(&cmd,"SET foo:rand:000000000000 %s",data); + benchmark("SET",cmd,len); + free(cmd); + } - len = redisFormatCommand(&cmd,"GET foo:rand:000000000000"); - benchmark("GET",cmd,len); - free(cmd); + if (test_is_selected("get")) { + len = redisFormatCommand(&cmd,"GET foo:rand:000000000000"); + benchmark("GET",cmd,len); + free(cmd); + } - len = redisFormatCommand(&cmd,"INCR counter:rand:000000000000"); - benchmark("INCR",cmd,len); - free(cmd); + if (test_is_selected("incr")) { + len = redisFormatCommand(&cmd,"INCR counter:rand:000000000000"); + benchmark("INCR",cmd,len); + free(cmd); + } - len = redisFormatCommand(&cmd,"LPUSH mylist %s",data); - benchmark("LPUSH",cmd,len); - free(cmd); + if (test_is_selected("lpush")) { + len = redisFormatCommand(&cmd,"LPUSH mylist %s",data); + benchmark("LPUSH",cmd,len); + free(cmd); + } - len = redisFormatCommand(&cmd,"LPOP mylist"); - benchmark("LPOP",cmd,len); - free(cmd); + if (test_is_selected("lpop")) { + len = redisFormatCommand(&cmd,"LPOP mylist"); + benchmark("LPOP",cmd,len); + free(cmd); + } - len = redisFormatCommand(&cmd,"SADD myset counter:rand:000000000000"); - benchmark("SADD",cmd,len); - free(cmd); + if (test_is_selected("sadd")) { + len = redisFormatCommand(&cmd, + "SADD myset counter:rand:000000000000"); + benchmark("SADD",cmd,len); + free(cmd); + } - len = redisFormatCommand(&cmd,"SPOP myset"); - benchmark("SPOP",cmd,len); - free(cmd); + if (test_is_selected("spop")) { + len = redisFormatCommand(&cmd,"SPOP myset"); + benchmark("SPOP",cmd,len); + free(cmd); + } - len = redisFormatCommand(&cmd,"LPUSH mylist %s",data); - benchmark("LPUSH (again, in order to bench LRANGE)",cmd,len); - free(cmd); + if (test_is_selected("lrange") || + test_is_selected("lrange_100") || + test_is_selected("lrange_300") || + test_is_selected("lrange_500") || + test_is_selected("lrange_600")) + { + len = redisFormatCommand(&cmd,"LPUSH mylist %s",data); + benchmark("LPUSH (needed to benchmark LRANGE)",cmd,len); + free(cmd); + } - len = redisFormatCommand(&cmd,"LRANGE mylist 0 99"); - benchmark("LRANGE (first 100 elements)",cmd,len); - free(cmd); + if (test_is_selected("lrange") || test_is_selected("lrange_100")) { + len = redisFormatCommand(&cmd,"LRANGE mylist 0 99"); + benchmark("LRANGE_100 (first 100 elements)",cmd,len); + free(cmd); + } - len = redisFormatCommand(&cmd,"LRANGE mylist 0 299"); - benchmark("LRANGE (first 300 elements)",cmd,len); - free(cmd); + if (test_is_selected("lrange") || test_is_selected("lrange_300")) { + len = redisFormatCommand(&cmd,"LRANGE mylist 0 299"); + benchmark("LRANGE_300 (first 300 elements)",cmd,len); + free(cmd); + } - len = redisFormatCommand(&cmd,"LRANGE mylist 0 449"); - benchmark("LRANGE (first 450 elements)",cmd,len); - free(cmd); + if (test_is_selected("lrange") || test_is_selected("lrange_500")) { + len = redisFormatCommand(&cmd,"LRANGE mylist 0 449"); + benchmark("LRANGE_500 (first 450 elements)",cmd,len); + free(cmd); + } - len = redisFormatCommand(&cmd,"LRANGE mylist 0 599"); - benchmark("LRANGE (first 600 elements)",cmd,len); - free(cmd); + if (test_is_selected("lrange") || test_is_selected("lrange_600")) { + len = redisFormatCommand(&cmd,"LRANGE mylist 0 599"); + benchmark("LRANGE_600 (first 600 elements)",cmd,len); + free(cmd); + } + + if (test_is_selected("mset")) { + const char *argv[21]; + argv[0] = "MSET"; + for (i = 1; i < 21; i += 2) { + argv[i] = "foo:rand:000000000000"; + argv[i+1] = data; + } + len = redisFormatCommandArgv(&cmd,21,argv,NULL); + benchmark("MSET (10 keys)",cmd,len); + free(cmd); + } if (!config.csv) printf("\n"); } while(config.loop); From 84c6bdfcd0cd12f741bbb117648d057c26c85b89 Mon Sep 17 00:00:00 2001 From: antirez Date: Mon, 7 Nov 2011 15:35:01 +0100 Subject: [PATCH 14/21] speed-regression.tcl script: obtain test names dynamically. --- utils/speed-regression.tcl | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/utils/speed-regression.tcl b/utils/speed-regression.tcl index 503a0799..a37f6028 100755 --- a/utils/speed-regression.tcl +++ b/utils/speed-regression.tcl @@ -49,13 +49,18 @@ proc get-result-with-name {output name} { return "n/a" } -proc combine-results {results} { - set tests { - ping set get incr lpush lpop sadd spop - "lrange (first 100 elements)" - "lrange (first 600 elements)" - "mset (10 keys)" +proc get-test-names output { + set names {} + foreach line [split $output "\n"] { + lassign [split $line ","] key value + set key [string tolower [string range $key 1 end-1]] + lappend names $key } + return $names +} + +proc combine-results {results} { + set tests [get-test-names [lindex $results 1]] foreach test $tests { puts $test foreach {branch output} $results { From 85bc9b06b747939dcf4151a9a820957d2b37168b Mon Sep 17 00:00:00 2001 From: antirez Date: Mon, 7 Nov 2011 16:00:15 +0100 Subject: [PATCH 15/21] speed-regression.tcl script: killing previously tested instance fixed. Don't run if there is already an instance running in the same port. --- utils/speed-regression.tcl | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/utils/speed-regression.tcl b/utils/speed-regression.tcl index a37f6028..af8dd31d 100755 --- a/utils/speed-regression.tcl +++ b/utils/speed-regression.tcl @@ -2,6 +2,9 @@ # Copyright (C) 2011 Salvatore Sanfilippo # Released under the BSD license like Redis itself +source ../tests/support/redis.tcl +set ::port 12123 + proc run-tests branches { set runs {} set branch_id 0 @@ -22,16 +25,21 @@ proc run-tests branches { # Start the Redis server puts " starting the server... [exec ./redis-server -v]" - set pids [exec echo "port 12123\nloglevel warning\n" | ./redis-server - > /dev/null 2> /dev/null &] + set pids [exec echo "port $::port\nloglevel warning\n" | ./redis-server - > /dev/null 2> /dev/null &] + puts " pids: $pids" after 1000 puts " running the benchmark" - set output [exec /tmp/redis-benchmark -n 100000 --csv -p 12123] + + set r [redis 127.0.0.1 $::port] + set i [$r info] + puts " redis INFO shows version: [lindex [split $i] 0]" + $r close + + set output [exec /tmp/redis-benchmark -n 100000 --csv -p $::port] lappend runs $b $output puts " killing server..." - catch { - exec kill -9 [lindex $pids 0] - exec kill -9 [lindex $pids 1] - } + catch {exec kill -9 [lindex $pids 0]} + catch {exec kill -9 [lindex $pids 1]} incr branch_id } return $runs @@ -87,4 +95,12 @@ if {![file exists speed-regression.tcl]} { puts "Example: cd utils; ./speed-regression.tcl" exit 1 } + +# Make sure there is not already a server runnign on port 12123 +set is_not_running [catch {set r [redis 127.0.0.1 $::port]}] +if {!$is_not_running} { + puts "Sorry, you have a running server on port $::port" + exit 1 +} + main From 55758a5fab86390b37b584c3ee6b6d80bb98c320 Mon Sep 17 00:00:00 2001 From: antirez Date: Mon, 7 Nov 2011 16:52:55 +0100 Subject: [PATCH 16/21] speed-regression.tcl: move tests, data size, requests in global vars that will be changed via command line options. --- utils/speed-regression.tcl | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/utils/speed-regression.tcl b/utils/speed-regression.tcl index af8dd31d..c3bf51c9 100755 --- a/utils/speed-regression.tcl +++ b/utils/speed-regression.tcl @@ -4,6 +4,9 @@ source ../tests/support/redis.tcl set ::port 12123 +set ::tests {PING,SET,GET,INCR,LPUSH,LPOP,SADD,SPOP,LRANGE_100,LRANGE_600,MSET} +set ::datasize 16 +set ::requests 100000 proc run-tests branches { set runs {} @@ -35,7 +38,7 @@ proc run-tests branches { puts " redis INFO shows version: [lindex [split $i] 0]" $r close - set output [exec /tmp/redis-benchmark -n 100000 --csv -p $::port] + set output [exec /tmp/redis-benchmark -n $::requests -t $::tests -d $::datasize --csv -p $::port] lappend runs $b $output puts " killing server..." catch {exec kill -9 [lindex $pids 0]} @@ -86,6 +89,8 @@ proc main {} { slowset 2.2.0 2.4.0 unstable slowset } set results [run-tests $branches] + puts "\n" + puts "# Test results: datasize=$::datasize requests=$::requests" puts [combine-results $results] } From d5a80182870140ff338c9ad9a35d3698f3911bc1 Mon Sep 17 00:00:00 2001 From: antirez Date: Mon, 7 Nov 2011 17:18:50 +0100 Subject: [PATCH 17/21] speed-regression.tcl: command line options to select tests, data size, and number of requests. --- utils/speed-regression.tcl | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/utils/speed-regression.tcl b/utils/speed-regression.tcl index c3bf51c9..86a7d8d8 100755 --- a/utils/speed-regression.tcl +++ b/utils/speed-regression.tcl @@ -108,4 +108,23 @@ if {!$is_not_running} { exit 1 } +# parse arguments +for {set j 0} {$j < [llength $argv]} {incr j} { + set opt [lindex $argv $j] + set arg [lindex $argv [expr $j+1]] + if {$opt eq {--tests}} { + set ::tests $arg + incr j + } elseif {$opt eq {--datasize}} { + set ::datasize $arg + incr j + } elseif {$opt eq {--requests}} { + set ::requests $arg + incr j + } else { + puts "Wrong argument: $opt" + exit 1 + } +} + main From 65330badb97206cd7cf0973d3f2267b0a523c05e Mon Sep 17 00:00:00 2001 From: antirez Date: Tue, 8 Nov 2011 10:59:35 +0100 Subject: [PATCH 18/21] hiredis/redis changes for speed with big payloads: read buffer size set to 16k, request buffer size is no longer destroyed when emtpy and large (better fix needed). Redis clients static output buffer set to 16k as well. --- deps/hiredis/hiredis.c | 5 +++-- src/redis.h | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/deps/hiredis/hiredis.c b/deps/hiredis/hiredis.c index b27c63b8..976e94f9 100644 --- a/deps/hiredis/hiredis.c +++ b/deps/hiredis/hiredis.c @@ -520,13 +520,14 @@ void redisReplyReaderFeed(void *reader, const char *buf, size_t len) { /* Copy the provided buffer. */ if (buf != NULL && len >= 1) { +#if 0 /* Destroy internal buffer when it is empty and is quite large. */ if (r->len == 0 && sdsavail(r->buf) > 16*1024) { sdsfree(r->buf); r->buf = sdsempty(); r->pos = 0; } - +#endif r->buf = sdscatlen(r->buf,buf,len); r->len = sdslen(r->buf); } @@ -901,7 +902,7 @@ static void __redisCreateReplyReader(redisContext *c) { * After this function is called, you may use redisContextReadReply to * see if there is a reply available. */ int redisBufferRead(redisContext *c) { - char buf[2048]; + char buf[1024*16]; int nread = read(c->fd,buf,sizeof(buf)); if (nread == -1) { if (errno == EAGAIN && !(c->flags & REDIS_BLOCK)) { diff --git a/src/redis.h b/src/redis.h index b5e954fd..8cc2474f 100644 --- a/src/redis.h +++ b/src/redis.h @@ -49,7 +49,7 @@ #define REDIS_MAX_WRITE_PER_EVENT (1024*64) #define REDIS_REQUEST_MAX_SIZE (1024*1024*256) /* max bytes in inline command */ #define REDIS_SHARED_INTEGERS 10000 -#define REDIS_REPLY_CHUNK_BYTES (5*1500) /* 5 TCP packets with default MTU */ +#define REDIS_REPLY_CHUNK_BYTES (16*1024) /* 16k output buffer */ #define REDIS_MAX_LOGMSG_LEN 1024 /* Default maximum length of syslog messages */ #define REDIS_AUTO_AOFREWRITE_PERC 100 #define REDIS_AUTO_AOFREWRITE_MIN_SIZE (1024*1024) From 53272781d0b7f664fc6e3fcd5c9032dd09103bbc Mon Sep 17 00:00:00 2001 From: antirez Date: Tue, 8 Nov 2011 11:22:40 +0100 Subject: [PATCH 19/21] Multi bulk optimization for creating big objects without copying data is no longer optional, #ifdefs removed. Also debugging messages removed. --- src/networking.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/networking.c b/src/networking.c index 51c0ac61..21064bf8 100644 --- a/src/networking.c +++ b/src/networking.c @@ -767,7 +767,6 @@ int processMultibulkBuffer(redisClient *c) { } pos += newline-(c->querybuf+pos)+2; -#ifdef REDIS_MBULK_BIG_ARG if (ll >= REDIS_MBULK_BIG_ARG) { /* If we are going to read a large object from network * try to make it likely that it will start at c->querybuf @@ -780,7 +779,6 @@ int processMultibulkBuffer(redisClient *c) { * going to contain. */ if (ll >= REDIS_MBULK_BIG_ARG) c->querybuf = sdsMakeRoomFor(c->querybuf,ll+2); -#endif c->bulklen = ll; } @@ -789,7 +787,6 @@ int processMultibulkBuffer(redisClient *c) { /* Not enough data (+2 == trailing \r\n) */ break; } else { -#ifdef REDIS_MBULK_BIG_ARG /* Optimization: if the buffer contanins JUST our bulk element * instead of creating a new object by *copying* the sds we * just use the current sds string. */ @@ -797,7 +794,6 @@ int processMultibulkBuffer(redisClient *c) { c->bulklen >= REDIS_MBULK_BIG_ARG && (signed) sdslen(c->querybuf) == c->bulklen+2) { - // printf("HERE (arg %d)\n",c->argc); c->argv[c->argc++] = createObject(REDIS_STRING,c->querybuf); sdsIncrLen(c->querybuf,-2); /* remove CRLF */ c->querybuf = sdsempty(); @@ -806,14 +802,10 @@ int processMultibulkBuffer(redisClient *c) { c->querybuf = sdsMakeRoomFor(c->querybuf,c->bulklen+2); pos = 0; } else { -#endif - // printf("NOT HERE (arg %d) (pos %d)\n",c->argc, pos); c->argv[c->argc++] = createStringObject(c->querybuf+pos,c->bulklen); pos += c->bulklen+2; -#ifdef REDIS_MBULK_BIG_ARG } -#endif c->bulklen = -1; c->multibulklen--; } From b0a2e34059b4b442a25fc7e25afde39706ad9034 Mon Sep 17 00:00:00 2001 From: antirez Date: Tue, 8 Nov 2011 11:24:12 +0100 Subject: [PATCH 20/21] yet another #if REDIS_MBULK_BIG_ARG removed. --- src/networking.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/networking.c b/src/networking.c index 21064bf8..3e698cd5 100644 --- a/src/networking.c +++ b/src/networking.c @@ -868,7 +868,6 @@ void readQueryFromClient(aeEventLoop *el, int fd, void *privdata, int mask) { REDIS_NOTUSED(mask); readlen = REDIS_IOBUF_LEN; -#if REDIS_MBULK_BIG_ARG /* If this is a multi bulk request, and we are processing a bulk reply * that is large enough, try to maximize the probabilty that the query * buffer contains excatly the SDS string representing the object, even @@ -882,7 +881,6 @@ void readQueryFromClient(aeEventLoop *el, int fd, void *privdata, int mask) { if (remaining < readlen) readlen = remaining; } -#endif qblen = sdslen(c->querybuf); c->querybuf = sdsMakeRoomFor(c->querybuf, readlen); From b90314588fc4863f2fdb6bec0a46d48385c66994 Mon Sep 17 00:00:00 2001 From: antirez Date: Tue, 8 Nov 2011 11:26:06 +0100 Subject: [PATCH 21/21] useless double if removed. --- src/networking.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/networking.c b/src/networking.c index 3e698cd5..edd7891d 100644 --- a/src/networking.c +++ b/src/networking.c @@ -774,11 +774,10 @@ int processMultibulkBuffer(redisClient *c) { * avoiding a large copy of data. */ c->querybuf = sdsrange(c->querybuf,pos,-1); pos = 0; - } - /* Hint the sds library about the amount of bytes this string is - * going to contain. */ - if (ll >= REDIS_MBULK_BIG_ARG) + /* Hint the sds library about the amount of bytes this string is + * going to contain. */ c->querybuf = sdsMakeRoomFor(c->querybuf,ll+2); + } c->bulklen = ll; }