From ec7405502d66cbde25be69f1130fb0115c369bb1 Mon Sep 17 00:00:00 2001
From: Henning Baldersheim <balder@yahoo-inc.com>
Date: Sun, 18 Feb 2024 23:34:14 +0000
Subject: Deinline dump methods. They are not at all time critical.

---
 eval/src/vespa/eval/eval/aggr.cpp                  |  14 +-
 eval/src/vespa/eval/eval/tensor_nodes.cpp          | 171 +++++++++++++++++++++
 eval/src/vespa/eval/eval/tensor_nodes.h            | 159 ++-----------------
 eval/src/vespa/eval/instruction/generic_reduce.cpp |   8 +-
 4 files changed, 193 insertions(+), 159 deletions(-)

diff --git a/eval/src/vespa/eval/eval/aggr.cpp b/eval/src/vespa/eval/eval/aggr.cpp
index a43341dac30..ebb8ab4d6b1 100644
--- a/eval/src/vespa/eval/eval/aggr.cpp
+++ b/eval/src/vespa/eval/eval/aggr.cpp
@@ -12,12 +12,12 @@ namespace eval {
 namespace {
 
 template <typename T>
-struct Wrapper : Aggregator {
+struct Wrapper final : Aggregator {
     T aggr;
-    virtual void first(double value) final override { aggr = T{value}; }
-    virtual void next(double value) final override { aggr.sample(value); }
-    virtual double result() const final override { return aggr.result(); }
-    virtual Aggr enum_value() const final override { return T::enum_value(); }
+    void first(double value) override { aggr = T{value}; }
+    void next(double value) override { aggr.sample(value); }
+    double result() const override { return aggr.result(); }
+    Aggr enum_value() const override { return T::enum_value(); }
 };
 
 } // namespace vespalib::eval::<unnamed>
@@ -66,9 +66,7 @@ AggrNames::from_name(const vespalib::string &name)
     return &(result->second);
 }
 
-Aggregator::~Aggregator()
-{
-}
+Aggregator::~Aggregator() = default;
 
 Aggregator &
 Aggregator::create(Aggr aggr, Stash &stash)
diff --git a/eval/src/vespa/eval/eval/tensor_nodes.cpp b/eval/src/vespa/eval/eval/tensor_nodes.cpp
index ef2718234b2..1143f182f34 100644
--- a/eval/src/vespa/eval/eval/tensor_nodes.cpp
+++ b/eval/src/vespa/eval/eval/tensor_nodes.cpp
@@ -17,4 +17,175 @@ void TensorCreate      ::accept(NodeVisitor &visitor) const { visitor.visit(*thi
 void TensorLambda      ::accept(NodeVisitor &visitor) const { visitor.visit(*this); }
 void TensorPeek        ::accept(NodeVisitor &visitor) const { visitor.visit(*this); }
 
+vespalib::string
+TensorMap::dump(DumpContext &ctx) const {
+    vespalib::string str;
+    str += "map(";
+    str += _child->dump(ctx);
+    str += ",";
+    str += _lambda->dump_as_lambda();
+    str += ")";
+    return str;
+}
+
+vespalib::string
+TensorMapSubspaces::dump(DumpContext &ctx) const {
+    vespalib::string str;
+    str += "map_subspaces(";
+    str += _child->dump(ctx);
+    str += ",";
+    str += _lambda->dump_as_lambda();
+    str += ")";
+    return str;
+}
+
+vespalib::string
+TensorJoin::dump(DumpContext &ctx) const {
+    vespalib::string str;
+    str += "join(";
+    str += _lhs->dump(ctx);
+    str += ",";
+    str += _rhs->dump(ctx);
+    str += ",";
+    str += _lambda->dump_as_lambda();
+    str += ")";
+    return str;
+}
+
+vespalib::string
+TensorMerge::dump(DumpContext &ctx) const {
+    vespalib::string str;
+    str += "join(";
+    str += _lhs->dump(ctx);
+    str += ",";
+    str += _rhs->dump(ctx);
+    str += ",";
+    str += _lambda->dump_as_lambda();
+    str += ")";
+    return str;
+}
+
+vespalib::string
+TensorReduce::dump(DumpContext &ctx) const {
+    vespalib::string str;
+    str += "reduce(";
+    str += _child->dump(ctx);
+    str += ",";
+    str += *AggrNames::name_of(_aggr);
+    for (const auto &dimension: _dimensions) {
+        str += ",";
+        str += dimension;
+    }
+    str += ")";
+    return str;
+}
+
+vespalib::string
+TensorRename::dump(DumpContext &ctx) const  {
+    vespalib::string str;
+    str += "rename(";
+    str += _child->dump(ctx);
+    str += ",";
+    str += flatten(_from);
+    str += ",";
+    str += flatten(_to);
+    str += ")";
+    return str;
+}
+
+vespalib::string
+TensorRename::flatten(const std::vector<vespalib::string> &list) {
+    if (list.size() == 1) {
+        return list[0];
+    }
+    vespalib::string str = "(";
+    for (size_t i = 0; i < list.size(); ++i) {
+        if (i > 0) {
+            str += ",";
+        }
+        str += list[i];
+    }
+    str += ")";
+    return str;
+}
+
+vespalib::string
+TensorConcat::dump(DumpContext &ctx) const {
+    vespalib::string str;
+    str += "concat(";
+    str += _lhs->dump(ctx);
+    str += ",";
+    str += _rhs->dump(ctx);
+    str += ",";
+    str += _dimension;
+    str += ")";
+    return str;
+}
+
+vespalib::string
+TensorCellCast::dump(DumpContext &ctx) const {
+    vespalib::string str;
+    str += "cell_cast(";
+    str += _child->dump(ctx);
+    str += ",";
+    str += value_type::cell_type_to_name(_cell_type);
+    str += ")";
+    return str;
+}
+
+vespalib::string
+TensorCreate::dump(DumpContext &ctx) const {
+    vespalib::string str = _type.to_spec();
+    str += ":{";
+    CommaTracker child_list;
+    for (const Child &child: _cells) {
+        child_list.maybe_add_comma(str);
+        str += as_string(child.first);
+        str += ":";
+        str += child.second->dump(ctx);
+    }
+    str += "}";
+    return str;
+}
+
+vespalib::string
+TensorLambda::dump(DumpContext &) const {
+    vespalib::string str = _type.to_spec();
+    vespalib::string expr = _lambda->dump();
+    if (starts_with(expr, "(")) {
+        str += expr;
+    } else {
+        str += "(";
+        str += expr;
+        str += ")";
+    }
+    return str;
+}
+
+vespalib::string
+TensorPeek::dump(DumpContext &ctx) const {
+    vespalib::string str = _param->dump(ctx);
+    str += "{";
+    CommaTracker dim_list;
+    for (const auto &dim : _dim_list) {
+        dim_list.maybe_add_comma(str);
+        str += dim.first;
+        str += ":";
+        if (dim.second.is_expr()) {
+            vespalib::string expr = dim.second.expr->dump(ctx);
+            if (starts_with(expr, "(")) {
+                str += expr;
+            } else {
+                str += "(";
+                str += expr;
+                str += ")";
+            }
+        } else {
+            str += as_quoted_string(dim.second.label);
+        }
+    }
+    str += "}";
+    return str;
+}
+
 }
diff --git a/eval/src/vespa/eval/eval/tensor_nodes.h b/eval/src/vespa/eval/eval/tensor_nodes.h
index 6ed19e81712..5de2e4bc80d 100644
--- a/eval/src/vespa/eval/eval/tensor_nodes.h
+++ b/eval/src/vespa/eval/eval/tensor_nodes.h
@@ -23,15 +23,7 @@ public:
         : _child(std::move(child)), _lambda(std::move(lambda)) {}
     const Node &child() const { return *_child; }
     const Function &lambda() const { return *_lambda; }
-    vespalib::string dump(DumpContext &ctx) const override {
-        vespalib::string str;
-        str += "map(";
-        str += _child->dump(ctx);
-        str += ",";
-        str += _lambda->dump_as_lambda();
-        str += ")";
-        return str;
-    }
+    vespalib::string dump(DumpContext &ctx) const override;
     void accept(NodeVisitor &visitor) const override;
     size_t num_children() const override { return 1; }
     const Node &get_child(size_t idx) const override {
@@ -53,15 +45,7 @@ public:
         : _child(std::move(child)), _lambda(std::move(lambda)) {}
     const Node &child() const { return *_child; }
     const Function &lambda() const { return *_lambda; }
-    vespalib::string dump(DumpContext &ctx) const override {
-        vespalib::string str;
-        str += "map_subspaces(";
-        str += _child->dump(ctx);
-        str += ",";
-        str += _lambda->dump_as_lambda();
-        str += ")";
-        return str;
-    }
+    vespalib::string dump(DumpContext &ctx) const override;
     void accept(NodeVisitor &visitor) const override;
     size_t num_children() const override { return 1; }
     const Node &get_child(size_t idx) const override {
@@ -85,17 +69,7 @@ public:
     const Node &lhs() const { return *_lhs; }
     const Node &rhs() const { return *_rhs; }
     const Function &lambda() const { return *_lambda; }
-    vespalib::string dump(DumpContext &ctx) const override {
-        vespalib::string str;
-        str += "join(";
-        str += _lhs->dump(ctx);
-        str += ",";
-        str += _rhs->dump(ctx);
-        str += ",";
-        str += _lambda->dump_as_lambda();
-        str += ")";
-        return str;
-    }
+    vespalib::string dump(DumpContext &ctx) const override;
     void accept(NodeVisitor &visitor) const override;
     size_t num_children() const override { return 2; }
     const Node &get_child(size_t idx) const override {
@@ -119,17 +93,7 @@ public:
     const Node &lhs() const { return *_lhs; }
     const Node &rhs() const { return *_rhs; }
     const Function &lambda() const { return *_lambda; }
-    vespalib::string dump(DumpContext &ctx) const override {
-        vespalib::string str;
-        str += "join(";
-        str += _lhs->dump(ctx);
-        str += ",";
-        str += _rhs->dump(ctx);
-        str += ",";
-        str += _lambda->dump_as_lambda();
-        str += ")";
-        return str;
-    }
+    vespalib::string dump(DumpContext &ctx) const override;
     void accept(NodeVisitor &visitor) const override;
     size_t num_children() const override { return 2; }
     const Node &get_child(size_t idx) const override {
@@ -153,19 +117,7 @@ public:
     const Node &child() const { return *_child; }
     Aggr aggr() const { return _aggr; }
     const std::vector<vespalib::string> &dimensions() const { return _dimensions; }
-    vespalib::string dump(DumpContext &ctx) const override {
-        vespalib::string str;
-        str += "reduce(";
-        str += _child->dump(ctx);
-        str += ",";
-        str += *AggrNames::name_of(_aggr);
-        for (const auto &dimension: _dimensions) {
-            str += ",";
-            str += dimension;
-        }
-        str += ")";
-        return str;
-    }
+    vespalib::string dump(DumpContext &ctx) const override;
     void accept(NodeVisitor &visitor) const override;
     size_t num_children() const override { return 1; }
     const Node &get_child(size_t idx) const override {
@@ -188,17 +140,7 @@ public:
     const Node &child() const { return *_child; }
     const std::vector<vespalib::string> &from() const { return _from; }
     const std::vector<vespalib::string> &to() const { return _to; }
-    vespalib::string dump(DumpContext &ctx) const override {
-        vespalib::string str;
-        str += "rename(";
-        str += _child->dump(ctx);
-        str += ",";
-        str += flatten(_from);
-        str += ",";
-        str += flatten(_to);
-        str += ")";
-        return str;
-    }
+    vespalib::string dump(DumpContext &ctx) const override;
     void accept(NodeVisitor &visitor) const override;
     size_t num_children() const override { return 1; }
     const Node &get_child(size_t idx) const override {
@@ -208,20 +150,7 @@ public:
     void detach_children(NodeHandler &handler) override {
         handler.handle(std::move(_child));
     }
-    static vespalib::string flatten(const std::vector<vespalib::string> &list) {
-        if (list.size() == 1) {
-            return list[0];
-        }
-        vespalib::string str = "(";
-        for (size_t i = 0; i < list.size(); ++i) {
-            if (i > 0) {
-                str += ",";
-            }
-            str += list[i];
-        }
-        str += ")";
-        return str;
-    }
+    static vespalib::string flatten(const std::vector<vespalib::string> &list);
 };
 
 class TensorConcat : public Node {
@@ -235,17 +164,7 @@ public:
     const Node &lhs() const { return *_lhs; }
     const Node &rhs() const { return *_rhs; }
     const vespalib::string &dimension() const { return _dimension; }
-    vespalib::string dump(DumpContext &ctx) const override {
-        vespalib::string str;
-        str += "concat(";
-        str += _lhs->dump(ctx);
-        str += ",";
-        str += _rhs->dump(ctx);
-        str += ",";
-        str += _dimension;
-        str += ")";
-        return str;
-    }
+    vespalib::string dump(DumpContext &ctx) const override;
     void accept(NodeVisitor &visitor) const override;
     size_t num_children() const override { return 2; }
     const Node &get_child(size_t idx) const override {
@@ -267,15 +186,7 @@ public:
         : _child(std::move(child)), _cell_type(cell_type) {}
     const Node &child() const { return *_child; }
     CellType cell_type() const { return _cell_type; }
-    vespalib::string dump(DumpContext &ctx) const override {
-        vespalib::string str;
-        str += "cell_cast(";
-        str += _child->dump(ctx);
-        str += ",";
-        str += value_type::cell_type_to_name(_cell_type);
-        str += ")";
-        return str;
-    }
+    vespalib::string dump(DumpContext &ctx) const override;
     void accept(NodeVisitor &visitor) const override;
     size_t num_children() const override { return 1; }
     const Node &get_child(size_t idx) const override {
@@ -305,19 +216,7 @@ public:
         }
     }
     const ValueType &type() const { return _type; }
-    vespalib::string dump(DumpContext &ctx) const override {
-        vespalib::string str = _type.to_spec();
-        str += ":{";
-        CommaTracker child_list;
-        for (const Child &child: _cells) {
-            child_list.maybe_add_comma(str);
-            str += as_string(child.first);
-            str += ":";
-            str += child.second->dump(ctx);
-        }
-        str += "}";
-        return str;
-    }
+    vespalib::string dump(DumpContext &ctx) const override;
     void accept(NodeVisitor &visitor) const override;
     size_t num_children() const override { return _cells.size(); }
     const Node &get_child(size_t idx) const override {
@@ -350,18 +249,7 @@ public:
     const ValueType &type() const { return _type; }
     const std::vector<size_t> &bindings() const { return _bindings; }
     const Function &lambda() const { return *_lambda; }
-    vespalib::string dump(DumpContext &) const override {
-        vespalib::string str = _type.to_spec();
-        vespalib::string expr = _lambda->dump();
-        if (starts_with(expr, "(")) {
-            str += expr;
-        } else {
-            str += "(";
-            str += expr;
-            str += ")";
-        }
-        return str;
-    }
+    vespalib::string dump(DumpContext &) const override;
     void accept(NodeVisitor &visitor) const override;
     size_t num_children() const override { return 0; }
     const Node &get_child(size_t) const override { abort(); }
@@ -400,30 +288,7 @@ public:
     }
     const Node &param() const { return *_param; }
     const DimList &dim_list() const { return _dim_list; }
-    vespalib::string dump(DumpContext &ctx) const override {
-        vespalib::string str = _param->dump(ctx);
-        str += "{";
-        CommaTracker dim_list;
-        for (const auto &dim: _dim_list) {
-            dim_list.maybe_add_comma(str);
-            str += dim.first;
-            str += ":";
-            if (dim.second.is_expr()) {
-                vespalib::string expr = dim.second.expr->dump(ctx);
-                if (starts_with(expr, "(")) {
-                    str += expr;
-                } else {
-                    str += "(";
-                    str += expr;
-                    str += ")";
-                }
-            } else {
-                str += as_quoted_string(dim.second.label);
-            }
-        }
-        str += "}";
-        return str;
-    }
+    vespalib::string dump(DumpContext &ctx) const override;
     void accept(NodeVisitor &visitor) const override;
     size_t num_children() const override { return (1 + _expr_dims.size()); }
     const Node &get_child(size_t idx) const override {
diff --git a/eval/src/vespa/eval/instruction/generic_reduce.cpp b/eval/src/vespa/eval/instruction/generic_reduce.cpp
index 6d845557496..dac8a5ceece 100644
--- a/eval/src/vespa/eval/instruction/generic_reduce.cpp
+++ b/eval/src/vespa/eval/instruction/generic_reduce.cpp
@@ -8,7 +8,6 @@
 #include <vespa/vespalib/util/typify.h>
 #include <vespa/vespalib/util/overload.h>
 #include <vespa/vespalib/util/visit_ranges.h>
-#include <algorithm>
 #include <cassert>
 #include <array>
 
@@ -51,7 +50,7 @@ struct SparseReduceState {
     SmallVector<string_id*> keep_address;
     size_t                  subspace;
 
-    SparseReduceState(const SparseReducePlan &plan)
+    explicit SparseReduceState(const SparseReducePlan &plan)
         : full_address(plan.keep_dims.size() + plan.num_reduce_dims),
           fetch_address(full_address.size(), nullptr),
           keep_address(plan.keep_dims.size(), nullptr),
@@ -97,7 +96,8 @@ generic_reduce(const Value &value, const ReduceParam &param) {
         auto zero = builder->add_subspace();
         std::fill(zero.begin(), zero.end(), OCT{});
     }
-    return builder->build(std::move(builder));
+    auto & builder_ref = *builder;
+    return builder_ref.build(std::move(builder));
 }
 
 template <typename ICT, typename OCT, typename AGGR>
@@ -311,7 +311,7 @@ GenericReduce::make_instruction(const ValueType &result_type,
     assert(result_type == param.res_type);
     assert(result_type.cell_meta().eq(input_type.cell_meta().reduce(result_type.is_double())));
     auto fun = typify_invoke<3,ReduceTypify,SelectGenericReduceOp>(input_type.cell_meta(), result_type.cell_meta().is_scalar, aggr, param);
-    return Instruction(fun, wrap_param<ReduceParam>(param));
+    return {fun, wrap_param<ReduceParam>(param)};
 }
 
 } // namespace
-- 
cgit v1.2.3