Micro optimize DenseTensorCellsIterator::next by expecting most probable branch and use 32 bits for sizes instead of 64 bits

14 files changed, 46 insertions, 52 deletions

diff --git a/eval/src/tests/tensor/dense_dot_product_function/dense_dot_product_function_test.cpp b/eval/src/tests/tensor/dense_dot_product_function/dense_dot_product_function_test.cpp
index ca77997bac7..0b8b98fc617 100644
--- a/eval/src/tests/tensor/dense_dot_product_function/dense_dot_product_function_test.cpp
+++ b/eval/src/tests/tensor/dense_dot_product_function/dense_dot_product_function_test.cpp
@@ -1,8 +1,5 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
-#include <vespa/log/log.h>
-LOG_SETUP("dense_dot_product_function_test");
-
 #include <vespa/vespalib/testkit/test_kit.h>
 #include <vespa/eval/eval/tensor_function.h>
 #include <vespa/eval/tensor/dense/dense_dot_product_function.h>
@@ -12,16 +9,13 @@ LOG_SETUP("dense_dot_product_function_test");
 #include <vespa/vespalib/util/stringfmt.h>
 #include <vespa/vespalib/util/stash.h>
 
+#include <vespa/log/log.h>
+LOG_SETUP("dense_dot_product_function_test");
+
 using namespace vespalib;
 using namespace vespalib::eval;
 using namespace vespalib::tensor;
 
-ValueType
-makeType(size_t numCells)
-{
-    return ValueType::tensor_type({{"x", numCells}});
-}
-
 tensor::Tensor::UP
 makeTensor(size_t numCells, double cellBias)
 {
diff --git a/eval/src/tests/tensor/dense_tensor_builder/dense_tensor_builder_test.cpp b/eval/src/tests/tensor/dense_tensor_builder/dense_tensor_builder_test.cpp
index 6f3cdd5f93f..61efdbe6d22 100644
--- a/eval/src/tests/tensor/dense_tensor_builder/dense_tensor_builder_test.cpp
+++ b/eval/src/tests/tensor/dense_tensor_builder/dense_tensor_builder_test.cpp
@@ -147,7 +147,7 @@ TEST_F("require that builder can be re-used", Fixture)
 }
 
 void
