From b857928ba7b1d40ef96de7b94bf115e2e59e3075 Mon Sep 17 00:00:00 2001 From: filipe oliveira Date: Wed, 23 Feb 2022 16:15:12 +0000 Subject: [PATCH] Optimize deferred replies to use shared objects instead of sprintf (#10334) Avoid sprintf/ll2string on setDeferredAggregateLen()/addReplyLongLongWithPrefix() when we can used shared objects. In some pipelined workloads this achieves about 10% improvement. Co-authored-by: Oran Agra --- src/networking.c | 29 +++++++++++++++++++++++++++-- src/server.c | 4 ++++ src/server.h | 5 ++++- 3 files changed, 35 insertions(+), 3 deletions(-) diff --git a/src/networking.c b/src/networking.c index 27bfbad8b..8412c0960 100644 --- a/src/networking.c +++ b/src/networking.c @@ -714,6 +714,24 @@ void setDeferredAggregateLen(client *c, void *node, long length, char prefix) { * we return NULL in addReplyDeferredLen() */ if (node == NULL) return; + /* Things like *2\r\n, %3\r\n or ~4\r\n are emitted very often by the protocol + * so we have a few shared objects to use if the integer is small + * like it is most of the times. */ + const size_t hdr_len = OBJ_SHARED_HDR_STRLEN(length); + const int opt_hdr = length < OBJ_SHARED_BULKHDR_LEN; + if (prefix == '*' && opt_hdr) { + setDeferredReply(c, node, shared.mbulkhdr[length]->ptr, hdr_len); + return; + } + if (prefix == '%' && opt_hdr) { + setDeferredReply(c, node, shared.maphdr[length]->ptr, hdr_len); + return; + } + if (prefix == '~' && opt_hdr) { + setDeferredReply(c, node, shared.sethdr[length]->ptr, hdr_len); + return; + } + char lenstr[128]; size_t lenstr_len = sprintf(lenstr, "%c%ld\r\n", prefix, length); setDeferredReply(c, node, lenstr, lenstr_len); @@ -806,12 +824,19 @@ void addReplyLongLongWithPrefix(client *c, long long ll, char prefix) { /* Things like $3\r\n or *2\r\n are emitted very often by the protocol * so we have a few shared objects to use if the integer is small * like it is most of the times. */ - if (prefix == '*' && ll < OBJ_SHARED_BULKHDR_LEN && ll >= 0) { + const int opt_hdr = ll < OBJ_SHARED_BULKHDR_LEN && ll >= 0; + if (prefix == '*' && opt_hdr) { addReply(c,shared.mbulkhdr[ll]); return; - } else if (prefix == '$' && ll < OBJ_SHARED_BULKHDR_LEN && ll >= 0) { + } else if (prefix == '$' && opt_hdr) { addReply(c,shared.bulkhdr[ll]); return; + } else if (prefix == '%' && opt_hdr) { + addReply(c,shared.maphdr[ll]); + return; + } else if (prefix == '~' && opt_hdr) { + addReply(c,shared.sethdr[ll]); + return; } buf[0] = prefix; diff --git a/src/server.c b/src/server.c index b3a340aaf..3f2dffa79 100644 --- a/src/server.c +++ b/src/server.c @@ -1790,6 +1790,10 @@ void createSharedObjects(void) { sdscatprintf(sdsempty(),"*%d\r\n",j)); shared.bulkhdr[j] = createObject(OBJ_STRING, sdscatprintf(sdsempty(),"$%d\r\n",j)); + shared.maphdr[j] = createObject(OBJ_STRING, + sdscatprintf(sdsempty(),"%%%d\r\n",j)); + shared.sethdr[j] = createObject(OBJ_STRING, + sdscatprintf(sdsempty(),"~%d\r\n",j)); } /* The following two shared objects, minstring and maxstring, are not * actually used for their value but as a special object meaning diff --git a/src/server.h b/src/server.h index 3fe8686c5..82a8549ff 100644 --- a/src/server.h +++ b/src/server.h @@ -109,6 +109,7 @@ typedef long long ustime_t; /* microsecond time type. */ #define PROTO_SHARED_SELECT_CMDS 10 #define OBJ_SHARED_INTEGERS 10000 #define OBJ_SHARED_BULKHDR_LEN 32 +#define OBJ_SHARED_HDR_STRLEN(_len_) (((_len_) < 10) ? 4 : 5) /* see shared.mbulkhdr etc. */ #define LOG_MAX_LEN 1024 /* Default maximum length of syslog messages.*/ #define AOF_REWRITE_ITEMS_PER_CMD 64 #define AOF_ANNOTATION_LINE_MAX_LEN 1024 @@ -1229,7 +1230,9 @@ struct sharedObjectsStruct { *select[PROTO_SHARED_SELECT_CMDS], *integers[OBJ_SHARED_INTEGERS], *mbulkhdr[OBJ_SHARED_BULKHDR_LEN], /* "*\r\n" */ - *bulkhdr[OBJ_SHARED_BULKHDR_LEN]; /* "$\r\n" */ + *bulkhdr[OBJ_SHARED_BULKHDR_LEN], /* "$\r\n" */ + *maphdr[OBJ_SHARED_BULKHDR_LEN], /* "%\r\n" */ + *sethdr[OBJ_SHARED_BULKHDR_LEN]; /* "~\r\n" */ sds minstring, maxstring; };