Merge pull request #15385 from vespa-engine/balder/reorder-for-smaller-footprint

Balder/reorder for smaller footprint

16 files changed, 86 insertions, 109 deletions

diff --git a/storage/src/tests/visiting/memory_bounded_trace_test.cpp b/storage/src/tests/visiting/memory_bounded_trace_test.cpp
index 543531ef55a..e43fa702fb5 100644
--- a/storage/src/tests/visiting/memory_bounded_trace_test.cpp
+++ b/storage/src/tests/visiting/memory_bounded_trace_test.cpp
@@ -32,12 +32,12 @@ TEST(MemoryBoundedTraceTest, memory_used_is_accumulated_recursively_for_non_leaf
 TEST(MemoryBoundedTraceTest, trace_nodes_can_be_moved_and_implicitly_cleared) {
     MemoryBoundedTrace trace(100);
     EXPECT_TRUE(trace.add(mbus::TraceNode("hello world", epoch)));
-    mbus::TraceNode target;
+    mbus::Trace target;
     trace.moveTraceTo(target);
     EXPECT_EQ(1, target.getNumChildren());
     EXPECT_EQ(0, trace.getApproxMemoryUsed());
     
-    mbus::TraceNode emptinessCheck;
+    mbus::Trace emptinessCheck;
     trace.moveTraceTo(emptinessCheck);
     EXPECT_EQ(0, emptinessCheck.getNumChildren());
 }
@@ -52,7 +52,7 @@ TEST(MemoryBoundedTraceTest, trace_nodes_can_be_moved_and_implicitly_cleared) {
 TEST(MemoryBoundedTraceTest, moved_trace_tree_is_marked_as_strict) {
     MemoryBoundedTrace trace(100);
     EXPECT_TRUE(trace.add(mbus::TraceNode("hello world", epoch)));
-    mbus::TraceNode target;
+    mbus::Trace target;
     trace.moveTraceTo(target);
     EXPECT_EQ(1, target.getNumChildren());
     EXPECT_TRUE(target.getChild(0).isStrict());
@@ -69,7 +69,7 @@ TEST(MemoryBoundedTraceTest, can_not_add_more_nodes_when_memory_used_exceeds_upp
                                            "the freeway", epoch)));
     EXPECT_EQ(11, trace.getApproxMemoryUsed());
 
-    mbus::TraceNode target;
+    mbus::Trace target;
     trace.moveTraceTo(target);
     // Twice nested node (root -> added trace tree -> leaf with txt).
     EXPECT_EQ(1, target.getNumChildren());
@@ -82,7 +82,7 @@ TEST(MemoryBoundedTraceTest, moved_tree_includes_stats_node_when_nodes_omitted)
     EXPECT_TRUE(trace.add(mbus::TraceNode("abcdef", epoch)));
     EXPECT_FALSE(trace.add(mbus::TraceNode("ghijkjlmn", epoch)));
 
-    mbus::TraceNode target;
+    mbus::Trace target;
     trace.moveTraceTo(target);
     EXPECT_EQ(1, target.getNumChildren());
     EXPECT_EQ(2, target.getChild(0).getNumChildren());
diff --git a/storage/src/tests/visiting/visitortest.cpp b/storage/src/tests/visiting/visitortest.cpp
index 72668564345..f727cdf8eb2 100644
--- a/storage/src/tests/visiting/visitortest.cpp
+++ b/storage/src/tests/visiting/visitortest.cpp
@@ -799,15 +799,16 @@ TEST_F(VisitorTest, no_mbus_tracing_if_trace_level_is_zero) {
     cmd->getTrace().setLevel(0);
     std::shared_ptr<api::CreateVisitorReply> reply;
     ASSERT_NO_FATAL_FAILURE(doCompleteVisitingSession(cmd, reply));
-    EXPECT_TRUE(reply->getTrace().getRoot().isEmpty());
+    EXPECT_TRUE(reply->getTrace().isEmpty());
 }
 
 TEST_F(VisitorTest, reply_contains_trace_if_trace_level_above_zero) {
     std::shared_ptr<api::CreateVisitorCommand> cmd(makeCreateVisitor());
     cmd->getTrace().setLevel(1);
+    cmd->getTrace().trace(1,"at least one trace.");
     std::shared_ptr<api::CreateVisitorReply> reply;
     ASSERT_NO_FATAL_FAILURE(doCompleteVisitingSession(cmd, reply));
-    EXPECT_FALSE(reply->getTrace().getRoot().isEmpty());
+    EXPECT_FALSE(reply->getTrace().isEmpty());
 }
 
 TEST_F(VisitorTest, no_more_iterators_sent_while_memory_used_above_limit) {
diff --git a/storage/src/vespa/storage/distributor/operations/external/getoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/getoperation.cpp
index c4fb9b8228c..c72e5731a59 100644
--- a/storage/src/vespa/storage/distributor/operations/external/getoperation.cpp
+++ b/storage/src/vespa/storage/distributor/operations/external/getoperation.cpp
@@ -144,9 +144,7 @@ GetOperation::onReceive(DistributorMessageSender& sender, const std::shared_ptr<
 
     LOG(debug, "Received %s", msg->toString(true).c_str());
 
-    if ( ! getreply->getTrace().getRoot().isEmpty()) {
-        _msg->getTrace().getRoot().addChild(getreply->getTrace().getRoot());
-    }
+    _msg->getTrace().addChild(getreply->steal_trace());
     bool allDone = true;
     for (auto& response : _responses) {
         for (uint32_t i = 0; i < response.second.size(); i++) {
diff --git a/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp
index 3866ee4e6f7..d7dd0be7f4f 100644
--- a/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp
+++ b/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.cpp
@@ -27,21 +27,22 @@ TwoPhaseUpdateOperation::TwoPhaseUpdateOperation(
         DistributorMetricSet& metrics,
         SequencingHandle sequencingHandle)
     : SequencedOperation(std::move(sequencingHandle)),
-    _updateMetric(metrics.updates[msg->getLoadType()]),
-    _putMetric(metrics.update_puts[msg->getLoadType()]),
-    _getMetric(metrics.update_gets[msg->getLoadType()]),
-    _metadata_get_metrics(metrics.update_metadata_gets[msg->getLoadType()]),
-    _updateCmd(std::move(msg)),
-    _updateReply(),
-    _manager(manager),
-    _bucketSpace(bucketSpace),
-    _sendState(SendState::NONE_SENT),
-    _mode(Mode::FAST_PATH),
-    _single_get_latency_timer(),
-    _fast_path_repair_source_node(0xffff),
-    _use_initial_cheap_metadata_fetch_phase(
+      _updateMetric(metrics.updates[msg->getLoadType()]),
+      _putMetric(metrics.update_puts[msg->getLoadType()]),
+      _getMetric(metrics.update_gets[msg->getLoadType()]),
+      _metadata_get_metrics(metrics.update_metadata_gets[msg->getLoadType()]),
+      _updateCmd(std::move(msg)),
+      _updateReply(),
+      _manager(manager),
+      _bucketSpace(bucketSpace),
+      _sendState(SendState::NONE_SENT),
+      _mode(Mode::FAST_PATH),
+      _trace(_updateCmd->getTrace().getLevel()),
+      _single_get_latency_timer(),
+      _fast_path_repair_source_node(0xffff),
+      _use_initial_cheap_metadata_fetch_phase(
             _manager.getDistributor().getConfig().enable_metadata_only_fetch_phase_for_inconsistent_updates()),
-    _replySent(false)
+      _replySent(false)
 {
     document::BucketIdFactory idFactory;
     _updateDocBucketId = idFactory.getBucketId(_updateCmd->getDocumentId());
@@ -132,9 +133,7 @@ TwoPhaseUpdateOperation::sendReply(
         std::shared_ptr<api::StorageReply>& reply)
 {
     assert(!_replySent);
-    if (!_trace.isEmpty()) {
-        reply->getTrace().getRoot().addChild(_trace);
-    }
+    reply->getTrace().addChild(std::move(_trace));
     sender.sendReply(reply);
     _replySent = true;
 }
@@ -645,11 +644,9 @@ TwoPhaseUpdateOperation::satisfiesUpdateTimestampConstraint(api::Timestamp ts) c
 }
 
 void
-TwoPhaseUpdateOperation::addTraceFromReply(const api::StorageReply& reply)
+TwoPhaseUpdateOperation::addTraceFromReply(api::StorageReply & reply)
 {
-    if ( ! reply.getTrace().getRoot().isEmpty()) {
-        _trace.addChild(reply.getTrace().getRoot());
-    }
+    _trace.addChild(reply.steal_trace());
 }
 
 void
diff --git a/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.h b/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.h
index 2d8f3e8494d..8527ff0ffba 100644
--- a/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.h
+++ b/storage/src/vespa/storage/distributor/operations/external/twophaseupdateoperation.h
@@ -125,7 +125,7 @@ private:
             DistributorMessageSender& sender,
             const document::Document& candidateDoc);
     bool satisfiesUpdateTimestampConstraint(api::Timestamp) const;
-    void addTraceFromReply(const api::StorageReply& reply);
+    void addTraceFromReply(api::StorageReply& reply);
     bool hasTasCondition() const noexcept;
     void replyWithTasFailure(DistributorMessageSender& sender,
                              vespalib::stringref message);
@@ -146,7 +146,7 @@ private:
     SentMessageMap _sentMessageMap;
     SendState _sendState;
     Mode _mode;
-    mbus::TraceNode _trace;
+    mbus::Trace _trace;
     document::BucketId _updateDocBucketId;
     std::vector<std::pair<document::BucketId, uint16_t>> _replicas_at_get_send_time;
     std::optional<framework::MilliSecTimer> _single_get_latency_timer;
diff --git a/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp b/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp
index 5a03e05d563..616fd312723 100644
--- a/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp
+++ b/storage/src/vespa/storage/distributor/operations/external/visitoroperation.cpp
@@ -171,7 +171,7 @@ VisitorOperation::onReceive(
 {
     api::CreateVisitorReply& reply = static_cast<api::CreateVisitorReply&>(*r);
 
-    _trace.add(reply.getTrace().getRoot());
+    _trace.add(reply.steal_trace());
 
     SentMessagesMap::iterator iter = _sentMessages.find(reply.getMsgId());
     assert(iter != _sentMessages.end());
@@ -828,8 +828,8 @@ VisitorOperation::sendReply(const api::ReturnCode& code, DistributorMessageSende
 {
     if (!_sentReply) {
         // Send create visitor reply
-        api::CreateVisitorReply::SP reply(new api::CreateVisitorReply(*_msg));
-        _trace.moveTraceTo(reply->getTrace().getRoot());
+        auto reply = std::make_shared<api::CreateVisitorReply>(*_msg);
+        _trace.moveTraceTo(reply->getTrace());
         reply->setLastBucket(getLastBucketVisited());
         reply->setResult(code);
 
diff --git a/storage/src/vespa/storage/distributor/operations/external/visitoroperation.h b/storage/src/vespa/storage/distributor/operations/external/visitoroperation.h
index 42b0bd56b9e..24c7157b481 100644
--- a/storage/src/vespa/storage/distributor/operations/external/visitoroperation.h
+++ b/storage/src/vespa/storage/distributor/operations/external/visitoroperation.h
@@ -38,7 +38,7 @@ public:
                      const Config& config,
                      VisitorMetricSet& metrics);
 
-    ~VisitorOperation();
+    ~VisitorOperation() override;
 
     void onClose(DistributorMessageSender& sender) override;
     void onStart(DistributorMessageSender& sender) override;
diff --git a/storage/src/vespa/storage/distributor/persistencemessagetracker.cpp b/storage/src/vespa/storage/distributor/persistencemessagetracker.cpp
index 6362db6c002..584cb379400 100644
--- a/storage/src/vespa/storage/distributor/persistencemessagetracker.cpp
+++ b/storage/src/vespa/storage/distributor/persistencemessagetracker.cpp
@@ -22,6 +22,7 @@ PersistenceMessageTrackerImpl::PersistenceMessageTrackerImpl(
       _reply(std::move(reply)),
       _manager(link),
       _revertTimestamp(revertTimestamp),
+      _trace(_reply->getTrace().getLevel()),
       _requestTimer(link.getClock()),
       _n_persistence_replies_total(0),
       _n_successful_persistence_replies(0),
@@ -163,25 +164,18 @@ PersistenceMessageTrackerImpl::addBucketInfoFromReply(
             bucket.toString().c_str(),
             bucketInfo.toString().c_str(),
             node);
-        _remapBucketInfo[bucket].push_back(
-                BucketCopy(_manager.getUniqueTimestamp(),
-                           node,
-                           bucketInfo));
+        _remapBucketInfo[bucket].emplace_back(_manager.getUniqueTimestamp(), node, bucketInfo);
     } else {
         LOG(debug, "Bucket %s: Received bucket info %s from node %d",
             bucket.toString().c_str(),
             bucketInfo.toString().c_str(),
             node);
-        _bucketInfo[bucket].push_back(
-                BucketCopy(_manager.getUniqueTimestamp(),
-                           node,
-                           bucketInfo));
+        _bucketInfo[bucket].emplace_back(_manager.getUniqueTimestamp(), node, bucketInfo);
     }
 }
 
 void
-PersistenceMessageTrackerImpl::logSuccessfulReply(uint16_t node, 
-                                              const api::BucketInfoReply& reply) const
+PersistenceMessageTrackerImpl::logSuccessfulReply(uint16_t node, const api::BucketInfoReply& reply) const
 {
     LOG(spam, "Bucket %s: Received successful reply %s",
         reply.getBucketId().toString().c_str(),
@@ -230,7 +224,7 @@ PersistenceMessageTrackerImpl::sendReply(MessageSender& sender)
     updateMetrics();
     _trace.setStrict(false);
     if ( ! _trace.isEmpty()) {
-        _reply->getTrace().getRoot().addChild(_trace);
+        _reply->getTrace().addChild(std::move(_trace));
     }
     
     sender.sendReply(_reply);
@@ -292,9 +286,7 @@ PersistenceMessageTrackerImpl::updateFromReply(
         api::BucketInfoReply& reply,
         uint16_t node)
 {
-    if ( ! reply.getTrace().getRoot().isEmpty()) {
-        _trace.addChild(reply.getTrace().getRoot());
-    }
+    _trace.addChild(reply.steal_trace());
 
     if (reply.getType() == api::MessageType::CREATEBUCKET_REPLY) {
         handleCreateBucketReply(reply, node);
diff --git a/storage/src/vespa/storage/distributor/persistencemessagetracker.h b/storage/src/vespa/storage/distributor/persistencemessagetracker.h
index 4392aa1fc30..cf9f4017eda 100644
--- a/storage/src/vespa/storage/distributor/persistencemessagetracker.h
+++ b/storage/src/vespa/storage/distributor/persistencemessagetracker.h
@@ -73,7 +73,7 @@ private:
     DistributorComponent& _manager;
     api::Timestamp _revertTimestamp;
     std::vector<BucketNodePair> _revertNodes;
-    mbus::TraceNode _trace;
+    mbus::Trace _trace;
     framework::MilliSecTimer _requestTimer;
     uint32_t _n_persistence_replies_total;
     uint32_t _n_successful_persistence_replies;
diff --git a/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp b/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp
index cba3969cd68..54c1c7606b2 100644
--- a/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp
+++ b/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp
@@ -676,7 +676,7 @@ FileStorManager::onInternal(const shared_ptr<api::InternalCommand>& msg)
         spi::Context context(msg->getLoadType(), msg->getPriority(), msg->getTrace().getLevel());
         shared_ptr<DestroyIteratorCommand> cmd(std::static_pointer_cast<DestroyIteratorCommand>(msg));
         _provider->destroyIterator(cmd->getIteratorId(), context);
-        msg->getTrace().getRoot().addChild(context.getTrace().getRoot());
+        msg->getTrace().addChild(context.steal_trace());
         return true;
     }
     case ReadBucketList::ID:
diff --git a/storage/src/vespa/storage/persistence/persistenceutil.cpp b/storage/src/vespa/storage/persistence/persistenceutil.cpp
index 42d67573763..9a7107fb583 100644
--- a/storage/src/vespa/storage/persistence/persistenceutil.cpp
+++ b/storage/src/vespa/storage/persistence/persistenceutil.cpp
@@ -93,9 +93,7 @@ MessageTracker::sendReply() {
               _msg->toString(true).c_str(), vespalib::to_s(duration));
     }
     if (hasReply()) {
-        if ( ! _context.getTrace().getRoot().isEmpty()) {
-            getReply().getTrace().getRoot().addChild(_context.getTrace().getRoot());
-        }
+        getReply().getTrace().addChild(_context.steal_trace());
         if (_updateBucketInfo) {
             if (getReply().getResult().success()) {
                 _env.setBucketInfo(*this, _bucketLock->getBucket());
@@ -104,13 +102,10 @@ MessageTracker::sendReply() {
         if (getReply().getResult().success()) {
             _metric->latency.addValue(_timer.getElapsedTimeAsDouble());
         }
-        LOG(spam, "Sending reply up: %s %" PRIu64,
-            getReply().toString().c_str(), getReply().getMsgId());
+        LOG(spam, "Sending reply up: %s %" PRIu64, getReply().toString().c_str(), getReply().getMsgId());
         _replySender.sendReplyDirectly(std::move(_reply));
     } else {
-        if ( ! _context.getTrace().getRoot().isEmpty()) {
-            _msg->getTrace().getRoot().addChild(_context.getTrace().getRoot());
-        }
+        _msg->getTrace().addChild(_context.steal_trace());
     }
 }
 
diff --git a/storage/src/vespa/storage/storageserver/communicationmanager.cpp b/storage/src/vespa/storage/storageserver/communicationmanager.cpp
index a35b9d1d59a..940bffdbcc6 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(docMsgPtr->steal_trace());
         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(storMsgPtr->steal_trace());
         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(reply->steal_trace());
                 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(reply->steal_trace());
             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(msg->steal_trace());
             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(msg->steal_trace());
             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(reply->steal_trace());
             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(reply->steal_trace());
             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(reply->steal_trace());
         replyUP->setMessage(std::move(context._storageProtocolMsg));
     }
 
diff --git a/storage/src/vespa/storage/storageserver/rpc/storage_api_rpc_service.cpp b/storage/src/vespa/storage/storageserver/rpc/storage_api_rpc_service.cpp
index cb1d0380bf1..c465315a5a6 100644
--- a/storage/src/vespa/storage/storageserver/rpc/storage_api_rpc_service.cpp
+++ b/storage/src/vespa/storage/storageserver/rpc/storage_api_rpc_service.cpp
@@ -208,7 +208,7 @@ void StorageApiRpcService::encode_rpc_v1_response(FRT_RPCRequest& request, api::
     // TODO skip encoding header altogether if no relevant fields set?
     protobuf::ResponseHeader hdr;
     if (reply.getTrace().getLevel() > 0) {
-        hdr.set_trace_payload(reply.getTrace().getRoot().encode());
+        hdr.set_trace_payload(reply.getTrace().encode());
     }
     // TODO consistent naming...
     encode_header_into_rpc_params(hdr, *ret);
@@ -289,7 +289,7 @@ void StorageApiRpcService::RequestDone(FRT_RPCRequest* raw_req) {
     assert(reply);
 
     if (!hdr.trace_payload().empty()) {
-        cmd.getTrace().getRoot().addChild(mbus::TraceNode::decode(hdr.trace_payload()));
+        cmd.getTrace().addChild(mbus::TraceNode::decode(hdr.trace_payload()));
     }
     if (cmd.getTrace().shouldTrace(TraceLevel::SEND_RECEIVE)) {
         cmd.getTrace().trace(TraceLevel::SEND_RECEIVE,
diff --git a/storage/src/vespa/storage/visiting/memory_bounded_trace.cpp b/storage/src/vespa/storage/visiting/memory_bounded_trace.cpp
index 9f9eb2942c1..05bb026f238 100644
--- a/storage/src/vespa/storage/visiting/memory_bounded_trace.cpp
+++ b/storage/src/vespa/storage/visiting/memory_bounded_trace.cpp
@@ -6,7 +6,7 @@
 namespace storage {
 
 MemoryBoundedTrace::MemoryBoundedTrace(size_t softMemoryUpperBound)
-    : _node(),
+    : _trace(),
       _currentMemoryUsed(0),
       _omittedNodes(0),
       _omittedBytes(0),
@@ -14,53 +14,49 @@ MemoryBoundedTrace::MemoryBoundedTrace(size_t softMemoryUpperBound)
 {
 }
 
-namespace {
-
-size_t
-computeTraceTreeMemoryUsage(const mbus::TraceNode& node)
+bool
+MemoryBoundedTrace::add(const mbus::TraceNode& node)
 {
-    if (node.isLeaf()) {
-        return node.getNote().size();
-    }
-    size_t childSum = 0;
-    const uint32_t childCount = node.getNumChildren();
-    for (uint32_t i = 0; i < childCount; ++i) {
-        childSum += computeTraceTreeMemoryUsage(node.getChild(i));
+    const size_t nodeFootprint = node.computeMemoryUsage();
+
+    if (_currentMemoryUsed >= _softMemoryUpperBound) {
+        ++_omittedNodes;
+        _omittedBytes += nodeFootprint;
+        return false;
     }
-    return childSum;
+    _trace.addChild(vespalib::TraceNode(node));
+    _currentMemoryUsed += nodeFootprint;
+    return true;
 }
 
-} // anon ns
-
 bool
-MemoryBoundedTrace::add(const mbus::TraceNode& node)
+MemoryBoundedTrace::add(mbus::Trace && node)
 {
-    const size_t nodeFootprint = computeTraceTreeMemoryUsage(node);
+    const size_t nodeFootprint = node.computeMemoryUsage();
 
     if (_currentMemoryUsed >= _softMemoryUpperBound) {
         ++_omittedNodes;
         _omittedBytes += nodeFootprint;
         return false;
     }
-    _node.addChild(node);
+    _trace.addChild(std::move(node));
     _currentMemoryUsed += nodeFootprint;
     return true;
 }
 
 void
-MemoryBoundedTrace::moveTraceTo(mbus::TraceNode& out)
+MemoryBoundedTrace::moveTraceTo(mbus::Trace& out)
 {
-    if (_node.isEmpty()) {
+    if (_trace.isEmpty()) {
         return;
     }
     if (_omittedNodes > 0) {
-        _node.addChild(vespalib::make_string(
+        _trace.trace(0, vespalib::make_string(
                 "Trace too large; omitted %zu subsequent trace trees "
                 "containing a total of %zu bytes",
-                _omittedNodes, _omittedBytes));
+                _omittedNodes, _omittedBytes), false);
     }
-    out.addChild(_node); // XXX rvalue support should be added to TraceNode.
-    _node.clear();
+    out.addChild(std::move(_trace));
     _currentMemoryUsed = 0;
     _omittedNodes = 0;
     _omittedBytes = 0;
diff --git a/storage/src/vespa/storage/visiting/memory_bounded_trace.h b/storage/src/vespa/storage/visiting/memory_bounded_trace.h
index 71ce1be51ac..2c75c809af6 100644
--- a/storage/src/vespa/storage/visiting/memory_bounded_trace.h
+++ b/storage/src/vespa/storage/visiting/memory_bounded_trace.h
@@ -23,6 +23,7 @@ public:
      * otherwise.
      */
     bool add(const mbus::TraceNode& node);
+    bool add(mbus::Trace && trace);
 
     /**
      * Append current trace tree to the output trace node and clear internal
@@ -33,14 +34,14 @@ public:
      *
      * If current trace is empty, no nodes are added to `out`.
      */
-    void moveTraceTo(mbus::TraceNode& out);
+    void moveTraceTo(mbus::Trace& out);
 
     size_t getApproxMemoryUsed() const noexcept {
         return _currentMemoryUsed;
     }
 
 private:
-    mbus::TraceNode _node;
+    mbus::Trace _trace;
     size_t _currentMemoryUsed;
     size_t _omittedNodes;
     size_t _omittedBytes;
diff --git a/storage/src/vespa/storage/visiting/visitor.cpp b/storage/src/vespa/storage/visiting/visitor.cpp
index 982bcd78f6f..9dee798de84 100644
--- a/storage/src/vespa/storage/visiting/visitor.cpp
+++ b/storage/src/vespa/storage/visiting/visitor.cpp
@@ -146,8 +146,7 @@ Visitor::BucketIterationState::~BucketIterationState()
 {
     if (_iteratorId != 0) {
         // Making the assumption that this is effectively nothrow.
-        std::shared_ptr<DestroyIteratorCommand> cmd(
-                new DestroyIteratorCommand(_iteratorId));
+        auto cmd = std::make_shared<DestroyIteratorCommand>(_iteratorId);
         cmd->setLoadType(_visitor._initiatingCmd->getLoadType());
         cmd->getTrace().setLevel(_visitor._traceLevel);
         cmd->setPriority(0);
@@ -178,7 +177,7 @@ Visitor::VisitorTarget::VisitorTarget()
 {
 }
 
-Visitor::VisitorTarget::~VisitorTarget() {}
+Visitor::VisitorTarget::~VisitorTarget() = default;
 
 Visitor::Visitor(StorageComponent& component)
     : _component(component),
@@ -372,10 +371,9 @@ Visitor::sendReplyOnce()
         std::shared_ptr<api::StorageReply> reply(_initiatingCmd->makeReply());
 
         _hitCounter->updateVisitorStatistics(_visitorStatistics);
-        static_cast<api::CreateVisitorReply*>(reply.get())
-            ->setVisitorStatistics(_visitorStatistics);
+        static_cast<api::CreateVisitorReply*>(reply.get())->setVisitorStatistics(_visitorStatistics);
         if (shouldAddMbusTrace()) {
-            _trace.moveTraceTo(reply->getTrace().getRoot());
+            _trace.moveTraceTo(reply->getTrace());
         }
         reply->setResult(_result);
         LOG(debug, "Sending %s", reply->toString(true).c_str());
@@ -603,15 +601,14 @@ bool
 Visitor::addBoundedTrace(uint32_t level, const vespalib::string &message) {
     mbus::Trace tempTrace;
     tempTrace.trace(level, message);
-    return _trace.add(tempTrace.getRoot());
+    return _trace.add(std::move(tempTrace));
 }
 
 void
-Visitor::handleDocumentApiReply(mbus::Reply::UP reply,
-                        VisitorThreadMetrics& metrics)
+Visitor::handleDocumentApiReply(mbus::Reply::UP reply, VisitorThreadMetrics& metrics)
 {
     if (shouldAddMbusTrace()) {
-        _trace.add(reply->getTrace().getRoot());
+        _trace.add(reply->steal_trace());
     }
 
     mbus::Message::UP message = reply->getMessage();
@@ -832,7 +829,7 @@ Visitor::onGetIterReply(const std::shared_ptr<GetIterReply>& reply,
     }
 
     if (shouldAddMbusTrace()) {
-        _trace.add(reply->getTrace().getRoot());
+        _trace.add(reply->steal_trace());
     }
 
     LOG(debug, "Continuing visitor %s.", _id.c_str());