MixedMap optimizer now only for inplace

3 files changed, 32 insertions, 44 deletions

diff --git a/eval/src/tests/instruction/mixed_map_function/mixed_map_function_test.cpp b/eval/src/tests/instruction/mixed_map_function/mixed_map_function_test.cpp
index dae147da216..6cf1a0c6f3a 100644
--- a/eval/src/tests/instruction/mixed_map_function/mixed_map_function_test.cpp
+++ b/eval/src/tests/instruction/mixed_map_function/mixed_map_function_test.cpp
@@ -16,6 +16,7 @@ const ValueBuilderFactory &prod_factory = FastValueBuilderFactory::get();
 EvalFixture::ParamRepo make_params() {
     return EvalFixture::ParamRepo()
         .add("a", GenSpec(1.5))
+        .add_mutable("@a", GenSpec(1.5))
         .add("b", GenSpec(2.5))
         .add_variants("sparse", GenSpec().map("x", {"a"}))
         .add_variants("mixed", GenSpec().map("x", {"a"}).idx("y", 5))
@@ -23,7 +24,7 @@ EvalFixture::ParamRepo make_params() {
 }
 EvalFixture::ParamRepo param_repo = make_params();
 
-void verify_optimized(const vespalib::string &expr, bool inplace) {
+void verify_optimized(const vespalib::string &expr) {
     EvalFixture slow_fixture(prod_factory, expr, param_repo, false);
     EvalFixture fixture(prod_factory, expr, param_repo, true, true);
     EXPECT_EQ(fixture.result(), EvalFixture::ref(expr, param_repo));
@@ -31,13 +32,10 @@ void verify_optimized(const vespalib::string &expr, bool inplace) {
     auto info = fixture.find_all<MixedMapFunction>();
     ASSERT_EQ(info.size(), 1u);
     EXPECT_TRUE(info[0]->result_is_mutable());
-    EXPECT_EQ(info[0]->inplace(), inplace);
+    EXPECT_EQ(info[0]->inplace(), true);
     ASSERT_EQ(fixture.num_params(), 1);
-    if (inplace) {
-        EXPECT_EQ(fixture.get_param(0), fixture.result());
-    } else {
-        EXPECT_TRUE(!(fixture.get_param(0) == fixture.result()));
-    }
+    // inplace check:
+    EXPECT_EQ(fixture.get_param(0), fixture.result());
 }
 
 void verify_not_optimized(const vespalib::string &expr) {
@@ -47,36 +45,38 @@ void verify_not_optimized(const vespalib::string &expr) {
     EXPECT_EQ(fixture.result(), slow_fixture.result());
     auto info = fixture.find_all<MixedMapFunction>();
     EXPECT_TRUE(info.empty());
+    EXPECT_TRUE(!(fixture.get_param(0) == fixture.result()));
 }
 
-TEST(MapTest, dense_map_is_optimized) {
-    verify_optimized("map(x5y3,f(x)(x+10))", false);
-    verify_optimized("map(x5y3_f,f(x)(x+10))", false);
+TEST(MapTest, dense_map_not_inplace) {
+    verify_not_optimized("map(x5y3,f(x)(x+10))");
+    verify_not_optimized("map(x5y3_f,f(x)(x+10))");
 }
 
 TEST(MapTest, simple_dense_map_can_be_inplace) {
-    verify_optimized("map(@x5y3,f(x)(x+10))", true);
-    verify_optimized("map(@x5y3_f,f(x)(x+10))", true);
+    verify_optimized("map(@x5y3,f(x)(x+10))");
+    verify_optimized("map(@x5y3_f,f(x)(x+10))");
 }
 
 TEST(MapTest, scalar_map_is_not_optimized) {
     verify_not_optimized("map(a,f(x)(x+10))");
+    verify_not_optimized("map(@a,f(x)(x+10))");
 }
 
-TEST(MapTest, sparse_map_is_optimized) {
-    verify_optimized("map(sparse,f(x)(x+10))", false);
+TEST(MapTest, sparse_map_not_inplace) {
+    verify_not_optimized("map(sparse,f(x)(x+10))");
 }
 
 TEST(MapTest, sparse_map_can_be_inplace) {
-    verify_optimized("map(@sparse,f(x)(x+10))", true);
+    verify_optimized("map(@sparse,f(x)(x+10))");
 }
 
-TEST(MapTest, mixed_map_is_optimized) {
-    verify_optimized("map(mixed,f(x)(x+10))", false);
+TEST(MapTest, mixed_map_not_inplace) {
+    verify_not_optimized("map(mixed,f(x)(x+10))");
 }
 
 TEST(MapTest, mixed_map_can_be_inplace) {
-    verify_optimized("map(@mixed,f(x)(x+10))", true);
+    verify_optimized("map(@mixed,f(x)(x+10))");
 }
 
 GTEST_MAIN_RUN_ALL_TESTS()
diff --git a/eval/src/vespa/eval/instruction/mixed_map_function.cpp b/eval/src/vespa/eval/instruction/mixed_map_function.cpp
index 69917ae94e0..23d5809fbca 100644
--- a/eval/src/vespa/eval/instruction/mixed_map_function.cpp
+++ b/eval/src/vespa/eval/instruction/mixed_map_function.cpp
@@ -8,47 +8,32 @@
 
 namespace vespalib::eval {
 
-using vespalib::ArrayRef;
+using operation::TypifyOp1;
+using tensor_function::map_fun_t;
 
-using namespace operation;
-using namespace tensor_function;
-
-using op_function = InterpretedFunction::op_function;
 using Instruction = InterpretedFunction::Instruction;
 using State = InterpretedFunction::State;
 
 namespace {
 
-template <typename CT, bool inplace>
-ArrayRef<CT> make_dst_cells(ConstArrayRef<CT> src_cells, Stash &stash) {
-    if (inplace) {
-        return unconstify(src_cells);
-    } else {
-        return stash.create_uninitialized_array<CT>(src_cells.size());
-    }
-}
-
-template <typename CT, typename Fun, bool inplace>
-void my_simple_map_op(State &state, uint64_t param) {
+template <typename CT, typename Fun>
+void my_inplace_map_op(State &state, uint64_t param) {
     Fun my_fun((map_fun_t)param);
     auto const &child = state.peek(0);
     auto src_cells = child.cells().typify<CT>();
-    auto dst_cells = make_dst_cells<CT, inplace>(src_cells, state.stash);
+    auto dst_cells = unconstify(src_cells);
     apply_op1_vec(dst_cells.begin(), src_cells.begin(), dst_cells.size(), my_fun);
-    if (!inplace) {
-        state.pop_push(state.stash.create<ValueView>(child.type(), child.index(), TypedCells(dst_cells)));
-    }
 }
 
 //-----------------------------------------------------------------------------
 
 struct MyGetFun {
-    template <typename R1, typename R2, typename R3> static auto invoke() {
-        return my_simple_map_op<R1, R2, R3::value>;
+    template <typename CT, typename Fun> static auto invoke() {
+        return my_inplace_map_op<CT, Fun>;
     }
 };
 
-using MyTypify = TypifyValue<TypifyCellType,TypifyOp1,TypifyBool>;
+using MyTypify = TypifyValue<TypifyCellType,TypifyOp1>;
 
 } // namespace vespalib::eval::<unnamed>
 
@@ -66,7 +51,7 @@ MixedMapFunction::~MixedMapFunction() = default;
 Instruction
 MixedMapFunction::compile_self(const ValueBuilderFactory &, Stash &) const
 {
-    auto op = typify_invoke<3,MyTypify,MyGetFun>(result_type().cell_type(), function(), inplace());
+    auto op = typify_invoke<2,MyTypify,MyGetFun>(result_type().cell_type(), function());
     static_assert(sizeof(uint64_t) == sizeof(function()));
     return Instruction(op, (uint64_t)(function()));
 }
@@ -75,7 +60,10 @@ const TensorFunction &
 MixedMapFunction::optimize(const TensorFunction &expr, Stash &stash)
 {
     if (auto map = as<Map>(expr)) {
-        if (! map->child().result_type().is_double()) {
+        if ((map->result_type() == map->child().result_type())
+            && (! map->child().result_type().is_double())
+            && map->child().result_is_mutable())
+        {
             return stash.create<MixedMapFunction>(map->result_type(), map->child(), map->function());
         }
     }
diff --git a/eval/src/vespa/eval/instruction/mixed_map_function.h b/eval/src/vespa/eval/instruction/mixed_map_function.h
index 3f3b821d15e..2e7ca7c9422 100644
--- a/eval/src/vespa/eval/instruction/mixed_map_function.h
+++ b/eval/src/vespa/eval/instruction/mixed_map_function.h
@@ -7,7 +7,7 @@
 namespace vespalib::eval {
 
 /**
- * Tensor function optimizing map operations on tensors.
+ * Tensor function optimizing in-place map operations on tensors.
  **/
 class MixedMapFunction : public tensor_function::Map
 {