- 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.

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<ProtonLocationTerm *>
+std::vector<LocationTerm *>
 find_location_terms(Node *tree) {
-    std::vector<ProtonLocationTerm *> retval;
+    std::vector<LocationTerm *> retval;
     std::vector<Node *> nodes;
     nodes.push_back(tree);
-    for (size_t i = 0; i < nodes.size(); ++i) {
-        if (auto loc = dynamic_cast<ProtonLocationTerm *>(nodes[i])) {
-            retval.push_back(loc);
+    for (Node * node : nodes) {
+        if (node->isLocationTerm()) {
+            retval.push_back(static_cast<LocationTerm *>(node));
         }
-        if (auto parent = dynamic_cast<const search::query::Intermediate *>(nodes[i])) {
+        if (node->isIntermediate()) {
+            auto parent = static_cast<const search::query::Intermediate *>(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);
@@ -101,6 +103,10 @@ GeoLocationSpec process_location_term(ProtonLocationTerm &pterm) {
 }
 
 void exchange_location_nodes(const string &location_str,
+                             Node::UP &query_tree,
+                             std::vector<GeoLocationSpec> &fef_locations) __attribute__((noinline));
+
+void exchange_location_nodes(const string &location_str,
                            Node::UP &query_tree,
                            std::vector<GeoLocationSpec> &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 <typename Base>
-struct ProtonTerm : public ProtonTermBase<Base> {
+struct ProtonTerm final : public ProtonTermBase<Base> {
     using ProtonTermBase<Base>::ProtonTermBase;
     ~ProtonTerm();
 };
@@ -116,8 +116,7 @@ typedef search::query::SimpleWeakAnd     ProtonWeakAnd;
 typedef search::query::SimpleSameElement ProtonSameElement;
 
 
-struct ProtonEquiv final : public ProtonTermBase<search::query::Equiv>
-{
+struct ProtonEquiv final : public ProtonTermBase<search::query::Equiv> {
     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<Node *> &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<Node> 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;
 };