Merge pull request #30234 from vespa-engine/balder/some-code-cleanup

- DocId is an overloaded type already. Just use string as directly fo…

8 files changed, 64 insertions, 67 deletions

diff --git a/searchlib/src/vespa/searchlib/aggregation/hitsaggregationresult.h b/searchlib/src/vespa/searchlib/aggregation/hitsaggregationresult.h
index 0bf33ea33e8..f90ee3c2312 100644
--- a/searchlib/src/vespa/searchlib/aggregation/hitsaggregationresult.h
+++ b/searchlib/src/vespa/searchlib/aggregation/hitsaggregationresult.h
@@ -16,8 +16,8 @@ public:
     class SummaryGenerator
     {
     public:
-        virtual ~SummaryGenerator() { }
-        virtual vespalib::ConstBufferRef fillSummary(DocId lid, const SummaryClassType & summaryClass) = 0;
+        virtual ~SummaryGenerator() = default;
+        virtual vespalib::ConstBufferRef fillSummary(DocId lid, vespalib::stringref summaryClass) = 0;
     };
 
 private:
@@ -26,7 +26,7 @@ private:
     void onAggregate(const ResultNode &result, const document::Document & doc, HitRank rank) override;
     const ResultNode & onGetRank() const override;
 
-    SummaryClassType          _summaryClass;
+    vespalib::string          _summaryClass;
     uint32_t                  _maxHits;
     HitList                   _hits;
     bool                      _isOrdered;
@@ -50,8 +50,8 @@ public:
     ~HitsAggregationResult() override;
     void postMerge() override { _hits.postMerge(_maxHits); }
     void setSummaryGenerator(SummaryGenerator & summaryGenerator) { _summaryGenerator = &summaryGenerator; }
