diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2024-01-02 19:35:09 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-01-02 19:35:09 +0100 |
commit | 16a9a6cacefb90fce4a49150fd8220afc22599a9 (patch) | |
tree | cb86e1f4ca3e3f83626f41ef11e64fdb81b8c9be /searchlib | |
parent | f031666ee60ab878782bc4490aa697635c43026d (diff) | |
parent | 008d81c1f2aee5498472f0aab1c20724c1b7f9b2 (diff) |
Merge pull request #29755 from vespa-engine/balder/avoid-generic-template
- Avoid inefficient generic template.
Diffstat (limited to 'searchlib')
7 files changed, 48 insertions, 46 deletions
diff --git a/searchlib/src/vespa/searchlib/query/streaming/query.cpp b/searchlib/src/vespa/searchlib/query/streaming/query.cpp index d2eee5d345f..3079ec31e8f 100644 --- a/searchlib/src/vespa/searchlib/query/streaming/query.cpp +++ b/searchlib/src/vespa/searchlib/query/streaming/query.cpp @@ -12,7 +12,7 @@ QueryConnector::visitMembers(vespalib::ObjectVisitor &visitor) const visit(visitor, "Operator", _opName); } -QueryConnector::QueryConnector(const char * opName) +QueryConnector::QueryConnector(const char * opName) noexcept : QueryNode(), _opName(opName), _index(), @@ -31,7 +31,7 @@ const HitList & QueryConnector::evaluateHits(HitList & hl) const { if (evaluate()) { - hl.push_back(Hit(1, 0, 0, 1)); + hl.emplace_back(1, 0, 0, 1); } return hl; } @@ -105,10 +105,10 @@ QueryConnector::create(ParseItem::ItemType type) { switch (type) { case search::ParseItem::ITEM_AND: return std::make_unique<AndQueryNode>(); - case search::ParseItem::ITEM_OR: return std::make_unique<OrQueryNode>(); + case search::ParseItem::ITEM_OR: case search::ParseItem::ITEM_WEAK_AND: return std::make_unique<OrQueryNode>(); + case search::ParseItem::ITEM_WEIGHTED_SET: case search::ParseItem::ITEM_EQUIV: return std::make_unique<EquivQueryNode>(); - case search::ParseItem::ITEM_WEIGHTED_SET: return std::make_unique<EquivQueryNode>(); case search::ParseItem::ITEM_WAND: return std::make_unique<OrQueryNode>(); case search::ParseItem::ITEM_NOT: return std::make_unique<AndNotQueryNode>(); case search::ParseItem::ITEM_PHRASE: return std::make_unique<PhraseQueryNode>(); @@ -340,7 +340,7 @@ Query::Query(const QueryNodeResultFactory & factory, vespalib::stringref queryRe bool Query::evaluate() const { - return valid() ? _root->evaluate() : false; + return valid() && _root->evaluate(); } bool diff --git a/searchlib/src/vespa/searchlib/query/streaming/query.h b/searchlib/src/vespa/searchlib/query/streaming/query.h index 42c3b94002c..3904f743d26 100644 --- a/searchlib/src/vespa/searchlib/query/streaming/query.h +++ b/searchlib/src/vespa/searchlib/query/streaming/query.h @@ -13,8 +13,8 @@ namespace search::streaming { class QueryConnector : public QueryNode { public: - QueryConnector(const char * opName); - ~QueryConnector(); + explicit QueryConnector(const char * opName) noexcept; + ~QueryConnector() override; const HitList & evaluateHits(HitList & hl) const override; void reset() override; void getLeaves(QueryTermList & tl) override; @@ -44,7 +44,7 @@ private: class TrueNode : public QueryConnector { public: - TrueNode() : QueryConnector("AND") { } + TrueNode() noexcept : QueryConnector("AND") { } bool evaluate() const override; }; @@ -52,7 +52,7 @@ public: class FalseNode : public QueryConnector { public: - FalseNode() : QueryConnector("AND") { } + FalseNode() noexcept : QueryConnector("AND") { } bool evaluate() const override; }; @@ -62,8 +62,8 @@ public: class AndQueryNode : public QueryConnector { public: - AndQueryNode() : QueryConnector("AND") { } - AndQueryNode(const char * opName) : QueryConnector(opName) { } + AndQueryNode() noexcept : QueryConnector("AND") { } + explicit AndQueryNode(const char * opName) noexcept : QueryConnector(opName) { } bool evaluate() const override; bool isFlattenable(ParseItem::ItemType type) const override { return type == ParseItem::ITEM_AND; } }; @@ -74,7 +74,7 @@ public: class AndNotQueryNode : public QueryConnector { public: - AndNotQueryNode() : QueryConnector("ANDNOT") { } + AndNotQueryNode() noexcept : QueryConnector("ANDNOT") { } bool evaluate() const override; bool isFlattenable(ParseItem::ItemType type) const override { return type == ParseItem::ITEM_NOT; } }; @@ -85,8 +85,8 @@ public: class OrQueryNode : public QueryConnector { public: - OrQueryNode() : QueryConnector("OR") { } - OrQueryNode(const char * opName) : QueryConnector(opName) { } + OrQueryNode() noexcept : QueryConnector("OR") { } + explicit OrQueryNode(const char * opName) noexcept : QueryConnector(opName) { } bool evaluate() const override; bool isFlattenable(ParseItem::ItemType type) const override { return (type == ParseItem::ITEM_OR) || @@ -102,7 +102,7 @@ public: class EquivQueryNode : public OrQueryNode { public: - EquivQueryNode() : OrQueryNode("EQUIV") { } + EquivQueryNode() noexcept : OrQueryNode("EQUIV") { } bool evaluate() const override; bool isFlattenable(ParseItem::ItemType type) const override { return (type == ParseItem::ITEM_EQUIV) || @@ -117,7 +117,7 @@ public: class PhraseQueryNode : public AndQueryNode { public: - PhraseQueryNode() : AndQueryNode("PHRASE"), _fieldInfo(32) { } + PhraseQueryNode() noexcept : AndQueryNode("PHRASE"), _fieldInfo(32) { } bool evaluate() const override; const HitList & evaluateHits(HitList & hl) const override; void getPhrases(QueryNodeRefList & tl) override; @@ -138,7 +138,7 @@ private: class SameElementQueryNode : public AndQueryNode { public: - SameElementQueryNode() : AndQueryNode("SAME_ELEMENT") { } + SameElementQueryNode() noexcept : AndQueryNode("SAME_ELEMENT") { } bool evaluate() const override; const HitList & evaluateHits(HitList & hl) const override; bool isFlattenable(ParseItem::ItemType type) const override { return type == ParseItem::ITEM_NOT; } @@ -151,8 +151,8 @@ public: class NearQueryNode : public AndQueryNode { public: - NearQueryNode() : AndQueryNode("NEAR"), _distance(0) { } - NearQueryNode(const char * opName) : AndQueryNode(opName), _distance(0) { } + NearQueryNode() noexcept : AndQueryNode("NEAR"), _distance(0) { } + explicit NearQueryNode(const char * opName) noexcept : AndQueryNode(opName), _distance(0) { } bool evaluate() const override; void distance(size_t dist) { _distance = dist; } size_t distance() const { return _distance; } @@ -169,8 +169,7 @@ private: class ONearQueryNode : public NearQueryNode { public: - ONearQueryNode() : NearQueryNode("ONEAR") { } - ~ONearQueryNode() { } + ONearQueryNode() noexcept : NearQueryNode("ONEAR") { } bool evaluate() const override; }; diff --git a/searchlib/src/vespa/searchlib/query/streaming/querynode.cpp b/searchlib/src/vespa/searchlib/query/streaming/querynode.cpp index db0fbd5b98e..6b3aa7a2fd0 100644 --- a/searchlib/src/vespa/searchlib/query/streaming/querynode.cpp +++ b/searchlib/src/vespa/searchlib/query/streaming/querynode.cpp @@ -13,12 +13,18 @@ LOG_SETUP(".vsm.querynode"); namespace search::streaming { namespace { - vespalib::stringref DEFAULT("default"); - bool disableRewrite(const QueryNode * qn) { - return dynamic_cast<const NearQueryNode *> (qn) || - dynamic_cast<const PhraseQueryNode *> (qn) || - dynamic_cast<const SameElementQueryNode *>(qn); - } + +vespalib::stringref DEFAULT("default"); +bool disableRewrite(const QueryNode * qn) { + return dynamic_cast<const NearQueryNode *> (qn) || + dynamic_cast<const PhraseQueryNode *> (qn) || + dynamic_cast<const SameElementQueryNode *>(qn); +} + +bool possibleFloat(const QueryTerm & qt, const QueryTerm::string & term) { + return !qt.encoding().isBase10Integer() && qt.encoding().isFloat() && (term.find('.') != QueryTerm::string::npos); +} + } QueryNode::UP @@ -43,8 +49,8 @@ QueryNode::Build(const QueryNode * parent, const QueryNodeResultFactory & factor { qn = QueryConnector::create(type); if (qn) { - QueryConnector * qc = dynamic_cast<QueryConnector *> (qn.get()); - NearQueryNode * nqn = dynamic_cast<NearQueryNode *> (qc); + auto * qc = dynamic_cast<QueryConnector *> (qn.get()); + auto * nqn = dynamic_cast<NearQueryNode *> (qc); if (nqn) { nqn->distance(queryRep.getNearDistance()); } @@ -150,21 +156,17 @@ QueryNode::Build(const QueryNode * parent, const QueryNodeResultFactory & factor qt->setFuzzyMaxEditDistance(queryRep.getFuzzyMaxEditDistance()); qt->setFuzzyPrefixLength(queryRep.getFuzzyPrefixLength()); } - if (qt->encoding().isBase10Integer() || - ! qt->encoding().isFloat() || - ! factory.getRewriteFloatTerms() || - ! allowRewrite || - (ssTerm.find('.') == vespalib::string::npos)) - { - qn = std::move(qt); - } else { + if (possibleFloat(*qt, ssTerm) && factory.getRewriteFloatTerms() && allowRewrite) { auto phrase = std::make_unique<PhraseQueryNode>(); - phrase->addChild(std::make_unique<QueryTerm>(factory.create(), ssTerm.substr(0, ssTerm.find('.')), ssIndex, TermType::WORD)); - phrase->addChild(std::make_unique<QueryTerm>(factory.create(), ssTerm.substr(ssTerm.find('.') + 1), ssIndex, TermType::WORD)); + auto dotPos = ssTerm.find('.'); + phrase->addChild(std::make_unique<QueryTerm>(factory.create(), ssTerm.substr(0, dotPos), ssIndex, TermType::WORD)); + phrase->addChild(std::make_unique<QueryTerm>(factory.create(), ssTerm.substr(dotPos + 1), ssIndex, TermType::WORD)); auto orqn = std::make_unique<EquivQueryNode>(); orqn->addChild(std::move(qt)); orqn->addChild(std::move(phrase)); qn = std::move(orqn); + } else { + qn = std::move(qt); } } } diff --git a/searchlib/src/vespa/searchlib/query/streaming/querynode.h b/searchlib/src/vespa/searchlib/query/streaming/querynode.h index 09c44d951d3..bfc840e4603 100644 --- a/searchlib/src/vespa/searchlib/query/streaming/querynode.h +++ b/searchlib/src/vespa/searchlib/query/streaming/querynode.h @@ -36,7 +36,7 @@ class QueryNode public: using UP = std::unique_ptr<QueryNode>; - virtual ~QueryNode() { } + virtual ~QueryNode() = default; /// This evalutes if the subtree starting here evaluates to true. virtual bool evaluate() const = 0; /// This return the hitList for this subtree. Does only give meaning in a diff --git a/searchlib/src/vespa/searchlib/query/streaming/querynoderesultbase.h b/searchlib/src/vespa/searchlib/query/streaming/querynoderesultbase.h index 62fc32a4575..10f3b7cbf21 100644 --- a/searchlib/src/vespa/searchlib/query/streaming/querynoderesultbase.h +++ b/searchlib/src/vespa/searchlib/query/streaming/querynoderesultbase.h @@ -21,7 +21,7 @@ class QueryNodeResultFactory { public: virtual ~QueryNodeResultFactory() = default; virtual bool getRewriteFloatTerms() const { return false; } - virtual std::unique_ptr<QueryNodeResultBase> create() const { return std::unique_ptr<QueryNodeResultBase>(); } + virtual std::unique_ptr<QueryNodeResultBase> create() const { return {}; } }; } diff --git a/searchlib/src/vespa/searchlib/query/streaming/queryterm.cpp b/searchlib/src/vespa/searchlib/query/streaming/queryterm.cpp index 9c45427d07d..a658ff5f3d6 100644 --- a/searchlib/src/vespa/searchlib/query/streaming/queryterm.cpp +++ b/searchlib/src/vespa/searchlib/query/streaming/queryterm.cpp @@ -15,6 +15,7 @@ private: }; CharInfo::CharInfo() + : _charInfo() { // XXX: Should refactor to reduce number of magic constants. memset(_charInfo, 0x01, 128); // All 7 bits are ascii7bit @@ -33,7 +34,7 @@ CharInfo::CharInfo() _charInfo[uint8_t('E')] = 0x05; } -static CharInfo _G_charTable; +CharInfo _G_charTable; } @@ -65,10 +66,10 @@ QueryTerm::QueryTerm(std::unique_ptr<QueryNodeResultBase> org, const string & te { if (!termS.empty()) { uint8_t enc(0xff); - for (size_t i(0), m(termS.size()); i < m; i++) { - enc &= _G_charTable.get(termS[i]); + for (char c : termS) { + enc &= _G_charTable.get(c); } - _encoding = enc; + _encoding = EncodingBitMap(enc); } } diff --git a/searchlib/src/vespa/searchlib/query/streaming/queryterm.h b/searchlib/src/vespa/searchlib/query/streaming/queryterm.h index 6e91437b1f9..2d1156a9c51 100644 --- a/searchlib/src/vespa/searchlib/query/streaming/queryterm.h +++ b/searchlib/src/vespa/searchlib/query/streaming/queryterm.h @@ -27,7 +27,7 @@ public: class EncodingBitMap { public: - EncodingBitMap(uint8_t bm=0) : _enc(bm) { } + explicit EncodingBitMap(uint8_t bm) : _enc(bm) { } bool isFloat() const { return _enc & Float; } bool isBase10Integer() const { return _enc & Base10Integer; } bool isAscii7Bit() const { return _enc & Ascii7Bit; } |