Skip to content

Commit 7eff8ba

Browse files
yujun411522yujun411522
authored
Feature:redis reply change from union to variant (#115)
1 parent fe2471a commit 7eff8ba

File tree

3 files changed

+77
-139
lines changed

3 files changed

+77
-139
lines changed

‎trpc/client/redis/reader.cc‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -239,8 +239,9 @@ void Reader::MoveToNextTask() {
239239
} else {
240240
// Reset type
241241
assert(cur->idx_ < prv->elements_);
242-
prv->obj_->u_.array_.push_back(Reply());
243-
cur->obj_ = &prv->obj_->u_.array_.back();
242+
std::vector<Reply>& array = std::get<std::vector<Reply>>(prv->obj_->u_);
243+
array.push_back(Reply());
244+
cur->obj_ = &(array.back());
244245
cur->elements_ = -1;
245246
cur->idx_++;
246247
return;
@@ -342,9 +343,10 @@ int Reader::ProcessMultiBulkItem(NoncontiguousBuffer& in, NoncontiguousBuffer::c
342343
// It need update rstack_ when count of elements > 1
343344
if (elements > 0) {
344345
cur->elements_ = elements;
345-
cur->obj_->u_.array_.emplace_back();
346+
std::vector<Reply>& array = std::get<std::vector<Reply>>(cur->obj_->u_);
347+
array.emplace_back();
346348
ridx_++;
347-
rstack_[ridx_].obj_ = &cur->obj_->u_.array_.back();
349+
rstack_[ridx_].obj_ = &(array.back());
348350
rstack_[ridx_].elements_ = -1;
349351
rstack_[ridx_].idx_ = 0;
350352
rstack_[ridx_].parent_ = cur;
@@ -448,13 +450,12 @@ bool Reader::GetReply(NoncontiguousBuffer& in, std::deque<std::any>& out, const
448450
if (pipeline_count > 1) {
449451
ReadTask* cur = &(rstack_[ridx_]);
450452
cur->obj_->type_ = Reply::Type::ARRAY;
451-
452453
cur->obj_->Set(ArrayReplyMarker{});
453-
454454
cur->elements_ = pipeline_count;
455-
cur->obj_->u_.array_.emplace_back();
455+
std::vector<Reply>& array = std::get<std::vector<Reply>>(cur->obj_->u_);
456+
array.emplace_back();
456457
ridx_++;
457-
rstack_[ridx_].obj_ = &cur->obj_->u_.array_.back();
458+
rstack_[ridx_].obj_ = &(array.back());
458459
rstack_[ridx_].elements_ = -1;
459460
rstack_[ridx_].idx_ = 0;
460461
rstack_[ridx_].parent_ = cur;

‎trpc/client/redis/reply.h‎

Lines changed: 38 additions & 131 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include <ostream>
1818
#include <string>
1919
#include <utility>
20+
#include <variant>
2021
#include <vector>
2122

2223
namespace trpc {
@@ -45,22 +46,8 @@ struct Reply {
4546
INVALID,
4647
};
4748

48-
union Any {
49-
Any() {}
50-
~Any() {}
51-
Any(std::string&& value) : value_(std::move(value)) {}
52-
Any(int64_t integer) : integer_(integer) {}
53-
Any(std::vector<Reply>&& array) : array_(std::move(array)) {}
54-
55-
std::string value_;
56-
57-
int64_t integer_{0};
58-
59-
std::vector<Reply> array_;
60-
};
61-
6249
Type type_ = Type::NONE;
63-
Any u_;
50+
std::variant<std::string, int64_t, std::vector<Reply>> u_;
6451
bool has_value_ = false;
6552
uint32_t serial_no_ = 0;
6653

@@ -83,148 +70,70 @@ struct Reply {
8370

8471
explicit Reply(InvalidReplyMarker) : type_(Type::INVALID), serial_no_(0) {}
8572

86-
[[gnu::always_inline]] Reply(Reply&& x) noexcept
87-
: type_(x.type_), has_value_(x.has_value_), serial_no_(x.serial_no_) {
88-
switch (type_) {
89-
case Type::NONE:
90-
break;
91-
case Type::STRING:
92-
case Type::STATUS:
93-
case Type::ERROR:
94-
new (&u_.value_) std::basic_string<char>(std::move(x.u_.value_));
95-
x.u_.value_.~basic_string();
96-
break;
97-
case Type::INTEGER:
98-
u_.integer_ = x.u_.integer_;
99-
break;
100-
case Type::ARRAY:
101-
new (&u_.array_) std::vector<Reply>(std::move(x.u_.array_));
102-
x.u_.array_.~vector();
103-
break;
104-
case Type::NIL:
105-
case Type::INVALID:
106-
break;
107-
default:
108-
break;
109-
}
73+
Reply(const Reply& x) = default;
74+
Reply(Reply&& x) = default;
11075

111-
x.type_ = Type::INVALID;
112-
}
113-
114-
[[gnu::always_inline]] ~Reply() noexcept {
115-
switch (type_) {
116-
case Type::STRING:
117-
case Type::STATUS:
118-
case Type::ERROR:
119-
if (has_value_) {
120-
u_.value_.~basic_string();
121-
}
122-
break;
123-
case Type::INTEGER:
124-
break;
125-
case Type::ARRAY:
126-
if (has_value_) {
127-
u_.array_.~vector();
128-
}
129-
break;
130-
case Type::NONE:
131-
case Type::NIL:
132-
case Type::INVALID:
133-
break;
134-
default:
135-
break;
136-
}
137-
}
138-
139-
Reply(const Reply& x) noexcept : type_(x.type_), has_value_(x.has_value_), serial_no_(x.serial_no_) {
140-
switch (type_) {
141-
case Type::NONE:
142-
break;
143-
case Type::STRING:
144-
case Type::STATUS:
145-
case Type::ERROR:
146-
new (&u_.value_) std::basic_string<char>(x.u_.value_);
147-
break;
148-
case Type::INTEGER:
149-
u_.integer_ = x.u_.integer_;
150-
break;
151-
case Type::ARRAY:
152-
new (&u_.array_) std::vector<Reply>(x.u_.array_);
153-
break;
154-
case Type::NIL:
155-
case Type::INVALID:
156-
break;
157-
default:
158-
break;
159-
}
160-
}
161-
162-
Reply& operator=(Reply&& x) noexcept {
163-
if (this != &x) {
164-
this->~Reply();
165-
new (this) Reply(std::move(x));
166-
}
167-
return *this;
168-
}
169-
170-
Reply& operator=(const Reply& x) noexcept {
171-
if (this != &x) {
172-
this->~Reply();
173-
new (this) Reply(x);
174-
}
175-
return *this;
176-
}
76+
Reply& operator=(const Reply& x) = default;
77+
Reply& operator=(Reply&& x) = default;
17778

17879
inline void Set(StringReplyMarker, const char* s, size_t len, size_t capacity) {
17980
has_value_ = true;
180-
new (&u_.value_) std::string();
181-
u_.value_.reserve(capacity);
182-
u_.value_.append(s, len);
81+
std::string value;
82+
value.reserve(capacity);
83+
value.append(s, len);
84+
u_ = std::move(value);
18385
}
18486

18587
inline void Set(StringReplyMarker, const char* s, size_t len) {
18688
has_value_ = true;
187-
new (&u_.value_) std::string();
18889
if (len < 1) {
18990
return;
19091
}
191-
u_.value_.reserve(len);
192-
u_.value_.append(s, len);
92+
93+
std::string value;
94+
value.reserve(len);
95+
value.append(s, len);
96+
u_ = std::move(value);
19397
}
19498

19599
inline void Set(StatusReplyMarker, const char* s, size_t len, size_t capacity) {
196100
has_value_ = true;
197-
new (&u_.value_) std::string();
198-
u_.value_.reserve(capacity);
199-
u_.value_.append(s, len);
101+
std::string value;
102+
value.reserve(capacity);
103+
value.append(s, len);
104+
u_ = std::move(value);
200105
}
201106

202107
inline void Set(StatusReplyMarker, const char* s, size_t len) {
203108
has_value_ = true;
204-
new (&u_.value_) std::string(s, s + len);
109+
u_ = std::string(s, s + len);
205110
}
206111

207112
inline void Set(ErrorReplyMarker, const char* s, size_t len, size_t capacity) {
208113
has_value_ = true;
209-
new (&u_.value_) std::string();
210-
u_.value_.reserve(capacity);
211-
u_.value_.append(s, len);
114+
std::string value;
115+
value.reserve(capacity);
116+
value.append(s, len);
117+
u_ = std::move(value);
212118
}
213119

214120
inline void Set(ErrorReplyMarker, const char* s, size_t len) {
215121
has_value_ = true;
216-
new (&u_.value_) std::string(s, s + len);
122+
u_ = std::string(s, s + len);
217123
}
218124

219-
inline void Set(IntegerReplyMarker, int64_t i) { u_.integer_ = i; }
125+
inline void Set(IntegerReplyMarker, int64_t i) { u_ = i; }
220126
inline void Set(ArrayReplyMarker) {
221127
has_value_ = true;
222-
new (&u_.array_) std::vector<Reply>();
128+
u_ = std::vector<Reply>();
223129
}
224130
inline void Set(NilReplyMarker) { type_ = Type::NIL; }
225131
inline void Set(InvalidReplyMarker) { type_ = Type::INVALID; }
226132

227-
inline void AppendString(const char* s, size_t len) { u_.value_.append(s, len); }
133+
inline void AppendString(const char* s, size_t len) {
134+
std::string& value = std::get<std::string>(u_);
135+
value.append(s, len);
136+
}
228137

229138
inline bool IsNone() const { return type_ == Type::NONE; }
230139
inline bool IsNil() const { return type_ == Type::NIL; }
@@ -236,18 +145,17 @@ struct Reply {
236145
inline bool IsInvalid() const { return type_ == Type::INVALID; }
237146

238147
/// @brief Get reply as string ONLY when type is in[string/status/error]
239-
inline const std::basic_string<char>& GetString() const { return u_.value_; }
148+
inline const std::basic_string<char>& GetString() const { return std::get<std::string>(u_); }
240149

241-
inline int64_t GetInteger() const { return u_.integer_; }
242-
inline const std::vector<Reply>& GetArray() const { return u_.array_; }
150+
inline int64_t GetInteger() const { return std::get<int64_t>(u_); }
151+
inline const std::vector<Reply>& GetArray() const { return std::get<std::vector<Reply>>(u_); }
243152

244153
/// @brief Get reply as string ONLY when type is string when need for high performance
245154
/// It will use std::move the move this reply,so DO NOT invoke repeatly
246155
inline int GetString(std::string& value) {
247156
if (has_value_ && type_ == Type::STRING) {
248-
if (!u_.value_.empty()) {
249-
value = std::move(u_.value_);
250-
}
157+
value = std::move(std::get<std::string>(u_));
158+
251159
has_value_ = false;
252160
return 0;
253161
}
@@ -258,9 +166,8 @@ struct Reply {
258166
/// It will use std::move the move this reply,so DO NOT invoke repeatly
259167
inline int GetArray(std::vector<Reply>& value) {
260168
if (has_value_ && type_ == Type::ARRAY) {
261-
if (!u_.array_.empty()) {
262-
value = std::move(u_.array_);
263-
}
169+
value = std::move(std::get<std::vector<Reply>>(u_));
170+
264171
has_value_ = false;
265172
return 0;
266173
}

‎trpc/client/redis/reply_test.cc‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,36 @@ TEST(Reply, EmptyArrayMove) {
247247
EXPECT_EQ(0, value.size());
248248
}
249249

250+
TEST(Reply, Construct) {
251+
trpc::redis::Reply r1;
252+
r1.type_ = trpc::redis::Reply::Type::STRING;
253+
r1.Set(trpc::redis::StringReplyMarker{}, "redis", 5);
254+
255+
// Default copy constructor
256+
trpc::redis::Reply copy_construct_reply(r1);
257+
EXPECT_EQ("redis", r1.GetString());
258+
EXPECT_EQ("redis", copy_construct_reply.GetString());
259+
260+
// Default move constructor
261+
trpc::redis::Reply move_construct_reply(std::move(r1));
262+
EXPECT_EQ("", r1.GetString());
263+
EXPECT_EQ("redis", move_construct_reply.GetString());
264+
265+
trpc::redis::Reply r2;
266+
r2.type_ = trpc::redis::Reply::Type::STRING;
267+
r2.Set(trpc::redis::StringReplyMarker{}, "redis", 5);
268+
269+
// Default copy assignment
270+
trpc::redis::Reply copy_assigning_reply = r2;
271+
EXPECT_EQ("redis", r2.GetString());
272+
EXPECT_EQ("redis", copy_assigning_reply.GetString());
273+
274+
// Default move assignment
275+
trpc::redis::Reply move_assigning_reply(std::move(r2));
276+
EXPECT_EQ("", r2.GetString());
277+
EXPECT_EQ("redis", move_assigning_reply.GetString());
278+
}
279+
250280
} // namespace testing
251281

252282
} // namespace trpc

0 commit comments

Comments
 (0)