This is an automated email from the ASF dual-hosted git repository.
chenBright pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brpc.git
The following commit(s) were added to refs/heads/master by this push:
new d0f3f933 Optimize Redis command formatting buffer handling (#3609)
d0f3f933 is described below
commit d0f3f933cc051af3e760f169a7e95a515d9faf29
Author: nkysg <[email protected]>
AuthorDate: Sun Oct 18 19:09:30 2026 +0800
Optimize Redis command formatting buffer handling (#3609)
* Optimize Redis command formatting buffer handling
* Handle Redis command formatting append failures
* remove unused function
---
src/brpc/redis_command.cpp | 70 +++++++++++++++++++++++++++++++-------------
test/brpc_redis_unittest.cpp | 25 ++++++++++++++++
2 files changed, 74 insertions(+), 21 deletions(-)
diff --git a/src/brpc/redis_command.cpp b/src/brpc/redis_command.cpp
index 2513d160..5f30d049 100644
--- a/src/brpc/redis_command.cpp
+++ b/src/brpc/redis_command.cpp
@@ -68,7 +68,7 @@ inline size_t AppendDecimal(char* outbuf, unsigned long d) {
// short or does not have many %. In a 100K-time call to formating of
// "GET key1", the time spent on RedisRequest.AddCommand() are ~700ns
// vs. ~400ns while using snprintf() vs. AppendDecimal() respectively.
-inline void AppendHeader(std::string& buf, char fc, unsigned long value) {
+inline void AppendHeader(butil::IOBuf& buf, char fc, unsigned long value) {
char header[32];
header[0] = fc;
size_t len = AppendDecimal(header + 1, value);
@@ -76,21 +76,31 @@ inline void AppendHeader(std::string& buf, char fc,
unsigned long value) {
header[len + 2] = '\n';
buf.append(header, len + 3);
}
-inline void AppendHeader(butil::IOBuf& buf, char fc, unsigned long value) {
+
+inline int AppendHeader(butil::IOBufAppender& buf, char fc,
+ unsigned long value) {
char header[32];
header[0] = fc;
size_t len = AppendDecimal(header + 1, value);
header[len + 1] = '\r';
header[len + 2] = '\n';
- buf.append(header, len + 3);
+ return buf.append(header, len + 3);
}
-static void FlushComponent(std::string* out, std::string* compbuf, int* ncomp)
{
- AppendHeader(*out, '$', compbuf->size());
- out->append(*compbuf);
- out->append("\r\n", 2);
+static int FlushComponent(butil::IOBufAppender* out, std::string* compbuf,
+ int* ncomp) {
+ if (AppendHeader(*out, '$', compbuf->size()) != 0 ||
+ out->append(compbuf->data(), compbuf->size()) != 0 ||
+ out->append("\r\n", 2) != 0) {
+ return -1;
+ }
compbuf->clear();
++*ncomp;
+ return 0;
+}
+
+inline butil::Status AppendFailed() {
+ return butil::Status(ENOMEM, "Fail to append Redis command");
}
// Support hiredis-style format, namely everything is same with printf except
@@ -105,8 +115,7 @@ RedisCommandFormatV(butil::IOBuf* outbuf, const char* fmt,
va_list ap) {
return butil::Status(EINVAL, "Param[outbuf] or [fmt] is NULL");
}
const size_t fmt_len = strlen(fmt);
- std::string nocount_buf;
- nocount_buf.reserve(fmt_len * 3 / 2 + 16);
+ butil::IOBufAppender nocount_buf;
std::string compbuf; // A component
compbuf.reserve(fmt_len + 16);
const char* c = fmt;
@@ -123,7 +132,9 @@ RedisCommandFormatV(butil::IOBuf* outbuf, const char* fmt,
va_list ap) {
if (quote_char) {
compbuf.push_back(*c);
} else if (!compbuf.empty() || format_arg_in_comp) {
- FlushComponent(&nocount_buf, &compbuf, &ncomponent);
+ if (FlushComponent(&nocount_buf, &compbuf, &ncomponent) !=
0) {
+ return AppendFailed();
+ }
format_arg_in_comp = false;
}
} else if (*c == '"' || *c == '\'') { // Check quotation.
@@ -131,7 +142,9 @@ RedisCommandFormatV(butil::IOBuf* outbuf, const char* fmt,
va_list ap) {
quote_char = *c;
quote_pos = c;
if (!compbuf.empty() || format_arg_in_comp) {
- FlushComponent(&nocount_buf, &compbuf, &ncomponent);
+ if (FlushComponent(&nocount_buf, &compbuf,
&ncomponent) != 0) {
+ return AppendFailed();
+ }
format_arg_in_comp = false;
}
} else if (quote_char == *c) {
@@ -144,7 +157,9 @@ RedisCommandFormatV(butil::IOBuf* outbuf, const char* fmt,
va_list ap) {
compbuf.push_back(*c);
} else { // end quote
quote_char = 0;
- FlushComponent(&nocount_buf, &compbuf, &ncomponent);
+ if (FlushComponent(&nocount_buf, &compbuf,
&ncomponent) != 0) {
+ return AppendFailed();
+ }
format_arg_in_comp = false;
}
} else {
@@ -323,7 +338,9 @@ RedisCommandFormatV(butil::IOBuf* outbuf, const char* fmt,
va_list ap) {
}
if (!compbuf.empty() || format_arg_in_comp) {
- FlushComponent(&nocount_buf, &compbuf, &ncomponent);
+ if (FlushComponent(&nocount_buf, &compbuf, &ncomponent) != 0) {
+ return AppendFailed();
+ }
format_arg_in_comp = false;
}
@@ -332,7 +349,9 @@ RedisCommandFormatV(butil::IOBuf* outbuf, const char* fmt,
va_list ap) {
"formatting of conversion specifiers)";
AppendHeader(*outbuf, '*', ncomponent);
- outbuf->append(nocount_buf);
+ if (ncomponent != 0) {
+ outbuf->append(nocount_buf.buf().movable());
+ }
return butil::Status::OK();
}
@@ -350,8 +369,7 @@ RedisCommandNoFormat(butil::IOBuf* outbuf, const
butil::StringPiece& cmd) {
return butil::Status(EINVAL, "Param[outbuf] or [cmd] is NULL");
}
const size_t cmd_len = cmd.size();
- std::string nocount_buf;
- nocount_buf.reserve(cmd_len * 3 / 2 + 16);
+ butil::IOBufAppender nocount_buf;
std::string compbuf; // A component
compbuf.reserve(cmd_len + 16);
int ncomponent = 0;
@@ -362,14 +380,18 @@ RedisCommandNoFormat(butil::IOBuf* outbuf, const
butil::StringPiece& cmd) {
if (quote_char) {
compbuf.push_back(*c);
} else if (!compbuf.empty()) {
- FlushComponent(&nocount_buf, &compbuf, &ncomponent);
+ if (FlushComponent(&nocount_buf, &compbuf, &ncomponent) != 0) {
+ return AppendFailed();
+ }
}
} else if (*c == '"' || *c == '\'') { // Check quotation.
if (!quote_char) { // begin quote
quote_char = *c;
quote_pos = c;
if (!compbuf.empty()) {
- FlushComponent(&nocount_buf, &compbuf, &ncomponent);
+ if (FlushComponent(&nocount_buf, &compbuf, &ncomponent) !=
0) {
+ return AppendFailed();
+ }
}
} else if (quote_char == *c) {
const char last_char = (compbuf.empty() ? 0 : compbuf.back());
@@ -381,7 +403,9 @@ RedisCommandNoFormat(butil::IOBuf* outbuf, const
butil::StringPiece& cmd) {
compbuf.push_back(*c);
} else { // end quote
quote_char = 0;
- FlushComponent(&nocount_buf, &compbuf, &ncomponent);
+ if (FlushComponent(&nocount_buf, &compbuf, &ncomponent) !=
0) {
+ return AppendFailed();
+ }
}
} else {
compbuf.push_back(*c);
@@ -400,11 +424,15 @@ RedisCommandNoFormat(butil::IOBuf* outbuf, const
butil::StringPiece& cmd) {
}
if (!compbuf.empty()) {
- FlushComponent(&nocount_buf, &compbuf, &ncomponent);
+ if (FlushComponent(&nocount_buf, &compbuf, &ncomponent) != 0) {
+ return AppendFailed();
+ }
}
AppendHeader(*outbuf, '*', ncomponent);
- outbuf->append(nocount_buf);
+ if (ncomponent != 0) {
+ outbuf->append(nocount_buf.buf().movable());
+ }
return butil::Status::OK();
}
diff --git a/test/brpc_redis_unittest.cpp b/test/brpc_redis_unittest.cpp
index 13806681..7e8d0be0 100644
--- a/test/brpc_redis_unittest.cpp
+++ b/test/brpc_redis_unittest.cpp
@@ -164,6 +164,31 @@ TEST(RedisCommandFormatTest,
vector_components_preserve_binary_payload) {
EXPECT_EQ(expected, buf.to_string());
}
+TEST(RedisCommandFormatTest, format_preserves_binary_payload) {
+ const std::string value("a\0b", 3);
+ butil::IOBuf buf;
+ ASSERT_TRUE(brpc::RedisCommandFormat(
+ &buf, "SET key %b", value.data(), value.size())
+ .ok());
+
+ std::string expected = "*3\r\n$3\r\nSET\r\n$3\r\nkey\r\n$3\r\n";
+ expected.append(value).append("\r\n");
+ EXPECT_EQ(expected, buf.to_string());
+}
+
+TEST(RedisCommandFormatTest, no_format_encodes_components) {
+ butil::IOBuf buf;
+ ASSERT_TRUE(brpc::RedisCommandNoFormat(&buf, "SET key value").ok());
+ EXPECT_EQ("*3\r\n$3\r\nSET\r\n$3\r\nkey\r\n$5\r\nvalue\r\n",
+ buf.to_string());
+}
+
+TEST(RedisCommandFormatTest, empty_format_encodes_empty_array) {
+ butil::IOBuf buf;
+ ASSERT_TRUE(brpc::RedisCommandFormat(&buf, "").ok());
+ EXPECT_EQ("*0\r\n", buf.to_string());
+}
+
TEST(RedisCommandFormatTest, authenticator_encodes_password_and_database) {
brpc::policy::RedisAuthenticator authenticator("a b", 3);
std::string credential;
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
