Less copying of cluster state bundles

Already decoded into a `shared_ptr` during RPC request
handling; might as well use it.

3 files changed, 30 insertions, 18 deletions

diff --git a/storage/src/vespa/storage/storageserver/rpc/cluster_controller_api_rpc_service.cpp b/storage/src/vespa/storage/storageserver/rpc/cluster_controller_api_rpc_service.cpp
index 4d74eb1974b..3b53c0c9584 100644
--- a/storage/src/vespa/storage/storageserver/rpc/cluster_controller_api_rpc_service.cpp
+++ b/storage/src/vespa/storage/storageserver/rpc/cluster_controller_api_rpc_service.cpp
@@ -124,7 +124,8 @@ void ClusterControllerApiRpcService::RPC_setSystemState2(FRT_RPCRequest* req) {
                                     req->GetParams()->GetValue(0)._string._len);
     lib::ClusterState systemState(systemStateStr);
 
-    auto cmd = std::make_shared<api::SetSystemStateCommand>(lib::ClusterStateBundle(systemState));
+    auto bundle = std::make_shared<const lib::ClusterStateBundle>(systemState);
+    auto cmd = std::make_shared<api::SetSystemStateCommand>(std::move(bundle));
     cmd->setPriority(api::StorageMessage::VERYHIGH);
 
     detach_and_forward_to_enqueuer(std::move(cmd), req);
@@ -167,8 +168,7 @@ void ClusterControllerApiRpcService::RPC_setDistributionStates(FRT_RPCRequest* r
     }
     LOG(debug, "Got state bundle %s", state_bundle->toString().c_str());
 
-    // TODO add constructor taking in shared_ptr directly instead?
-    auto cmd = std::make_shared<api::SetSystemStateCommand>(*state_bundle);
+    auto cmd = std::make_shared<api::SetSystemStateCommand>(std::move(state_bundle));
     cmd->setPriority(api::StorageMessage::VERYHIGH);
 
     detach_and_forward_to_enqueuer(std::move(cmd), req);
diff --git a/storage/src/vespa/storageapi/message/state.cpp b/storage/src/vespa/storageapi/message/state.cpp
index 5a50167f584..b4e8655d783 100644
--- a/storage/src/vespa/storageapi/message/state.cpp
+++ b/storage/src/vespa/storageapi/message/state.cpp
@@ -5,8 +5,7 @@
 #include <vespa/vdslib/state/clusterstate.h>
 #include <ostream>
 
-namespace storage {
-namespace api {
+namespace storage::api {
 
 IMPLEMENT_COMMAND(GetNodeStateCommand, GetNodeStateReply)
 IMPLEMENT_REPLY(GetNodeStateReply)
@@ -45,7 +44,7 @@ GetNodeStateReply::GetNodeStateReply(const GetNodeStateCommand& cmd)
 GetNodeStateReply::GetNodeStateReply(const GetNodeStateCommand& cmd,
                                      const lib::NodeState& state)
     : StorageReply(cmd),
-      _state(new lib::NodeState(state))
+      _state(std::make_unique<lib::NodeState>(state))
 {
 }
 
@@ -64,23 +63,31 @@ GetNodeStateReply::print(std::ostream& out, bool verbose,
     }
 }
 
+SetSystemStateCommand::SetSystemStateCommand(std::shared_ptr<const lib::ClusterStateBundle> state)
+    : StorageCommand(MessageType::SETSYSTEMSTATE),
+      _state(std::move(state))
+{
+}
+
 SetSystemStateCommand::SetSystemStateCommand(const lib::ClusterStateBundle& state)
     : StorageCommand(MessageType::SETSYSTEMSTATE),
-      _state(state)
+      _state(std::make_shared<const lib::ClusterStateBundle>(state))
 {
 }
 
 SetSystemStateCommand::SetSystemStateCommand(const lib::ClusterState& state)
     : StorageCommand(MessageType::SETSYSTEMSTATE),
-      _state(state)
+      _state(std::make_shared<const lib::ClusterStateBundle>(state))
 {
 }
 
+SetSystemStateCommand::~SetSystemStateCommand() = default;
+
 void
 SetSystemStateCommand::print(std::ostream& out, bool verbose,
                              const std::string& indent) const
 {
-    out << "SetSystemStateCommand(" << *_state.getBaselineClusterState() << ")";
+    out << "SetSystemStateCommand(" << *_state->getBaselineClusterState() << ")";
     if (verbose) {
         out << " : ";
         StorageCommand::print(out, verbose, indent);
@@ -89,7 +96,7 @@ SetSystemStateCommand::print(std::ostream& out, bool verbose,
 
 SetSystemStateReply::SetSystemStateReply(const SetSystemStateCommand& cmd)
     : StorageReply(cmd),
-      _state(cmd.getClusterStateBundle())
+      _state(cmd.cluster_state_bundle_ptr())
 {
 }
 
@@ -138,5 +145,4 @@ void ActivateClusterStateVersionReply::print(std::ostream& out, bool verbose,
     }
 }
 
-} // api
-} // storage
+} // storage::api
diff --git a/storage/src/vespa/storageapi/message/state.h b/storage/src/vespa/storageapi/message/state.h
index 900355b12a2..4aa4c8a8f31 100644
--- a/storage/src/vespa/storageapi/message/state.h
+++ b/storage/src/vespa/storageapi/message/state.h
@@ -61,13 +61,19 @@ public:
  *  put/get/remove etx)
  */
 class SetSystemStateCommand : public StorageCommand {
-    lib::ClusterStateBundle _state;
+    std::shared_ptr<const lib::ClusterStateBundle> _state;
 
 public:
+    explicit SetSystemStateCommand(std::shared_ptr<const lib::ClusterStateBundle> state);
     explicit SetSystemStateCommand(const lib::ClusterStateBundle &state);
     explicit SetSystemStateCommand(const lib::ClusterState &state);
-    const lib::ClusterState& getSystemState() const { return *_state.getBaselineClusterState(); }
-    const lib::ClusterStateBundle& getClusterStateBundle() const { return _state; }
+    ~SetSystemStateCommand() override;
+
+    [[nodiscard]] const lib::ClusterState& getSystemState() const { return *_state->getBaselineClusterState(); }
+    [[nodiscard]] const lib::ClusterStateBundle& getClusterStateBundle() const { return *_state; }
+    [[nodiscard]] std::shared_ptr<const lib::ClusterStateBundle> cluster_state_bundle_ptr() const noexcept {
+        return _state;
+    }
     void print(std::ostream& out, bool verbose, const std::string& indent) const override;
 
     DECLARE_STORAGECOMMAND(SetSystemStateCommand, onSetSystemState)
@@ -80,14 +86,14 @@ public:
  * @brief Reply received after a SetSystemStateCommand.
  */
 class SetSystemStateReply : public StorageReply {
-    lib::ClusterStateBundle _state;
+    std::shared_ptr<const lib::ClusterStateBundle> _state;
 
 public:
     explicit SetSystemStateReply(const SetSystemStateCommand& cmd);
 
     // Not serialized. Available locally
-    const lib::ClusterState& getSystemState() const { return *_state.getBaselineClusterState(); }
-    const lib::ClusterStateBundle& getClusterStateBundle() const { return _state; }
+    const lib::ClusterState& getSystemState() const { return *_state->getBaselineClusterState(); }
+    const lib::ClusterStateBundle& getClusterStateBundle() const { return *_state; }
     void print(std::ostream& out, bool verbose, const std::string& indent) const override;
 
     DECLARE_STORAGEREPLY(SetSystemStateReply, onSetSystemStateReply)