-    const SummaryClassType & getSummaryClass() const { return _summaryClass; }
-    HitsAggregationResult setSummaryClass(const SummaryClassType & summaryClass) { _summaryClass = summaryClass; return *this; }
+    const vespalib::string & getSummaryClass() const { return _summaryClass; }
+    HitsAggregationResult setSummaryClass(vespalib::stringref summaryClass) { _summaryClass = summaryClass; return *this; }
     HitsAggregationResult &setMaxHits(uint32_t maxHits) {
         _maxHits = (maxHits == 0) ? std::numeric_limits<uint32_t>::max() : maxHits;
         return *this;
diff --git a/searchlib/src/vespa/searchlib/aggregation/vdshit.h b/searchlib/src/vespa/searchlib/aggregation/vdshit.h
index 11cfe9b3b18..32a35c22977 100644
--- a/searchlib/src/vespa/searchlib/aggregation/vdshit.h
+++ b/searchlib/src/vespa/searchlib/aggregation/vdshit.h
@@ -2,7 +2,6 @@
 #pragma once
 
 #include "hit.h"
-#include "aggregationresult.h"
 
 namespace search::aggregation {
 
@@ -10,19 +9,18 @@ class VdsHit : public Hit
 {
 public:
     using Summary = std::vector<uint8_t>;
-    using DocId = vespalib::string;
     DECLARE_IDENTIFIABLE_NS2(search, aggregation, VdsHit);
     DECLARE_NBO_SERIALIZE;
     VdsHit() noexcept : Hit(), _docId(), _summary() {}
-    VdsHit(DocId docId, HitRank rank) noexcept : Hit(rank), _docId(docId), _summary() {}
-    ~VdsHit();
+    VdsHit(vespalib::stringref docId, HitRank rank) noexcept : Hit(rank), _docId(docId), _summary() {}
+    ~VdsHit() override;
     VdsHit *clone() const override { return new VdsHit(*this); }
     void visitMembers(vespalib::ObjectVisitor &visitor) const override;
-    const   DocId &   getDocId() const noexcept { return _docId; }
+    const vespalib::string &   getDocId() const noexcept { return _docId; }
     const Summary & getSummary() const noexcept { return _summary; }
-    VdsHit &   setDocId(DocId & docId) noexcept { _docId = docId; return *this; }
+    VdsHit & setDocId(vespalib::stringref docId) noexcept { _docId = docId; return *this; }
     VdsHit & setSummary(const void * buf, size_t sz) noexcept {
-        const uint8_t * v(static_cast<const uint8_t *>(buf));
+        const auto * v(static_cast<const uint8_t *>(buf));
         Summary n(v, v+sz);
         _summary.swap(n);
         return *this;
@@ -30,8 +28,8 @@ public:
     bool operator < (const VdsHit &b) const noexcept { return cmp(b) < 0; }
 
 private:
-    DocId     _docId;
-    Summary   _summary;
+    vespalib::string  _docId;
+    Summary           _summary;
 };
 
 }
diff --git a/searchlib/src/vespa/searchlib/index/indexbuilder.h b/searchlib/src/vespa/searchlib/index/indexbuilder.h
index 9615bfd9428..71b826698ef 100644
--- a/searchlib/src/vespa/searchlib/index/indexbuilder.h
+++ b/searchlib/src/vespa/searchlib/index/indexbuilder.h
@@ -10,6 +10,13 @@ class DocIdAndFeatures;
 class Schema;
 class WordDocElementWordPosFeatures;
 
+/**
+ * Interface for building an index for a single field
+ * The index should be built as follows:
+ *   Add the set of unique words in sorted order.
+ *   For each word add the set of document ids in sorted order.
+ *   For each document id add the position information for that document.
+ */
 class FieldIndexBuilder {
 public:
     virtual ~FieldIndexBuilder() = default;
@@ -20,16 +27,11 @@ public:
 
 /**
  * Interface used to build an index for the set of index fields specified in a schema.
- *
- * The index should be built as follows:
- *   For each field add the set of unique words in sorted order.
- *   For each word add the set of document ids in sorted order.
- *   For each document id add the position information for that document.
+ * Create and complete one field builder at the time.
  */
 class IndexBuilder {
 protected:
     const Schema &_schema;
-
 public:
     explicit IndexBuilder(const Schema &schema);
     virtual ~IndexBuilder();
diff --git a/streamingvisitors/src/vespa/searchvisitor/hitcollector.cpp b/streamingvisitors/src/vespa/searchvisitor/hitcollector.cpp
index 229a6f455c4..96bd3c733ec 100644
--- a/streamingvisitors/src/vespa/searchvisitor/hitcollector.cpp
+++ b/streamingvisitors/src/vespa/searchvisitor/hitcollector.cpp
@@ -5,13 +5,10 @@
 #include <vespa/searchlib/fef/utils.h>
 #include <vespa/vespalib/util/stringfmt.h>
 #include <algorithm>
-#include <vespa/eval/eval/value_codec.h>
-#include <vespa/vespalib/objects/nbostream.h>
 
 #include <vespa/log/log.h>
 LOG_SETUP(".searchvisitor.hitcollector");
 
-using search::fef::MatchData;
 using vespalib::FeatureSet;
 using vespalib::FeatureValues;
 using vdslib::SearchResult;
@@ -20,7 +17,7 @@ using FefUtils = search::fef::Utils;
 
 namespace streaming {
 
-HitCollector::Hit::Hit(const vsm::StorageDocument *  doc, uint32_t docId, const search::fef::MatchData & matchData,
+HitCollector::Hit::Hit(const vsm::StorageDocument *  doc, uint32_t docId, const MatchData & matchData,
                        double score, const void * sortData, size_t sortDataLen) :
     _docid(docId),
     _score(score),
@@ -34,7 +31,7 @@ HitCollector::Hit::Hit(const vsm::StorageDocument *  doc, uint32_t docId, const
     }
 }
 
-HitCollector::Hit::~Hit() { }
+HitCollector::Hit::~Hit() = default;
 
 HitCollector::HitCollector(size_t wantedHits) :
     _hits(),
@@ -55,13 +52,13 @@ HitCollector::getDocSum(const search::DocumentIdT & docId) const
 }
 
 bool
-HitCollector::addHit(const vsm::StorageDocument * doc, uint32_t docId, const search::fef::MatchData & data, double score)
+HitCollector::addHit(const vsm::StorageDocument * doc, uint32_t docId, const MatchData & data, double score)
 {
     return addHit(Hit(doc, docId, data, score));
 }
 
 bool
-HitCollector::addHit(const vsm::StorageDocument * doc, uint32_t docId, const search::fef::MatchData & data,
+HitCollector::addHit(const vsm::StorageDocument * doc, uint32_t docId, const MatchData & data,
                      double score, const void * sortData, size_t sortDataLen)
 {
     return addHit(Hit(doc, docId, data, score, sortData, sortDataLen));
@@ -154,7 +151,7 @@ HitCollector::fillSearchResult(vdslib::SearchResult & searchResult)
 
 FeatureSet::SP
 HitCollector::getFeatureSet(IRankProgram &rankProgram,
-                            const search::fef::FeatureResolver &resolver,
+                            const FeatureResolver &resolver,
                             const search::StringStringMap &feature_rename_map)
 {
     if (resolver.num_features() == 0 || _hits.empty()) {
@@ -174,7 +171,7 @@ HitCollector::getFeatureSet(IRankProgram &rankProgram,
 
 FeatureValues
 HitCollector::get_match_features(IRankProgram& rank_program,
-                                 const search::fef::FeatureResolver& resolver,
+                                 const FeatureResolver& resolver,
                                  const search::StringStringMap& feature_rename_map)
 {
     FeatureValues match_features;
diff --git a/streamingvisitors/src/vespa/searchvisitor/hitcollector.h b/streamingvisitors/src/vespa/searchvisitor/hitcollector.h
index 76c94840c7b..4c3a49fa5cc 100644
--- a/streamingvisitors/src/vespa/searchvisitor/hitcollector.h
+++ b/streamingvisitors/src/vespa/searchvisitor/hitcollector.h
@@ -10,7 +10,7 @@
 #include <vespa/vespalib/stllike/string.h>
 #include <vespa/vespalib/util/featureset.h>
 
-namespace search { namespace fef { class FeatureResolver; } }
+namespace search::fef { class FeatureResolver; }
 
 namespace streaming {
 
@@ -20,54 +20,55 @@ namespace streaming {
 class HitCollector : public vsm::IDocSumCache
 {
 private:
+    using TermFieldMatchData = search::fef::TermFieldMatchData;
+    using MatchData = search::fef::MatchData;
+    using FeatureResolver = search::fef::FeatureResolver;
     class Hit
     {
     public:
-        Hit(const vsm::StorageDocument * doc, uint32_t docId, const search::fef::MatchData & matchData,
+        Hit(const vsm::StorageDocument * doc, uint32_t docId, const MatchData & matchData,
             double score, const void * sortData, size_t sortDataLen);
-        Hit(const vsm::StorageDocument * doc, uint32_t docId, const search::fef::MatchData & matchData, double score)
+        Hit(const vsm::StorageDocument * doc, uint32_t docId, const MatchData & matchData, double score)
             : Hit(doc, docId, matchData, score, nullptr, 0)
         { }
         ~Hit();
         Hit(const Hit &) = delete;
         Hit & operator = (const Hit &) = delete;
-        Hit(Hit && rhs) = default;
-        Hit & operator = (Hit && rhs) = default;
-        search::DocumentIdT getDocId() const { return _docid; }
-        const vsm::StorageDocument & getDocument() const { return *_document; }
-        const std::vector<search::fef::TermFieldMatchData> &getMatchData() const { return _matchData; }
-        search::feature_t getRankScore() const { return _score; }
-        const vespalib::string & getSortBlob() const { return _sortBlob; }
-        bool operator < (const Hit & b) const { return getDocId() < b.getDocId(); }
-        int cmpDocId(const Hit & b) const { return getDocId() - b.getDocId(); }
-        int cmpRank(const Hit & b) const {
+        Hit(Hit && rhs) noexcept = default;
+        Hit & operator = (Hit && rhs) noexcept = default;
+        search::DocumentIdT getDocId() const noexcept { return _docid; }
+        const vsm::StorageDocument & getDocument() const noexcept { return *_document; }
+        const std::vector<TermFieldMatchData> &getMatchData() const noexcept { return _matchData; }
+        search::feature_t getRankScore() const noexcept { return _score; }
+        const vespalib::string & getSortBlob() const noexcept { return _sortBlob; }
+        bool operator < (const Hit & b) const noexcept { return getDocId() < b.getDocId(); }
+        int cmpDocId(const Hit & b) const noexcept { return getDocId() - b.getDocId(); }
+        int cmpRank(const Hit & b) const noexcept {
             return (getRankScore() > b.getRankScore()) ?
                 -1 : ((getRankScore() < b.getRankScore()) ? 1 : cmpDocId(b));
         }
-        int cmpSort(const Hit & b) const {
+        int cmpSort(const Hit & b) const noexcept {
             int diff = _sortBlob.compare(b._sortBlob.c_str(), b._sortBlob.size());
             return (diff == 0) ? cmpDocId(b) : diff;
         }
         class RankComparator {
         public:
-            RankComparator() {}
-            bool operator() (const Hit & lhs, const Hit & rhs) const {
+            bool operator() (const Hit & lhs, const Hit & rhs) const noexcept {
                 return lhs.cmpRank(rhs) < 0;
             }
         };
         class SortComparator {
         public:
-            SortComparator() {}
-            bool operator() (const Hit & lhs, const Hit & rhs) const {
+            bool operator() (const Hit & lhs, const Hit & rhs) const noexcept {
                 return lhs.cmpSort(rhs) < 0;
             }
         };
 
     private:
         uint32_t _docid;
-        double _score;
+        double   _score;
         const vsm::StorageDocument * _document;
-        std::vector<search::fef::TermFieldMatchData> _matchData;
+        std::vector<TermFieldMatchData> _matchData;
         vespalib::string _sortBlob;
     };
     using HitVector = std::vector<Hit>;
@@ -82,11 +83,11 @@ public:
     using UP = std::unique_ptr<HitCollector>;
 
     struct IRankProgram {
-        virtual ~IRankProgram() {}
-        virtual void run(uint32_t docid, const std::vector<search::fef::TermFieldMatchData> &matchData) = 0;
+        virtual ~IRankProgram() = default;
+        virtual void run(uint32_t docid, const std::vector<TermFieldMatchData> &matchData) = 0;
     };
 
-    HitCollector(size_t wantedHits);
+    explicit HitCollector(size_t wantedHits);
 
     virtual const vsm::Document & getDocSum(const search::DocumentIdT & docId) const override;
 
@@ -100,7 +101,7 @@ public:
      * @param data  The match data for the hit.
      * @return true if the document was added to the heap
      **/
-    bool addHit(const vsm::StorageDocument * doc, uint32_t docId, const search::fef::MatchData & data, double score);
+    bool addHit(const vsm::StorageDocument * doc, uint32_t docId, const MatchData & data, double score);
 
     /**
      * Adds a hit to this hit collector.
@@ -114,7 +115,7 @@ public:
      * @param sortDataLen The length of the sortdata.
      * @return true if the document was added to the heap
      **/
-    bool addHit(const vsm::StorageDocument * doc, uint32_t docId, const search::fef::MatchData & data,
+    bool addHit(const vsm::StorageDocument * doc, uint32_t docId, const MatchData & data,
                 double score, const void * sortData, size_t sortDataLen);
 
     /**
@@ -134,13 +135,12 @@ public:
      * @param resolver   feature resolver, gives feature names and values
      **/
     vespalib::FeatureSet::SP getFeatureSet(IRankProgram &rankProgram,
-                                           const search::fef::FeatureResolver &resolver,
+                                           const FeatureResolver &resolver,
                                            const search::StringStringMap &feature_rename_map);
 
     vespalib::FeatureValues get_match_features(IRankProgram& rank_program,
-                                               const search::fef::FeatureResolver& resolver,
+                                               const FeatureResolver& resolver,
                                                const search::StringStringMap& feature_rename_map);
 };
 
 } // namespace streaming
-
diff --git a/streamingvisitors/src/vespa/searchvisitor/rankprocessor.cpp b/streamingvisitors/src/vespa/searchvisitor/rankprocessor.cpp
index a54d2adee78..7db1542ca53 100644
--- a/streamingvisitors/src/vespa/searchvisitor/rankprocessor.cpp
+++ b/streamingvisitors/src/vespa/searchvisitor/rankprocessor.cpp
@@ -247,7 +247,7 @@ FeatureSet::SP
 RankProcessor::calculateFeatureSet()
 {
     LOG(debug, "Calculate feature set");
-    RankProgram &rankProgram = *(_summaryProgram.get() != nullptr ? _summaryProgram : _rankProgram);
+    RankProgram &rankProgram = *(_summaryProgram ? _summaryProgram : _rankProgram);
     search::fef::FeatureResolver resolver(rankProgram.get_seeds(false));
     LOG(debug, "Feature handles: numNames(%ld)", resolver.num_features());
     RankProgramWrapper wrapper(*_match_data);
@@ -260,7 +260,7 @@ FeatureValues
 RankProcessor::calculate_match_features()
 {
     if (!_match_features_program) {
-        return FeatureValues();
+        return {};
     }
     RankProgramWrapper wrapper(*_match_data);
     search::fef::FeatureResolver resolver(_match_features_program->get_seeds(false));
@@ -284,7 +284,7 @@ void
 RankProcessor::unpack_match_data(uint32_t docid, MatchData &matchData, QueryWrapper& query)
 {
     for (auto& term : query.getTermList()) {
-        QueryTermData & qtd = static_cast<QueryTermData &>(term->getQueryItem());
+        auto & qtd = static_cast<QueryTermData &>(term->getQueryItem());
         const ITermData &td = qtd.getTermData();
         term->unpack_match_data(docid, td, matchData);
     }
diff --git a/streamingvisitors/src/vespa/searchvisitor/searchvisitor.cpp b/streamingvisitors/src/vespa/searchvisitor/searchvisitor.cpp
index 0a4664a4878..9755f20588f 100644
--- a/streamingvisitors/src/vespa/searchvisitor/searchvisitor.cpp
+++ b/streamingvisitors/src/vespa/searchvisitor/searchvisitor.cpp
@@ -194,7 +194,7 @@ SearchVisitor::SummaryGenerator::SummaryGenerator(const search::IAttributeManage
 SearchVisitor::SummaryGenerator::~SummaryGenerator() = default;
 
 SearchVisitor::StreamingDocsumsState&
-SearchVisitor::SummaryGenerator::get_streaming_docsums_state(const vespalib::string& summary_class) {
+SearchVisitor::SummaryGenerator::get_streaming_docsums_state(vespalib::stringref summary_class) {
     auto itr = _docsum_states.find(summary_class);
     if (itr != _docsum_states.end()) {
         return *itr->second;
@@ -224,7 +224,7 @@ SearchVisitor::SummaryGenerator::get_streaming_docsums_state(const vespalib::str
 }
 
 vespalib::ConstBufferRef
-SearchVisitor::SummaryGenerator::fillSummary(AttributeVector::DocId lid, const HitsAggregationResult::SummaryClassType & summaryClass)
+SearchVisitor::SummaryGenerator::fillSummary(AttributeVector::DocId lid, vespalib::stringref summaryClass)
 {
     if (_docsumWriter != nullptr) {
         vespalib::Slime slime;
diff --git a/streamingvisitors/src/vespa/searchvisitor/searchvisitor.h b/streamingvisitors/src/vespa/searchvisitor/searchvisitor.h
index 945750ef679..21f89da3d18 100644
--- a/streamingvisitors/src/vespa/searchvisitor/searchvisitor.h
+++ b/streamingvisitors/src/vespa/searchvisitor/searchvisitor.h
@@ -419,13 +419,13 @@ private:
         void setFilter(std::unique_ptr<vsm::DocsumFilter> filter) { _docsumFilter = std::move(filter); }
         void setDocsumCache(const vsm::IDocSumCache & cache) { _docsumFilter->setDocSumStore(cache); }
         void setDocsumWriter(IDocsumWriter & docsumWriter) { _docsumWriter = & docsumWriter; }
-        vespalib::ConstBufferRef fillSummary(search::AttributeVector::DocId lid, const HitsAggregationResult::SummaryClassType & summaryClass) override;
+        vespalib::ConstBufferRef fillSummary(search::AttributeVector::DocId lid, vespalib::stringref summaryClass) override;
         void set_dump_features(bool dump_features) { _dump_features = dump_features; }
         void set_location(const vespalib::string& location) { _location = location; }
         void set_stack_dump(std::vector<char> stack_dump) { _stack_dump = std::move(stack_dump); }
         void add_summary_field(vespalib::stringref field) { _summaryFields.emplace_back(field); }
     private:
-        StreamingDocsumsState& get_streaming_docsums_state(const vespalib::string& summary_class);
+        StreamingDocsumsState& get_streaming_docsums_state(vespalib::stringref summary_class);
         vsm::GetDocsumsStateCallback            _callback;
         vespalib::hash_map<vespalib::string, std::unique_ptr<StreamingDocsumsState>> _docsum_states;
         std::vector<vespalib::string>           _summaryFields;
@@ -442,9 +442,9 @@ private:
     class HitsResultPreparator : public vespalib::ObjectOperation, public vespalib::ObjectPredicate
     {
     public:
-        explicit HitsResultPreparator(SummaryGenerator & summaryGenerator) :
-           _summaryGenerator(summaryGenerator),
-           _numHitsAggregators(0)
+        explicit HitsResultPreparator(SummaryGenerator & summaryGenerator)
+            : _summaryGenerator(summaryGenerator),
+              _numHitsAggregators(0)
         { }
         size_t getNumHitsAggregators() const  { return _numHitsAggregators; }
     private: