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

Balder/reorder for smaller footprint

42 files changed, 304 insertions, 302 deletions

diff --git a/documentapi/src/vespa/documentapi/loadtypes/loadtypeset.cpp b/documentapi/src/vespa/documentapi/loadtypes/loadtypeset.cpp
index cde15776b62..42aa79332db 100644
--- a/documentapi/src/vespa/documentapi/loadtypes/loadtypeset.cpp
+++ b/documentapi/src/vespa/documentapi/loadtypes/loadtypeset.cpp
@@ -38,7 +38,7 @@ LoadTypeSet::LoadTypeSet(const LoadTypeConfig& config)
     configure(config);
 }
 
-LoadTypeSet::~LoadTypeSet() { }
+LoadTypeSet::~LoadTypeSet() = default;
 
 void
 LoadTypeSet::addLoadType(uint32_t id, const string& name, Priority::Value priority) {
diff --git a/messagebus/src/tests/simpleprotocol/simpleprotocol.cpp b/messagebus/src/tests/simpleprotocol/simpleprotocol.cpp
index 715ef597767..bc4fc80abad 100644
--- a/messagebus/src/tests/simpleprotocol/simpleprotocol.cpp
+++ b/messagebus/src/tests/simpleprotocol/simpleprotocol.cpp
@@ -6,9 +6,7 @@
 #include <vespa/messagebus/testlib/simplereply.h>
 #include <vespa/messagebus/testlib/slobrok.h>
 #include <vespa/messagebus/testlib/testserver.h>
-#include <vespa/messagebus/errorcode.h>
 #include <vespa/messagebus/ireplyhandler.h>
-#include <vespa/messagebus/network/identity.h>
 #include <vespa/messagebus/routing/routingcontext.h>
 #include <vespa/vespalib/testkit/testapp.h>
 #include <vespa/vespalib/component/vtag.h>
@@ -29,12 +27,15 @@ Test::Main()
     {
         // test protocol
         IRoutingPolicy::UP bogus = protocol.createPolicy("bogus", "");
-        EXPECT_TRUE(bogus.get() == 0);
+        EXPECT_FALSE(bogus);
     }
     TEST_FLUSH();
     {
         // test SimpleMessage
-        Message::UP msg(new SimpleMessage("test"));
+        EXPECT_EQUAL(56u, sizeof(Routable));
+        EXPECT_EQUAL(104u, sizeof(Message));
+        EXPECT_EQUAL(184u, sizeof(SimpleMessage));
+        auto msg = std::make_unique<SimpleMessage>("test");
         EXPECT_TRUE(!msg->isReply());
         EXPECT_TRUE(msg->getProtocol() == SimpleProtocol::NAME);
         EXPECT_TRUE(msg->getType() == SimpleProtocol::MESSAGE);
@@ -42,7 +43,7 @@ Test::Main()
         Blob b = protocol.encode(version, *msg);
         EXPECT_TRUE(b.size() > 0);
         Routable::UP tmp = protocol.decode(version, BlobRef(b));
-        ASSERT_TRUE(tmp.get() != 0);
+        ASSERT_TRUE(tmp);
         EXPECT_TRUE(!tmp->isReply());
         EXPECT_TRUE(tmp->getProtocol() == SimpleProtocol::NAME);
         EXPECT_TRUE(tmp->getType() == SimpleProtocol::MESSAGE);
@@ -51,7 +52,10 @@ Test::Main()
     TEST_FLUSH();
     {
         // test SimpleReply
-        Reply::UP reply(new SimpleReply("reply"));
+        EXPECT_EQUAL(56u, sizeof(Routable));
+        EXPECT_EQUAL(96u, sizeof(Reply));
+        EXPECT_EQUAL(160u, sizeof(SimpleReply));
+        auto reply = std::make_unique<SimpleReply>("reply");
         EXPECT_TRUE(reply->isReply());
         EXPECT_TRUE(reply->getProtocol() == SimpleProtocol::NAME);
         EXPECT_TRUE(reply->getType() == SimpleProtocol::REPLY);
@@ -59,7 +63,7 @@ Test::Main()
         Blob b = protocol.encode(version, *reply);
         EXPECT_TRUE(b.size() > 0);
         Routable::UP tmp = protocol.decode(version, BlobRef(b));
-        ASSERT_TRUE(tmp.get() != 0);
+        ASSERT_TRUE(tmp);
         EXPECT_TRUE(tmp->isReply());
         EXPECT_TRUE(tmp->getProtocol() == SimpleProtocol::NAME);
         EXPECT_TRUE(tmp->getType() == SimpleProtocol::REPLY);
diff --git a/messagebus/src/tests/trace-roundtrip/trace-roundtrip.cpp b/messagebus/src/tests/trace-roundtrip/trace-roundtrip.cpp
index 344d38c8263..f97fc6a0010 100644
--- a/messagebus/src/tests/trace-roundtrip/trace-roundtrip.cpp
+++ b/messagebus/src/tests/trace-roundtrip/trace-roundtrip.cpp
@@ -120,6 +120,6 @@ Test::Main()
                   .addChild("Server reply")
                   .addChild("Proxy reply")
                   .addChild("Client reply");
-    EXPECT_TRUE(reply->getTrace().getRoot().encode() == t.encode());
+    EXPECT_TRUE(reply->getTrace().encode() == t.encode());
     TEST_DONE();
 }
diff --git a/messagebus/src/vespa/messagebus/network/rpcsend.cpp b/messagebus/src/vespa/messagebus/network/rpcsend.cpp
index dca7f0c997f..86c9b139f1a 100644
--- a/messagebus/src/vespa/messagebus/network/rpcsend.cpp
+++ b/messagebus/src/vespa/messagebus/network/rpcsend.cpp
@@ -178,7 +178,7 @@ RPCSend::doRequestDone(FRT_RPCRequest *req) {
         }
     } else {
         FRT_Values &ret = *req->GetReturn();
-        reply = createReply(ret, serviceName, error, trace.getRoot());
+        reply = createReply(ret, serviceName, error, trace);
     }
     if (trace.shouldTrace(TraceLevel::SEND_RECEIVE)) {
         trace.trace(TraceLevel::SEND_RECEIVE,
diff --git a/messagebus/src/vespa/messagebus/network/rpcsend.h b/messagebus/src/vespa/messagebus/network/rpcsend.h
index f3a9177d236..1ccdea6fbc5 100644
--- a/messagebus/src/vespa/messagebus/network/rpcsend.h
+++ b/messagebus/src/vespa/messagebus/network/rpcsend.h
@@ -12,7 +12,7 @@ class FRT_ReflectionBuilder;
 
 namespace vespalib::slime { struct Cursor; }
 namespace vespalib { struct Memory; }
-namespace vespalib { class TraceNode; }
+namespace vespalib { class Trace; }
 namespace mbus {
 
 class Error;
@@ -56,7 +56,7 @@ protected:
 
     virtual void build(FRT_ReflectionBuilder & builder) = 0;
     virtual std::unique_ptr<Reply> createReply(const FRT_Values & response, const string & serviceName,
-                                               Error & error, vespalib::TraceNode & rootTrace) const = 0;
+                                               Error & error, vespalib::Trace & trace) const = 0;
     virtual void encodeRequest(FRT_RPCRequest &req, const vespalib::Version &version, const Route & route,
                                const RPCServiceAddress & address, const Message & msg, uint32_t traceLevel,
                                const PayLoadFiller &filler, duration timeRemaining) const = 0;
diff --git a/messagebus/src/vespa/messagebus/network/rpcsendv1.cpp b/messagebus/src/vespa/messagebus/network/rpcsendv1.cpp
index 388ab3309c4..a8f7b47f3e3 100644
--- a/messagebus/src/vespa/messagebus/network/rpcsendv1.cpp
+++ b/messagebus/src/vespa/messagebus/network/rpcsendv1.cpp
@@ -115,7 +115,7 @@ RPCSendV1::toParams(const FRT_Values &args) const
 
 
 std::unique_ptr<Reply>
-RPCSendV1::createReply(const FRT_Values & ret, const string & serviceName, Error & error, vespalib::TraceNode & rootTrace) const
+RPCSendV1::createReply(const FRT_Values & ret, const string & serviceName, Error & error, vespalib::Trace & trace) const
 {
     vespalib::Version version          = vespalib::Version(ret[0]._string._str);
     double            retryDelay       = ret[1]._double;
@@ -127,7 +127,7 @@ RPCSendV1::createReply(const FRT_Values & ret, const string & serviceName, Error
     uint32_t          errorServicesLen = ret[4]._string_array._len;
     const char       *protocolName     = ret[5]._string._str;
     BlobRef payload(ret[6]._data._buf, ret[6]._data._len);
-    const char       *trace            = ret[7]._string._str;
+    const char       *traceStr         = ret[7]._string._str;
 
     Reply::UP reply;
     if (payload.size() > 0) {
@@ -141,7 +141,7 @@ RPCSendV1::createReply(const FRT_Values & ret, const string & serviceName, Error
         reply->addError(Error(errorCodes[i], errorMessages[i]._str,
                               errorServices[i]._len > 0 ? errorServices[i]._str : serviceName.c_str()));
     }
-    rootTrace.addChild(TraceNode::decode(trace));
+    trace.addChild(TraceNode::decode(traceStr));
     return reply;
 }
 
@@ -163,7 +163,7 @@ RPCSendV1::createResponse(FRT_Values & ret, const string & version, Reply & repl
     ret.AddString(reply.getProtocol().c_str());
     ret.AddData(std::move(payload.payload()), payload.size());
     if (reply.getTrace().getLevel() > 0) {
-        ret.AddString(reply.getTrace().getRoot().encode().c_str());
+        ret.AddString(reply.getTrace().encode().c_str());
     } else {
         ret.AddString("");
     }
diff --git a/messagebus/src/vespa/messagebus/network/rpcsendv1.h b/messagebus/src/vespa/messagebus/network/rpcsendv1.h
index 249acc50e0c..f9b4b0bdfe8 100644
--- a/messagebus/src/vespa/messagebus/network/rpcsendv1.h
+++ b/messagebus/src/vespa/messagebus/network/rpcsendv1.h
@@ -17,7 +17,7 @@ private:
                        const PayLoadFiller &filler, duration timeRemaining) const override;
 
     std::unique_ptr<Reply> createReply(const FRT_Values & response, const string & serviceName,
-                                       Error & error, vespalib::TraceNode & rootTrace) const override;
+                                       Error & error, vespalib::Trace & trace) const override;
     void createResponse(FRT_Values & ret, const string & version, Reply & reply, Blob payload) const override;
 };
 
diff --git a/messagebus/src/vespa/messagebus/network/rpcsendv2.cpp b/messagebus/src/vespa/messagebus/network/rpcsendv2.cpp
index f7303ece20f..16175fea1c2 100644
--- a/messagebus/src/vespa/messagebus/network/rpcsendv2.cpp
+++ b/messagebus/src/vespa/messagebus/network/rpcsendv2.cpp
@@ -188,7 +188,7 @@ RPCSendV2::toParams(const FRT_Values &args) const
 
 std::unique_ptr<Reply>
 RPCSendV2::createReply(const FRT_Values & ret, const string & serviceName,
-                       Error & error, vespalib::TraceNode & rootTrace) const
+                       Error & error, vespalib::Trace & rootTrace) const
 {
     uint8_t encoding = ret[3]._intval8;
     uint32_t uncompressedSize = ret[4]._intval32;
@@ -240,7 +240,7 @@ RPCSendV2::createResponse(FRT_Values & ret, const string & version, Reply & repl
     root.setString(PROTOCOL_F, reply.getProtocol());
     root.setData(BLOB_F, vespalib::Memory(payload.data(), payload.size()));
     if (reply.getTrace().getLevel() > 0) {
-        root.setString(TRACE_F, reply.getTrace().getRoot().encode());
+        root.setString(TRACE_F, reply.getTrace().encode());
     }
 
     if (reply.getNumErrors() > 0) {
diff --git a/messagebus/src/vespa/messagebus/network/rpcsendv2.h b/messagebus/src/vespa/messagebus/network/rpcsendv2.h
index c48aa90a9fb..da4154e70a8 100644
--- a/messagebus/src/vespa/messagebus/network/rpcsendv2.h
+++ b/messagebus/src/vespa/messagebus/network/rpcsendv2.h
@@ -17,7 +17,7 @@ private:
                        const PayLoadFiller &filler, duration timeRemaining) const override;
 
     std::unique_ptr<Reply> createReply(const FRT_Values & response, const string & serviceName,
-                                       Error & error, vespalib::TraceNode & rootTrace) const override;
+                                       Error & error, vespalib::Trace & trace) const override;
     void createResponse(FRT_Values & ret, const string & version, Reply & reply, Blob payload) const override;
 };
 
diff --git a/messagebus/src/vespa/messagebus/routable.h b/messagebus/src/vespa/messagebus/routable.h
index 50cb4e090ce..2c83daded1e 100644
--- a/messagebus/src/vespa/messagebus/routable.h
+++ b/messagebus/src/vespa/messagebus/routable.h
@@ -24,9 +24,9 @@ namespace mbus {
  */
 class Routable {
 private:
-    Context   _context;
-    CallStack _stack;
-    Trace     _trace;
+    Context    _context;
+    CallStack  _stack;
+    Trace      _trace;
 
 public:
     /**
@@ -90,6 +90,7 @@ public:
      * @return Trace object
      */
     Trace &getTrace() { return _trace; }
+    Trace && steal_trace() { return std::move(_trace); }
 
     /**
      * Access the Trace object for this Routable. The Trace is part of the
@@ -106,7 +107,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/messagebus/src/vespa/messagebus/routing/routingnode.cpp b/messagebus/src/vespa/messagebus/routing/routingnode.cpp
index f24afbc07ca..5a70f510dcc 100644
--- a/messagebus/src/vespa/messagebus/routing/routingnode.cpp
+++ b/messagebus/src/vespa/messagebus/routing/routingnode.cpp
@@ -184,10 +184,7 @@ RoutingNode::setReply(Reply::UP reply)
 {
     if (reply) {
         _shouldRetry = _resender != nullptr && _resender->shouldRetry(*reply);
-        if ( ! reply->getTrace().getRoot().isEmpty()) {
-            _trace.getRoot().addChild(std::move(reply->getTrace().getRoot()));
-            reply->getTrace().clear();
-        }
+        _trace.addChild(reply->steal_trace());
     }
     _reply = std::move(reply);
 }
@@ -268,14 +265,12 @@ RoutingNode::notifyMerge()
     // Merges the trace information from all children into this. This method takes care not to spend cycles
     // manipulating the trace in case tracing is disabled.
     if (_trace.getLevel() > 0) {
-        TraceNode tail;
+        Trace tail;
         for (auto * child : _children) {
-            TraceNode &root = child->_trace.getRoot();
-            tail.addChild(root);
-            root.clear();
+            tail.addChild(std::move(child->_trace));
         }
         tail.setStrict(false);
-        _trace.getRoot().addChild(tail);
+        _trace.addChild(std::move(tail));
     }
 
     // Execute the {@link RoutingPolicy#merge(RoutingContext)} method of the current routing policy. If a
diff --git a/messagebus/src/vespa/messagebus/sendproxy.cpp b/messagebus/src/vespa/messagebus/sendproxy.cpp
index c884932664b..ff514f788cd 100644
--- a/messagebus/src/vespa/messagebus/sendproxy.cpp
+++ b/messagebus/src/vespa/messagebus/sendproxy.cpp
@@ -53,11 +53,10 @@ SendProxy::handleReply(Reply::UP reply)
         } else if (logger.wants(ns_log::Logger::spam)) {
             LOG(spam, "Trace for reply:\n%s", reply->getTrace().toString().c_str());
         }
-        Trace empty;
-        trace.swap(empty);
+        trace.clear();
     } else if (trace.getLevel() > 0) {
-        trace.getRoot().addChild(reply->getTrace().getRoot());
-        trace.getRoot().normalize();
+        trace.addChild(reply->steal_trace());
+        trace.normalize();
     }
     reply->swapState(*_msg);
     reply->setMessage(std::move(_msg));
diff --git a/messagebus_test/src/tests/trace/trace.cpp b/messagebus_test/src/tests/trace/trace.cpp
index 334f00745da..1ab30303d2c 100644
--- a/messagebus_test/src/tests/trace/trace.cpp
+++ b/messagebus_test/src/tests/trace/trace.cpp
@@ -100,14 +100,14 @@ Test::Main()
     Reply::UP reply;
     SourceSession::UP ss = mb.getMessageBus().createSourceSession(src, SourceSessionParams());
     for (int i = 0; i < 50; ++i) {
-        Message::UP msg(new SimpleMessage("test"));
+        auto msg = std::make_unique<SimpleMessage>("test");
         msg->getTrace().setLevel(1);
         ss->send(std::move(msg), "test");
         reply = src.getReply(10s);
         if (reply) {
-            reply->getTrace().getRoot().normalize();
+            reply->getTrace().normalize();
             // resending breaks the trace, so retry until it has expected form
-            if (!reply->hasErrors() && reply->getTrace().getRoot().encode() == expect.encode()) {
+            if (!reply->hasErrors() && reply->getTrace().encode() == expect.encode()) {
                 break;
             }
         }
@@ -116,7 +116,7 @@ Test::Main()
     }
 
     EXPECT_TRUE(!reply->hasErrors());
-    EXPECT_EQUAL(reply->getTrace().getRoot().encode(), expect.encode());
+    EXPECT_EQUAL(reply->getTrace().encode(), expect.encode());
     EXPECT_TRUE(system((ctl_script + " stop all").c_str()) == 0);
     TEST_DONE();
 }
diff --git a/persistence/src/vespa/persistence/spi/context.h b/persistence/src/vespa/persistence/spi/context.h
index 8c31439ee75..a1cb9e68f31 100644
--- a/persistence/src/vespa/persistence/spi/context.h
+++ b/persistence/src/vespa/persistence/spi/context.h
@@ -76,6 +76,7 @@ public:
         return _readConsistency;
     }
 
+    vespalib::Trace && steal_trace() { return std::move(_trace); }
     vespalib::Trace& getTrace() { return _trace; }
     const vespalib::Trace& getTrace() const { return _trace; }
 
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());
diff --git a/storageapi/src/tests/messageapi/storage_message_address_test.cpp b/storageapi/src/tests/messageapi/storage_message_address_test.cpp
index c340cba4b28..9db282c98eb 100644
--- a/storageapi/src/tests/messageapi/storage_message_address_test.cpp
+++ b/storageapi/src/tests/messageapi/storage_message_address_test.cpp
@@ -31,6 +31,11 @@ TEST(StorageMessageAddressTest, storage_hash_covers_all_expected_fields) {
               hash_of("foo", lib::NodeType::DISTRIBUTOR, 0));
     EXPECT_NE(hash_of("foo", lib::NodeType::STORAGE, 0),
               hash_of("foo", lib::NodeType::STORAGE, 1));
+
+    EXPECT_EQ(112u, sizeof(StorageMessageAddress));
+    EXPECT_EQ(144u, sizeof(StorageMessage));
+    EXPECT_EQ(80u, sizeof(documentapi::LoadType));
+    EXPECT_EQ(16u, sizeof(mbus::Trace));
 }
 
 } // storage::api
diff --git a/storageapi/src/vespa/storageapi/messageapi/bucketreply.cpp b/storageapi/src/vespa/storageapi/messageapi/bucketreply.cpp
index 1385fc331ac..e5fb7764aeb 100644
--- a/storageapi/src/vespa/storageapi/messageapi/bucketreply.cpp
+++ b/storageapi/src/vespa/storageapi/messageapi/bucketreply.cpp
@@ -7,8 +7,7 @@
 using document::Bucket;
 using document::BucketId;
 
-namespace storage {
-namespace api {
+namespace storage::api {
 
 BucketReply::BucketReply(const BucketCommand& cmd,
                          const ReturnCode& code)
@@ -42,5 +41,4 @@ BucketReply::print(std::ostream& out, bool verbose,
     }
 }
 
-} // api
-} // storage
+}
diff --git a/storageapi/src/vespa/storageapi/messageapi/storagecommand.cpp b/storageapi/src/vespa/storageapi/messageapi/storagecommand.cpp
index d9bbf34141a..397c869f4fb 100644
--- a/storageapi/src/vespa/storageapi/messageapi/storagecommand.cpp
+++ b/storageapi/src/vespa/storageapi/messageapi/storagecommand.cpp
@@ -11,7 +11,6 @@ StorageCommand::StorageCommand(const StorageCommand& other)
       _timeout(other._timeout),
       _sourceIndex(other._sourceIndex)
 {
-    setTrace(other.getTrace());
 }
 
 StorageCommand::StorageCommand(const MessageType& type, Priority p)
diff --git a/storageapi/src/vespa/storageapi/messageapi/storagecommand.h b/storageapi/src/vespa/storageapi/messageapi/storagecommand.h
index c835168c5b7..bd55ee57a8f 100644
--- a/storageapi/src/vespa/storageapi/messageapi/storagecommand.h
+++ b/storageapi/src/vespa/storageapi/messageapi/storagecommand.h
@@ -30,7 +30,7 @@ protected:
 public:
     DECLARE_POINTER_TYPEDEFS(StorageCommand);
 
-    virtual ~StorageCommand();
+    ~StorageCommand() override;
 
     bool sourceIndexSet() const { return (_sourceIndex != 0xffff); }
     void setSourceIndex(uint16_t sourceIndex) { _sourceIndex = sourceIndex; }
diff --git a/storageapi/src/vespa/storageapi/messageapi/storagemessage.cpp b/storageapi/src/vespa/storageapi/messageapi/storagemessage.cpp
index 0f5546a5510..700669739ce 100644
--- a/storageapi/src/vespa/storageapi/messageapi/storagemessage.cpp
+++ b/storageapi/src/vespa/storageapi/messageapi/storagemessage.cpp
@@ -141,10 +141,10 @@ MessageType::print(std::ostream& out, bool verbose, const std::string& indent) c
 
 StorageMessageAddress::StorageMessageAddress(const mbus::Route& route)
     : _route(route),
-      _protocol(DOCUMENT),
+      _cluster(),
       _precomputed_storage_hash(0),
-      _cluster(""),
       _type(nullptr),
+      _protocol(DOCUMENT),
       _index(0xFFFF)
 { }
 
@@ -163,10 +163,10 @@ createAddress(vespalib::stringref cluster, const lib::NodeType& type, uint16_t i
 // TODO we ideally want this removed. Currently just in place to support usage as map key when emplacement not available
 StorageMessageAddress::StorageMessageAddress()
     : _route(),
-      _protocol(Protocol::STORAGE),
-      _precomputed_storage_hash(0),
       _cluster(),
+      _precomputed_storage_hash(0),
       _type(nullptr),
+      _protocol(Protocol::STORAGE),
       _index(0)
 {}
 
@@ -174,10 +174,10 @@ StorageMessageAddress::StorageMessageAddress()
 StorageMessageAddress::StorageMessageAddress(vespalib::stringref cluster, const lib::NodeType& type,
                                              uint16_t index, Protocol protocol)
     : _route(),
-      _protocol(protocol),
-      _precomputed_storage_hash(0),
       _cluster(cluster),
+      _precomputed_storage_hash(0),
       _type(&type),
+      _protocol(protocol),
       _index(index)
 {
     std::vector<mbus::IHopDirective::SP> directives;
@@ -269,20 +269,22 @@ StorageMessage::generateMsgId()
 StorageMessage::StorageMessage(const MessageType& type, Id id)
     : _type(type),
       _msgId(id),
-      _priority(NORMAL),
       _address(),
       _loadType(documentapi::LoadType::DEFAULT),
-      _approxByteSize(50)
+      _trace(),
+      _approxByteSize(50),
+      _priority(NORMAL)
 {
 }
 
 StorageMessage::StorageMessage(const StorageMessage& other, Id id)
     : _type(other._type),
       _msgId(id),
-      _priority(other._priority),
       _address(),
       _loadType(other._loadType),
-      _approxByteSize(other._approxByteSize)
+      _trace(other.getTrace().getLevel()),
+      _approxByteSize(other._approxByteSize),
+      _priority(other._priority)
 {
 }
 
diff --git a/storageapi/src/vespa/storageapi/messageapi/storagemessage.h b/storageapi/src/vespa/storageapi/messageapi/storagemessage.h
index aa23d3a0cae..58f167f5002 100644
--- a/storageapi/src/vespa/storageapi/messageapi/storagemessage.h
+++ b/storageapi/src/vespa/storageapi/messageapi/storagemessage.h
@@ -268,12 +268,12 @@ public:
     enum Protocol { STORAGE, DOCUMENT };
 
 private:
-    mbus::Route _route;
-    Protocol    _protocol;
+    mbus::Route       _route;
+    vespalib::string  _cluster;
     // Used for internal VDS addresses only
     size_t               _precomputed_storage_hash;
-    vespalib::string     _cluster;
     const lib::NodeType* _type;
+    Protocol             _protocol;
     uint16_t             _index;
 
 public:
@@ -359,12 +359,12 @@ protected:
     static Id generateMsgId();
 
     const MessageType& _type;
-    Id _msgId;
-    Priority _priority;
+    Id                 _msgId;
     std::unique_ptr<StorageMessageAddress> _address;
-    documentapi::LoadType _loadType;
-    mbus::Trace _trace;
-    uint32_t _approxByteSize;
+    documentapi::LoadType  _loadType;
+    vespalib::Trace        _trace;
+    uint32_t    _approxByteSize;
+    Priority    _priority;
 
     StorageMessage(const MessageType& code, Id id);
     StorageMessage(const StorageMessage&, Id id);
@@ -373,7 +373,7 @@ protected:
 public:
     StorageMessage& operator=(const StorageMessage&) = delete;
     StorageMessage(const StorageMessage&) = delete;
-    virtual ~StorageMessage();
+    ~StorageMessage() override;
 
     Id getMsgId() const { return _msgId; }
 
@@ -394,7 +394,7 @@ public:
     const StorageMessageAddress* getAddress() const { return _address.get(); }
 
     void setAddress(const StorageMessageAddress& address) {
-        _address.reset(new StorageMessageAddress(address));
+        _address = std::make_unique<StorageMessageAddress>(address);
     }
 
     /**
@@ -435,13 +435,14 @@ public:
     const documentapi::LoadType& getLoadType() const { return _loadType; }
     void setLoadType(const documentapi::LoadType& type) { _loadType = type; }
 
+    mbus::Trace && steal_trace() { return std::move(_trace); }
     mbus::Trace& getTrace() { return _trace; }
     const mbus::Trace& getTrace() const { return _trace; }
 
     /**
        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 81cdadb3623..2bb9fabd7d5 100644
--- a/storageapi/src/vespa/storageapi/messageapi/storagereply.cpp
+++ b/storageapi/src/vespa/storageapi/messageapi/storagereply.cpp
@@ -14,7 +14,12 @@ StorageReply::StorageReply(const StorageCommand& cmd, ReturnCode code)
     if (cmd.getAddress()) {
         setAddress(*cmd.getAddress());
     }
-    setTrace(cmd.getTrace());
+    // TODD do we really need copy construction
+    if ( ! cmd.getTrace().isEmpty()) {
+        setTrace(vespalib::Trace(cmd.getTrace()));
+    }  else {
+        getTrace().setLevel(cmd.getTrace().getLevel());
+    }
     setTransportContext(cmd.getTransportContext());
 }
 
diff --git a/vespalib/src/tests/trace/trace.cpp b/vespalib/src/tests/trace/trace.cpp
index 92bee3231b0..992317b0289 100644
--- a/vespalib/src/tests/trace/trace.cpp
+++ b/vespalib/src/tests/trace/trace.cpp
@@ -146,25 +146,25 @@ TEST("testTraceLevel")
     t.setLevel(4);
     EXPECT_EQUAL(4u, t.getLevel());
     t.trace(9, "no");
-    EXPECT_EQUAL(0u, t.getRoot().getNumChildren());
+    EXPECT_EQUAL(0u, t.getNumChildren());
     t.trace(8, "no");
-    EXPECT_EQUAL(0u, t.getRoot().getNumChildren());
+    EXPECT_EQUAL(0u, t.getNumChildren());
     t.trace(7, "no");
-    EXPECT_EQUAL(0u, t.getRoot().getNumChildren());
+    EXPECT_EQUAL(0u, t.getNumChildren());
     t.trace(6, "no");
-    EXPECT_EQUAL(0u, t.getRoot().getNumChildren());
+    EXPECT_EQUAL(0u, t.getNumChildren());
     t.trace(5, "no");
-    EXPECT_EQUAL(0u, t.getRoot().getNumChildren());
+    EXPECT_EQUAL(0u, t.getNumChildren());
     t.trace(4, "yes");
-    EXPECT_EQUAL(1u, t.getRoot().getNumChildren());
+    EXPECT_EQUAL(1u, t.getNumChildren());
     t.trace(3, "yes");
-    EXPECT_EQUAL(2u, t.getRoot().getNumChildren());
+    EXPECT_EQUAL(2u, t.getNumChildren());
     t.trace(2, "yes");
-    EXPECT_EQUAL(3u, t.getRoot().getNumChildren());
+    EXPECT_EQUAL(3u, t.getNumChildren());
     t.trace(1, "yes");
-    EXPECT_EQUAL(4u, t.getRoot().getNumChildren());
+    EXPECT_EQUAL(4u, t.getNumChildren());
     t.trace(0, "yes");
-    EXPECT_EQUAL(5u, t.getRoot().getNumChildren());
+    EXPECT_EQUAL(5u, t.getNumChildren());
 }
 
 TEST("testCompact")
@@ -261,10 +261,10 @@ TEST("testTraceDump")
             b1.addChild(b2);
         }
         for (int i = 0; i < 10; ++i) {
-            big.getRoot().addChild(b1);
+            big.addChild(TraceNode(b1));
         }
         string normal = big.toString();
-        string full = big.getRoot().toString();
+        string full = big.toString(100000);
         EXPECT_GREATER(normal.size(), 30000u);
         EXPECT_LESS(normal.size(), 32000u);
         EXPECT_GREATER(full.size(), 50000u);
diff --git a/vespalib/src/vespa/vespalib/trace/trace.cpp b/vespalib/src/vespa/vespalib/trace/trace.cpp
index 8ca2ef6561c..be370aebbd2 100644
--- a/vespalib/src/vespa/vespalib/trace/trace.cpp
+++ b/vespalib/src/vespa/vespalib/trace/trace.cpp
@@ -6,43 +6,13 @@
 
 namespace vespalib {
 
-Trace::Trace() :
-    _level(0),
-    _root()
+Trace::Trace(const Trace &rhs)
+    : _root(),
+      _level(rhs._level)
 {
-    // empty
-}
-
-Trace::Trace(uint32_t level) :
-    _level(level),
-    _root()
-{
-    // empty
-}
-
-Trace::~Trace() = default;
-
-Trace &
-Trace::clear()
-{
-    _level = 0;
-    _root.clear();
-    return *this;
-}
-
-Trace &
-Trace::swap(Trace &other)
-{
-    std::swap(_level, other._level);
-    _root.swap(other._root);
-    return *this;
-}
-
-Trace &
-Trace::setLevel(uint32_t level)
-{
-    _level = std::min(level, 9u);
-    return *this;
+    if (!rhs.isEmpty()) {
+        _root = std::make_unique<TraceNode>(rhs.getRoot());
+    }
 }
 
 bool
@@ -53,12 +23,36 @@ Trace::trace(uint32_t level, const string &note, bool addTime)
     }
     if (addTime) {
         struct timeval tv;
-        gettimeofday(&tv, NULL);
-        _root.addChild(make_string("[%ld.%06ld] %s", tv.tv_sec, static_cast<long>(tv.tv_usec), note.c_str()));
+        gettimeofday(&tv, nullptr);
+        ensureRoot().addChild(make_string("[%ld.%06ld] %s", tv.tv_sec, static_cast<long>(tv.tv_usec), note.c_str()));
     } else {
-        _root.addChild(note);
+        ensureRoot().addChild(note);
     }
     return true;
 }
 
+string
+Trace::toString(size_t limit) const {
+    return _root ? _root->toString(limit) : "";
+}
+
+string
+Trace::encode() const {
+    return isEmpty() ? "" : getRoot().encode();
+}
+
+void
+Trace::clear() {
+    _level = 0;
+    _root.reset();
+}
+
+TraceNode &
+Trace::ensureRoot() {
+    if (!_root) {
+        _root = std::make_unique<TraceNode>();
+    }
+    return *_root;
+}
+
 } // namespace vespalib
diff --git a/vespalib/src/vespa/vespalib/trace/trace.h b/vespalib/src/vespa/vespalib/trace/trace.h
index d1ca5c5d7e7..2f70785d77d 100644
--- a/vespalib/src/vespa/vespalib/trace/trace.h
+++ b/vespalib/src/vespa/vespalib/trace/trace.h
@@ -18,31 +18,23 @@ namespace vespalib {
  * information will be traced.
  */
 class Trace {
-private:
-    uint32_t  _level;
-    TraceNode _root;
-
 public:
 
     /**
      * Create an empty Trace with level set to 0 (no tracing)
      */
-    Trace();
-    ~Trace();
-
-    /**
-     * Create an empty trace with given level.
-     *
-     * @param level Level to set.
-     */
-    explicit Trace(uint32_t level);
+    Trace() : Trace(0) {}
+    explicit Trace(uint32_t level) : _root(), _level(level) { }
+    Trace & operator = (Trace &&) = default;
+    Trace(Trace &&) = default;
+    Trace(const Trace &);
+    Trace & operator = (const Trace &) = delete;
+    ~Trace() = default;
 
     /**
      * Remove all trace information and set the trace level to 0.
-     *
-     * @return This, to allow chaining.
      */
-    Trace &clear();
+    void clear();
 
     /**
      * Swap the internals of this with another.
@@ -50,21 +42,16 @@ public:
      * @param other The trace to swap internals with.
      * @return This, to allow chaining.
      */
-    Trace &swap(Trace &other);
+    Trace &swap(Trace &other) {
+        std::swap(_level, other._level);
+        _root.swap(other._root);
+        return *this;
+    }
 
-    /**
-     * Set the trace level. 0 means no tracing, 9 means enable all tracing.
-     *
-     * @param level The level to set.
-     * @return This, to allow chaining.
-     */
-    Trace &setLevel(uint32_t level);
+    void setLevel(uint32_t level) {
+        _level = std::min(level, 9u);
+    }
 
-    /**
-     * Returns the trace level.
-     *
-     * @return The trace level.
-     */
     uint32_t getLevel() const { return _level; }
 
     /**
@@ -91,19 +78,29 @@ public:
      */
     bool trace(uint32_t level, const string &note, bool addTime = true);
 
-    /**
-     * Returns the root of the trace tree.
-     *
-     * @return The root.
-     */
-    TraceNode &getRoot() { return _root; }
+    void normalize() {
+        if (_root) {
+            _root->normalize();
+        }
+    }
 
-    /**
-     * Returns a const reference to the root of the trace tree.
-     *
-     * @return The root.
-     */
-    const TraceNode &getRoot() const { return _root; }
+    void setStrict(bool strict) {
+        ensureRoot().setStrict(strict);
+    }
+    void addChild(TraceNode && child) {
+        ensureRoot().addChild(std::move(child));
+    }
+    void addChild(Trace && child) {
+        if (!child.isEmpty()) {
+            addChild(std::move(*child._root));
+        }
+    }
+
+    bool isEmpty() const { return !_root || _root->isEmpty(); }
+
+    uint32_t getNumChildren() const { return _root ? _root->getNumChildren() : 0; }
+    const TraceNode & getChild(uint32_t child) const { return getRoot().getChild(child); }
+    string encode() const;
 
     /**
      * Returns a string representation of the contained trace tree. This is a
@@ -111,7 +108,16 @@ public:
      *
      * @return Readable trace string.
      */
-    string toString() const { return _root.toString(31337); }
+    string toString(size_t limit=31337) const;
+    size_t computeMemoryUsage() const {
+        return _root ? _root->computeMemoryUsage() : 0;
+    }
+private:
+    const TraceNode &getRoot() const { return *_root; }
+    TraceNode &ensureRoot();
+
+    std::unique_ptr<TraceNode> _root;
+    uint32_t  _level;
 };
 
 #define VESPALIB_TRACE2(ttrace, level, note, addTime) \
diff --git a/vespalib/src/vespa/vespalib/trace/tracenode.cpp b/vespalib/src/vespa/vespalib/trace/tracenode.cpp
index d34b45025d3..12dd51ac677 100644
--- a/vespalib/src/vespa/vespalib/trace/tracenode.cpp
+++ b/vespalib/src/vespa/vespalib/trace/tracenode.cpp
@@ -40,22 +40,22 @@ struct Cmp {
 } // namespace <unnamed>
 
 
-TraceNode::TraceNode() :
-    _parent(nullptr),
-    _strict(true),
-    _hasNote(false),
-    _note(""),
-    _children(),
-    _timestamp()
+TraceNode::TraceNode()
+    : _note(""),
+      _children(),
+      _parent(nullptr),
+      _timestamp(),
+      _strict(true),
+      _hasNote(false)
 { }
 
-TraceNode::TraceNode(const TraceNode &rhs) :
-    _parent(nullptr),
-    _strict(rhs._strict),
-    _hasNote(rhs._hasNote),
-    _note(rhs._note),
-    _children(),
-    _timestamp(rhs._timestamp)
+TraceNode::TraceNode(const TraceNode &rhs)
+    : _note(rhs._note),
+      _children(),
+      _parent(nullptr),
+      _timestamp(rhs._timestamp),
+      _strict(rhs._strict),
+      _hasNote(rhs._hasNote)
 {
     addChildren(rhs._children);
 }
@@ -65,22 +65,22 @@ TraceNode & TraceNode::operator =(const TraceNode &) = default;
 
 TraceNode::~TraceNode() = default;
 
-TraceNode::TraceNode(const string &note, system_time timestamp) :
-    _parent(nullptr),
-    _strict(true),
-    _hasNote(true),
-    _note(note),
-    _children(),
-    _timestamp(timestamp)
+TraceNode::TraceNode(const string &note, system_time timestamp)
+    : _note(note),
+      _children(),
+      _parent(nullptr),
+      _timestamp(timestamp),
+      _strict(true),
+      _hasNote(true)
 { }
 
-TraceNode::TraceNode(system_time timestamp) :
-    _parent(nullptr),
-    _strict(true),
-    _hasNote(false),
-    _note(""),
-    _children(),
-    _timestamp(timestamp)
+TraceNode::TraceNode(system_time timestamp)
+    : _note(""),
+      _children(),
+      _parent(nullptr),
+      _timestamp(timestamp),
+      _strict(true),
+      _hasNote(false)
 { }
 
 TraceNode &
@@ -342,4 +342,17 @@ TraceNode::accept(TraceVisitor & visitor) const
     return visitor;
 }
 
+size_t
+TraceNode::computeMemoryUsage() const
+{
+    if (isLeaf()) {
+        return getNote().size();
+    }
+    size_t childSum = 0;
+    for (const TraceNode & child : _children) {
+        childSum += child.computeMemoryUsage();
+    }
+    return childSum;
+}
+
 } // namespace vespalib
diff --git a/vespalib/src/vespa/vespalib/trace/tracenode.h b/vespalib/src/vespa/vespalib/trace/tracenode.h
index 63e7bcd6dc0..7a7cdb89c69 100644
--- a/vespalib/src/vespa/vespalib/trace/tracenode.h
+++ b/vespalib/src/vespa/vespalib/trace/tracenode.h
@@ -23,12 +23,12 @@ struct TraceVisitor;
  */
 class TraceNode {
 private:
-    TraceNode             *_parent;
-    bool                   _strict;
-    bool                   _hasNote;
     string                 _note;
     std::vector<TraceNode> _children;
+    TraceNode             *_parent;
     system_time            _timestamp;
+    bool                   _strict;
+    bool                   _hasNote;
 
 public:
     /**
@@ -253,6 +253,8 @@ public:
      */
     TraceVisitor & accept(TraceVisitor & visitor) const;
 
+    size_t computeMemoryUsage() const;
+
 };
 
 } // namespace vespalib