From d4c1509a3b277480440fc00ac207564104fb5164 Mon Sep 17 00:00:00 2001 From: Eddy Ashton Date: Wed, 7 Jul 2021 22:27:26 +0100 Subject: [PATCH] Remove unnecessary copies into OArray (#2777) --- src/consensus/aft/async_execution.h | 39 +++++++++------------------ src/consensus/aft/raft.h | 13 +++------ src/consensus/aft/raft_consensus.h | 5 ++-- src/consensus/aft/test/logging_stub.h | 4 ++- src/kv/kv_types.h | 3 ++- src/kv/test/stub_consensus.h | 4 ++- src/node/node_state.h | 12 +++++---- src/node/node_to_node.h | 8 +++--- src/node/test/channels.cpp | 3 ++- 9 files changed, 39 insertions(+), 52 deletions(-) diff --git a/src/consensus/aft/async_execution.h b/src/consensus/aft/async_execution.h index c3f3deefadef..0ccab74a4f7a 100644 --- a/src/consensus/aft/async_execution.h +++ b/src/consensus/aft/async_execution.h @@ -53,28 +53,23 @@ namespace aft const ccf::NodeId& from_, AppendEntries&& hdr_, const uint8_t* data_, - size_t size_, - OArray&& oarray_) : + size_t size_) : store(store_), from(from_), hdr(std::move(hdr_)), - data(data_), - size(size_), - oarray(std::move(oarray_)) + body(data_, data_ + size_) {} void execute() override { - store.recv_append_entries(from, hdr, data, size); + store.recv_append_entries(from, hdr, body.data(), body.size()); } private: AbstractConsensusCallback& store; ccf::NodeId from; AppendEntries hdr; - const uint8_t* data; - size_t size; - OArray oarray; + std::vector body; }; class AppendEntryResponseCallback : public AbstractMsgCallback @@ -223,28 +218,23 @@ namespace aft const ccf::NodeId& from_, RequestViewChangeMsg&& hdr_, const uint8_t* data_, - size_t size_, - OArray&& oarray_) : + size_t size_) : store(store_), from(from_), hdr(std::move(hdr_)), - data(data_), - size(size_), - oarray(std::move(oarray_)) + body(data_, data_ + size_) {} void execute() override { - store.recv_view_change(from, hdr, data, size); + store.recv_view_change(from, hdr, body.data(), body.size()); } private: AbstractConsensusCallback& store; ccf::NodeId from; RequestViewChangeMsg hdr; - const uint8_t* data; - size_t size; - OArray oarray; + std::vector body; }; class SkipViewCallback : public AbstractMsgCallback @@ -278,27 +268,22 @@ namespace aft const ccf::NodeId& from_, ViewChangeEvidenceMsg&& hdr_, const uint8_t* data_, - size_t size_, - OArray&& oarray_) : + size_t size_) : store(store_), from(from_), hdr(std::move(hdr_)), - data(data_), - size(size_), - oarray(std::move(oarray_)) + body(data_, data_ + size_) {} void execute() override { - store.recv_view_change_evidence(from, hdr, data, size); + store.recv_view_change_evidence(from, hdr, body.data(), body.size()); } private: AbstractConsensusCallback& store; ccf::NodeId from; ViewChangeEvidenceMsg hdr; - const uint8_t* data; - size_t size; - OArray oarray; + std::vector body; }; } diff --git a/src/consensus/aft/raft.h b/src/consensus/aft/raft.h index 1af71d1b4651..235fa2cf9ba8 100644 --- a/src/consensus/aft/raft.h +++ b/src/consensus/aft/raft.h @@ -711,15 +711,8 @@ namespace aft } void recv_message(const ccf::NodeId& from, const uint8_t* data, size_t size) - { - recv_message(from, OArray({data, data + size})); - } - - void recv_message(const ccf::NodeId& from, OArray&& d) { std::unique_ptr aee; - const uint8_t* data = d.data(); - size_t size = d.size(); RaftMsgType type = serialized::peek(data, size); try @@ -732,7 +725,7 @@ namespace aft channels->template recv_authenticated( from, data, size); aee = std::make_unique( - *this, from, std::move(r), data, size, std::move(d)); + *this, from, std::move(r), data, size); break; } case raft_append_entries_response: @@ -800,7 +793,7 @@ namespace aft ->template recv_authenticated_with_load( from, data, size); aee = std::make_unique( - *this, from, std::move(r), data, size, std::move(d)); + *this, from, std::move(r), data, size); break; } @@ -821,7 +814,7 @@ namespace aft from, data, size); aee = std::make_unique( - *this, from, std::move(r), data, size, std::move(d)); + *this, from, std::move(r), data, size); break; } diff --git a/src/consensus/aft/raft_consensus.h b/src/consensus/aft/raft_consensus.h index ab179c050984..7d2da1796f8f 100644 --- a/src/consensus/aft/raft_consensus.h +++ b/src/consensus/aft/raft_consensus.h @@ -121,9 +121,10 @@ namespace aft return aft->active_nodes(); } - void recv_message(const ccf::NodeId& from, OArray&& data) override + void recv_message( + const ccf::NodeId& from, const uint8_t* data, size_t size) override { - return aft->recv_message(from, std::move(data)); + return aft->recv_message(from, data, size); } void add_configuration( diff --git a/src/consensus/aft/test/logging_stub.h b/src/consensus/aft/test/logging_stub.h index 7eb86c152052..acc31750fade 100644 --- a/src/consensus/aft/test/logging_stub.h +++ b/src/consensus/aft/test/logging_stub.h @@ -140,7 +140,9 @@ namespace aft return true; } - void recv_message(const ccf::NodeId& from, OArray&& oa) override {} + void recv_message( + const ccf::NodeId& from, const uint8_t* data, size_t size) override + {} void initialize( const ccf::NodeId& self_id, diff --git a/src/kv/kv_types.h b/src/kv/kv_types.h index edf36f6c47ea..51a214cf26dc 100644 --- a/src/kv/kv_types.h +++ b/src/kv/kv_types.h @@ -409,7 +409,8 @@ namespace kv virtual bool view_change_in_progress() = 0; virtual std::set active_nodes() = 0; - virtual void recv_message(const NodeId& from, OArray&& oa) = 0; + virtual void recv_message( + const NodeId& from, const uint8_t* data, size_t size) = 0; virtual bool on_request(const TxHistory::RequestCallbackArgs&) { diff --git a/src/kv/test/stub_consensus.h b/src/kv/test/stub_consensus.h index 059e0453e042..b37c7494c126 100644 --- a/src/kv/test/stub_consensus.h +++ b/src/kv/test/stub_consensus.h @@ -141,7 +141,9 @@ namespace kv::test view_history.initialise(view_history_); } - void recv_message(const NodeId& from, OArray&& oa) override {} + void recv_message( + const NodeId& from, const uint8_t* data, size_t size) override + {} void add_configuration( ccf::SeqNo seqno, diff --git a/src/node/node_state.h b/src/node/node_state.h index 20a57c5c0ecf..3229a2ab1c23 100644 --- a/src/node/node_state.h +++ b/src/node/node_state.h @@ -1449,21 +1449,23 @@ namespace ccf return; } - OArray oa(std::move(data)); + const uint8_t* payload_data = data.data(); + size_t payload_size = data.size(); + NodeMsgType msg_type = - serialized::overlay(oa.data(), oa.size()); - NodeId from = serialized::read(oa.data(), oa.size()); + serialized::overlay(payload_data, payload_size); + NodeId from = serialized::read(payload_data, payload_size); switch (msg_type) { case channel_msg: { - n2n_channels->recv_message(from, std::move(oa)); + n2n_channels->recv_message(from, payload_data, payload_size); break; } case consensus_msg: { - consensus->recv_message(from, std::move(oa)); + consensus->recv_message(from, payload_data, payload_size); break; } diff --git a/src/node/node_to_node.h b/src/node/node_to_node.h index 3ab08e1eda7b..9285bef24702 100644 --- a/src/node/node_to_node.h +++ b/src/node/node_to_node.h @@ -93,7 +93,8 @@ namespace ccf virtual bool recv_authenticated( const NodeId& from, CBuffer cb, const uint8_t*& data, size_t& size) = 0; - virtual void recv_message(const NodeId& from, OArray&& oa) = 0; + virtual void recv_message( + const NodeId& from, const uint8_t* data, size_t size) = 0; virtual void initialize( const NodeId& self_id, @@ -294,12 +295,11 @@ namespace ccf } } - void recv_message(const NodeId& from, OArray&& oa) override + void recv_message( + const NodeId& from, const uint8_t* data, size_t size) override { try { - const uint8_t* data = oa.data(); - size_t size = oa.size(); auto chmsg = serialized::read(data, size); switch (chmsg) { diff --git a/src/node/test/channels.cpp b/src/node/test/channels.cpp index f75b9722c951..e4076ff1b864 100644 --- a/src/node/test/channels.cpp +++ b/src/node/test/channels.cpp @@ -739,7 +739,8 @@ TEST_CASE("Full NodeToNode test") { case NodeMsgType::channel_msg: { - n2n.recv_message(msg.from, msg.data()); + const auto msg_body = msg.data(); + n2n.recv_message(msg.from, msg_body.data(), msg_body.size()); auto d = msg.data(); const uint8_t* data = d.data();