redis_server_protocol: refine code by cr

example/redis_c++/redis_server.cpp

@@ -26,6 +26,8 @@
 #include <gflags/gflags.h>
 #include <unordered_map>
 
+#include <butil/time.h>
+
 class RedisServiceImpl : public brpc::RedisService {
 public:
     bool Set(const std::string& key, const std::string& value) {

src/brpc/redis_command.cpp

@@ -18,7 +18,6 @@
 // Authors: Ge,Jun (gejun@baidu.com)
 
 #include "butil/logging.h"
-#include "butil/string_printf.h"
 #include "brpc/log.h"
 #include "brpc/redis_command.h"
 
@@ -26,6 +25,19 @@ namespace brpc {
 
 const size_t CTX_WIDTH = 5;
 
+// Much faster than snprintf(..., "%lu", d);
+inline size_t AppendDecimal(char* outbuf, unsigned long d) {
+    char buf[24];  // enough for decimal 64-bit integers
+    size_t n = sizeof(buf);
+    do {
+        const unsigned long q = d / 10;
+        buf[--n] = d - q * 10 + '0';
+        d = q;
+    } while (d);
+    fast_memcpy(outbuf, buf + n, sizeof(buf) - n);
+    return sizeof(buf) - n;
+}
+
 // This function is the hotspot of RedisCommandFormatV() when format is
 // short or does not have many %. In a 100K-time call to formating of
 // "GET key1", the time spent on RedisRequest.AddCommand() are ~700ns
@@ -33,7 +45,7 @@ const size_t CTX_WIDTH = 5;
 inline void AppendHeader(std::string& buf, char fc, unsigned long value) {
     char header[32];
     header[0] = fc;
-    size_t len = butil::AppendDecimal(header + 1, value);
+    size_t len = AppendDecimal(header + 1, value);
     header[len + 1] = '\r';
     header[len + 2] = '\n';
     buf.append(header, len + 3);
@@ -41,7 +53,7 @@ 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 = butil::AppendDecimal(header + 1, value);
+    size_t len = AppendDecimal(header + 1, value);
     header[len + 1] = '\r';
     header[len + 2] = '\n';
     buf.append(header, len + 3);

src/brpc/redis_reply.cpp

@@ -51,12 +51,12 @@ bool RedisReply::SerializeTo(butil::IOBufAppender* appender) {
                 appender->append(_data.long_str, _length);
             }
             appender->append("\r\n", 2);
-            break;
+            return true;
         case REDIS_REPLY_INTEGER:
             appender->push_back(':');
             appender->append_decimal(_data.integer);
             appender->append("\r\n", 2);
-            break;
+            return true;
         case REDIS_REPLY_STRING:
             appender->push_back('$');
             appender->append_decimal(_length);
@@ -69,7 +69,7 @@ bool RedisReply::SerializeTo(butil::IOBufAppender* appender) {
                 }
                 appender->append("\r\n", 2);
             }
-            break;
+            return true;
         case REDIS_REPLY_ARRAY:
             appender->push_back('*');
             appender->append_decimal(_length);
@@ -81,15 +81,13 @@ bool RedisReply::SerializeTo(butil::IOBufAppender* appender) {
                     }
                 }
             }
-            break;
+            return true;
         case REDIS_REPLY_NIL:
             LOG(ERROR) << "Do you forget to call SetXXX()?";
             return false;
-        default:
-            CHECK(false) << "unknown redis type=" << _type;
-            return false;
     }
