Skip to content

Commit 3ccdbb3

Browse files
committed
fix: Improve reply latency of HELLO
For high connection rate cases it can be significant. In addition, refactor reply_builder so we could use SendStringArrInternal for more cases like SendScoredArray that also has high latency overhead for replies. Signed-off-by: Roman Gershman <[email protected]>
1 parent e352edd commit 3ccdbb3

File tree

3 files changed

+24
-20
lines changed

3 files changed

+24
-20
lines changed

src/facade/reply_builder.cc

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -524,7 +524,9 @@ void RedisReplyBuilder::SendStringArr(StrSpan arr, CollectionType type) {
524524
return;
525525
}
526526

527-
SendStringArrInternal(warr, type);
527+
auto cb = [&](size_t i) { return warr[i]; };
528+
529+
SendStringArrInternal(warr.Size(), std::move(cb), type);
528530
}
529531

530532
void RedisReplyBuilder::StartArray(unsigned len) {
@@ -558,9 +560,8 @@ void RedisReplyBuilder::StartCollection(unsigned len, CollectionType type) {
558560
// to low numbers (around 1024). Therefore, to make it robust we send the array in batches.
559561
// We limit the vector length to 256 and when it fills up we flush it to the socket and continue
560562
// iterating.
561-
void RedisReplyBuilder::SendStringArrInternal(WrappedStrSpan arr, CollectionType type) {
562-
size_t size = arr.Size();
563-
563+
void RedisReplyBuilder::SendStringArrInternal(
564+
size_t size, absl::FunctionRef<std::string_view(unsigned)> producer, CollectionType type) {
564565
size_t header_len = size;
565566
string_view type_char = "*";
566567
if (is_resp3_) {
@@ -575,40 +576,41 @@ void RedisReplyBuilder::SendStringArrInternal(WrappedStrSpan arr, CollectionType
575576
}
576577

577578
// When vector length is too long, Send returns EMSGSIZE.
578-
size_t vec_len = std::min<size_t>(256u, size);
579+
size_t vec_len = std::min<size_t>(128u, size);
579580

580581
absl::FixedArray<iovec, 16> vec(vec_len * 2 + 2);
581-
absl::FixedArray<char, 64> meta((vec_len + 1) * 16);
582+
absl::FixedArray<char, 64> meta((vec_len + 1) * 16); // 16 bytes per length.
582583
char* next = meta.data();
583584

584-
*next++ = type_char[0];
585-
next = absl::numbers_internal::FastIntToBuffer(header_len, next);
586-
*next++ = '\r';
587-
*next++ = '\n';
585+
auto serialize_len = [&](char prefix, size_t len) {
586+
*next++ = prefix;
587+
next = absl::numbers_internal::FastIntToBuffer(len, next);
588+
*next++ = '\r';
589+
*next++ = '\n';
590+
};
591+
592+
serialize_len(type_char[0], header_len);
588593
vec[0] = IoVec(string_view{meta.data(), size_t(next - meta.data())});
589594
char* start = next;
590595

591596
unsigned vec_indx = 1;
592597
string_view src;
593598
for (unsigned i = 0; i < size; ++i) {
594-
src = arr[i];
595-
*next++ = '$';
596-
next = absl::numbers_internal::FastIntToBuffer(src.size(), next);
597-
*next++ = '\r';
598-
*next++ = '\n';
599-
vec[vec_indx] = IoVec(string_view{start, size_t(next - start)});
599+
src = producer(i);
600+
serialize_len('$', src.size());
601+
// add serialized len blob
602+
vec[vec_indx++] = IoVec(string_view{start, size_t(next - start)});
600603
DCHECK_GT(next - start, 0);
601604

602605
start = next;
603-
++vec_indx;
604606

605-
vec[vec_indx] = IoVec(src);
607+
vec[vec_indx++] = IoVec(src);
606608

607609
*next++ = '\r';
608610
*next++ = '\n';
609-
++vec_indx;
610611

611612
if (vec_indx + 1 >= vec.size()) {
613+
// Flush the iovec array.
612614
if (i < size - 1 || vec_indx == vec.size()) {
613615
Send(vec.data(), vec_indx);
614616
if (ec_)

src/facade/reply_builder.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,8 @@ class RedisReplyBuilder : public SinkReplyBuilder {
247247
};
248248

249249
private:
250-
void SendStringArrInternal(WrappedStrSpan arr, CollectionType type);
250+
void SendStringArrInternal(size_t size, absl::FunctionRef<std::string_view(unsigned)> producer,
251+
CollectionType type);
251252

252253
bool is_resp3_ = false;
253254
};

src/server/server_family.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2327,6 +2327,7 @@ void ServerFamily::Hello(CmdArgList args, ConnectionContext* cntx) {
23272327
rb->SetResp3(false);
23282328
}
23292329

2330+
SinkReplyBuilder::ReplyAggregator agg(rb);
23302331
rb->StartCollection(7, RedisReplyBuilder::MAP);
23312332
rb->SendBulkString("server");
23322333
rb->SendBulkString("redis");

0 commit comments

Comments
 (0)