From b7f77d40ad9ed91e525ace4ad2fc9809a1ddaae6 Mon Sep 17 00:00:00 2001
From: Teon Banek <teon.banek@memgraph.io>
Date: Wed, 28 Feb 2018 10:52:22 +0100
Subject: [PATCH] Stack allocate the RPC Request
Summary:
There's no need to heap allocate the request which is used only to be
serialized and sent over the network. Unfortunately, this change
complicates the reading a bit, because we need to deserialize the raw
pointer and put it in std::unique_ptr instead of simply deserializing
the unique_ptr.
Heap allocation shows up visibly in perf during benchmarks, so this
change may yield some improvement.
Reviewers: mferencevic, mtomic
Reviewed By: mferencevic
Subscribers: pullbot
Differential Revision: https://phabricator.memgraph.io/D1254
---
src/communication/rpc/client.cpp | 6 ++++--
src/communication/rpc/client.hpp | 5 ++---
src/communication/rpc/protocol.cpp | 11 +++++++----
3 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/src/communication/rpc/client.cpp b/src/communication/rpc/client.cpp
index faf84483e..ea442c4be 100644
--- a/src/communication/rpc/client.cpp
+++ b/src/communication/rpc/client.cpp
@@ -19,7 +19,7 @@ namespace communication::rpc {
Client::Client(const io::network::Endpoint &endpoint) : endpoint_(endpoint) {}
-std::unique_ptr<Message> Client::Call(std::unique_ptr<Message> request) {
+std::unique_ptr<Message> Client::Call(const Message &request) {
std::lock_guard<std::mutex> guard(mutex_);
if (FLAGS_rpc_random_latency) {
@@ -50,7 +50,9 @@ std::unique_ptr<Message> Client::Call(std::unique_ptr<Message> request) {
std::stringstream request_stream(std::ios_base::out | std::ios_base::binary);
{
boost::archive::binary_oarchive request_archive(request_stream);
- request_archive << request;
+ // Serialize reference as pointer (to serialize the derived class). The
+ // request is read in protocol.cpp.
+ request_archive << &request;
// Archive destructor ensures everything is written.
}
diff --git a/src/communication/rpc/client.hpp b/src/communication/rpc/client.hpp
index 31ecdb5c5..579b0dedc 100644
--- a/src/communication/rpc/client.hpp
+++ b/src/communication/rpc/client.hpp
@@ -36,8 +36,7 @@ class Client {
utils::Demangle(typeid(Req).name()).value_or("unknown"));
std::unique_ptr<Message> response = nullptr;
stats::Stopwatch(request_name, [&] {
- response = Call(std::unique_ptr<Message>(
- std::make_unique<Req>(std::forward<Args>(args)...)));
+ response = Call(Req(std::forward<Args>(args)...));
});
auto *real_response = dynamic_cast<Res *>(response.get());
if (!real_response && response) {
@@ -55,7 +54,7 @@ class Client {
void Abort();
private:
- std::unique_ptr<Message> Call(std::unique_ptr<Message> request);
+ std::unique_ptr<Message> Call(const Message &request);
io::network::Endpoint endpoint_;
std::experimental::optional<io::network::Socket> socket_;
diff --git a/src/communication/rpc/protocol.cpp b/src/communication/rpc/protocol.cpp
index d586f79d4..9b20d83ba 100644
--- a/src/communication/rpc/protocol.cpp
+++ b/src/communication/rpc/protocol.cpp
@@ -24,15 +24,18 @@ void Session::Execute() {
buffer_.Resize(request_size);
if (buffer_.size() < request_size) return;
- std::unique_ptr<Message> request;
- {
+ // Read the request message.
+ std::unique_ptr<Message> request([this, request_len]() {
+ Message *req_ptr = nullptr;
std::stringstream stream(std::ios_base::in | std::ios_base::binary);
stream.str(std::string(
reinterpret_cast<char *>(buffer_.data() + sizeof(MessageSize)),
request_len));
boost::archive::binary_iarchive archive(stream);
- archive >> request;
- }
+ // Sent from client.cpp
+ archive >> req_ptr;
+ return req_ptr;
+ }());
buffer_.Shift(sizeof(MessageSize) + request_len);
auto callbacks_accessor = server_.callbacks_.access();