-    return true;
+    CHECK(false) << "unknown redis type=" << _type;
+    return false;
 }
 
 ParseError RedisReply::ConsumePartialIOBuf(butil::IOBuf& buf) {
@@ -438,7 +436,7 @@ void RedisReply::SetArray(int size) {
     _data.array.replies = subs;
 }
 
-void RedisReply::SetStringImpl(const std::string& str, RedisReplyType type) {
+void RedisReply::SetStringImpl(const butil::StringPiece& str, RedisReplyType type) {
     if (!_arena) {
         return;
     }
@@ -447,7 +445,7 @@ void RedisReply::SetStringImpl(const std::string& str, RedisReplyType type) {
     }
     const size_t size = str.size();
     if (size < sizeof(_data.short_str)) {
-        memcpy(_data.short_str, str.c_str(), size);
+        memcpy(_data.short_str, str.data(), size);
         _data.short_str[size] = '\0';
     } else {
         char* d = (char*)_arena->allocate((size/8 + 1) * 8);

src/brpc/redis_reply.h

@@ -69,16 +69,16 @@ public:
     void SetArray(int size);
 
     // Set the reply to status message `str'.
-    void SetStatus(const std::string& str);
+    void SetStatus(const butil::StringPiece& str);
 
     // Set the reply to error message `str'.
-    void SetError(const std::string& str);
+    void SetError(const butil::StringPiece& str);
 
     // Set the reply to integer `value'.
     void SetInteger(int64_t value);
 
     // Set the reply to string `str'.
-    void SetString(const std::string& str);
+    void SetString(const butil::StringPiece& str);
 
     // Convert the reply into a signed 64-bit integer(according to
     // http://redis.io/topics/protocol). If the reply is not an integer,
@@ -144,7 +144,7 @@ private:
     // by calling CopyFrom[Different|Same]Arena.
     DISALLOW_COPY_AND_ASSIGN(RedisReply);
 
-    void SetStringImpl(const std::string& str, RedisReplyType type);
+    void SetStringImpl(const butil::StringPiece& str, RedisReplyType type);
     void Reset();
     
     RedisReplyType _type;
@@ -174,6 +174,7 @@ inline void RedisReply::Reset() {
     _length = 0;
     _data.array.last_index = -1;
     _data.array.replies = NULL;
+    // _arena should not be reset because further memory allocation needs it.
 }
 
 inline RedisReply::RedisReply(butil::Arena* arena)
@@ -215,11 +216,11 @@ inline void RedisReply::SetNullString() {
     _length = npos;
 }
 
-inline void RedisReply::SetStatus(const std::string& str) {
+inline void RedisReply::SetStatus(const butil::StringPiece& str) {
     return SetStringImpl(str, REDIS_REPLY_STATUS);
 }
 
-inline void RedisReply::SetError(const std::string& str) {
+inline void RedisReply::SetError(const butil::StringPiece& str) {
     return SetStringImpl(str, REDIS_REPLY_ERROR);
 }
 
@@ -232,7 +233,7 @@ inline void RedisReply::SetInteger(int64_t value) {
     _data.integer = value;
 }
 
-inline void RedisReply::SetString(const std::string& str) {
+inline void RedisReply::SetString(const butil::StringPiece& str) {
     return SetStringImpl(str, REDIS_REPLY_STRING);
 }
 
@@ -285,7 +286,7 @@ inline RedisReply& RedisReply::operator[](size_t index) {
 }
 
 inline const RedisReply& RedisReply::operator[](size_t index) const {
-    if (is_array() && (int)index < _length) {
+    if (is_array() && _length > 0 && index < (size_t)_length) {
         return _data.array.replies[index];
     }
     static RedisReply redis_nil(NULL);
@@ -297,6 +298,7 @@ inline void RedisReply::Swap(RedisReply& other) {
     std::swap(_length, other._length);
     std::swap(_data.padding[0], other._data.padding[0]);
     std::swap(_data.padding[1], other._data.padding[1]);
+    std::swap(_arena, other._arena);
 }
 
 inline void RedisReply::Clear() {
@@ -304,6 +306,7 @@ inline void RedisReply::Clear() {
     _length = 0;
     _data.array.last_index = -1;
     _data.array.replies = NULL;
+    // _arena should not be cleared because it may be shared between RedisReply;
 }
 
 inline void RedisReply::CopyFromSameArena(const RedisReply& other) {
@@ -311,6 +314,7 @@ inline void RedisReply::CopyFromSameArena(const RedisReply& other) {
     _length = other._length;
     _data.padding[0] = other._data.padding[0];
     _data.padding[1] = other._data.padding[1];
+    _arena = other._arena;
 }
 
 } // namespace brpc

src/butil/string_printf.h

@@ -46,28 +46,6 @@ int string_appendf(std::string* output, const char* format, ...)
 // Returns 0 on success, -1 otherwise.
 int string_vappendf(std::string* output, const char* format, va_list args);
 
-// Format integer |d| to |output|, which is much faster than snprintf(..., "%lu", d);
-// Return written length.
-inline size_t AppendDecimal(char* outbuf, long d) {
-    char buf[24];  // enough for decimal 64-bit integers
-    size_t n = sizeof(buf);
-    bool negative = false;
-    if (d < 0) {
-        negative = true;
-        d = -d;
-    }
-    do {
-        const long q = d / 10;
-        buf[--n] = d - q * 10 + '0';
-        d = q;
-    } while (d);
-    if (negative) {
-        buf[--n] = '-';
-    }
-    memcpy(outbuf, buf + n, sizeof(buf) - n);
-    return sizeof(buf) - n;
-}
-
 }  // namespace butil
 
 #endif  // BUTIL_STRING_PRINTF_H