From 3968f683fd4a6883d896cda698a34729e7338148 Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Tue, 10 Nov 2020 16:15:14 +0000 Subject: optimize join with number, with unit test --- .../instruction/join_with_number/CMakeLists.txt | 8 ++ .../join_with_number_function_test.cpp | 136 +++++++++++++++++++++ .../vespa/eval/eval/optimize_tensor_function.cpp | 4 +- eval/src/vespa/eval/instruction/CMakeLists.txt | 1 + .../eval/instruction/join_with_number_function.cpp | 116 ++++++++++++++++++ .../eval/instruction/join_with_number_function.h | 35 ++++++ 6 files changed, 298 insertions(+), 2 deletions(-) create mode 100644 eval/src/tests/instruction/join_with_number/CMakeLists.txt create mode 100644 eval/src/tests/instruction/join_with_number/join_with_number_function_test.cpp create mode 100644 eval/src/vespa/eval/instruction/join_with_number_function.cpp create mode 100644 eval/src/vespa/eval/instruction/join_with_number_function.h (limited to 'eval/src') diff --git a/eval/src/tests/instruction/join_with_number/CMakeLists.txt b/eval/src/tests/instruction/join_with_number/CMakeLists.txt new file mode 100644 index 00000000000..9bb170eae17 --- /dev/null +++ b/eval/src/tests/instruction/join_with_number/CMakeLists.txt @@ -0,0 +1,8 @@ +# Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +vespa_add_executable(eval_join_with_number_function_test_app TEST + SOURCES + join_with_number_function_test.cpp + DEPENDS + vespaeval +) +vespa_add_test(NAME eval_join_with_number_function_test_app COMMAND eval_join_with_number_function_test_app) diff --git a/eval/src/tests/instruction/join_with_number/join_with_number_function_test.cpp b/eval/src/tests/instruction/join_with_number/join_with_number_function_test.cpp new file mode 100644 index 00000000000..a67fc3725ca --- /dev/null +++ b/eval/src/tests/instruction/join_with_number/join_with_number_function_test.cpp @@ -0,0 +1,136 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include +#include +#include +#include +#include +#include + +#include + +using namespace vespalib; +using namespace vespalib::eval; +using namespace vespalib::eval::test; +using namespace vespalib::eval::tensor_function; + +using vespalib::make_string_short::fmt; + +using Primary = JoinWithNumberFunction::Primary; + +namespace vespalib::eval { + +std::ostream &operator<<(std::ostream &os, Primary primary) +{ + switch(primary) { + case Primary::LHS: return os << "LHS"; + case Primary::RHS: return os << "RHS"; + } + abort(); +} + +} + +const ValueBuilderFactory &prod_factory = FastValueBuilderFactory::get(); + +EvalFixture::ParamRepo make_params() { + return EvalFixture::ParamRepo() + .add("a", spec(1.5)) + .add("number", spec(2.5)) + .add("sparse", spec({x({"a"})}, N())) + .add("dense", spec({y(5)}, N())) + .add("mixed", spec({x({"a"}),y(5)}, N())) + .add("mixed_float", spec(float_cells({x({"a"}),y(5)}), N())) + .add("mixed_inplace", spec({x({"a"}),y(5)}, N()), true) + .add_matrix("x", 3, "y", 5); +} +EvalFixture::ParamRepo param_repo = make_params(); + +void verify_optimized(const vespalib::string &expr, Primary primary, bool inplace) { + EvalFixture slow_fixture(prod_factory, expr, param_repo, false); + EvalFixture fixture(prod_factory, expr, param_repo, true, true); + EXPECT_EQUAL(fixture.result(), EvalFixture::ref(expr, param_repo)); + EXPECT_EQUAL(fixture.result(), slow_fixture.result()); + auto info = fixture.find_all(); + ASSERT_EQUAL(info.size(), 1u); + EXPECT_TRUE(info[0]->result_is_mutable()); + EXPECT_EQUAL(info[0]->primary(), primary); + EXPECT_EQUAL(info[0]->inplace(), inplace); + int p_inplace = inplace ? ((primary == Primary::LHS) ? 0 : 1) : -1; + EXPECT_TRUE((p_inplace == -1) || (fixture.num_params() > size_t(p_inplace))); + for (size_t i = 0; i < fixture.num_params(); ++i) { + if (i == size_t(p_inplace)) { + EXPECT_EQUAL(fixture.get_param(i), fixture.result()); + } else { + EXPECT_NOT_EQUAL(fixture.get_param(i), fixture.result()); + } + } +} + +void verify_not_optimized(const vespalib::string &expr) { + EvalFixture slow_fixture(prod_factory, expr, param_repo, false); + EvalFixture fixture(prod_factory, expr, param_repo, true); + EXPECT_EQUAL(fixture.result(), EvalFixture::ref(expr, param_repo)); + EXPECT_EQUAL(fixture.result(), slow_fixture.result()); + auto info = fixture.find_all(); + EXPECT_TRUE(info.empty()); +} + +TEST("require that dense number join can be optimized") { + TEST_DO(verify_optimized("x3y5+a", Primary::LHS, false)); + TEST_DO(verify_optimized("a+x3y5", Primary::RHS, false)); + TEST_DO(verify_optimized("x3y5f*a", Primary::LHS, false)); + TEST_DO(verify_optimized("a*x3y5f", Primary::RHS, false)); +} + +TEST("require that dense number join can be inplace") { + TEST_DO(verify_optimized("@x3y5*a", Primary::LHS, true)); + TEST_DO(verify_optimized("a*@x3y5", Primary::RHS, true)); + TEST_DO(verify_optimized("@x3y5f+a", Primary::LHS, true)); + TEST_DO(verify_optimized("a+@x3y5f", Primary::RHS, true)); +} + +TEST("require that asymmetric operations work") { + TEST_DO(verify_optimized("x3y5/a", Primary::LHS, false)); + TEST_DO(verify_optimized("a/x3y5", Primary::RHS, false)); + TEST_DO(verify_optimized("x3y5f-a", Primary::LHS, false)); + TEST_DO(verify_optimized("a-x3y5f", Primary::RHS, false)); +} + +TEST("require that mixed number join can be optimized") { + TEST_DO(verify_optimized("mixed+a", Primary::LHS, false)); + TEST_DO(verify_optimized("a+mixed", Primary::RHS, false)); + TEST_DO(verify_optimized("mixed #include #include -#include +#include #include #include #include @@ -73,7 +73,7 @@ const TensorFunction &optimize_for_factory(const ValueBuilderFactory &factory, c child.set(DensePowAsMapOptimizer::optimize(child.get(), stash)); child.set(DenseSimpleMapFunction::optimize(child.get(), stash)); child.set(DenseSimpleJoinFunction::optimize(child.get(), stash)); - child.set(DenseNumberJoinFunction::optimize(child.get(), stash)); + child.set(JoinWithNumberFunction::optimize(child.get(), stash)); child.set(DenseSingleReduceFunction::optimize(child.get(), stash)); nodes.pop_back(); } diff --git a/eval/src/vespa/eval/instruction/CMakeLists.txt b/eval/src/vespa/eval/instruction/CMakeLists.txt index 926a69bd291..7211ebbfb49 100644 --- a/eval/src/vespa/eval/instruction/CMakeLists.txt +++ b/eval/src/vespa/eval/instruction/CMakeLists.txt @@ -11,4 +11,5 @@ vespa_add_library(eval_instruction OBJECT generic_peek.cpp generic_reduce.cpp generic_rename.cpp + join_with_number_function.cpp ) diff --git a/eval/src/vespa/eval/instruction/join_with_number_function.cpp b/eval/src/vespa/eval/instruction/join_with_number_function.cpp new file mode 100644 index 00000000000..dd3512a5e74 --- /dev/null +++ b/eval/src/vespa/eval/instruction/join_with_number_function.cpp @@ -0,0 +1,116 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "join_with_number_function.h" +#include +#include +#include +#include +#include + +using namespace vespalib::eval::tensor_function; +using namespace vespalib::eval::operation; + +namespace vespalib::eval { + +using Instruction = InterpretedFunction::Instruction; +using State = InterpretedFunction::State; + +namespace { + +template +ArrayRef make_dst_cells(ConstArrayRef src_cells, Stash &stash) { + if (inplace) { + return unconstify(src_cells); + } else { + return stash.create_uninitialized_array(src_cells.size()); + } +} + +template +void my_number_join_op(State &state, uint64_t param) { + using OP = typename std::conditional,Fun>::type; + OP my_op((join_fun_t)param); + const Value &tensor = state.peek(swap ? 0 : 1); + CT number = state.peek(swap ? 1 : 0).as_double(); + auto src_cells = tensor.cells().typify(); + auto dst_cells = make_dst_cells(src_cells, state.stash); + apply_op2_vec_num(dst_cells.begin(), src_cells.begin(), number, dst_cells.size(), my_op); + if (inplace) { + state.pop_pop_push(tensor); + } else { + state.pop_pop_push(state.stash.create(tensor.type(), tensor.index(), TypedCells(dst_cells))); + } +} + +struct SelectJoinWithNumberOp { + template + static auto invoke() { + return my_number_join_op; + } +}; + +} // namespace + +JoinWithNumberFunction::JoinWithNumberFunction(const Join &original, bool tensor_was_right) + : tensor_function::Op2(original.result_type(), original.lhs(), original.rhs()), + _primary(tensor_was_right ? Primary::RHS : Primary::LHS), + _function(original.function()) +{ +} + +JoinWithNumberFunction::~JoinWithNumberFunction() = default; + +bool +JoinWithNumberFunction::inplace() const { + if (_primary == Primary::LHS) { + return lhs().result_is_mutable(); + } else { + return rhs().result_is_mutable(); + } +} + +using MyTypify = TypifyValue; + +InterpretedFunction::Instruction +JoinWithNumberFunction::compile_self(EngineOrFactory, Stash &) const +{ + auto op = typify_invoke<4,MyTypify,SelectJoinWithNumberOp>(result_type().cell_type(), + _function, + inplace(), + (_primary == Primary::RHS)); + return Instruction(op, (uint64_t)(_function)); +} + +void +JoinWithNumberFunction::visit_self(vespalib::ObjectVisitor &visitor) const +{ + Super::visit_self(visitor); + visitor.visitBool("tensor_was_right", (_primary == Primary::RHS)); + visitor.visitBool("is_inplace", inplace()); +} + +const TensorFunction & +JoinWithNumberFunction::optimize(const TensorFunction &expr, Stash &stash) +{ + if (! expr.result_type().is_scalar()) { + if (const auto *join = as(expr)) { + const ValueType &result_type = join->result_type(); + const TensorFunction &lhs = join->lhs(); + const TensorFunction &rhs = join->rhs(); + if (lhs.result_type().is_double() && + (result_type == rhs.result_type())) + { + return stash.create(*join, true); + } + if (rhs.result_type().is_double() && + (result_type == lhs.result_type())) + { + return stash.create(*join, false); + } + } + } + return expr; +} + +} // namespace diff --git a/eval/src/vespa/eval/instruction/join_with_number_function.h b/eval/src/vespa/eval/instruction/join_with_number_function.h new file mode 100644 index 00000000000..6e3f9aa4106 --- /dev/null +++ b/eval/src/vespa/eval/instruction/join_with_number_function.h @@ -0,0 +1,35 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include +#include + +namespace vespalib::eval { + +/** + * Tensor function for joining a general tensor with a number + */ +class JoinWithNumberFunction : public tensor_function::Op2 +{ +public: + enum class Primary : uint8_t { LHS, RHS }; +private: + using Super = tensor_function::Op2; + Primary _primary; + tensor_function::join_fun_t _function; +public: + + JoinWithNumberFunction(const vespalib::eval::tensor_function::Join &original_join, bool number_on_left); + ~JoinWithNumberFunction(); + Primary primary() const { return _primary; } + bool inplace() const; + bool result_is_mutable() const override { return true; } + + InterpretedFunction::Instruction compile_self(EngineOrFactory engine, Stash &stash) const override; + void visit_self(vespalib::ObjectVisitor &visitor) const override; + static const TensorFunction &optimize(const TensorFunction &expr, Stash &stash); +}; + +} // namespace vespalib::tensor + -- cgit v1.2.3