-assertTensorCell(const std::vector<size_t> &expAddress,
+assertTensorCell(const DenseTensor::Address &expAddress,
                  double expCell,
                  const DenseTensor::CellsIterator &itr)
 {
diff --git a/eval/src/vespa/eval/eval/simple_tensor.cpp b/eval/src/vespa/eval/eval/simple_tensor.cpp
index 0e58d292334..1836f2088f3 100644
--- a/eval/src/vespa/eval/eval/simple_tensor.cpp
+++ b/eval/src/vespa/eval/eval/simple_tensor.cpp
@@ -57,14 +57,14 @@ Address select(const Address &a, const Address &b, const IndexList &selector) {
     return result;
 }
 
-size_t get_dimension_size(const ValueType &type, size_t dim_idx) {
+size_t get_dimension_size(const ValueType &type, ValueType::Dimension::size_type dim_idx) {
     if (dim_idx == ValueType::Dimension::npos) {
         return 1;
     }
     return type.dimensions()[dim_idx].size;
 }
 
-size_t get_dimension_index(const Address &addr, size_t dim_idx) {
+size_t get_dimension_index(const Address &addr, ValueType::Dimension::size_type dim_idx) {
     if (dim_idx == ValueType::Dimension::npos) {
         return 0;
     }
diff --git a/eval/src/vespa/eval/eval/value_type.cpp b/eval/src/vespa/eval/eval/value_type.cpp
index 03e6d2bbcdf..1c4973a78ca 100644
--- a/eval/src/vespa/eval/eval/value_type.cpp
+++ b/eval/src/vespa/eval/eval/value_type.cpp
@@ -101,9 +101,9 @@ struct Renamer {
 
 } // namespace vespalib::tensor::<unnamed>
 
-constexpr size_t ValueType::Dimension::npos;
+constexpr ValueType::Dimension::size_type ValueType::Dimension::npos;
 
-ValueType::~ValueType() { }
+ValueType::~ValueType() = default;
 bool
 ValueType::is_sparse() const
 {
diff --git a/eval/src/vespa/eval/eval/value_type.h b/eval/src/vespa/eval/eval/value_type.h
index e304f51436f..a4762acd4c0 100644
--- a/eval/src/vespa/eval/eval/value_type.h
+++ b/eval/src/vespa/eval/eval/value_type.h
@@ -18,12 +18,13 @@ class ValueType
 public:
     enum class Type { ANY, ERROR, DOUBLE, TENSOR };
     struct Dimension {
-        static constexpr size_t npos = -1;
+        using size_type = uint32_t;
+        static constexpr size_type npos = -1;
         vespalib::string name;
-        size_t size;
+        size_type size;
         Dimension(const vespalib::string &name_in)
             : name(name_in), size(npos) {}
-        Dimension(const vespalib::string &name_in, size_t size_in)
+        Dimension(const vespalib::string &name_in, size_type size_in)
             : name(name_in), size(size_in) {}
         bool operator==(const Dimension &rhs) const {
             return ((name == rhs.name) && (size == rhs.size));
diff --git a/eval/src/vespa/eval/tensor/dense/dense_tensor.h b/eval/src/vespa/eval/tensor/dense/dense_tensor.h
index 0fa9062d223..c45d3c7ccb6 100644
--- a/eval/src/vespa/eval/tensor/dense/dense_tensor.h
+++ b/eval/src/vespa/eval/tensor/dense/dense_tensor.h
@@ -28,12 +28,9 @@ private:
 public:
     DenseTensor();
     ~DenseTensor() {}
-    DenseTensor(const eval::ValueType &type_in,
-                const Cells &cells_in);
-    DenseTensor(const eval::ValueType &type_in,
-                Cells &&cells_in);
-    DenseTensor(eval::ValueType &&type_in,
-                Cells &&cells_in);
+    DenseTensor(const eval::ValueType &type_in, const Cells &cells_in);
+    DenseTensor(const eval::ValueType &type_in, Cells &&cells_in);
+    DenseTensor(eval::ValueType &&type_in, Cells &&cells_in);
     bool operator==(const DenseTensor &rhs) const;
     const Cells &cells() const { return _cells; }
 
diff --git a/eval/src/vespa/eval/tensor/dense/dense_tensor_address_combiner.h b/eval/src/vespa/eval/tensor/dense/dense_tensor_address_combiner.h
index 094d642513c..dd1fc6572a9 100644
--- a/eval/src/vespa/eval/tensor/dense/dense_tensor_address_combiner.h
+++ b/eval/src/vespa/eval/tensor/dense/dense_tensor_address_combiner.h
@@ -18,7 +18,7 @@ namespace vespalib::tensor {
 class DenseTensorAddressCombiner
 {
 public:
-    using Address = std::vector<size_t>;
+    using Address = DenseTensorCellsIterator::Address;
 
 private:
     enum class AddressOp {
@@ -33,16 +33,13 @@ private:
     Address _combinedAddress;
 
 public:
-    DenseTensorAddressCombiner(const eval::ValueType &lhs,
-                               const eval::ValueType &rhs);
+    DenseTensorAddressCombiner(const eval::ValueType &lhs, const eval::ValueType &rhs);
     ~DenseTensorAddressCombiner();
 
-    bool combine(const CellsIterator &lhsItr,
-                 const CellsIterator &rhsItr);
+    bool combine(const CellsIterator &lhsItr, const CellsIterator &rhsItr);
     const Address &address() const { return _combinedAddress; }
 
     static eval::ValueType combineDimensions(const eval::ValueType &lhs, const eval::ValueType &rhs);
-
 };
 
 }
diff --git a/eval/src/vespa/eval/tensor/dense/dense_tensor_builder.cpp b/eval/src/vespa/eval/tensor/dense/dense_tensor_builder.cpp
index 0b66dd51206..5d52e5f6e0e 100644
--- a/eval/src/vespa/eval/tensor/dense/dense_tensor_builder.cpp
+++ b/eval/src/vespa/eval/tensor/dense/dense_tensor_builder.cpp
@@ -83,7 +83,7 @@ DenseTensorBuilder::calculateCellAddress()
         const auto &dim = _dimensions[i];
         if (label == UNDEFINED_LABEL) {
             throw IllegalArgumentException(make_string("Label for dimension '%s' is undefined. "
-                    "Expected a value in the range [0, %zu>",
+                    "Expected a value in the range [0, %u>",
                     dim.name.c_str(), dim.size));
         }
         result += (label * multiplier);
diff --git a/eval/src/vespa/eval/tensor/dense/dense_tensor_cells_iterator.cpp b/eval/src/vespa/eval/tensor/dense/dense_tensor_cells_iterator.cpp
index eaf80f95c83..320366234d8 100644
--- a/eval/src/vespa/eval/tensor/dense/dense_tensor_cells_iterator.cpp
+++ b/eval/src/vespa/eval/tensor/dense/dense_tensor_cells_iterator.cpp
@@ -4,19 +4,25 @@
 
 namespace vespalib::tensor {
 
+DenseTensorCellsIterator::DenseTensorCellsIterator(const eval::ValueType &type_in, CellsRef cells)
+        : _type(type_in),
+          _cells(cells),
+          _cellIdx(0),
+          _address(type_in.dimensions().size(), 0)
+{}
+DenseTensorCellsIterator::~DenseTensorCellsIterator() = default;
+
 void
 DenseTensorCellsIterator::next()
 {
     ++_cellIdx;
-    if (valid()) {
-        for (int64_t i = (_address.size() - 1); i >= 0; --i) {
-            _address[i]++;
-            if (_address[i] != _type.dimensions()[i].size) {
-                // Outer dimension labels can only be increased when this label wraps around.
-                break;
-            } else {
-                _address[i] = 0;
-            }
+    for (int64_t i = (_address.size() - 1); i >= 0; --i) {
+        _address[i]++;
+        if (__builtin_expect((_address[i] != _type.dimensions()[i].size), true)) {
+            // Outer dimension labels can only be increased when this label wraps around.
+            break;
+        } else {
+            _address[i] = 0;
         }
     }
 }
diff --git a/eval/src/vespa/eval/tensor/dense/dense_tensor_cells_iterator.h b/eval/src/vespa/eval/tensor/dense/dense_tensor_cells_iterator.h
index beef1b9e5f2..094e7459405 100644
--- a/eval/src/vespa/eval/tensor/dense/dense_tensor_cells_iterator.h
+++ b/eval/src/vespa/eval/tensor/dense/dense_tensor_cells_iterator.h
@@ -15,24 +15,22 @@ namespace vespalib::tensor {
  */
 class DenseTensorCellsIterator
 {
+public:
+    using size_type = eval::ValueType::Dimension::size_type;
+    using Address = std::vector<size_type>;
 private:
     using CellsRef = vespalib::ConstArrayRef<double>;
     const eval::ValueType &_type;
     CellsRef _cells;
     size_t   _cellIdx;
-    std::vector<size_t> _address;
-
+    Address  _address;
 public:
-    DenseTensorCellsIterator(const eval::ValueType &type_in, CellsRef cells)
-        : _type(type_in),
-          _cells(cells),
-          _cellIdx(0),
-          _address(type_in.dimensions().size(), 0)
-    {}
-    bool valid() const { return _cellIdx < _cells.size(); }
+    DenseTensorCellsIterator(const eval::ValueType &type_in, CellsRef cells);
+    ~DenseTensorCellsIterator();
     void next();
+    bool valid() const { return _cellIdx < _cells.size(); }
     double cell() const { return _cells[_cellIdx]; }
-    const std::vector<size_t> &address() const { return _address; }
+    const Address &address() const { return _address; }
     const eval::ValueType &fast_type() const { return _type; }
 };
 
diff --git a/eval/src/vespa/eval/tensor/dense/dense_tensor_view.cpp b/eval/src/vespa/eval/tensor/dense/dense_tensor_view.cpp
index feec93713e4..74c8981168d 100644
--- a/eval/src/vespa/eval/tensor/dense/dense_tensor_view.cpp
+++ b/eval/src/vespa/eval/tensor/dense/dense_tensor_view.cpp
@@ -227,7 +227,7 @@ DenseTensorView::accept(TensorVisitor &visitor) const
         addressBuilder.clear();
         auto rawIndex = iterator.address().begin();
         for (const auto &dimension : _typeRef.dimensions()) {
-            label = vespalib::make_string("%zu", *rawIndex);
+            label = vespalib::make_string("%u", *rawIndex);
             addressBuilder.add(dimension.name, label);
             ++rawIndex;
         }
diff --git a/eval/src/vespa/eval/tensor/dense/dense_tensor_view.h b/eval/src/vespa/eval/tensor/dense/dense_tensor_view.h
index 071df1dde24..fd95c8555f4 100644
--- a/eval/src/vespa/eval/tensor/dense/dense_tensor_view.h
+++ b/eval/src/vespa/eval/tensor/dense/dense_tensor_view.h
@@ -21,6 +21,7 @@ public:
     using Cells = std::vector<double>;
     using CellsRef = ConstArrayRef<double>;
     using CellsIterator = DenseTensorCellsIterator;
+    using Address = std::vector<eval::ValueType::Dimension::size_type>;
 
 private:
     const eval::ValueType &_typeRef;
diff --git a/eval/src/vespa/eval/tensor/dense/direct_dense_tensor_builder.h b/eval/src/vespa/eval/tensor/dense/direct_dense_tensor_builder.h
index 8566eedf246..865decd9fb8 100644
--- a/eval/src/vespa/eval/tensor/dense/direct_dense_tensor_builder.h
+++ b/eval/src/vespa/eval/tensor/dense/direct_dense_tensor_builder.h
@@ -13,7 +13,7 @@ class DirectDenseTensorBuilder
 {
 public:
     using Cells = DenseTensor::Cells;
-    using Address = std::vector<size_t>;
+    using Address = DenseTensor::Address;
 
 private:
     eval::ValueType _type;
diff --git a/eval/src/vespa/eval/tensor/dense/mutable_dense_tensor_view.h b/eval/src/vespa/eval/tensor/dense/mutable_dense_tensor_view.h
index 1575e27ae26..b68a1594905 100644
--- a/eval/src/vespa/eval/tensor/dense/mutable_dense_tensor_view.h
+++ b/eval/src/vespa/eval/tensor/dense/mutable_dense_tensor_view.h
@@ -17,7 +17,7 @@ private:
     {
     private:
         eval::ValueType _type;
-        std::vector<size_t *> _unboundDimSizes;
+        std::vector<eval::ValueType::Dimension::size_type *> _unboundDimSizes;
 
     public:
         MutableValueType(eval::ValueType type_in);