Merge pull request #22854 from vespa-engine/balder/inline-tensor-access-methods

- Optimize get_tensor_ref optimizing for no branches on happy path.

6 files changed, 26 insertions, 45 deletions

diff --git a/searchlib/src/vespa/searchlib/tensor/direct_tensor_attribute.cpp b/searchlib/src/vespa/searchlib/tensor/direct_tensor_attribute.cpp
index fbb55bf396c..9085ff6bb00 100644
--- a/searchlib/src/vespa/searchlib/tensor/direct_tensor_attribute.cpp
+++ b/searchlib/src/vespa/searchlib/tensor/direct_tensor_attribute.cpp
@@ -122,24 +122,18 @@ DirectTensorAttribute::getTensor(DocId docId) const
 const vespalib::eval::Value &
 DirectTensorAttribute::get_tensor_ref(DocId docId) const
 {
-    EntryRef ref;
-    if (docId < getCommittedDocIdLimit()) {
-        ref = acquire_entry_ref(docId);
-    }
-    if (ref.valid()) {
-        auto ptr = _direct_store.get_tensor(ref);
-        if (ptr) {
-            return *ptr;
-        }
-    }
-    return *_emptyTensor;
+    if (docId >= getCommittedDocIdLimit()) { return *_emptyTensor; }
+
+    auto ptr = _direct_store.get_tensor(acquire_entry_ref(docId));
+    if ( ptr == nullptr) { return *_emptyTensor; }
+
+    return *ptr;
 }
 
 std::unique_ptr<AttributeSaver>
 DirectTensorAttribute::onInitSave(vespalib::stringref fileName)
 {
-    vespalib::GenerationHandler::Guard guard(getGenerationHandler().
-                                             takeGuard());
+    vespalib::GenerationHandler::Guard guard(getGenerationHandler().takeGuard());
     return std::make_unique<DirectTensorAttributeSaver>
         (std::move(guard),
          this->createAttributeHeader(fileName),
diff --git a/searchlib/src/vespa/searchlib/tensor/direct_tensor_attribute.h b/searchlib/src/vespa/searchlib/tensor/direct_tensor_attribute.h
index 779cca59abb..931fb969978 100644
--- a/searchlib/src/vespa/searchlib/tensor/direct_tensor_attribute.h
+++ b/searchlib/src/vespa/searchlib/tensor/direct_tensor_attribute.h
@@ -9,25 +9,25 @@ namespace vespalib::eval { struct Value; }
 
 namespace search::tensor {
 
-class DirectTensorAttribute : public TensorAttribute
+class DirectTensorAttribute final : public TensorAttribute
 {
     DirectTensorStore _direct_store;
 
 public:
     DirectTensorAttribute(vespalib::stringref baseFileName, const Config &cfg);
-    virtual ~DirectTensorAttribute();
-    virtual void setTensor(DocId docId, const vespalib::eval::Value &tensor) override;
+    ~DirectTensorAttribute() override;
+    void setTensor(DocId docId, const vespalib::eval::Value &tensor) override;
     void update_tensor(DocId docId,
                        const document::TensorUpdate &update,
                        bool create_empty_if_non_existing) override;
-    virtual std::unique_ptr<vespalib::eval::Value> getTensor(DocId docId) const override;
-    virtual bool onLoad(vespalib::Executor *executor) override;
-    virtual std::unique_ptr<AttributeSaver> onInitSave(vespalib::stringref fileName) override;
-    virtual void compactWorst() override;
+    std::unique_ptr<vespalib::eval::Value> getTensor(DocId docId) const override;
+    bool onLoad(vespalib::Executor *executor) override;
+    std::unique_ptr<AttributeSaver> onInitSave(vespalib::stringref fileName) override;
+    void compactWorst() override;
 
     void set_tensor(DocId docId, std::unique_ptr<vespalib::eval::Value> tensor);
     const vespalib::eval::Value &get_tensor_ref(DocId docId) const override;
-    virtual bool supports_get_tensor_ref() const override { return true; }
+    bool supports_get_tensor_ref() const override { return true; }
 };
 
 }  // namespace search::tensor
diff --git a/searchlib/src/vespa/searchlib/tensor/direct_tensor_store.cpp b/searchlib/src/vespa/searchlib/tensor/direct_tensor_store.cpp
index 3f2a1bb1969..e6084c9cde4 100644
--- a/searchlib/src/vespa/searchlib/tensor/direct_tensor_store.cpp
+++ b/searchlib/src/vespa/searchlib/tensor/direct_tensor_store.cpp
@@ -47,17 +47,6 @@ DirectTensorStore::DirectTensorStore()
 
 DirectTensorStore::~DirectTensorStore() = default;
 
-const vespalib::eval::Value *
-DirectTensorStore::get_tensor(EntryRef ref) const
-{
-    if (!ref.valid()) {
-        return nullptr;
-    }
-    const auto& entry = _tensor_store.getEntry(ref);
-    assert(entry);
-    return entry.get();
-}
-
 EntryRef
 DirectTensorStore::store_tensor(std::unique_ptr<vespalib::eval::Value> tensor)
 {
diff --git a/searchlib/src/vespa/searchlib/tensor/direct_tensor_store.h b/searchlib/src/vespa/searchlib/tensor/direct_tensor_store.h
index 1f112f1ea28..587ded2209c 100644
--- a/searchlib/src/vespa/searchlib/tensor/direct_tensor_store.h
+++ b/searchlib/src/vespa/searchlib/tensor/direct_tensor_store.h
@@ -3,7 +3,6 @@
 #pragma once
 
 #include "tensor_store.h"
-#include <memory>
 
 namespace vespalib::eval { struct Value; }
 
@@ -28,7 +27,7 @@ private:
         using CleanContext = typename ParentType::CleanContext;
     public:
         TensorBufferType();
-        virtual void cleanHold(void* buffer, size_t offset, ElemCount num_elems, CleanContext clean_ctx) override;
+        void cleanHold(void* buffer, size_t offset, ElemCount num_elems, CleanContext clean_ctx) override;
     };
 
     TensorStoreType _tensor_store;
@@ -40,7 +39,12 @@ public:
     ~DirectTensorStore() override;
     using RefType = TensorStoreType::RefType;
 
-    const vespalib::eval::Value * get_tensor(EntryRef ref) const;
+    const vespalib::eval::Value * get_tensor(EntryRef ref) const {
+        if (!ref.valid()) {
+            return nullptr;
+        }
+        return _tensor_store.getEntry(ref).get();
+    }
     EntryRef store_tensor(std::unique_ptr<vespalib::eval::Value> tensor);
 
     void holdTensor(EntryRef ref) override;
diff --git a/vespalib/src/vespa/vespalib/datastore/datastore.h b/vespalib/src/vespa/vespalib/datastore/datastore.h
index be74c2a60d5..3ede2ada953 100644
--- a/vespalib/src/vespa/vespalib/datastore/datastore.h
+++ b/vespalib/src/vespa/vespalib/datastore/datastore.h
@@ -118,7 +118,10 @@ public:
     ~DataStore();
 
     EntryRef addEntry(const EntryType &e);
-    const EntryType &getEntry(EntryRef ref) const;
+
+    const EntryType &getEntry(EntryRef ref) const {
+        return *this->template getEntry<EntryType>(RefType(ref));
+    }
 };
 
 extern template class DataStoreT<EntryRefT<22> >;
diff --git a/vespalib/src/vespa/vespalib/datastore/datastore.hpp b/vespalib/src/vespa/vespalib/datastore/datastore.hpp
index 23dcb9222a1..e46b6cf111e 100644
--- a/vespalib/src/vespa/vespalib/datastore/datastore.hpp
+++ b/vespalib/src/vespa/vespalib/datastore/datastore.hpp
@@ -164,15 +164,6 @@ DataStore<EntryType, RefT>::addEntry(const EntryType &e)
     return FreeListAllocator<EntryType, RefT, NoOpReclaimer>(*this, 0).alloc(e).ref;
 }
 
-template <typename EntryType, typename RefT>
-const EntryType &
-DataStore<EntryType, RefT>::getEntry(EntryRef ref) const
-{
-    RefType intRef(ref);
-    const EntryType *be = this->template getEntry<EntryType>(intRef);
-    return *be;
-}
-
 extern template class DataStoreT<EntryRefT<22> >;
 
 }