Explicit copy construction of Trace.

6 files changed, 21 insertions, 20 deletions

diff --git a/messagebus/src/vespa/messagebus/routable.h b/messagebus/src/vespa/messagebus/routable.h
index 50cb4e090ce..dbae90c9aa1 100644
--- a/messagebus/src/vespa/messagebus/routable.h
+++ b/messagebus/src/vespa/messagebus/routable.h
@@ -106,7 +106,7 @@ public:
      *
      * @param trace The trace to set.
      */
-    void setTrace(const Trace &trace) { _trace = trace; }
+    void setTrace(Trace &&trace) { _trace = std::move(trace); }
 
     /**
      * Swaps the state that makes this routable unique to another routable. The
diff --git a/storage/src/vespa/storage/storageserver/communicationmanager.cpp b/storage/src/vespa/storage/storageserver/communicationmanager.cpp
index a35b9d1d59a..f0c62f128cf 100644
--- a/storage/src/vespa/storage/storageserver/communicationmanager.cpp
+++ b/storage/src/vespa/storage/storageserver/communicationmanager.cpp
@@ -102,7 +102,7 @@ CommunicationManager::handleMessage(std::unique_ptr<mbus::Message> msg)
             return;
         }
 
-        cmd->setTrace(docMsgPtr->getTrace());
+        cmd->setTrace(vespalib::Trace(docMsgPtr->getTrace())); // Can it be moved ?
         cmd->setTransportContext(std::make_unique<StorageTransportContext>(std::move(docMsgPtr)));
 
         enqueue_or_process(std::move(cmd));
@@ -114,7 +114,7 @@ CommunicationManager::handleMessage(std::unique_ptr<mbus::Message> msg)
         //TODO: Can it be moved ?
         std::shared_ptr<api::StorageCommand> cmd = storMsgPtr->getCommand();
         cmd->setTimeout(storMsgPtr->getTimeRemaining());
-        cmd->setTrace(storMsgPtr->getTrace());
+        cmd->setTrace(vespalib::Trace(storMsgPtr->getTrace())); // Can it be moved ?
         cmd->setTransportContext(std::make_unique<StorageTransportContext>(std::move(storMsgPtr)));
 
         enqueue_or_process(std::move(cmd));
@@ -203,12 +203,12 @@ CommunicationManager::handleReply(std::unique_ptr<mbus::Reply> reply)
                     _docApiConverter.toStorageAPI(static_cast<documentapi::DocumentReply&>(*reply), *originalCommand));
 
             if (sar) {
-                sar->setTrace(reply->getTrace());
+                sar->setTrace(vespalib::Trace(reply->getTrace())); // Can it be moved ?
                 receiveStorageReply(sar);
             }
         } else if (protocolName == mbusprot::StorageProtocol::NAME) {
             mbusprot::StorageReply* sr(static_cast<mbusprot::StorageReply*>(reply.get()));
-            sr->getReply()->setTrace(reply->getTrace());
+            sr->getReply()->setTrace(vespalib::Trace(reply->getTrace()));
             receiveStorageReply(sr->getReply());
         } else {
             LOGBM(warning, "Received unsupported reply type %d for protocol '%s'.",
@@ -583,7 +583,7 @@ CommunicationManager::sendCommand(
             cmd->setContext(mbus::Context(msg->getMsgId()));
             cmd->setRetryEnabled(false);
             cmd->setTimeRemaining(msg->getTimeout());
-            cmd->setTrace(msg->getTrace());
+            cmd->setTrace(vespalib::Trace(msg->getTrace()));  //TODO Refactor so that we can move instead
             sendMessageBusMessage(msg, std::move(cmd), address.getRoute());
         }
         break;
@@ -596,7 +596,7 @@ CommunicationManager::sendCommand(
 
         if (mbusMsg) {
             MBUS_TRACE(msg->getTrace(), 7, "Communication manager: Converted OK");
-            mbusMsg->setTrace(msg->getTrace());
+            mbusMsg->setTrace(vespalib::Trace(msg->getTrace())); //TODO Refactor so that we can move instead
             mbusMsg->setRetryEnabled(false);
 
             {
@@ -690,13 +690,13 @@ CommunicationManager::sendMessageBusReply(
         if (reply->getResult().getResult() == api::ReturnCode::WRONG_DISTRIBUTION) {
             replyUP = std::make_unique<documentapi::WrongDistributionReply>(reply->getResult().getMessage());
             replyUP->swapState(*context._docAPIMsg);
-            replyUP->setTrace(reply->getTrace());
+            replyUP->setTrace(std::move(reply->getTrace()));
             replyUP->addError(mbus::Error(documentapi::DocumentProtocol::ERROR_WRONG_DISTRIBUTION,
                                           reply->getResult().getMessage()));
         } else {
             replyUP = context._docAPIMsg->createReply();
             replyUP->swapState(*context._docAPIMsg);
-            replyUP->setTrace(reply->getTrace());
+            replyUP->setTrace(std::move(reply->getTrace()));
             replyUP->setMessage(std::move(context._docAPIMsg));
             _docApiConverter.transferReplyState(*reply, *replyUP);
         }
@@ -707,7 +707,7 @@ CommunicationManager::sendMessageBusReply(
         }
 
         replyUP->swapState(*context._storageProtocolMsg);
-        replyUP->setTrace(reply->getTrace());
+        replyUP->setTrace(std::move(reply->getTrace()));
         replyUP->setMessage(std::move(context._storageProtocolMsg));
     }
 
diff --git a/storageapi/src/vespa/storageapi/messageapi/storagecommand.cpp b/storageapi/src/vespa/storageapi/messageapi/storagecommand.cpp
index 3e886ac872e..82d3dd8f6a8 100644
--- a/storageapi/src/vespa/storageapi/messageapi/storagecommand.cpp
+++ b/storageapi/src/vespa/storageapi/messageapi/storagecommand.cpp
@@ -11,8 +11,9 @@ StorageCommand::StorageCommand(const StorageCommand& other)
       _timeout(other._timeout),
       _sourceIndex(other._sourceIndex)
 {
+    // TODD do we really need copy construction, seems only use by CreateVisitorCommand ?
     if ( !other.getTrace().isEmpty()) {
-        setTrace(other.getTrace());
+        setTrace(vespalib::Trace(other.getTrace()));
     } else {
         getTrace().setLevel(other.getTrace().getLevel());
     }
diff --git a/storageapi/src/vespa/storageapi/messageapi/storagemessage.h b/storageapi/src/vespa/storageapi/messageapi/storagemessage.h
index 366ba684445..9135a6619ef 100644
--- a/storageapi/src/vespa/storageapi/messageapi/storagemessage.h
+++ b/storageapi/src/vespa/storageapi/messageapi/storagemessage.h
@@ -362,7 +362,7 @@ protected:
     Id                 _msgId;
     std::unique_ptr<StorageMessageAddress> _address;
     documentapi::LoadType _loadType;
-    mbus::Trace _trace;
+    vespalib::Trace _trace;
     uint32_t    _approxByteSize;
     Priority    _priority;
 
@@ -441,7 +441,7 @@ public:
     /**
        Sets the trace object for this message.
     */
-    void setTrace(const mbus::Trace &trace) { _trace = trace; }
+    void setTrace(vespalib::Trace && trace) { _trace = std::move(trace); }
 
     /**
      * Cheap version of tostring().
diff --git a/storageapi/src/vespa/storageapi/messageapi/storagereply.cpp b/storageapi/src/vespa/storageapi/messageapi/storagereply.cpp
index 57dab64e0bd..2bb9fabd7d5 100644
--- a/storageapi/src/vespa/storageapi/messageapi/storagereply.cpp
+++ b/storageapi/src/vespa/storageapi/messageapi/storagereply.cpp
@@ -14,8 +14,9 @@ StorageReply::StorageReply(const StorageCommand& cmd, ReturnCode code)
     if (cmd.getAddress()) {
         setAddress(*cmd.getAddress());
     }
+    // TODD do we really need copy construction
     if ( ! cmd.getTrace().isEmpty()) {
-        setTrace(cmd.getTrace());
+        setTrace(vespalib::Trace(cmd.getTrace()));
     }  else {
         getTrace().setLevel(cmd.getTrace().getLevel());
     }
diff --git a/vespalib/src/vespa/vespalib/trace/trace.h b/vespalib/src/vespa/vespalib/trace/trace.h
index 55d35f55617..5be9b662c89 100644
--- a/vespalib/src/vespa/vespalib/trace/trace.h
+++ b/vespalib/src/vespa/vespalib/trace/trace.h
@@ -24,13 +24,12 @@ public:
      * Create an empty Trace with level set to 0 (no tracing)
      */
     Trace() : Trace(0) {}
-
-    /**
-     * Create an empty trace with given level.
-     *
-     * @param level Level to set.
-     */
     explicit Trace(uint32_t level) : _root(), _level(level) { }
+    Trace & operator = (Trace &&) = default;
+    Trace(Trace &&) = default;
+    Trace(const Trace &) = default;
+    Trace & operator = (const Trace &) = delete;
+    ~Trace() = default;
 
     /**
      * Remove all trace information and set the trace level to 0.