Skip to content

Commit

Permalink
Reduce redundant call of prepareClientToWrite when call addReply* con…
Browse files Browse the repository at this point in the history
…tinuously (redis#13412)

This PR is based on the commits from PR
valkey-io/valkey#670.

## Description

While exploring hotspots with profiling some benchmark workloads, we
noticed the high cycles ratio of `prepareClientToWrite`, taking about 9%
of the CPU of `smembers`, `lrange` commands. After deep dive the code
logic, we thought we can gain the performance by reducing the redundant
call of `prepareClientToWrite` when call addReply* continuously.

For example: In

https://github.com/valkey-io/valkey/blob/unstable/src/networking.c#L1080-L1082,
`prepareClientToWrite` is called three times in a row.

---------

Signed-off-by: Lipeng Zhu <lipeng.zhu@intel.com>
Co-authored-by: Lipeng Zhu <lipeng.zhu@intel.com>
Co-authored-by: Wangyang Guo <wangyang.guo@intel.com>
  • Loading branch information
3 people committed Aug 29, 2024
1 parent 2734724 commit c4e7424
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 24 deletions.
58 changes: 36 additions & 22 deletions src/networking.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,13 @@
* Copyright (c) 2009-Present, Redis Ltd.
* All rights reserved.
*
* Copyright (c) 2024-present, Valkey contributors.
* All rights reserved.
*
* Licensed under your choice of the Redis Source Available License 2.0
* (RSALv2) or the Server Side Public License v1 (SSPLv1).
*
* Portions of this file are available under BSD3 terms; see REDISCONTRIBUTIONS for more information.
*/

#include "server.h"
Expand Down Expand Up @@ -924,7 +929,7 @@ void addReplyHumanLongDouble(client *c, long double d) {

/* Add a long long as integer reply or bulk len / multi bulk count.
* Basically this is used to output <prefix><long long><crlf>. */
void addReplyLongLongWithPrefix(client *c, long long ll, char prefix) {
static void _addReplyLongLongWithPrefix(client *c, long long ll, char prefix) {
char buf[128];
int len;

Expand All @@ -934,38 +939,41 @@ void addReplyLongLongWithPrefix(client *c, long long ll, char prefix) {
const int opt_hdr = ll < OBJ_SHARED_BULKHDR_LEN && ll >= 0;
const size_t hdr_len = OBJ_SHARED_HDR_STRLEN(ll);
if (prefix == '*' && opt_hdr) {
addReplyProto(c,shared.mbulkhdr[ll]->ptr,hdr_len);
_addReplyToBufferOrList(c, shared.mbulkhdr[ll]->ptr, hdr_len);
return;
} else if (prefix == '$' && opt_hdr) {
addReplyProto(c,shared.bulkhdr[ll]->ptr,hdr_len);
_addReplyToBufferOrList(c, shared.bulkhdr[ll]->ptr, hdr_len);
return;
} else if (prefix == '%' && opt_hdr) {
addReplyProto(c,shared.maphdr[ll]->ptr,hdr_len);
_addReplyToBufferOrList(c, shared.maphdr[ll]->ptr, hdr_len);
return;
} else if (prefix == '~' && opt_hdr) {
addReplyProto(c,shared.sethdr[ll]->ptr,hdr_len);
_addReplyToBufferOrList(c, shared.sethdr[ll]->ptr, hdr_len);
return;
}

buf[0] = prefix;
len = ll2string(buf+1,sizeof(buf)-1,ll);
buf[len+1] = '\r';
buf[len+2] = '\n';
addReplyProto(c,buf,len+3);
len = ll2string(buf + 1, sizeof(buf) - 1, ll);
buf[len + 1] = '\r';
buf[len + 2] = '\n';
_addReplyToBufferOrList(c, buf, len + 3);
}

void addReplyLongLong(client *c, long long ll) {
if (ll == 0)
addReply(c,shared.czero);
else if (ll == 1)
addReply(c,shared.cone);
else
addReplyLongLongWithPrefix(c,ll,':');
addReply(c, shared.cone);
else {
if (prepareClientToWrite(c) != C_OK) return;
_addReplyLongLongWithPrefix(c, ll, ':');
}
}

void addReplyAggregateLen(client *c, long length, int prefix) {
serverAssert(length >= 0);
addReplyLongLongWithPrefix(c,length,prefix);
if (prepareClientToWrite(c) != C_OK) return;
_addReplyLongLongWithPrefix(c, length, prefix);
}

void addReplyArrayLen(client *c, long length) {
Expand Down Expand Up @@ -1025,8 +1033,8 @@ void addReplyNullArray(client *c) {
/* Create the length prefix of a bulk reply, example: $2234 */
void addReplyBulkLen(client *c, robj *obj) {
size_t len = stringObjectLen(obj);

addReplyLongLongWithPrefix(c,len,'$');
if (prepareClientToWrite(c) != C_OK) return;
_addReplyLongLongWithPrefix(c, len, '$');
}

/* Add a Redis Object as a bulk reply */
Expand All @@ -1038,16 +1046,22 @@ void addReplyBulk(client *c, robj *obj) {

/* Add a C buffer as bulk reply */
void addReplyBulkCBuffer(client *c, const void *p, size_t len) {
addReplyLongLongWithPrefix(c,len,'$');
addReplyProto(c,p,len);
addReplyProto(c,"\r\n",2);
if (prepareClientToWrite(c) != C_OK) return;
_addReplyLongLongWithPrefix(c, len, '$');
_addReplyToBufferOrList(c, p, len);
_addReplyToBufferOrList(c, "\r\n", 2);
}

/* Add sds to reply (takes ownership of sds and frees it) */
void addReplyBulkSds(client *c, sds s) {
addReplyLongLongWithPrefix(c,sdslen(s),'$');
addReplySds(c,s);
addReplyProto(c,"\r\n",2);
void addReplyBulkSds(client *c, sds s) {
if (prepareClientToWrite(c) != C_OK) {
sdsfree(s);
return;
}
_addReplyLongLongWithPrefix(c, sdslen(s), '$');
_addReplyToBufferOrList(c, s, sdslen(s));
sdsfree(s);
_addReplyToBufferOrList(c, "\r\n", 2);
}

/* Set sds to a deferred reply (for symmetry with addReplyBulkSds it also frees the sds) */
Expand Down
6 changes: 4 additions & 2 deletions src/server.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
* Copyright (c) 2009-Present, Redis Ltd.
* All rights reserved.
*
* Copyright (c) 2024-present, Valkey contributors.
* All rights reserved.
*
* Licensed under your choice of the Redis Source Available License 2.0
* (RSALv2) or the Server Side Public License v1 (SSPLv1).
*
Expand Down Expand Up @@ -2620,8 +2623,7 @@ void addReplyErrorArity(client *c);
void addReplyErrorExpireTime(client *c);
void addReplyStatus(client *c, const char *status);
void addReplyDouble(client *c, double d);
void addReplyLongLongWithPrefix(client *c, long long ll, char prefix);
void addReplyBigNum(client *c, const char* num, size_t len);
void addReplyBigNum(client *c, const char *num, size_t len);
void addReplyHumanLongDouble(client *c, long double d);
void addReplyLongLong(client *c, long long ll);
void addReplyArrayLen(client *c, long length);
Expand Down

0 comments on commit c4e7424

Please sign in to comment.