diff --git a/trpc/client/redis/reader.cc b/trpc/client/redis/reader.cc index 2a36b8e3..b591706b 100644 --- a/trpc/client/redis/reader.cc +++ b/trpc/client/redis/reader.cc @@ -239,8 +239,9 @@ void Reader::MoveToNextTask() { } else { // Reset type assert(cur->idx_ < prv->elements_); - prv->obj_->u_.array_.push_back(Reply()); - cur->obj_ = &prv->obj_->u_.array_.back(); + std::vector& array = std::get>(prv->obj_->u_); + array.push_back(Reply()); + cur->obj_ = &(array.back()); cur->elements_ = -1; cur->idx_++; return; @@ -342,9 +343,10 @@ int Reader::ProcessMultiBulkItem(NoncontiguousBuffer& in, NoncontiguousBuffer::c // It need update rstack_ when count of elements > 1 if (elements > 0) { cur->elements_ = elements; - cur->obj_->u_.array_.emplace_back(); + std::vector& array = std::get>(cur->obj_->u_); + array.emplace_back(); ridx_++; - rstack_[ridx_].obj_ = &cur->obj_->u_.array_.back(); + rstack_[ridx_].obj_ = &(array.back()); rstack_[ridx_].elements_ = -1; rstack_[ridx_].idx_ = 0; rstack_[ridx_].parent_ = cur; @@ -448,13 +450,12 @@ bool Reader::GetReply(NoncontiguousBuffer& in, std::deque& out, const if (pipeline_count > 1) { ReadTask* cur = &(rstack_[ridx_]); cur->obj_->type_ = Reply::Type::ARRAY; - cur->obj_->Set(ArrayReplyMarker{}); - cur->elements_ = pipeline_count; - cur->obj_->u_.array_.emplace_back(); + std::vector& array = std::get>(cur->obj_->u_); + array.emplace_back(); ridx_++; - rstack_[ridx_].obj_ = &cur->obj_->u_.array_.back(); + rstack_[ridx_].obj_ = &(array.back()); rstack_[ridx_].elements_ = -1; rstack_[ridx_].idx_ = 0; rstack_[ridx_].parent_ = cur; diff --git a/trpc/client/redis/reply.h b/trpc/client/redis/reply.h index baf41dcc..76640e35 100644 --- a/trpc/client/redis/reply.h +++ b/trpc/client/redis/reply.h @@ -17,6 +17,7 @@ #include #include #include +#include #include namespace trpc { @@ -45,22 +46,8 @@ struct Reply { INVALID, }; - union Any { - Any() {} - ~Any() {} - Any(std::string&& value) : value_(std::move(value)) {} - Any(int64_t integer) : integer_(integer) {} - Any(std::vector&& array) : array_(std::move(array)) {} - - std::string value_; - - int64_t integer_{0}; - - std::vector array_; - }; - Type type_ = Type::NONE; - Any u_; + std::variant> u_; bool has_value_ = false; uint32_t serial_no_ = 0; @@ -83,148 +70,70 @@ struct Reply { explicit Reply(InvalidReplyMarker) : type_(Type::INVALID), serial_no_(0) {} - [[gnu::always_inline]] Reply(Reply&& x) noexcept - : type_(x.type_), has_value_(x.has_value_), serial_no_(x.serial_no_) { - switch (type_) { - case Type::NONE: - break; - case Type::STRING: - case Type::STATUS: - case Type::ERROR: - new (&u_.value_) std::basic_string(std::move(x.u_.value_)); - x.u_.value_.~basic_string(); - break; - case Type::INTEGER: - u_.integer_ = x.u_.integer_; - break; - case Type::ARRAY: - new (&u_.array_) std::vector(std::move(x.u_.array_)); - x.u_.array_.~vector(); - break; - case Type::NIL: - case Type::INVALID: - break; - default: - break; - } + Reply(const Reply& x) = default; + Reply(Reply&& x) = default; - x.type_ = Type::INVALID; - } - - [[gnu::always_inline]] ~Reply() noexcept { - switch (type_) { - case Type::STRING: - case Type::STATUS: - case Type::ERROR: - if (has_value_) { - u_.value_.~basic_string(); - } - break; - case Type::INTEGER: - break; - case Type::ARRAY: - if (has_value_) { - u_.array_.~vector(); - } - break; - case Type::NONE: - case Type::NIL: - case Type::INVALID: - break; - default: - break; - } - } - - Reply(const Reply& x) noexcept : type_(x.type_), has_value_(x.has_value_), serial_no_(x.serial_no_) { - switch (type_) { - case Type::NONE: - break; - case Type::STRING: - case Type::STATUS: - case Type::ERROR: - new (&u_.value_) std::basic_string(x.u_.value_); - break; - case Type::INTEGER: - u_.integer_ = x.u_.integer_; - break; - case Type::ARRAY: - new (&u_.array_) std::vector(x.u_.array_); - break; - case Type::NIL: - case Type::INVALID: - break; - default: - break; - } - } - - Reply& operator=(Reply&& x) noexcept { - if (this != &x) { - this->~Reply(); - new (this) Reply(std::move(x)); - } - return *this; - } - - Reply& operator=(const Reply& x) noexcept { - if (this != &x) { - this->~Reply(); - new (this) Reply(x); - } - return *this; - } + Reply& operator=(const Reply& x) = default; + Reply& operator=(Reply&& x) = default; inline void Set(StringReplyMarker, const char* s, size_t len, size_t capacity) { has_value_ = true; - new (&u_.value_) std::string(); - u_.value_.reserve(capacity); - u_.value_.append(s, len); + std::string value; + value.reserve(capacity); + value.append(s, len); + u_ = std::move(value); } inline void Set(StringReplyMarker, const char* s, size_t len) { has_value_ = true; - new (&u_.value_) std::string(); if (len < 1) { return; } - u_.value_.reserve(len); - u_.value_.append(s, len); + + std::string value; + value.reserve(len); + value.append(s, len); + u_ = std::move(value); } inline void Set(StatusReplyMarker, const char* s, size_t len, size_t capacity) { has_value_ = true; - new (&u_.value_) std::string(); - u_.value_.reserve(capacity); - u_.value_.append(s, len); + std::string value; + value.reserve(capacity); + value.append(s, len); + u_ = std::move(value); } inline void Set(StatusReplyMarker, const char* s, size_t len) { has_value_ = true; - new (&u_.value_) std::string(s, s + len); + u_ = std::string(s, s + len); } inline void Set(ErrorReplyMarker, const char* s, size_t len, size_t capacity) { has_value_ = true; - new (&u_.value_) std::string(); - u_.value_.reserve(capacity); - u_.value_.append(s, len); + std::string value; + value.reserve(capacity); + value.append(s, len); + u_ = std::move(value); } inline void Set(ErrorReplyMarker, const char* s, size_t len) { has_value_ = true; - new (&u_.value_) std::string(s, s + len); + u_ = std::string(s, s + len); } - inline void Set(IntegerReplyMarker, int64_t i) { u_.integer_ = i; } + inline void Set(IntegerReplyMarker, int64_t i) { u_ = i; } inline void Set(ArrayReplyMarker) { has_value_ = true; - new (&u_.array_) std::vector(); + u_ = std::vector(); } inline void Set(NilReplyMarker) { type_ = Type::NIL; } inline void Set(InvalidReplyMarker) { type_ = Type::INVALID; } - inline void AppendString(const char* s, size_t len) { u_.value_.append(s, len); } + inline void AppendString(const char* s, size_t len) { + std::string& value = std::get(u_); + value.append(s, len); + } inline bool IsNone() const { return type_ == Type::NONE; } inline bool IsNil() const { return type_ == Type::NIL; } @@ -236,18 +145,17 @@ struct Reply { inline bool IsInvalid() const { return type_ == Type::INVALID; } /// @brief Get reply as string ONLY when type is in[string/status/error] - inline const std::basic_string& GetString() const { return u_.value_; } + inline const std::basic_string& GetString() const { return std::get(u_); } - inline int64_t GetInteger() const { return u_.integer_; } - inline const std::vector& GetArray() const { return u_.array_; } + inline int64_t GetInteger() const { return std::get(u_); } + inline const std::vector& GetArray() const { return std::get>(u_); } /// @brief Get reply as string ONLY when type is string when need for high performance /// It will use std::move the move this reply,so DO NOT invoke repeatly inline int GetString(std::string& value) { if (has_value_ && type_ == Type::STRING) { - if (!u_.value_.empty()) { - value = std::move(u_.value_); - } + value = std::move(std::get(u_)); + has_value_ = false; return 0; } @@ -258,9 +166,8 @@ struct Reply { /// It will use std::move the move this reply,so DO NOT invoke repeatly inline int GetArray(std::vector& value) { if (has_value_ && type_ == Type::ARRAY) { - if (!u_.array_.empty()) { - value = std::move(u_.array_); - } + value = std::move(std::get>(u_)); + has_value_ = false; return 0; } diff --git a/trpc/client/redis/reply_test.cc b/trpc/client/redis/reply_test.cc index 923e0110..79f1ee12 100644 --- a/trpc/client/redis/reply_test.cc +++ b/trpc/client/redis/reply_test.cc @@ -247,6 +247,36 @@ TEST(Reply, EmptyArrayMove) { EXPECT_EQ(0, value.size()); } +TEST(Reply, Construct) { + trpc::redis::Reply r1; + r1.type_ = trpc::redis::Reply::Type::STRING; + r1.Set(trpc::redis::StringReplyMarker{}, "redis", 5); + + // Default copy constructor + trpc::redis::Reply copy_construct_reply(r1); + EXPECT_EQ("redis", r1.GetString()); + EXPECT_EQ("redis", copy_construct_reply.GetString()); + + // Default move constructor + trpc::redis::Reply move_construct_reply(std::move(r1)); + EXPECT_EQ("", r1.GetString()); + EXPECT_EQ("redis", move_construct_reply.GetString()); + + trpc::redis::Reply r2; + r2.type_ = trpc::redis::Reply::Type::STRING; + r2.Set(trpc::redis::StringReplyMarker{}, "redis", 5); + + // Default copy assignment + trpc::redis::Reply copy_assigning_reply = r2; + EXPECT_EQ("redis", r2.GetString()); + EXPECT_EQ("redis", copy_assigning_reply.GetString()); + + // Default move assignment + trpc::redis::Reply move_assigning_reply(std::move(r2)); + EXPECT_EQ("", r2.GetString()); + EXPECT_EQ("redis", move_assigning_reply.GetString()); +} + } // namespace testing } // namespace trpc