Skip to content

Commit 590d5e6

Browse files
romangeromange
committed
chore: use BackedArguments for memcache_parser
Signed-off-by: Roman Gershman <roman@dragonflydb.io>
1 parent ba791bf commit 590d5e6

File tree

8 files changed

+155
-124
lines changed

8 files changed

+155
-124
lines changed

‎src/common/backed_args.h‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,35 @@ class BackedArguments {
1818
BackedArguments() {
1919
}
2020

21+
class iterator {
22+
public:
23+
iterator(const BackedArguments* ba, size_t index) : ba_(ba), index_(index) {
24+
}
25+
26+
iterator& operator++() {
27+
offset_ += ba_->elem_capacity(index_);
28+
++index_;
29+
return *this;
30+
}
31+
32+
bool operator==(const iterator& other) const {
33+
return index_ == other.index_;
34+
}
35+
36+
bool operator!=(const iterator& other) const {
37+
return !(*this == other);
38+
}
39+
40+
std::string_view operator*() const {
41+
return ba_->at(index_, offset_);
42+
}
43+
44+
private:
45+
const BackedArguments* ba_;
46+
size_t index_;
47+
size_t offset_ = 0;
48+
};
49+
2150
// Construct the arguments from iterator range.
2251
// TODO: In general we could get away without the len argument,
2352
// but that would require fixing base::it::CompoundIterator to support subtraction.
@@ -45,6 +74,10 @@ class BackedArguments {
4574
return lens_.size();
4675
}
4776

77+
bool empty() const {
78+
return lens_.empty();
79+
}
80+
4881
size_t elem_len(size_t i) const {
4982
return lens_[i];
5083
}
@@ -58,6 +91,19 @@ class BackedArguments {
5891
return std::string_view{ptr, lens_[index]};
5992
}
6093

94+
iterator begin() const {
95+
return {this, 0};
96+
}
97+
98+
iterator end() const {
99+
return {this, lens_.size()};
100+
}
101+
102+
void clear() {
103+
lens_.clear();
104+
storage_.clear();
105+
}
106+
61107
protected:
62108
absl::InlinedVector<uint32_t, kLenCap> lens_;
63109
StorageType storage_;

‎src/facade/dragonfly_connection.cc‎

Lines changed: 4 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -381,39 +381,9 @@ class PipelineCacheSizeTracker {
381381

382382
thread_local PipelineCacheSizeTracker tl_pipe_cache_sz_tracker;
383383

384-
Connection::MCPipelineMessage::MCPipelineMessage(MemcacheParser::Command cmd_in,
384+
Connection::MCPipelineMessage::MCPipelineMessage(MemcacheParser::Command&& cmd_in,
385385
std::string_view value_in)
386-
: cmd{std::move(cmd_in)}, value{value_in}, backing_size{0} {
387-
// Note: The process of laundering string_views should be placed in an utility function,
388-
// but there are no other uses like this so far.
389-
390-
// Compute total size and create backing
391-
backing_size = cmd.key.size() + value.size();
392-
for (const auto& ext_key : cmd.keys_ext)
393-
backing_size += ext_key.size();
394-
395-
backing = make_unique<char[]>(backing_size);
396-
397-
// Copy everything into backing
398-
if (!cmd.key.empty())
399-
memcpy(backing.get(), cmd.key.data(), cmd.key.size());
400-
if (!value.empty())
401-
memcpy(backing.get() + cmd.key.size(), value.data(), value.size());
402-
size_t offset = cmd.key.size() + value.size();
403-
for (const auto& ext_key : cmd.keys_ext) {
404-
if (!ext_key.empty())
405-
memcpy(backing.get() + offset, ext_key.data(), ext_key.size());
406-
offset += ext_key.size();
407-
}
408-
409-
// Update string_views
410-
cmd.key = string_view{backing.get(), cmd.key.size()};
411-
value = string_view{backing.get() + cmd.key.size(), value.size()};
412-
offset = cmd.key.size() + value.size();
413-
for (auto& key : cmd.keys_ext) {
414-
key = {backing.get() + offset, key.size()};
415-
offset += key.size();
416-
}
386+
: cmd{std::move(cmd_in)}, value{value_in} {
417387
}
418388

419389
size_t Connection::MessageHandle::UsedMemory() const {
@@ -445,8 +415,7 @@ size_t Connection::MessageHandle::UsedMemory() const {
445415
return 0;
446416
}
447417
size_t operator()(const MCPipelineMessagePtr& msg) {
448-
return sizeof(MCPipelineMessage) + msg->backing_size +
449-
msg->cmd.keys_ext.size() * sizeof(string_view);
418+
return sizeof(MCPipelineMessage) + msg->cmd.HeapMemory() + msg->value.capacity();
450419
}
451420
};
452421

@@ -2002,8 +1971,7 @@ bool Connection::IsHttp() const {
20021971
Connection::MemoryUsage Connection::GetMemoryUsage() const {
20031972
size_t mem = sizeof(*this) + cmn::HeapSize(dispatch_q_) + cmn::HeapSize(name_) +
20041973
cmn::HeapSize(tmp_parse_args_) + cmn::HeapSize(tmp_cmd_vec_) +
2005-
cmn::HeapSize(memcache_parser_) + cmn::HeapSize(redis_parser_) + cmn::HeapSize(cc_) +
2006-
cmn::HeapSize(reply_builder_);
1974+
cmn::HeapSize(redis_parser_) + cmn::HeapSize(cc_) + cmn::HeapSize(reply_builder_);
20071975

20081976
// We add a hardcoded 9k value to accomodate for the part of the Fiber stack that is in use.
20091977
// The allocated stack is actually larger (~130k), but only a small fraction of that (9k

‎src/facade/dragonfly_connection.h‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,12 +85,10 @@ class Connection : public util::Connection {
8585

8686
// Pipeline message, accumulated Memcached command to be executed.
8787
struct MCPipelineMessage {
88-
MCPipelineMessage(MemcacheParser::Command cmd, std::string_view value);
88+
MCPipelineMessage(MemcacheParser::Command&& cmd, std::string_view value);
8989

9090
MemcacheParser::Command cmd;
91-
std::string_view value;
92-
size_t backing_size;
93-
std::unique_ptr<char[]> backing; // backing for cmd and value
91+
std::string value;
9492
};
9593

9694
// Monitor message, carries a simple payload with the registered event to be sent.

‎src/facade/memcache_parser.cc‎

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,10 @@ MP::CmdType From(string_view token) {
6161
return MP::INVALID;
6262
}
6363

64+
void MaterializeArgs(absl::Span<std::string_view> args, MP::Command* dest) {
65+
dest->Assign(args.begin(), args.end(), args.size());
66+
}
67+
6468
MP::Result ParseStore(ArgSlice tokens, MP::Command* res) {
6569
const size_t num_tokens = tokens.size();
6670
unsigned opt_pos = 3;
@@ -93,7 +97,7 @@ MP::Result ParseStore(ArgSlice tokens, MP::Command* res) {
9397
return MP::OK;
9498
}
9599

96-
MP::Result ParseValueless(ArgSlice tokens, MP::Command* res) {
100+
MP::Result ParseValueless(ArgSlice tokens, vector<string_view>* args, MP::Command* res) {
97101
const size_t num_tokens = tokens.size();
98102
size_t key_pos = 0;
99103
if (res->type == MP::GAT || res->type == MP::GATS) {
@@ -105,13 +109,15 @@ MP::Result ParseValueless(ArgSlice tokens, MP::Command* res) {
105109

106110
// We support only `flushall` or `flushall 0`
107111
if (key_pos < num_tokens && res->type == MP::FLUSHALL) {
112+
DCHECK(args->empty());
113+
108114
int delay = 0;
109115
if (key_pos + 1 == num_tokens && absl::SimpleAtoi(tokens[key_pos], &delay) && delay == 0)
110116
return MP::OK;
111117
return MP::PARSE_ERROR;
112118
}
113119

114-
res->key = tokens[key_pos++];
120+
args->push_back(tokens[key_pos++]);
115121

116122
if (key_pos < num_tokens && res->type == MP::STATS)
117123
return MP::PARSE_ERROR; // we don't support additional arguments to stats for now
@@ -126,16 +132,17 @@ MP::Result ParseValueless(ArgSlice tokens, MP::Command* res) {
126132
}
127133

128134
while (key_pos < num_tokens) {
129-
res->keys_ext.push_back(tokens[key_pos++]);
135+
args->push_back(tokens[key_pos++]);
130136
}
131137

132138
if (res->type >= MP::DELETE) { // write commands
133-
if (!res->keys_ext.empty() && res->keys_ext.back() == "noreply") {
139+
if (args->size() > 1 && args->back() == "noreply") {
134140
res->no_reply = true;
135-
res->keys_ext.pop_back();
141+
args->pop_back();
136142
}
137143
}
138144

145+
MaterializeArgs(absl::MakeSpan(*args), res);
139146
return MP::OK;
140147
}
141148

@@ -180,7 +187,7 @@ bool ParseMetaMode(char m, MP::Command* res) {
180187
}
181188

182189
// See https://raw.githubusercontent.com/memcached/memcached/refs/heads/master/doc/protocol.txt
183-
MP::Result ParseMeta(ArgSlice tokens, MP::Command* res) {
190+
MP::Result ParseMeta(ArgSlice tokens, vector<string_view>* args, MP::Command* res) {
184191
DCHECK(!tokens.empty());
185192

186193
if (res->type == MP::META_DEBUG) {
@@ -192,11 +199,11 @@ MP::Result ParseMeta(ArgSlice tokens, MP::Command* res) {
192199
return MP::PARSE_ERROR;
193200

194201
res->meta = true;
195-
res->key = tokens[0];
196202
res->bytes_len = 0;
197203
res->flags = 0;
198204
res->expire_ts = 0;
199205

206+
args->push_back(tokens[0]);
200207
tokens.remove_prefix(1);
201208

202209
// We emulate the behavior by returning the high level commands.
@@ -228,6 +235,8 @@ MP::Result ParseMeta(ArgSlice tokens, MP::Command* res) {
228235
return MP::PARSE_ERROR;
229236
}
230237

238+
string blob;
239+
231240
for (size_t i = 0; i < tokens.size(); ++i) {
232241
string_view token = tokens[i];
233242

@@ -239,9 +248,9 @@ MP::Result ParseMeta(ArgSlice tokens, MP::Command* res) {
239248
case 'b':
240249
if (token.size() != 1)
241250
return MP::PARSE_ERROR;
242-
if (!absl::Base64Unescape(res->key, &res->blob))
251+
if (!absl::Base64Unescape(args->front(), &blob))
243252
return MP::PARSE_ERROR;
244-
res->key = res->blob;
253+
args->front() = blob;
245254
res->base64 = true;
246255
break;
247256
case 'F':
@@ -282,6 +291,7 @@ MP::Result ParseMeta(ArgSlice tokens, MP::Command* res) {
282291
return MP::PARSE_ERROR;
283292
}
284293
}
294+
MaterializeArgs(absl::MakeSpan(*args), res);
285295

286296
return MP::OK;
287297
}
@@ -321,20 +331,27 @@ auto MP::Parse(string_view str, uint32_t* consumed, Command* cmd) -> Result {
321331

322332
ArgSlice tokens_view{tokens};
323333
tokens_view.remove_prefix(1);
334+
cmd->clear();
335+
tmp_args_.clear();
324336

325337
if (cmd->type <= CAS) { // Store command
326338
if (tokens_view.size() < 4 || tokens[0].size() > 250) { // key length limit
327339
return MP::PARSE_ERROR;
328340
}
329341

330-
cmd->key = tokens_view[0];
331-
342+
tmp_args_.push_back(tokens_view[0]);
332343
tokens_view.remove_prefix(1);
333-
return ParseStore(tokens_view, cmd);
344+
auto res = ParseStore(tokens_view, cmd);
345+
if (res != MP::OK) {
346+
return res;
347+
}
348+
MaterializeArgs(absl::MakeSpan(tmp_args_), cmd);
349+
350+
return MP::OK;
334351
}
335352

336353
if (cmd->type >= META_SET) {
337-
return tokens_view.empty() ? MP::PARSE_ERROR : ParseMeta(tokens_view, cmd);
354+
return tokens_view.empty() ? MP::PARSE_ERROR : ParseMeta(tokens_view, &tmp_args_, cmd);
338355
}
339356

340357
if (tokens_view.empty()) {
@@ -344,7 +361,7 @@ auto MP::Parse(string_view str, uint32_t* consumed, Command* cmd) -> Result {
344361
return MP::PARSE_ERROR;
345362
}
346363

347-
return ParseValueless(tokens_view, cmd);
364+
return ParseValueless(tokens_view, &tmp_args_, cmd);
348365
};
349366

350367
} // namespace facade

‎src/facade/memcache_parser.h‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
#include <string_view>
1010
#include <vector>
1111

12+
#include "common/backed_args.h"
13+
1214
namespace facade {
1315

1416
// Memcache parser does not parse value blobs, only the commands.
@@ -51,11 +53,16 @@ class MemcacheParser {
5153
};
5254

5355
// According to https://github.com/memcached/memcached/wiki/Commands#standard-protocol
54-
struct Command {
56+
struct Command : public cmn::BackedArguments {
57+
Command() = default;
58+
Command(const Command&) = delete;
59+
Command(Command&&) noexcept = default;
60+
5561
CmdType type = INVALID;
56-
std::string_view key;
57-
std::vector<std::string_view> keys_ext;
5862

63+
std::string_view key() const {
64+
return lens_.empty() ? std::string_view{} : Front();
65+
}
5966
union {
6067
uint64_t cas_unique = 0; // for CAS COMMAND
6168
uint64_t delta; // for DECR/INCR commands.
@@ -76,9 +83,6 @@ class MemcacheParser {
7683
bool return_access_time = false; // l
7784
bool return_hit = false; // h
7885
bool return_version = false; // c
79-
80-
// Used internally by meta parsing.
81-
std::string blob;
8286
};
8387

8488
enum Result {
@@ -95,6 +99,9 @@ class MemcacheParser {
9599
}
96100

97101
Result Parse(std::string_view str, uint32_t* consumed, Command* res);
102+
103+
private:
104+
std::vector<std::string_view> tmp_args_;
98105
};
99106

100107
} // namespace facade

0 commit comments

Comments
 (0)