From c18ef1f907c6212722027726e389c61f88834409 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Thu, 25 Mar 2021 09:20:38 +0000 Subject: - Avoid inlining exchange_location_nodes to be able to see its execution cost with. - Add isIntermediate, and isLocation virtual methods to avoid very expensive dynamic_cast. - Use search::query::LocationTerm, instead of ProtonLocationTerm when possible. --- .../src/vespa/searchcore/proton/matching/query.cpp | 22 ++++++++++++++-------- .../vespa/searchcore/proton/matching/querynodes.h | 5 ++--- .../src/vespa/searchlib/query/tree/intermediate.h | 1 + searchlib/src/vespa/searchlib/query/tree/node.h | 5 +++-- .../src/vespa/searchlib/query/tree/termnodes.h | 1 + 5 files changed, 21 insertions(+), 13 deletions(-) diff --git a/searchcore/src/vespa/searchcore/proton/matching/query.cpp b/searchcore/src/vespa/searchcore/proton/matching/query.cpp index 994e26081e7..39936126a99 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/query.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/query.cpp @@ -36,6 +36,7 @@ using search::queryeval::IntermediateBlueprint; using search::queryeval::Blueprint; using search::queryeval::IRequestContext; using search::queryeval::SearchIterator; +using search::query::LocationTerm; using vespalib::string; using std::vector; @@ -59,16 +60,17 @@ inject(Node::UP query, Node::UP to_inject) { return query; } -std::vector +std::vector find_location_terms(Node *tree) { - std::vector retval; + std::vector retval; std::vector nodes; nodes.push_back(tree); - for (size_t i = 0; i < nodes.size(); ++i) { - if (auto loc = dynamic_cast(nodes[i])) { - retval.push_back(loc); + for (Node * node : nodes) { + if (node->isLocationTerm()) { + retval.push_back(static_cast(node)); } - if (auto parent = dynamic_cast(nodes[i])) { + if (node->isIntermediate()) { + auto parent = static_cast(node); for (Node * child : parent->getChildren()) { nodes.push_back(child); } @@ -92,7 +94,7 @@ GeoLocationSpec parse_location_string(string str) { return empty; } -GeoLocationSpec process_location_term(ProtonLocationTerm &pterm) { +GeoLocationSpec process_location_term(LocationTerm &pterm) { auto old_view = pterm.getView(); auto new_view = PositionDataType::getZCurveFieldName(old_view); pterm.setView(new_view); @@ -100,6 +102,10 @@ GeoLocationSpec process_location_term(ProtonLocationTerm &pterm) { return GeoLocationSpec{new_view, loc}; } +void exchange_location_nodes(const string &location_str, + Node::UP &query_tree, + std::vector &fef_locations) __attribute__((noinline)); + void exchange_location_nodes(const string &location_str, Node::UP &query_tree, std::vector &fef_locations) @@ -110,7 +116,7 @@ void exchange_location_nodes(const string &location_str, if (parsed.location.valid()) { locationSpecs.push_back(parsed); } - for (ProtonLocationTerm * pterm : find_location_terms(query_tree.get())) { + for (LocationTerm * pterm : find_location_terms(query_tree.get())) { auto spec = process_location_term(*pterm); if (spec.location.valid()) { locationSpecs.push_back(spec); diff --git a/searchcore/src/vespa/searchcore/proton/matching/querynodes.h b/searchcore/src/vespa/searchcore/proton/matching/querynodes.h index 6cb608c8f8c..65a236e4e6e 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/querynodes.h +++ b/searchcore/src/vespa/searchcore/proton/matching/querynodes.h @@ -98,7 +98,7 @@ struct ProtonTermBase : public Base, }; template -struct ProtonTerm : public ProtonTermBase { +struct ProtonTerm final : public ProtonTermBase { using ProtonTermBase::ProtonTermBase; ~ProtonTerm(); }; @@ -116,8 +116,7 @@ typedef search::query::SimpleWeakAnd ProtonWeakAnd; typedef search::query::SimpleSameElement ProtonSameElement; -struct ProtonEquiv final : public ProtonTermBase -{ +struct ProtonEquiv final : public ProtonTermBase { search::fef::MatchDataLayout children_mdl; using ProtonTermBase::ProtonTermBase; }; diff --git a/searchlib/src/vespa/searchlib/query/tree/intermediate.h b/searchlib/src/vespa/searchlib/query/tree/intermediate.h index 2f4323f8e87..2bdc0104927 100644 --- a/searchlib/src/vespa/searchlib/query/tree/intermediate.h +++ b/searchlib/src/vespa/searchlib/query/tree/intermediate.h @@ -17,6 +17,7 @@ class Intermediate : public Node Intermediate() = default; virtual ~Intermediate() = 0; + bool isIntermediate() const override { return true; } const std::vector &getChildren() const { return _children; } Intermediate &reserve(size_t sz) { _children.reserve(sz); return *this; } diff --git a/searchlib/src/vespa/searchlib/query/tree/node.h b/searchlib/src/vespa/searchlib/query/tree/node.h index 4ef0d3b6fc8..2ad6237a1fd 100644 --- a/searchlib/src/vespa/searchlib/query/tree/node.h +++ b/searchlib/src/vespa/searchlib/query/tree/node.h @@ -15,9 +15,10 @@ class Node { public: typedef std::unique_ptr UP; - virtual ~Node() {} - + virtual ~Node() = default; virtual void accept(QueryVisitor &visitor) = 0; + virtual bool isIntermediate() const { return false; } + virtual bool isLocationTerm() const { return false; } }; } diff --git a/searchlib/src/vespa/searchlib/query/tree/termnodes.h b/searchlib/src/vespa/searchlib/query/tree/termnodes.h index e112fd6e295..3eda0732470 100644 --- a/searchlib/src/vespa/searchlib/query/tree/termnodes.h +++ b/searchlib/src/vespa/searchlib/query/tree/termnodes.h @@ -86,6 +86,7 @@ public: int32_t id, Weight weight) : QueryNodeMixinType(term, view, id, weight) {} + bool isLocationTerm() const override { return true; } virtual ~LocationTerm() = 0; }; -- cgit v1.2.3