Use tranfer by value and std::move to avoid copying shared pointer to the Update.

9 files changed, 32 insertions, 33 deletions

diff --git a/searchcore/src/tests/proton/persistenceengine/persistence_handler_map/persistence_handler_map_test.cpp b/searchcore/src/tests/proton/persistenceengine/persistence_handler_map/persistence_handler_map_test.cpp
index 0c72d11899a..8eace5ac657 100644
--- a/searchcore/src/tests/proton/persistenceengine/persistence_handler_map/persistence_handler_map_test.cpp
+++ b/searchcore/src/tests/proton/persistenceengine/persistence_handler_map/persistence_handler_map_test.cpp
@@ -16,7 +16,7 @@ struct DummyPersistenceHandler : public IPersistenceHandler {
     using SP = std::shared_ptr<DummyPersistenceHandler>;
     void initialize() override {}
     void handlePut(FeedToken, const storage::spi::Bucket &, storage::spi::Timestamp, DocumentSP) override {}
-    void handleUpdate(FeedToken, const storage::spi::Bucket &, storage::spi::Timestamp, const document::DocumentUpdate::SP &) override {}
+    void handleUpdate(FeedToken, const storage::spi::Bucket &, storage::spi::Timestamp, DocumentUpdateSP) override {}
     void handleRemove(FeedToken, const storage::spi::Bucket &, storage::spi::Timestamp, const document::DocumentId &) override {}
     void handleListBuckets(IBucketIdListResultHandler &) override {}
     void handleSetClusterState(const storage::spi::ClusterState &, IGenericResultHandler &) override {}
diff --git a/searchcore/src/tests/proton/persistenceengine/persistenceengine_test.cpp b/searchcore/src/tests/proton/persistenceengine/persistenceengine_test.cpp
index f5346213a7a..214ca186656 100644
--- a/searchcore/src/tests/proton/persistenceengine/persistenceengine_test.cpp
+++ b/searchcore/src/tests/proton/persistenceengine/persistenceengine_test.cpp
@@ -204,7 +204,7 @@ struct MyHandler : public IPersistenceHandler, IBucketFreezer {
     }
 
     void handleUpdate(FeedToken token, const Bucket& bucket,
-                      Timestamp timestamp, const document::DocumentUpdate::SP& upd) override {
+                      Timestamp timestamp, DocumentUpdateSP upd) override {
         token->setResult(std::make_unique<UpdateResult>(existingTimestamp), existingTimestamp > 0);
         handle(token, bucket, timestamp, upd->getId());
     }
diff --git a/searchcore/src/vespa/searchcore/proton/feedoperation/updateoperation.cpp b/searchcore/src/vespa/searchcore/proton/feedoperation/updateoperation.cpp
index 75086f7a2dd..051e9726452 100644
--- a/searchcore/src/vespa/searchcore/proton/feedoperation/updateoperation.cpp
+++ b/searchcore/src/vespa/searchcore/proton/feedoperation/updateoperation.cpp
@@ -31,18 +31,20 @@ UpdateOperation::UpdateOperation(Type type)
 
 
 UpdateOperation::UpdateOperation(Type type, const BucketId &bucketId,
-                                 const Timestamp &timestamp, const DocumentUpdate::SP &upd)
+                                 const Timestamp &timestamp, DocumentUpdate::SP upd)
     : DocumentOperation(type, bucketId, timestamp),
-      _upd(upd)
+      _upd(std::move(upd))
 {
 }
 
 
-UpdateOperation::UpdateOperation(const BucketId &bucketId, const Timestamp &timestamp, const DocumentUpdate::SP &upd)
-    : UpdateOperation(FeedOperation::UPDATE, bucketId, timestamp, upd)
+UpdateOperation::UpdateOperation(const BucketId &bucketId, const Timestamp &timestamp, DocumentUpdate::SP upd)
+    : UpdateOperation(FeedOperation::UPDATE, bucketId, timestamp, std::move(upd))
 {
 }
 
+UpdateOperation::~UpdateOperation() = default;
+
 void
 UpdateOperation::serializeUpdate(vespalib::nbostream &os) const
 {
diff --git a/searchcore/src/vespa/searchcore/proton/feedoperation/updateoperation.h b/searchcore/src/vespa/searchcore/proton/feedoperation/updateoperation.h
index 83e87fea096..7975452cfc6 100644
--- a/searchcore/src/vespa/searchcore/proton/feedoperation/updateoperation.h
+++ b/searchcore/src/vespa/searchcore/proton/feedoperation/updateoperation.h
@@ -17,7 +17,7 @@ private:
     DocumentUpdateSP _upd;
     UpdateOperation(Type type, const document::BucketId &bucketId,
                     const storage::spi::Timestamp &timestamp,
-                    const DocumentUpdateSP &upd);
+                    DocumentUpdateSP upd);
     void serializeUpdate(vespalib::nbostream &os) const;
     void deserializeUpdate(vespalib::nbostream && is, const document::DocumentTypeRepo &repo);
 public:
@@ -25,8 +25,8 @@ public:
     UpdateOperation(Type type);
     UpdateOperation(const document::BucketId &bucketId,
                     const storage::spi::Timestamp &timestamp,
-                    const DocumentUpdateSP &upd);
-    ~UpdateOperation() override {}
+                    DocumentUpdateSP upd);
+    ~UpdateOperation() override;
     const DocumentUpdateSP &getUpdate() const { return _upd; }
     void serialize(vespalib::nbostream &os) const override;
     void deserialize(vespalib::nbostream &is, const document::DocumentTypeRepo &repo) override;
diff --git a/searchcore/src/vespa/searchcore/proton/persistenceengine/ipersistencehandler.h b/searchcore/src/vespa/searchcore/proton/persistenceengine/ipersistencehandler.h
index c95f51b29dc..a4bede28d7b 100644
--- a/searchcore/src/vespa/searchcore/proton/persistenceengine/ipersistencehandler.h
+++ b/searchcore/src/vespa/searchcore/proton/persistenceengine/ipersistencehandler.h
@@ -30,10 +30,7 @@ public:
     IPersistenceHandler(const IPersistenceHandler &) = delete;
     IPersistenceHandler & operator = (const IPersistenceHandler &) = delete;
 
-    /**
-     * Virtual destructor to allow inheritance.
-     */
-    virtual ~IPersistenceHandler() { }
+    virtual ~IPersistenceHandler() = default;
 
     /**
      * Called before all other functions so that the persistence handler
@@ -45,7 +42,7 @@ public:
                            storage::spi::Timestamp timestamp, DocumentSP doc) = 0;
 
     virtual void handleUpdate(FeedToken token, const storage::spi::Bucket &bucket,
-                              storage::spi::Timestamp timestamp, const DocumentUpdateSP &upd) = 0;
+                              storage::spi::Timestamp timestamp, DocumentUpdateSP upd) = 0;
 
     virtual void handleRemove(FeedToken token, const storage::spi::Bucket &bucket,
                               storage::spi::Timestamp timestamp, const document::DocumentId &id) = 0;
diff --git a/searchcore/src/vespa/searchcore/proton/server/persistencehandlerproxy.cpp b/searchcore/src/vespa/searchcore/proton/server/persistencehandlerproxy.cpp
index cd6df7c5816..7d151a9ef14 100644
--- a/searchcore/src/vespa/searchcore/proton/server/persistencehandlerproxy.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/persistencehandlerproxy.cpp
@@ -44,9 +44,9 @@ PersistenceHandlerProxy::handlePut(FeedToken token, const Bucket &bucket, Timest
 }
 
 void
-PersistenceHandlerProxy::handleUpdate(FeedToken token, const Bucket &bucket, Timestamp timestamp, const DocumentUpdateSP &upd)
+PersistenceHandlerProxy::handleUpdate(FeedToken token, const Bucket &bucket, Timestamp timestamp, DocumentUpdateSP upd)
 {
-    auto op = std::make_unique<UpdateOperation>(bucket.getBucketId().stripUnused(), timestamp, upd);
+    auto op = std::make_unique<UpdateOperation>(bucket.getBucketId().stripUnused(), timestamp, std::move(upd));
     _feedHandler.handleOperation(std::move(token), std::move(op));
 }
 
diff --git a/searchcore/src/vespa/searchcore/proton/server/persistencehandlerproxy.h b/searchcore/src/vespa/searchcore/proton/server/persistencehandlerproxy.h
index 38ddb667213..ee6e8a7ee42 100644
--- a/searchcore/src/vespa/searchcore/proton/server/persistencehandlerproxy.h
+++ b/searchcore/src/vespa/searchcore/proton/server/persistencehandlerproxy.h
@@ -28,7 +28,7 @@ public:
                    storage::spi::Timestamp timestamp, DocumentSP doc) override;
 
     void handleUpdate(FeedToken token, const storage::spi::Bucket &bucket,
-                      storage::spi::Timestamp timestamp, const DocumentUpdateSP &upd) override;
+                      storage::spi::Timestamp timestamp, DocumentUpdateSP upd) override;
 
     void handleRemove(FeedToken token, const storage::spi::Bucket &bucket,
                       storage::spi::Timestamp timestamp,
diff --git a/storage/src/vespa/storage/bucketdb/storagebucketinfo.h b/storage/src/vespa/storage/bucketdb/storagebucketinfo.h
index 2a3d9aa8f91..d9081432feb 100644
--- a/storage/src/vespa/storage/bucketdb/storagebucketinfo.h
+++ b/storage/src/vespa/storage/bucketdb/storagebucketinfo.h
@@ -23,7 +23,7 @@ struct StorageBucketInfo {
         info.setTotalDocumentSize(0);
     }
     bool verifyLegal() const { return (disk != 0xff); }
-    uint32_t getMetaCount() { return info.getMetaCount(); }
+    uint32_t getMetaCount() const { return info.getMetaCount(); }
     void setChecksum(uint32_t crc) { info.setChecksum(crc); }
     bool operator == (const StorageBucketInfo & b) const;
     bool operator != (const StorageBucketInfo & b) const;
diff --git a/storage/src/vespa/storage/persistence/persistencethread.cpp b/storage/src/vespa/storage/persistence/persistencethread.cpp
index c77b8a1b80d..d31157cb11d 100644
--- a/storage/src/vespa/storage/persistence/persistencethread.cpp
+++ b/storage/src/vespa/storage/persistence/persistencethread.cpp
@@ -45,10 +45,10 @@ private:
 template<class FunctionType>
 class LambdaResultTask : public ResultTask {
 public:
-    LambdaResultTask(FunctionType && func)
+    explicit LambdaResultTask(FunctionType && func)
         : _func(std::move(func))
     {}
-    ~LambdaResultTask() override {}
+    ~LambdaResultTask() override = default;
     void run() override {
         handle(*_result);
         _func(std::move(_result));
@@ -215,7 +215,7 @@ PersistenceThread::handleRemove(api::RemoveCommand& cmd, MessageTracker::UP trac
     } else {
         // Note that the &cmd capture is OK since its lifetime is guaranteed by the tracker
         auto task = makeResultTask([&metrics, &cmd, tracker = std::move(trackerUP)](spi::Result::UP responseUP) {
-            const spi::RemoveResult & response = dynamic_cast<const spi::RemoveResult &>(*responseUP);
+            auto & response = dynamic_cast<const spi::RemoveResult &>(*responseUP);
             if (tracker->checkForError(response)) {
                 tracker->setReply(std::make_shared<api::RemoveReply>(cmd, response.wasFound() ? cmd.getTimestamp() : 0));
             }
@@ -253,7 +253,7 @@ PersistenceThread::handleUpdate(api::UpdateCommand& cmd, MessageTracker::UP trac
     } else {
         // Note that the &cmd capture is OK since its lifetime is guaranteed by the tracker
         auto task = makeResultTask([&cmd, tracker = std::move(trackerUP)](spi::Result::UP responseUP) {
-            const spi::UpdateResult & response = dynamic_cast<const spi::UpdateResult &>(*responseUP);
+            auto & response = dynamic_cast<const spi::UpdateResult &>(*responseUP);
             if (tracker->checkForError(response)) {
                 auto reply = std::make_shared<api::UpdateReply>(cmd);
                 reply->setOldTimestamp(response.getExistingTimestamp());
@@ -569,30 +569,30 @@ PersistenceThread::handleSplitBucket(api::SplitBucketCommand& cmd, MessageTracke
     _env._fileStorHandler.remapQueueAfterSplit(source, targets[0].second, targets[1].second);
     bool ownershipChanged(!_bucketOwnershipNotifier->distributorOwns(cmd.getSourceIndex(), cmd.getBucket()));
     // Now release all the bucketdb locks.
-    for (uint32_t i = 0; i < targets.size(); i++) {
+    for (auto & target : targets) {
         if (ownershipChanged) {
-            notifyGuard.notifyAlways(targets[i].second.bucket, targets[i].first->getBucketInfo());
+            notifyGuard.notifyAlways(target.second.bucket, target.first->getBucketInfo());
         }
         // The entries vector has the source bucket in element zero, so indexing
         // that with i+1
-        if (targets[i].second.foundInQueue || targets[i].first->getMetaCount() > 0) {
-            if (targets[i].first->getMetaCount() == 0) {
+        if (target.second.foundInQueue || target.first->getMetaCount() > 0) {
+            if (target.first->getMetaCount() == 0) {
                 // Fake that the bucket has content so it is not deleted.
-                targets[i].first->info.setMetaCount(1);
+                target.first->info.setMetaCount(1);
                 // Must make sure target bucket exists when we have pending ops
                 // to an empty target bucket, since the provider will have
                 // implicitly erased it by this point.
-                spi::Bucket createTarget(spi::Bucket(targets[i].second.bucket,
-                                                     spi::PartitionId(targets[i].second.diskIndex)));
+                spi::Bucket createTarget(spi::Bucket(target.second.bucket,
+                                                     spi::PartitionId(target.second.diskIndex)));
                 LOG(debug, "Split target %s was empty, but re-creating it since there are remapped operations queued to it",
                     createTarget.toString().c_str());
                 _spi.createBucket(createTarget, tracker->context());
             }
-            splitReply.getSplitInfo().emplace_back(targets[i].second.bucket.getBucketId(),
-                                                   targets[i].first->getBucketInfo());
-            targets[i].first.write();
+            splitReply.getSplitInfo().emplace_back(target.second.bucket.getBucketId(),
+                                                   target.first->getBucketInfo());
+            target.first.write();
         } else {
-            targets[i].first.remove();
+            target.first.remove();
         }
     }
     if (sourceEntry.exist()) {