Merge pull request #11822 from vespa-engine/balder/reduce-bytebuffer-exposure

Balder/reduce bytebuffer exposure

16 files changed, 60 insertions, 150 deletions

diff --git a/storage/src/tests/common/message_sender_stub.cpp b/storage/src/tests/common/message_sender_stub.cpp
index c127f9071e5..a82d45b0b99 100644
--- a/storage/src/tests/common/message_sender_stub.cpp
+++ b/storage/src/tests/common/message_sender_stub.cpp
@@ -22,9 +22,7 @@ MessageSenderStub::getLastCommand(bool verbose) const
 }
 
 std::string
-MessageSenderStub::dumpMessage(const api::StorageMessage& msg,
-                               bool includeAddress,
-                               bool verbose) const
+MessageSenderStub::dumpMessage(const api::StorageMessage& msg, bool includeAddress, bool verbose) const
 {
     std::ostringstream ost;
 
@@ -67,9 +65,7 @@ MessageSenderStub::getLastReply(bool verbose) const
         throw std::logic_error("Expected reply where there was none");
     }
 
-    return dumpMessage(*replies.back(),
-                       true,
-                       verbose);
+    return dumpMessage(*replies.back(),true, verbose);
 
 }
 
diff --git a/storage/src/tests/persistence/filestorage/filestormanagertest.cpp b/storage/src/tests/persistence/filestorage/filestormanagertest.cpp
index 64306fa7c24..4576f8a08f8 100644
--- a/storage/src/tests/persistence/filestorage/filestormanagertest.cpp
+++ b/storage/src/tests/persistence/filestorage/filestormanagertest.cpp
@@ -1501,7 +1501,7 @@ TEST_F(FileStorManagerTest, visiting) {
         for (uint32_t i=3; i<docCount; ++i) {
             auto reply = std::dynamic_pointer_cast<api::BucketInfoReply>(top.getReply(i));
             ASSERT_TRUE(reply.get());
-            ASSERT_TRUE(reply->getResult().success()) << reply->getResult().toString();
+            ASSERT_TRUE(reply->getResult().success()) << reply->getResult();
 
             info = reply->getBucketInfo();
         }
diff --git a/storage/src/tests/visiting/visitormanagertest.cpp b/storage/src/tests/visiting/visitormanagertest.cpp
index b7eb7fee3ec..20934d04eaa 100644
--- a/storage/src/tests/visiting/visitormanagertest.cpp
+++ b/storage/src/tests/visiting/visitormanagertest.cpp
@@ -20,6 +20,7 @@
 #include <vespa/documentapi/messagebus/messages/visitor.h>
 #include <vespa/config/common/exceptions.h>
 #include <vespa/vespalib/gtest/gtest.h>
+#include <vespa/vespalib/objects/nbostream.h>
 #include <gmock/gmock.h>
 #include <optional>
 #include <thread>
@@ -337,7 +338,7 @@ int getTotalSerializedSize(const std::vector<document::Document::SP>& docs)
 {
     int total = 0;
     for (size_t i = 0; i < docs.size(); ++i) {
-        total += int(docs[i]->serialize()->getLength());
+        total += int(docs[i]->serialize().size());
     }
     return total;
 }
diff --git a/storage/src/vespa/storage/common/bucketmessages.cpp b/storage/src/vespa/storage/common/bucketmessages.cpp
index e92e2d4c3bf..1a4dc61a3ce 100644
--- a/storage/src/vespa/storage/common/bucketmessages.cpp
+++ b/storage/src/vespa/storage/common/bucketmessages.cpp
@@ -14,7 +14,7 @@ ReadBucketList::ReadBucketList(BucketSpace bucketSpace, spi::PartitionId partiti
       _partition(partition)
 { }
 
-ReadBucketList::~ReadBucketList() { }
+ReadBucketList::~ReadBucketList() = default;
 
 document::Bucket
 ReadBucketList::getBucket() const
@@ -38,7 +38,7 @@ ReadBucketListReply::ReadBucketListReply(const ReadBucketList& cmd)
       _partition(cmd.getPartition())
 { }
 
-ReadBucketListReply::~ReadBucketListReply() { }
+ReadBucketListReply::~ReadBucketListReply() = default;
 
 document::Bucket
 ReadBucketListReply::getBucket() const
@@ -66,7 +66,7 @@ ReadBucketInfo::ReadBucketInfo(const document::Bucket &bucket)
       _bucket(bucket)
 { }
 
-ReadBucketInfo::~ReadBucketInfo() { }
+ReadBucketInfo::~ReadBucketInfo() = default;
 
 void
 ReadBucketInfo::print(std::ostream& out, bool verbose, const std::string& indent) const
@@ -92,7 +92,7 @@ ReadBucketInfoReply::ReadBucketInfoReply(const ReadBucketInfo& cmd)
      _bucket(cmd.getBucket())
 { }
 
-ReadBucketInfoReply::~ReadBucketInfoReply() { }
+ReadBucketInfoReply::~ReadBucketInfoReply() = default;
 void
 ReadBucketInfoReply::print(std::ostream& out, bool verbose, const std::string& indent) const {
     out << "ReadBucketInfoReply()";
@@ -117,7 +117,7 @@ RepairBucketCommand::RepairBucketCommand(const document::Bucket &bucket, uint16_
     setPriority(LOW);
 }
 
-RepairBucketCommand::~RepairBucketCommand() { }
+RepairBucketCommand::~RepairBucketCommand() = default;
 
 void
 RepairBucketCommand::print(std::ostream& out, bool verbose, const std::string& indent) const {
@@ -153,7 +153,7 @@ RepairBucketReply::RepairBucketReply(const RepairBucketCommand& cmd, const api::
       _altered(false)
 { }
 
-RepairBucketReply::~RepairBucketReply() { }
+RepairBucketReply::~RepairBucketReply() = default;
 
 void
 RepairBucketReply::print(std::ostream& out, bool verbose, const std::string& indent) const {
@@ -180,7 +180,7 @@ BucketDiskMoveCommand::BucketDiskMoveCommand(const document::Bucket &bucket,
     setPriority(LOW);
 }
 
-BucketDiskMoveCommand::~BucketDiskMoveCommand() { }
+BucketDiskMoveCommand::~BucketDiskMoveCommand() = default;
 
 void
 BucketDiskMoveCommand::setBucketId(const document::BucketId& id)
@@ -208,14 +208,13 @@ BucketDiskMoveReply::BucketDiskMoveReply(const BucketDiskMoveCommand& cmd,
       _dstDisk(cmd.getDstDisk())
 { }
 
-BucketDiskMoveReply::~BucketDiskMoveReply() { }
+BucketDiskMoveReply::~BucketDiskMoveReply() = default;
 
 void
 BucketDiskMoveReply::print(std::ostream& out, bool, const std::string&) const
 {
     out << "BucketDiskMoveReply(" << _bucket.getBucketId() << ", source " << _srcDisk
-        << ", target " << _dstDisk << ", " << _bucketInfo << ", "
-        << getResult() << ")";
+        << ", target " << _dstDisk << ", " << _bucketInfo << ", " << getResult() << ")";
 }
 
 std::unique_ptr<api::StorageReply>
@@ -236,7 +235,7 @@ InternalBucketJoinCommand::InternalBucketJoinCommand(const document::Bucket &buc
                        // them higher than getting more bucket info lists.
 }
 
-InternalBucketJoinCommand::~InternalBucketJoinCommand() { }
+InternalBucketJoinCommand::~InternalBucketJoinCommand() = default;
 
 void
 InternalBucketJoinCommand::print(std::ostream& out, bool verbose, const std::string& indent) const {
@@ -255,7 +254,7 @@ InternalBucketJoinReply::InternalBucketJoinReply(const InternalBucketJoinCommand
       _bucketInfo(info)
 { }
 
-InternalBucketJoinReply::~InternalBucketJoinReply() { }
+InternalBucketJoinReply::~InternalBucketJoinReply() = default;
 
 void
 InternalBucketJoinReply::print(std::ostream& out, bool verbose, const std::string& indent) const
diff --git a/storage/src/vespa/storage/common/storagelink.cpp b/storage/src/vespa/storage/common/storagelink.cpp
index f73eb3ea36d..431c90b27f2 100644
--- a/storage/src/vespa/storage/common/storagelink.cpp
+++ b/storage/src/vespa/storage/common/storagelink.cpp
@@ -4,6 +4,7 @@
 #include "bucketmessages.h"
 #include <vespa/vespalib/util/backtrace.h>
 #include <sstream>
+#include <cassert>
 
 #include <vespa/log/bufferedlogger.h>
 LOG_SETUP(".application.link");
@@ -141,7 +142,7 @@ void StorageLink::sendDown(const StorageMessage::SP& msg)
                 sendUp(reply);
             }
         } else {
-            ost << " Return code: " << static_cast<StorageReply&>(*msg).getResult();
+            ost << " Return code: " << static_cast<const StorageReply&>(*msg).getResult();
             LOGBP(warning, "%s", ost.str().c_str());
         }
     } else if (!_down->onDown(msg)) {
@@ -181,7 +182,7 @@ void StorageLink::sendUp(const shared_ptr<StorageMessage> & msg)
                 sendDown(reply);
             }
         } else {
-            ost << " Return code: " << static_cast<StorageReply&>(*msg).getResult();
+            ost << " Return code: " << static_cast<const StorageReply&>(*msg).getResult();
             LOGBP(warning, "%s", ost.str().c_str());
         }
     } else if (!_up->onUp(msg)) {
diff --git a/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.cpp b/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.cpp
index e143f4d8570..15a57c1e7ee 100644
--- a/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.cpp
+++ b/storage/src/vespa/storage/distributor/maintenance/simplemaintenancescanner.cpp
@@ -2,6 +2,7 @@
 #include "simplemaintenancescanner.h"
 #include <vespa/storage/distributor/distributor_bucket_space.h>
 #include <ostream>
+#include <cassert>
 
 namespace storage::distributor {
 
@@ -16,10 +17,10 @@ SimpleMaintenanceScanner::SimpleMaintenanceScanner(BucketPriorityDatabase& bucke
 {
 }
 
-SimpleMaintenanceScanner::~SimpleMaintenanceScanner() {}
+SimpleMaintenanceScanner::~SimpleMaintenanceScanner() = default;
 
-SimpleMaintenanceScanner::PendingMaintenanceStats::PendingMaintenanceStats() {}
-SimpleMaintenanceScanner::PendingMaintenanceStats::~PendingMaintenanceStats() {}
+SimpleMaintenanceScanner::PendingMaintenanceStats::PendingMaintenanceStats() = default;
+SimpleMaintenanceScanner::PendingMaintenanceStats::~PendingMaintenanceStats() = default;
 SimpleMaintenanceScanner::PendingMaintenanceStats::PendingMaintenanceStats(const PendingMaintenanceStats &) = default;
 SimpleMaintenanceScanner::PendingMaintenanceStats &
 SimpleMaintenanceScanner::PendingMaintenanceStats::operator = (const PendingMaintenanceStats &) = default;
diff --git a/storage/src/vespa/storage/distributor/operations/external/statbucketoperation.cpp b/storage/src/vespa/storage/distributor/operations/external/statbucketoperation.cpp
index 8e6494a588d..60c1137bd6d 100644
--- a/storage/src/vespa/storage/distributor/operations/external/statbucketoperation.cpp
+++ b/storage/src/vespa/storage/distributor/operations/external/statbucketoperation.cpp
@@ -8,8 +8,7 @@
 #include <vespa/log/log.h>
 LOG_SETUP(".distributor.callback.statbucket");
 
-namespace storage {
-namespace distributor {
+namespace storage::distributor {
 
 StatBucketOperation::StatBucketOperation(
         [[maybe_unused]] DistributorComponent& manager,
@@ -21,7 +20,7 @@ StatBucketOperation::StatBucketOperation(
 {
 }
 
-StatBucketOperation::~StatBucketOperation() {}
+StatBucketOperation::~StatBucketOperation() = default;
 
 void
 StatBucketOperation::onClose(DistributorMessageSender& sender)
@@ -36,8 +35,7 @@ StatBucketOperation::onStart(DistributorMessageSender& sender)
 {
     std::vector<uint16_t> nodes;
 
-    BucketDatabase::Entry entry(
-            _bucketSpace.getBucketDatabase().get(_command->getBucketId()));
+    BucketDatabase::Entry entry(_bucketSpace.getBucketDatabase().get(_command->getBucketId()));
 
     if (entry.valid()) {
         nodes = entry->getNodes();
@@ -103,5 +101,4 @@ StatBucketOperation::onReceive(DistributorMessageSender& sender, const std::shar
     }
 }
 
-} // distributor
-} // storage
+}
diff --git a/storage/src/vespa/storage/distributor/throttlingoperationstarter.cpp b/storage/src/vespa/storage/distributor/throttlingoperationstarter.cpp
index abd9778d72c..9e3230a0f34 100644
--- a/storage/src/vespa/storage/distributor/throttlingoperationstarter.cpp
+++ b/storage/src/vespa/storage/distributor/throttlingoperationstarter.cpp
@@ -1,6 +1,7 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include "throttlingoperationstarter.h"
+#include <cassert>
 
 namespace storage::distributor {
 
@@ -10,8 +11,7 @@ ThrottlingOperationStarter::ThrottlingOperation::~ThrottlingOperation()
 }
 
 bool
-ThrottlingOperationStarter::canStart(uint32_t currentOperationCount,
-                                    Priority priority) const
+ThrottlingOperationStarter::canStart(uint32_t currentOperationCount, Priority priority) const
 {
     uint32_t variablePending(_maxPending - _minPending);
     uint32_t maxPendingForPri(_minPending + variablePending*((255.0 - priority) / 255.0));
diff --git a/storage/src/vespa/storage/persistence/filestorage/filestormanager.h b/storage/src/vespa/storage/persistence/filestorage/filestormanager.h
index bfc89a70a85..65d4035a3dd 100644
--- a/storage/src/vespa/storage/persistence/filestorage/filestormanager.h
+++ b/storage/src/vespa/storage/persistence/filestorage/filestormanager.h
@@ -36,11 +36,8 @@ namespace api {
     class StorageReply;
 }
 
-class BucketMergeTest;
-class DiskInfo;
 struct FileStorManagerTest;
 class ReadBucketList;
-class ModifiedBucketCheckerThread;
 class BucketOwnershipNotifier;
 class AbortBucketOperationsCommand;
 
diff --git a/storage/src/vespa/storage/persistence/mergehandler.cpp b/storage/src/vespa/storage/persistence/mergehandler.cpp
index 37e1d818bb8..e5e358cbb60 100644
--- a/storage/src/vespa/storage/persistence/mergehandler.cpp
+++ b/storage/src/vespa/storage/persistence/mergehandler.cpp
@@ -506,18 +506,11 @@ MergeHandler::fetchLocalData(
             assert(doc != 0);
             assertContainedInBucket(doc->getId(), bucket, idFactory);
             e._docName = doc->getId().toString();
-            {
-                vespalib::nbostream stream;
-                doc->serializeHeader(stream);
-                e._headerBlob.resize(stream.size());
-                memcpy(&e._headerBlob[0], stream.peek(), stream.size());
-            }
-            {
-                vespalib::nbostream stream;
-                doc->serializeBody(stream);
-                e._bodyBlob.resize(stream.size());
-                memcpy(&e._bodyBlob[0], stream.peek(), stream.size());
-            }
+            vespalib::nbostream stream;
+            doc->serialize(stream);
+            e._headerBlob.resize(stream.size());
+            memcpy(&e._headerBlob[0], stream.peek(), stream.size());
+            e._bodyBlob.clear();
         } else {
             const DocumentId* docId = docEntry.getDocumentId();
             assert(docId != 0);
@@ -556,11 +549,11 @@ MergeHandler::deserializeDiffDocument(
         const api::ApplyBucketDiffCommand::Entry& e,
         const document::DocumentTypeRepo& repo) const
 {
-    Document::UP doc(new Document);
-    using document::ByteBuffer;
-    ByteBuffer hbuf(&e._headerBlob[0], e._headerBlob.size());
+    auto doc = std::make_unique<Document>();
+    vespalib::nbostream hbuf(&e._headerBlob[0], e._headerBlob.size());
     if (e._bodyBlob.size() > 0) {
-        ByteBuffer bbuf(&e._bodyBlob[0], e._bodyBlob.size());
+        // TODO Remove this branch and add warning on error.
+        vespalib::nbostream bbuf(&e._bodyBlob[0], e._bodyBlob.size());
         doc->deserialize(repo, hbuf, bbuf);
     } else {
         doc->deserialize(repo, hbuf);
diff --git a/storage/src/vespa/storage/storageserver/opslogger.cpp b/storage/src/vespa/storage/storageserver/opslogger.cpp
index 6fc9795993e..b6bceabf7a1 100644
--- a/storage/src/vespa/storage/storageserver/opslogger.cpp
+++ b/storage/src/vespa/storage/storageserver/opslogger.cpp
@@ -77,7 +77,7 @@ OpsLogger::onPutReply(const std::shared_ptr<api::PutReply>& msg)
     std::ostringstream ost;
     ost << _component.getClock().getTimeInSeconds().getTime()
         << "\tPUT\t" << msg->getDocumentId() << "\t"
-        << msg->getResult().toString() << "\n";
+        << msg->getResult() << "\n";
     {
         vespalib::LockGuard lock(_lock);
         if (_targetFile == nullptr) return false;
@@ -94,7 +94,7 @@ OpsLogger::onUpdateReply(const std::shared_ptr<api::UpdateReply>& msg)
     std::ostringstream ost;
     ost << _component.getClock().getTimeInSeconds().getTime()
         << "\tUPDATE\t" << msg->getDocumentId() << "\t"
-        << msg->getResult().toString() << "\n";
+        << msg->getResult() << "\n";
     {
         vespalib::LockGuard lock(_lock);
         if (_targetFile == nullptr) return false;
@@ -111,7 +111,7 @@ OpsLogger::onRemoveReply(const std::shared_ptr<api::RemoveReply>& msg)
     std::ostringstream ost;
     ost << _component.getClock().getTimeInSeconds().getTime()
         << "\tREMOVE\t" << msg->getDocumentId() << "\t"
-        << msg->getResult().toString() << "\n";
+        << msg->getResult() << "\n";
     {
         vespalib::LockGuard lock(_lock);
         if (_targetFile == nullptr) return false;
@@ -128,7 +128,7 @@ OpsLogger::onGetReply(const std::shared_ptr<api::GetReply>& msg)
     std::ostringstream ost;
     ost << _component.getClock().getTimeInSeconds().getTime()
         << "\tGET\t" << msg->getDocumentId() << "\t"
-        << msg->getResult().toString() << "\n";
+        << msg->getResult() << "\n";
     {
         vespalib::LockGuard lock(_lock);
         if (_targetFile == nullptr) return false;
diff --git a/storage/src/vespa/storage/storageserver/storagemetricsset.cpp b/storage/src/vespa/storage/storageserver/storagemetricsset.cpp
index f0e64f0dfd1..f3240f0663b 100644
--- a/storage/src/vespa/storage/storageserver/storagemetricsset.cpp
+++ b/storage/src/vespa/storage/storageserver/storagemetricsset.cpp
@@ -16,37 +16,6 @@ MessageMemoryUseMetricSet::MessageMemoryUseMetricSet(metrics::MetricSet* owner)
 
 MessageMemoryUseMetricSet::~MessageMemoryUseMetricSet() = default;
 
-DocumentSerializationMetricSet::DocumentSerializationMetricSet(metrics::MetricSet* owner)
-    : metrics::MetricSet("document_serialization", {{"docserialization"}},
-            "Counts of document serialization of various types", owner),
-      usedCachedSerializationCount(
-            "cached_serialization_count", {{"docserialization"}},
-            "Number of times we didn't need to serialize the document as "
-            "we already had serialized version cached", this),
-      compressedDocumentCount(
-            "compressed_serialization_count", {{"docserialization"}},
-            "Number of times we compressed document when serializing",
-            this),
-      compressionDidntHelpCount(
-            "compressed_didnthelp_count", {{"docserialization"}},
-            "Number of times we compressed document when serializing, but "
-            "the compressed version was bigger, so it was dumped", this),
-      uncompressableCount(
-            "uncompressable_serialization_count", {{"docserialization"}},
-            "Number of times we didn't attempt compression as document "
-            "had already been tagged uncompressable", this),
-      serializedUncompressed(
-            "uncompressed_serialization_count", {{"docserialization"}},
-            "Number of times we serialized a document uncompressed", this),
-      inputWronglySerialized(
-            "input_wrongly_serialized_count", {{"docserialization"}},
-            "Number of times we reserialized a document because the "
-            "compression it had in cache did not match what was configured",
-            this)
-{}
-
-DocumentSerializationMetricSet::~DocumentSerializationMetricSet() = default;
-
 StorageMetricSet::StorageMetricSet()
     : metrics::MetricSet("server", {{"memory"}},
           "Metrics for VDS applications"),
@@ -54,28 +23,12 @@ StorageMetricSet::StorageMetricSet()
       memoryUse_messages(this),
       memoryUse_visiting("memoryusage_visiting", {{"memory"}},
             "Message use from visiting", this),
-      documentSerialization(this),
       tls_metrics(this)
 {}
 
 StorageMetricSet::~StorageMetricSet() = default;
 
 void StorageMetricSet::updateMetrics() {
-    document::SerializableArray::Statistics stats(
-            document::SerializableArray::getStatistics());
-
-    documentSerialization.usedCachedSerializationCount.set(
-            stats._usedCachedSerializationCount);
-    documentSerialization.compressedDocumentCount.set(
-            stats._compressedDocumentCount);
-    documentSerialization.compressionDidntHelpCount.set(
-            stats._compressionDidntHelpCount);
-    documentSerialization.uncompressableCount.set(
-            stats._uncompressableCount);
-    documentSerialization.serializedUncompressed.set(
-            stats._serializedUncompressed);
-    documentSerialization.inputWronglySerialized.set(
-            stats._inputWronglySerialized);
 
     // Delta snapshotting is destructive, so if an explicit snapshot is triggered
     // (instead of just regular periodic snapshots), some events will effectively
diff --git a/storage/src/vespa/storage/storageserver/storagemetricsset.h b/storage/src/vespa/storage/storageserver/storagemetricsset.h
index e9378010540..49795c63324 100644
--- a/storage/src/vespa/storage/storageserver/storagemetricsset.h
+++ b/storage/src/vespa/storage/storageserver/storagemetricsset.h
@@ -3,7 +3,6 @@
 #pragma once
 
 #include "tls_statistics_metrics_wrapper.h"
-
 #include <vespa/metrics/metrics.h>
 
 namespace storage {
@@ -17,21 +16,8 @@ public:
     metrics::LongValueMetric highpri;
     metrics::LongValueMetric veryhighpri;
 
-    MessageMemoryUseMetricSet(metrics::MetricSet* owner);
-    ~MessageMemoryUseMetricSet();
-};
-
-struct DocumentSerializationMetricSet : public metrics::MetricSet
-{
-    metrics::LongCountMetric usedCachedSerializationCount;
-    metrics::LongCountMetric compressedDocumentCount;
-    metrics::LongCountMetric compressionDidntHelpCount;
-    metrics::LongCountMetric uncompressableCount;
-    metrics::LongCountMetric serializedUncompressed;
-    metrics::LongCountMetric inputWronglySerialized;
-
-    DocumentSerializationMetricSet(metrics::MetricSet* owner);
-    ~DocumentSerializationMetricSet();
+    explicit MessageMemoryUseMetricSet(metrics::MetricSet* owner);
+    ~MessageMemoryUseMetricSet() override;
 };
 
 struct StorageMetricSet : public metrics::MetricSet
@@ -39,12 +25,11 @@ struct StorageMetricSet : public metrics::MetricSet
     metrics::LongValueMetric memoryUse;
     MessageMemoryUseMetricSet memoryUse_messages;
     metrics::LongValueMetric memoryUse_visiting;
-    DocumentSerializationMetricSet documentSerialization;
 
     TlsStatisticsMetricsWrapper tls_metrics;
 
     StorageMetricSet();
-    ~StorageMetricSet();
+    ~StorageMetricSet() override;
     void updateMetrics();
 };
 
diff --git a/storage/src/vespa/storage/visiting/recoveryvisitor.cpp b/storage/src/vespa/storage/visiting/recoveryvisitor.cpp
index 7c69a232af0..80e74e890a1 100644
--- a/storage/src/vespa/storage/visiting/recoveryvisitor.cpp
+++ b/storage/src/vespa/storage/visiting/recoveryvisitor.cpp
@@ -2,7 +2,7 @@
 
 
 #include "recoveryvisitor.h"
-
+#include <vespa/vespalib/objects/nbostream.h>
 #include <vespa/documentapi/messagebus/messages/visitor.h>
 #include <vespa/vespalib/text/stringtokenizer.h>
 #include <vespa/vespalib/stllike/hash_map.hpp>
@@ -36,10 +36,9 @@ RecoveryVisitor::handleDocuments(const document::BucketId& bid,
 {
     vespalib::LockGuard guard(_mutex);
 
-    LOG(debug, "Visitor %s handling block of %zu documents.",
-               _id.c_str(), entries.size());
+    LOG(debug, "Visitor %s handling block of %zu documents.", _id.c_str(), entries.size());
 
-    documentapi::DocumentListMessage* cmd = NULL;
+    documentapi::DocumentListMessage* cmd = nullptr;
 
     {
         CommandMap::iterator iter = _activeCommands.find(bid);
@@ -71,7 +70,7 @@ RecoveryVisitor::handleDocuments(const document::BucketId& bid,
             }
         }
 
-        hitCounter.addHit(doc->getId(), doc->serialize()->getLength());
+        hitCounter.addHit(doc->getId(), doc->serialize().size());
 
         int64_t timestamp = doc->getLastModified();
         cmd->getDocuments().push_back(documentapi::DocumentListMessage::Entry(
diff --git a/storage/src/vespa/storage/visiting/recoveryvisitor.h b/storage/src/vespa/storage/visiting/recoveryvisitor.h
index e68a8fdbc8c..1da2acfed9c 100644
--- a/storage/src/vespa/storage/visiting/recoveryvisitor.h
+++ b/storage/src/vespa/storage/visiting/recoveryvisitor.h
@@ -12,16 +12,13 @@
 #include "visitor.h"
 #include <vespa/storageapi/message/datagram.h>
 
-namespace documentapi {
-class DocumentListMessage;
-}
+namespace documentapi { class DocumentListMessage; }
 
 namespace storage {
 
 class RecoveryVisitor : public Visitor {
 public:
-    RecoveryVisitor(StorageComponent&,
-                    const vdslib::Parameters& params);
+    RecoveryVisitor(StorageComponent&, const vdslib::Parameters& params);
 
 private:
     void handleDocuments(const document::BucketId& bucketId,
@@ -43,12 +40,11 @@ struct RecoveryVisitorFactory : public VisitorFactory {
 
     VisitorEnvironment::UP
     makeVisitorEnvironment(StorageComponent&) override {
-        return VisitorEnvironment::UP(new VisitorEnvironment);
+        return std::make_unique<VisitorEnvironment>();
     };
 
     Visitor*
-    makeVisitor(StorageComponent& c, VisitorEnvironment&,
-                const vdslib::Parameters& params) override
+    makeVisitor(StorageComponent& c, VisitorEnvironment&, const vdslib::Parameters& params) override
     {
         return new RecoveryVisitor(c, params);
     }
diff --git a/storage/src/vespa/storage/visiting/visitor.cpp b/storage/src/vespa/storage/visiting/visitor.cpp
index 4b213dff1d5..bdd066e8a4a 100644
--- a/storage/src/vespa/storage/visiting/visitor.cpp
+++ b/storage/src/vespa/storage/visiting/visitor.cpp
@@ -44,17 +44,12 @@ Visitor::HitCounter::addHit(const document::DocumentId& , uint32_t size)
 }
 
 void
-Visitor::HitCounter::updateVisitorStatistics(
-        vdslib::VisitorStatistics& statistics)
+Visitor::HitCounter::updateVisitorStatistics(vdslib::VisitorStatistics& statistics)
 {
-    statistics.setDocumentsReturned(
-            statistics.getDocumentsReturned() + _firstPassHits);
-    statistics.setBytesReturned(
-            statistics.getBytesReturned() + _firstPassBytes);
-    statistics.setSecondPassDocumentsReturned(
-            statistics.getSecondPassDocumentsReturned() + _secondPassHits);
-    statistics.setSecondPassBytesReturned(
-            statistics.getSecondPassBytesReturned() + _secondPassBytes);
+    statistics.setDocumentsReturned(statistics.getDocumentsReturned() + _firstPassHits);
+    statistics.setBytesReturned(statistics.getBytesReturned() + _firstPassBytes);
+    statistics.setSecondPassDocumentsReturned(statistics.getSecondPassDocumentsReturned() + _secondPassHits);
+    statistics.setSecondPassBytesReturned(statistics.getSecondPassBytesReturned() + _secondPassBytes);
 }
 
 Visitor::VisitorTarget::MessageMeta::MessageMeta(
@@ -68,8 +63,7 @@ Visitor::VisitorTarget::MessageMeta::MessageMeta(
 {
 }
 
-Visitor::VisitorTarget::MessageMeta::MessageMeta(
-        Visitor::VisitorTarget::MessageMeta&& rhs) noexcept
+Visitor::VisitorTarget::MessageMeta::MessageMeta(Visitor::VisitorTarget::MessageMeta&& rhs) noexcept
     : messageId(rhs.messageId),
       retryCount(rhs.retryCount),
       memoryUsage(rhs.memoryUsage),
@@ -78,9 +72,7 @@ Visitor::VisitorTarget::MessageMeta::MessageMeta(
 {
 }
 
-Visitor::VisitorTarget::MessageMeta::~MessageMeta()
-{
-}
+Visitor::VisitorTarget::MessageMeta::~MessageMeta() = default;
 
 Visitor::VisitorTarget::MessageMeta&
 Visitor::VisitorTarget::MessageMeta::operator=(