clp-s: Improve performance by removing unnecessary allocations of the…

… expression state object during search scans. (#364)
y-scope · May 1, 2024 · 64fa68b · 64fa68b
1 parent 1e0dfb0
commit 64fa68b
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 37 deletions.
diff --git a/components/core/src/clp_s/search/Output.cpp b/components/core/src/clp_s/search/Output.cpp
@@ -188,12 +188,6 @@ bool Output::filter(
     return true;
 }
 
-enum CurExpr {
-    AND,
-    OR,
-    FILTER
-};
-
 bool Output::evaluate(
         Expression* expr,
         int32_t schema,
@@ -203,47 +197,40 @@ bool Output::evaluate(
         return true;
     }
 
-    std::stack<CurExpr, std::vector<CurExpr>> parent_type;
-    std::stack<OpList::iterator, std::vector<OpList::iterator>> parent_it;
-
     Expression* cur = expr;
-    CurExpr cur_type = CurExpr::FILTER;
+    ExpressionType cur_type = ExpressionType::Filter;
     bool ret = false;
 
     if (dynamic_cast<AndExpr*>(cur)) {
-        cur_type = CurExpr::AND;
-        parent_type.push(CurExpr::AND);
-        parent_it.push(cur->op_begin());
+        cur_type = ExpressionType::And;
+        m_expression_state.emplace(cur_type, cur->op_begin());
         ret = true;
     } else if (dynamic_cast<OrExpr*>(cur)) {
-        cur_type = CurExpr::OR;
-        parent_type.push(CurExpr::OR);
-        parent_it.push(cur->op_begin());
+        cur_type = ExpressionType::Or;
+        m_expression_state.emplace(cur_type, cur->op_begin());
         ret = false;
     }
 
     do {
         switch (cur_type) {
-            case CurExpr::AND:
-                if (false == ret || parent_it.top() == cur->op_end()) {
-                    parent_type.pop();
-                    parent_it.pop();
+            case ExpressionType::And:
+                if (false == ret || m_expression_state.top().second == cur->op_end()) {
+                    m_expression_state.pop();
                     break;
                 } else {
-                    cur = static_cast<Expression*>((parent_it.top()++)->get());
+                    cur = static_cast<Expression*>((m_expression_state.top().second++)->get());
                     if (dynamic_cast<FilterExpr*>(cur)) {
-                        cur_type = CurExpr::FILTER;
+                        cur_type = ExpressionType::Filter;
                     } else {
                         // must be an OR-expr because AST would have been simplified
                         // to eliminate nested AND
-                        cur_type = CurExpr::OR;
-                        parent_type.push(CurExpr::OR);
-                        parent_it.push(cur->op_begin());
+                        cur_type = ExpressionType::Or;
+                        m_expression_state.emplace(cur_type, cur->op_begin());
                         ret = false;
                     }
                     continue;
                 }
-            case CurExpr::FILTER:
+            case ExpressionType::Filter:
                 if (static_cast<FilterExpr*>(cur)->get_column()->is_pure_wildcard()) {
                     ret = evaluate_wildcard_filter(
                             static_cast<FilterExpr*>(cur),
@@ -254,30 +241,28 @@ bool Output::evaluate(
                     ret = evaluate_filter(static_cast<FilterExpr*>(cur), schema, extracted_values);
                 }
                 break;
-            case CurExpr::OR:
-                if (ret || parent_it.top() == cur->op_end()) {
-                    parent_type.pop();
-                    parent_it.pop();
+            case ExpressionType::Or:
+                if (ret || m_expression_state.top().second == cur->op_end()) {
+                    m_expression_state.pop();
                     break;
                 } else {
-                    cur = static_cast<Expression*>((parent_it.top()++)->get());
+                    cur = static_cast<Expression*>((m_expression_state.top().second++)->get());
                     if (dynamic_cast<FilterExpr*>(cur)) {
-                        cur_type = CurExpr::FILTER;
+                        cur_type = ExpressionType::Filter;
                     } else {
                         // must be an AND-expr because AST would have been simplified
                         // to eliminate nested OR
-                        cur_type = CurExpr::AND;
-                        parent_type.push(CurExpr::AND);
-                        parent_it.push(cur->op_begin());
+                        cur_type = ExpressionType::And;
+                        m_expression_state.emplace(cur_type, cur->op_begin());
                         ret = true;
                     }
                     continue;
                 }
         }
 
         ret = cur->is_inverted() ? !ret : ret;
-        if (false == parent_type.empty()) {
-            cur_type = parent_type.top();
+        if (false == m_expression_state.empty()) {
+            cur_type = m_expression_state.top().first;
         }
         cur = cur->get_parent();
     } while (cur != nullptr);

diff --git a/components/core/src/clp_s/search/Output.hpp b/components/core/src/clp_s/search/Output.hpp
@@ -3,6 +3,7 @@
 
 #include <map>
 #include <set>
+#include <stack>
 #include <string>
 #include <unordered_set>
 #include <utility>
@@ -48,6 +49,12 @@ class Output : public FilterClass {
     bool filter();
 
 private:
+    enum class ExpressionType {
+        And,
+        Or,
+        Filter
+    };
+
     std::shared_ptr<ArchiveReader> m_archive_reader;
     std::shared_ptr<Expression> m_expr;
     SchemaMatch& m_match;
@@ -86,6 +93,11 @@ class Output : public FilterClass {
     std::map<ColumnDescriptor*, std::vector<int32_t>> m_wildcard_to_searched_datestrings;
     std::map<ColumnDescriptor*, std::vector<int32_t>> m_wildcard_to_searched_columns;
 
+    std::stack<
+            std::pair<ExpressionType, OpList::iterator>,
+            std::vector<std::pair<ExpressionType, OpList::iterator>>>
+            m_expression_state;
+
     simdjson::ondemand::parser m_array_parser;
     std::string m_array_search_string;
     bool m_maybe_string, m_maybe_number;