Merge pull request #23180 from vespa-engine/balder/inline-attrdfw-get_attribute

Inline return AttrDFW::get_attribute() and clean up some old code wit…

11 files changed, 29 insertions, 85 deletions

diff --git a/searchcore/src/vespa/searchcore/proton/docsummary/docsumcontext.cpp b/searchcore/src/vespa/searchcore/proton/docsummary/docsumcontext.cpp
index 74e135f021d..2b7f5509acb 100644
--- a/searchcore/src/vespa/searchcore/proton/docsummary/docsumcontext.cpp
+++ b/searchcore/src/vespa/searchcore/proton/docsummary/docsumcontext.cpp
@@ -41,7 +41,6 @@ Memory DOCSUM("docsum");
 Memory ERRORS("errors");
 Memory TYPE("type");
 Memory MESSAGE("message");
-Memory DETAILS("details");
 Memory TIMEOUT("timeout");
 
 }
@@ -51,14 +50,10 @@ DocsumContext::initState()
 {
     const DocsumRequest & req = _request;
     _docsumState._args.initFromDocsumRequest(req);
-    _docsumState._docsumcnt = req.hits.size();
-
-    _docsumState._docsumbuf = (_docsumState._docsumcnt > 0)
-                              ? (uint32_t*)malloc(sizeof(uint32_t) * _docsumState._docsumcnt)
-                              : nullptr;
-
-    for (uint32_t i = 0; i < _docsumState._docsumcnt; i++) {
-        _docsumState._docsumbuf[i] = req.hits[i].docid;
+    _docsumState._docsumbuf.clear();
+    _docsumState._docsumbuf.reserve(req.hits.size());
+    for (uint32_t i = 0; i < req.hits.size(); i++) {
+        _docsumState._docsumbuf.push_back(req.hits[i].docid);
     }
 }
 
@@ -77,7 +72,7 @@ vespalib::Slime::UP
 DocsumContext::createSlimeReply()
 {
     _docsumWriter.InitState(_attrMgr, &_docsumState);
-    const size_t estimatedChunkSize(std::min(0x200000ul, _docsumState._docsumcnt*0x400ul));
+    const size_t estimatedChunkSize(std::min(0x200000ul, _docsumState._docsumbuf.size()*0x400ul));
     vespalib::Slime::UP response(std::make_unique<vespalib::Slime>(makeSlimeParams(estimatedChunkSize)));
     Cursor & root = response->setObject();
     Cursor & array = root.setArray(DOCSUMS);
@@ -85,17 +80,18 @@ DocsumContext::createSlimeReply()
     IDocsumWriter::ResolveClassInfo rci = _docsumWriter.resolveClassInfo(_docsumState._args.getResultClassName(),
                                                                          _docsumStore.getSummaryClassId());
     _docsumState._omit_summary_features = rci.outputClass->omit_summary_features();
-    uint32_t i(0);
-    for (i = 0; (i < _docsumState._docsumcnt) && !_request.expired(); ++i) {
-        uint32_t docId = _docsumState._docsumbuf[i];
-        Cursor & docSumC = array.addObject();
+    uint32_t num_ok(0);
+    for (uint32_t docId : _docsumState._docsumbuf) {
+        if (_request.expired() ) { break; }
+        Cursor &docSumC = array.addObject();
         ObjectSymbolInserter inserter(docSumC, docsumSym);
         if ((docId != search::endDocId) && !rci.mustSkip) {
             _docsumWriter.insertDocsum(rci, docId, &_docsumState, &_docsumStore, *response, inserter);
         }
+        num_ok++;
     }
-    if (i != _docsumState._docsumcnt) {
-        const uint32_t numTimedOut = _docsumState._docsumcnt - i;
+    if (num_ok != _docsumState._docsumbuf.size()) {
+        const uint32_t numTimedOut = _docsumState._docsumbuf.size() - num_ok;
         Cursor & errors = root.setArray(ERRORS);
         Cursor & timeout = errors.addObject();
         timeout.setString(TYPE, TIMEOUT);
diff --git a/searchlib/src/vespa/searchlib/common/geo_location.h b/searchlib/src/vespa/searchlib/common/geo_location.h
index 07e6fd055cc..09c77037b03 100644
--- a/searchlib/src/vespa/searchlib/common/geo_location.h
+++ b/searchlib/src/vespa/searchlib/common/geo_location.h
@@ -2,10 +2,10 @@
 
 #pragma once
 
+#include <vespa/vespalib/geo/zcurve.h>
 #include <string>
 #include <cstdint>
 #include <limits>
-#include <vespa/vespalib/geo/zcurve.h>
 
 namespace search::common {
 
diff --git a/searchlib/src/vespa/searchlib/common/geo_location_spec.h b/searchlib/src/vespa/searchlib/common/geo_location_spec.h
index ea0104aa058..f1e3671181d 100644
--- a/searchlib/src/vespa/searchlib/common/geo_location_spec.h
+++ b/searchlib/src/vespa/searchlib/common/geo_location_spec.h
@@ -2,9 +2,9 @@
 
 #pragma once
 
+#include "geo_location.h"
 #include <string>
 #include <cstdint>
-#include "geo_location.h"
 
 namespace search::common {
 
diff --git a/searchlib/src/vespa/searchlib/engine/docsumrequest.h b/searchlib/src/vespa/searchlib/engine/docsumrequest.h
index 27fb5b25a96..d4f3a1ec340 100644
--- a/searchlib/src/vespa/searchlib/engine/docsumrequest.h
+++ b/searchlib/src/vespa/searchlib/engine/docsumrequest.h
@@ -2,7 +2,6 @@
 
 #pragma once
 
-#include "propertiesmap.h"
 #include "request.h"
 #include "lazy_source.h"
 #include <vespa/document/base/globalid.h>
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/attributedfw.cpp b/searchsummary/src/vespa/searchsummary/docsummary/attributedfw.cpp
index eb4f1a19e06..d5fdee096b1 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/attributedfw.cpp
+++ b/searchsummary/src/vespa/searchsummary/docsummary/attributedfw.cpp
@@ -1,7 +1,6 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include "attributedfw.h"
-#include "docsumstate.h"
 #include "docsumwriter.h"
 #include "docsum_field_writer_state.h"
 #include <vespa/eval/eval/value.h>
@@ -39,11 +38,6 @@ AttrDFW::AttrDFW(const vespalib::string & attrName) :
 {
 }
 
-const attribute::IAttributeVector &
-AttrDFW::get_attribute(const GetDocsumsState& s) const {
-    return *s.getAttribute(getIndex());
-}
-
 namespace {
 
 class SingleAttrDFW : public AttrDFW
@@ -53,14 +47,11 @@ public:
         AttrDFW(attrName)
     { }
     void insertField(uint32_t docid, GetDocsumsState *state, ResType type, Inserter &target) override;
-    bool isDefaultValue(uint32_t docid, const GetDocsumsState * state) const override;
+    bool isDefaultValue(uint32_t docid, const GetDocsumsState * state) const override {
+        return get_attribute(*state).isUndefined(docid);
+    }
 };
 
-bool SingleAttrDFW::isDefaultValue(uint32_t docid, const GetDocsumsState * state) const
-{
-    return get_attribute(*state).isUndefined(docid);
-}
-
 void
 SingleAttrDFW::insertField(uint32_t docid, GetDocsumsState * state, ResType type, Inserter &target)
 {
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/attributedfw.h b/searchsummary/src/vespa/searchsummary/docsummary/attributedfw.h
index cf42dac75c0..35f67fd5446 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/attributedfw.h
+++ b/searchsummary/src/vespa/searchsummary/docsummary/attributedfw.h
@@ -3,7 +3,7 @@
 #pragma once
 
 #include "docsumfieldwriter.h"
-
+#include "docsumstate.h"
 
 namespace search { class MatchingElementsFields; }
 namespace search::attribute { class IAttributeVector; }
@@ -27,7 +27,9 @@ class AttrDFW : public ISimpleDFW
 private:
     vespalib::string _attrName;
 protected:
-    const attribute::IAttributeVector& get_attribute(const GetDocsumsState& s) const;
+    const attribute::IAttributeVector& get_attribute(const GetDocsumsState& s) const {
+        return *s.getAttribute(getIndex());
+    }
     const vespalib::string & getAttributeName() const override { return _attrName; }
 public:
     AttrDFW(const vespalib::string & attrName);
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/docsumfieldwriter.h b/searchsummary/src/vespa/searchsummary/docsummary/docsumfieldwriter.h
index ed43dde1c34..19e22eba4de 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/docsumfieldwriter.h
+++ b/searchsummary/src/vespa/searchsummary/docsummary/docsumfieldwriter.h
@@ -18,13 +18,11 @@ class IDocsumFieldWriter
 public:
     using UP = std::unique_ptr<IDocsumFieldWriter>;
     IDocsumFieldWriter() : _index(0) { }
-    virtual ~IDocsumFieldWriter() {}
+    virtual ~IDocsumFieldWriter() = default;
 
-    static bool IsBinaryCompatible(ResType a, ResType b)
-    { return ResultConfig::IsBinaryCompatible(a, b); }
-
-    static bool IsRuntimeCompatible(ResType a, ResType b)
-    { return ResultConfig::IsRuntimeCompatible(a, b); }
+    static bool IsRuntimeCompatible(ResType a, ResType b) {
+        return ResultConfig::IsRuntimeCompatible(a, b);
+    }
 
     virtual bool IsGenerated() const = 0;
     virtual void insertField(uint32_t docid, GeneralResult *gres, GetDocsumsState *state, ResType type,
@@ -84,5 +82,4 @@ public:
                      vespalib::slime::Inserter &target) override;
 };
 
-
 }
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/docsumstate.cpp b/searchsummary/src/vespa/searchsummary/docsummary/docsumstate.cpp
index e538af3839e..80e57666ad3 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/docsumstate.cpp
+++ b/searchsummary/src/vespa/searchsummary/docsummary/docsumstate.cpp
@@ -24,10 +24,8 @@ namespace search::docsummary {
 
 GetDocsumsState::GetDocsumsState(GetDocsumsStateCallback &callback)
     : _args(),
-      _docsumbuf(nullptr),
-      _docsumcnt(0),
+      _docsumbuf(),
       _kwExtractor(nullptr),
-      _keywords(nullptr),
       _callback(callback),
       _dynteaser(),
       _attrCtx(),
@@ -53,8 +51,6 @@ GetDocsumsState::GetDocsumsState(GetDocsumsStateCallback &callback)
 
 GetDocsumsState::~GetDocsumsState()
 {
-    free(_docsumbuf);
-    free(_keywords);
     if (_dynteaser._result != nullptr) {
         juniper::ReleaseResult(_dynteaser._result);
     }
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/docsumstate.h b/searchsummary/src/vespa/searchsummary/docsummary/docsumstate.h
index c25aca15200..cb8a6851c33 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/docsumstate.h
+++ b/searchsummary/src/vespa/searchsummary/docsummary/docsumstate.h
@@ -53,12 +53,8 @@ public:
     const search::attribute::IAttributeVector * getAttribute(size_t index) const { return _attributes[index]; }
 
     GetDocsumArgs               _args;      // from getdocsums request
-
-    uint32_t                   *_docsumbuf; // from getdocsums request
-    uint32_t                    _docsumcnt; // from getdocsums request
-
+    std::vector<uint32_t>       _docsumbuf; // from getdocsums request
     KeywordExtractor           *_kwExtractor;
-    char                       *_keywords;  // list of keywords from query
 
     GetDocsumsStateCallback    &_callback;
 
@@ -85,12 +81,12 @@ public:
     void parse_locations();
 
     // used by SummaryFeaturesDFW
-    FeatureSet::SP _summaryFeatures;
+    std::shared_ptr<FeatureSet> _summaryFeatures;
     bool           _summaryFeaturesCached;
     bool           _omit_summary_features;
 
     // used by RankFeaturesDFW
-    FeatureSet::SP _rankFeatures;
+    std::shared_ptr<FeatureSet> _rankFeatures;
 
     // Used by AttributeCombinerDFW and MultiAttrDFW when filtering is enabled
     std::unique_ptr<search::MatchingElements> _matching_elements;
diff --git a/searchsummary/src/vespa/searchsummary/docsummary/getdocsumargs.h b/searchsummary/src/vespa/searchsummary/docsummary/getdocsumargs.h
index 9be90aba672..7c346a4b9bb 100644
--- a/searchsummary/src/vespa/searchsummary/docsummary/getdocsumargs.h
+++ b/searchsummary/src/vespa/searchsummary/docsummary/getdocsumargs.h
@@ -4,7 +4,6 @@
 
 #include <vespa/searchlib/fef/properties.h>
 #include <vespa/searchlib/engine/docsumrequest.h>
-#include <vespa/searchlib/engine/propertiesmap.h>
 
 namespace search::docsummary {
 
diff --git a/vespalib/src/vespa/vespalib/util/jsonwriter.h b/vespalib/src/vespa/vespalib/util/jsonwriter.h
index 57380b46e5c..966a5e4b073 100644
--- a/vespalib/src/vespa/vespalib/util/jsonwriter.h
+++ b/vespalib/src/vespa/vespalib/util/jsonwriter.h
@@ -75,36 +75,4 @@ public:
     stringref toString() const;
 };
 
-template<typename T>
-struct JSONPrinter
-{
-    static void printJSON(JSONWriter& w, T v) {
-        w.appendInt64(v);
-    }
-};
-
-template<>
-struct JSONPrinter<uint64_t>
-{
-    static void printJSON(JSONWriter& w, uint64_t v) {
-        w.appendUInt64(v);
-    }
-};
-
-template<>
-struct JSONPrinter<float>
-{
-    static void printJSON(JSONWriter& w, float v) {
-        w.appendDouble(v);
-    }
-};
-
-template<>
-struct JSONPrinter<double>
-{
-    static void printJSON(JSONWriter& w, double v) {
-        w.appendDouble(v);
-    }
-};
-
 }