Revert "Revert "Revert "Revert "Use a hash_set<int32_t> to quickly check if a field i…""

6 files changed, 70 insertions, 61 deletions

diff --git a/searchcore/src/tests/proton/documentdb/feedview/feedview_test.cpp b/searchcore/src/tests/proton/documentdb/feedview/feedview_test.cpp
index 2957a2a015d..9d9bb64e6b1 100644
--- a/searchcore/src/tests/proton/documentdb/feedview/feedview_test.cpp
+++ b/searchcore/src/tests/proton/documentdb/feedview/feedview_test.cpp
@@ -367,7 +367,7 @@ struct MyAttributeWriter : public IAttributeWriter
         _updateLid = lid;
         for (const auto & fieldUpdate : upd.getUpdates()) {
             search::AttributeVector * attr = getWritableAttribute(fieldUpdate.getField().getName());
-            onUpdate.onUpdateField(fieldUpdate.getField().getName(), attr);
+            onUpdate.onUpdateField(fieldUpdate.getField(), attr);
         }
     }
     void update(SerialNum serialNum, const document::Document &doc, DocumentIdT lid, OnWriteDoneType) override {
diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.cpp b/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.cpp
index bb25b3da7be..021fc4717af 100644
--- a/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.cpp
+++ b/searchcore/src/vespa/searchcore/proton/attribute/attribute_writer.cpp
@@ -764,9 +764,9 @@ AttributeWriter::update(SerialNum serialNum, const DocumentUpdate &upd, Document
 
     for (const auto &fupd : upd.getUpdates()) {
         LOG(debug, "Retrieving guard for attribute vector '%s'.", fupd.getField().getName().data());
-        auto itr = _attrMap.find(fupd.getField().getName());
-        AttributeVector * attrp = (itr != _attrMap.end()) ? itr->second.attribute : nullptr;
-        onUpdate.onUpdateField(fupd.getField().getName(), attrp);
+        auto found = _attrMap.find(fupd.getField().getName());
+        AttributeVector * attrp = (found != _attrMap.end()) ? found->second.attribute : nullptr;
+        onUpdate.onUpdateField(fupd.getField(), attrp);
         if (__builtin_expect(attrp == nullptr, false)) {
             LOG(spam, "Failed to find attribute vector %s", fupd.getField().getName().data());
             continue;
@@ -776,16 +776,15 @@ AttributeWriter::update(SerialNum serialNum, const DocumentUpdate &upd, Document
         if (__builtin_expect(attrp->getStatus().getLastSyncToken() >= serialNum, false)) {
             continue;
         }
-        if (itr->second.use_two_phase_put_for_assign_updates &&
-                is_single_assign_update(fupd)) {
+        if (found->second.use_two_phase_put_for_assign_updates && is_single_assign_update(fupd)) {
             auto prepare_task = std::make_unique<PreparePutTask>(serialNum, lid, *attrp, get_single_assign_update_field_value(fupd));
             auto complete_task = std::make_unique<CompletePutTask>(*prepare_task, onWriteDone);
             LOG(debug, "About to handle assign update as two phase put for docid %u in attribute vector '%s'",
                 lid, attrp->getName().c_str());
             _shared_executor.execute(CpuUsage::wrap(std::move(prepare_task), CpuUsage::Category::WRITE));
-            _attributeFieldWriter.executeTask(itr->second.executor_id, std::move(complete_task));
+            _attributeFieldWriter.executeTask(found->second.executor_id, std::move(complete_task));
         } else {
-            args[itr->second.executor_id.getId()]->_updates.emplace_back(attrp, &fupd);
+            args[found->second.executor_id.getId()]->_updates.emplace_back(attrp, &fupd);
             LOG(debug, "About to apply update for docId %u in attribute vector '%s'.", lid, attrp->getName().c_str());
         }
     }
diff --git a/searchcore/src/vespa/searchcore/proton/attribute/ifieldupdatecallback.h b/searchcore/src/vespa/searchcore/proton/attribute/ifieldupdatecallback.h
index d8872607b44..d3ab970fb39 100644
--- a/searchcore/src/vespa/searchcore/proton/attribute/ifieldupdatecallback.h
+++ b/searchcore/src/vespa/searchcore/proton/attribute/ifieldupdatecallback.h
@@ -5,16 +5,17 @@
 #include <vespa/vespalib/stllike/string.h>
 
 namespace search { class AttributeVector; }
+namespace document { class Field; }
 
 namespace proton {
 
 struct IFieldUpdateCallback {
-    virtual ~IFieldUpdateCallback() { }
-    virtual void onUpdateField(vespalib::stringref fieldName, const search::AttributeVector * attr) = 0;
+    virtual ~IFieldUpdateCallback() = default;
+    virtual void onUpdateField(const document::Field & field, const search::AttributeVector * attr) = 0;
 };
 
 struct DummyFieldUpdateCallback : IFieldUpdateCallback {
-    void onUpdateField(vespalib::stringref, const search::AttributeVector *) override {}
+    void onUpdateField(const document::Field & , const search::AttributeVector *) override {}
 };
 
 }
diff --git a/searchcore/src/vespa/searchcore/proton/server/searchable_feed_view.cpp b/searchcore/src/vespa/searchcore/proton/server/searchable_feed_view.cpp
index 207f1d813d8..7a78b4ba82a 100644
--- a/searchcore/src/vespa/searchcore/proton/server/searchable_feed_view.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/searchable_feed_view.cpp
@@ -34,7 +34,7 @@ SearchableFeedView::SearchableFeedView(StoreOnlyFeedView::Context storeOnlyCtx,
                                        const FastAccessFeedView::Context &fastUpdateCtx, Context ctx)
     : Parent(std::move(storeOnlyCtx), params, fastUpdateCtx),
       _indexWriter(ctx._indexWriter),
-      _hasIndexedFields(_schema->getNumIndexFields() > 0)
+      _hasIndexedFields(getSchema()->getNumIndexFields() > 0)
 { }
 
 SearchableFeedView::~SearchableFeedView() = default;
diff --git a/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp b/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp
index a537742b79b..d7c3246cb4b 100644
--- a/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp
+++ b/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.cpp
@@ -7,14 +7,14 @@
 #include "putdonecontext.h"
 #include "removedonecontext.h"
 #include "updatedonecontext.h"
-#include <vespa/document/datatype/documenttype.h>
-#include <vespa/document/fieldvalue/document.h>
-#include <vespa/document/repo/documenttyperepo.h>
 #include <vespa/searchcore/proton/attribute/ifieldupdatecallback.h>
 #include <vespa/searchcore/proton/common/feedtoken.h>
 #include <vespa/searchcore/proton/feedoperation/operations.h>
 #include <vespa/searchcore/proton/reference/i_gid_to_lid_change_handler.h>
 #include <vespa/searchcore/proton/reference/i_pending_gid_to_lid_changes.h>
+#include <vespa/document/repo/documenttyperepo.h>
+#include <vespa/document/datatype/documenttype.h>
+#include <vespa/document/fieldvalue/document.h>
 #include <vespa/vespalib/util/cpu_usage.h>
 #include <vespa/vespalib/util/destructor_callbacks.h>
 #include <vespa/vespalib/util/exceptions.h>
@@ -88,8 +88,8 @@ SummaryPutDoneContext::SummaryPutDoneContext(FeedToken token, IPendingLidTracker
 
 SummaryPutDoneContext::~SummaryPutDoneContext() = default;
 
-std::vector<document::GlobalId> getGidsToRemove(const IDocumentMetaStore &metaStore,
-                                                const LidVectorContext::LidVector &lidsToRemove)
+std::vector<document::GlobalId>
+getGidsToRemove(const IDocumentMetaStore &metaStore, const LidVectorContext::LidVector &lidsToRemove)
 {
     std::vector<document::GlobalId> gids;
     gids.reserve(lidsToRemove.size());
@@ -102,8 +102,9 @@ std::vector<document::GlobalId> getGidsToRemove(const IDocumentMetaStore &metaSt
     return gids;
 }
 
-void putMetaData(documentmetastore::IStore &meta_store, const DocumentId & doc_id,
-                 const DocumentOperation &op, bool is_removed_doc)
+void
+putMetaData(documentmetastore::IStore &meta_store, const DocumentId & doc_id,
+            const DocumentOperation &op, bool is_removed_doc)
 {
     documentmetastore::IStore::Result putRes(
             meta_store.put(doc_id.getGlobalId(), op.getBucketId(), op.getTimestamp(),
@@ -117,8 +118,9 @@ void putMetaData(documentmetastore::IStore &meta_store, const DocumentId & doc_i
     assert(op.getLid() == putRes._lid);
 }
 
-void removeMetaData(documentmetastore::IStore &meta_store, const GlobalId & gid, const DocumentId &doc_id,
-                    const DocumentOperation &op, bool is_removed_doc)
+void
+removeMetaData(documentmetastore::IStore &meta_store, const GlobalId & gid, const DocumentId &doc_id,
+               const DocumentOperation &op, bool is_removed_doc)
 {
     assert(meta_store.validLid(op.getPrevLid()));
     assert(is_removed_doc == op.getPrevMarkedAsRemoved());
@@ -147,6 +149,37 @@ moveMetaData(documentmetastore::IStore &meta_store, const DocumentId & doc_id, c
     meta_store.move(op.getPrevLid(), op.getLid(), op.get_prepare_serial_num());
 }
 
+class UpdateScope final : public IFieldUpdateCallback
+{
+private:
+    const vespalib::hash_set<int32_t> & _indexedFields;
+    bool _nonAttributeFields;
+public:
+    bool _hasIndexedFields;
+
+    UpdateScope(const vespalib::hash_set<int32_t> & indexedFields, const DocumentUpdate & upd);
+    bool hasIndexOrNonAttributeFields() const {
+        return _hasIndexedFields || _nonAttributeFields;
+    }
+    void onUpdateField(const document::Field & field, const search::AttributeVector * attr) override;
+};
+
+UpdateScope::UpdateScope(const vespalib::hash_set<int32_t> & indexedFields, const DocumentUpdate & upd)
+    : _indexedFields(indexedFields),
+      _nonAttributeFields(!upd.getFieldPathUpdates().empty()),
+      _hasIndexedFields(false)
+{}
+
+void
+UpdateScope::onUpdateField(const document::Field & field, const search::AttributeVector * attr) {
+    if (!_nonAttributeFields && (attr == nullptr || !attr->isUpdateableInMemoryOnly())) {
+        _nonAttributeFields = true;
+    }
+    if (!_hasIndexedFields && (_indexedFields.find(field.getId()) != _indexedFields.end())) {
+        _hasIndexedFields = true;
+    }
+}
+
 }  // namespace
 
 StoreOnlyFeedView::StoreOnlyFeedView(Context ctx, const PersistentParams &params)
@@ -160,12 +193,20 @@ StoreOnlyFeedView::StoreOnlyFeedView(Context ctx, const PersistentParams &params
       _pendingLidsForDocStore(),
       _pendingLidsForCommit(std::move(ctx._pendingLidsForCommit)),
       _schema(std::move(ctx._schema)),
+      _indexedFields(),
       _writeService(ctx._writeService),
       _params(params),
       _metaStore(_documentMetaStoreContext->get()),
       _gidToLidChangeHandler(ctx._gidToLidChangeHandler)
 {
     _docType = _repo->getDocumentType(_params._docTypeName.getName());
+    if (_schema && _docType) {
+        for (const auto &indexField : _schema->getIndexFields()) {
+            document::FieldPath fieldPath;
+            _docType->buildFieldPath(fieldPath, indexField.getName());
+            _indexedFields.insert(fieldPath.back().getFieldRef().getId());
+        }
+    }
 }
 
 StoreOnlyFeedView::~StoreOnlyFeedView() = default;
@@ -207,7 +248,7 @@ void
 StoreOnlyFeedView::putAttributes(SerialNum, Lid, const Document &, OnPutDoneType) {}
 
 void
-StoreOnlyFeedView::putIndexedFields(SerialNum, Lid, const Document::SP &, OnOperationDoneType) {}
+StoreOnlyFeedView::putIndexedFields(SerialNum, Lid, const std::shared_ptr<Document> &, OnOperationDoneType) {}
 
 void
 StoreOnlyFeedView::preparePut(PutOperation &putOp)
@@ -285,7 +326,7 @@ StoreOnlyFeedView::updateAttributes(SerialNum, Lid, const DocumentUpdate & upd,
                                     OnOperationDoneType, IFieldUpdateCallback & onUpdate)
 {
     for (const auto & fieldUpdate : upd.getUpdates()) {
-        onUpdate.onUpdateField(fieldUpdate.getField().getName(), nullptr);
+        onUpdate.onUpdateField(fieldUpdate.getField(), nullptr);
     }
 }
 
@@ -387,22 +428,6 @@ StoreOnlyFeedView::heartBeatSummary(SerialNum serialNum, DoneCallback onDone) {
             }));
 }
 
-StoreOnlyFeedView::UpdateScope::UpdateScope(const search::index::Schema & schema, const DocumentUpdate & upd)
-    : _schema(&schema),
-      _indexedFields(false),
-      _nonAttributeFields(!upd.getFieldPathUpdates().empty())
-{}
-
-void
-StoreOnlyFeedView::UpdateScope::onUpdateField(vespalib::stringref fieldName, const search::AttributeVector * attr) {
-    if (!_nonAttributeFields && (attr == nullptr || !attr->isUpdateableInMemoryOnly())) {
-        _nonAttributeFields = true;
-    }
-    if (!_indexedFields && _schema->isIndexField(fieldName)) {
-        _indexedFields = true;
-    }
-}
-
 void
 StoreOnlyFeedView::internalUpdate(FeedToken token, const UpdateOperation &updOp) {
     if ( ! updOp.getUpdate()) {
@@ -432,7 +457,7 @@ StoreOnlyFeedView::internalUpdate(FeedToken token, const UpdateOperation &updOp)
     }
 
     auto onWriteDone = createUpdateDoneContext(std::move(token), get_pending_lid_token(updOp), updOp.getUpdate());
-    UpdateScope updateScope(*_schema, upd);
+    UpdateScope updateScope(_indexedFields, upd);
     updateAttributes(serialNum, lid, upd, onWriteDone, updateScope);
 
     if (updateScope.hasIndexOrNonAttributeFields()) {
@@ -440,7 +465,7 @@ StoreOnlyFeedView::internalUpdate(FeedToken token, const UpdateOperation &updOp)
         FutureDoc futureDoc = promisedDoc.get_future().share();
         onWriteDone->setDocument(futureDoc);
         _pendingLidsForDocStore.waitComplete(lid);
-        if (updateScope._indexedFields) {
+        if (updateScope._hasIndexedFields) {
             updateIndexedFields(serialNum, lid, futureDoc, onWriteDone);
         }
         PromisedStream promisedStream;
diff --git a/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.h b/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.h
index 2822aa70525..25a98da7ce7 100644
--- a/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.h
+++ b/searchcore/src/vespa/searchcore/proton/server/storeonlyfeedview.h
@@ -9,7 +9,6 @@
 #include "searchcontext.h"
 #include <vespa/searchcore/proton/common/pendinglidtracker.h>
 #include <vespa/searchcore/proton/common/doctypename.h>
-#include <vespa/searchcore/proton/attribute/ifieldupdatecallback.h>
 #include <vespa/searchcore/proton/common/feeddebugger.h>
 #include <vespa/searchcore/proton/documentmetastore/documentmetastore.h>
 #include <vespa/searchcore/proton/documentmetastore/documentmetastorecontext.h>
@@ -18,9 +17,10 @@
 #include <vespa/searchcore/proton/persistenceengine/resulthandler.h>
 #include <vespa/searchcorespi/index/ithreadingservice.h>
 #include <vespa/searchlib/query/base.h>
+#include <vespa/searchcore/proton/feedoperation/operations.h>
 #include <vespa/vespalib/util/threadstackexecutorbase.h>
+#include <vespa/vespalib/stllike/hash_set.h>
 #include <future>
-#include <vespa/searchcore/proton/feedoperation/operations.h>
 
 namespace vespalib { class IDestructorCallback; }
 
@@ -118,22 +118,6 @@ public:
         {}
     };
 
-protected:
-    class UpdateScope : public IFieldUpdateCallback
-    {
-    private:
-        const search::index::Schema *_schema;
-    public:
-        bool _indexedFields;
-        bool _nonAttributeFields;
-
-        UpdateScope(const search::index::Schema & schema, const DocumentUpdate & upd);
-        bool hasIndexOrNonAttributeFields() const {
-            return _indexedFields || _nonAttributeFields;
-        }
-        void onUpdateField(vespalib::stringref fieldName, const search::AttributeVector * attr) override;
-    };
-
 private:
     const ISummaryAdapter::SP                                _summaryAdapter;
     const IDocumentMetaStoreContext::SP                      _documentMetaStoreContext;
@@ -142,9 +126,9 @@ private:
     LidReuseDelayer                                          _lidReuseDelayer;
     PendingLidTracker                                        _pendingLidsForDocStore;
     std::shared_ptr<PendingLidTrackerBase>                   _pendingLidsForCommit;
-
+    const search::index::Schema::SP                          _schema;
+    vespalib::hash_set<int32_t>                              _indexedFields;
 protected:
-    const search::index::Schema::SP          _schema;
     searchcorespi::index::IThreadingService &_writeService;
     PersistentParams                         _params;
     IDocumentMetaStore                      &_metaStore;