From b2840c8baef5b6ad6bdc8b14fecfb7d69db37a33 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Wed, 7 Sep 2022 16:11:38 +0200 Subject: Revert "Revert "Unify access to assets needed during rank-setup."" --- .../features/onnx_feature/onnx_feature_test.cpp | 22 +++++++++++----------- searchlib/src/vespa/searchlib/fef/onnx_model.cpp | 6 ++++-- searchlib/src/vespa/searchlib/fef/onnx_model.h | 4 ++++ .../vespa/searchlib/fef/test/indexenvironment.cpp | 18 ++++++++---------- .../vespa/searchlib/fef/test/indexenvironment.h | 20 +++++++++----------- 5 files changed, 36 insertions(+), 34 deletions(-) (limited to 'searchlib') diff --git a/searchlib/src/tests/features/onnx_feature/onnx_feature_test.cpp b/searchlib/src/tests/features/onnx_feature/onnx_feature_test.cpp index 54b574abbf3..bf7cbeeb5a2 100644 --- a/searchlib/src/tests/features/onnx_feature/onnx_feature_test.cpp +++ b/searchlib/src/tests/features/onnx_feature/onnx_feature_test.cpp @@ -61,14 +61,14 @@ struct OnnxFeatureTest : ::testing::Test { factory.addPrototype(std::make_shared("onnx")); factory.addPrototype(std::make_shared("onnxModel")); } - ~OnnxFeatureTest(); + ~OnnxFeatureTest() override; void add_expr(const vespalib::string &name, const vespalib::string &expr) { vespalib::string feature_name = expr_feature(name); vespalib::string expr_name = feature_name + ".rankingScript"; indexEnv.getProperties().add(expr_name, expr); } - void add_onnx(const OnnxModel &model) { - indexEnv.addOnnxModel(model); + void add_onnx(OnnxModel model) { + indexEnv.addOnnxModel(std::move(model)); } bool try_compile(const vespalib::string &seed) { resolver->addSeed(seed); @@ -84,16 +84,16 @@ struct OnnxFeatureTest : ::testing::Test { void compile(const vespalib::string &seed) { ASSERT_TRUE(try_compile(seed)); } - TensorSpec get(const vespalib::string &feature, uint32_t docid) { + TensorSpec get(const vespalib::string &feature, uint32_t docid) const { auto result = program.get_all_features(false); for (size_t i = 0; i < result.num_features(); ++i) { if (result.name_of(i) == feature) { return TensorSpec::from_value(result.resolve(i).as_object(docid)); } } - return TensorSpec("error"); + return {"error"}; } - TensorSpec get(uint32_t docid) { + TensorSpec get(uint32_t docid) const { auto result = program.get_seeds(false); EXPECT_EQ(1u, result.num_features()); return TensorSpec::from_value(result.resolve(0).as_object(docid)); @@ -153,11 +153,11 @@ TEST_F(OnnxFeatureTest, strange_input_and_output_names_are_normalized) { TEST_F(OnnxFeatureTest, input_features_and_output_names_can_be_specified) { add_expr("my_first_input", "tensor(a[2]):[10,20]"); add_expr("my_second_input", "tensor(a[2]):[5,10]"); - add_onnx(OnnxModel("custom_names", strange_names_model) + add_onnx(std::move(OnnxModel("custom_names", strange_names_model) .input_feature("input:0", "rankingExpression(my_first_input)") .input_feature("input/1", "rankingExpression(my_second_input)") .output_name("foo/bar", "my_first_output") - .output_name("-baz:0", "my_second_output")); + .output_name("-baz:0", "my_second_output"))); compile(onnx_feature("custom_names")); auto expect_add = TensorSpec("tensor(d0[2])").add({{"d0",0}},15).add({{"d0",1}},30); auto expect_sub = TensorSpec("tensor(d0[2])").add({{"d0",0}},5).add({{"d0",1}},10); @@ -169,7 +169,7 @@ TEST_F(OnnxFeatureTest, input_features_and_output_names_can_be_specified) { TEST_F(OnnxFeatureTest, fragile_model_can_be_evaluated) { add_expr("in1", "tensor(x[2]):[docid,5]"); add_expr("in2", "tensor(x[2]):[docid,10]"); - add_onnx(OnnxModel("fragile", fragile_model).dry_run_on_setup(true)); + add_onnx(std::move(OnnxModel("fragile", fragile_model).dry_run_on_setup(true))); EXPECT_TRUE(try_compile(onnx_feature("fragile"))); EXPECT_EQ(get(1), TensorSpec::from_expr("tensor(d0[2]):[2,15]")); EXPECT_EQ(get(3), TensorSpec::from_expr("tensor(d0[2]):[6,15]")); @@ -185,7 +185,7 @@ struct MyIssues : Issue::Handler { TEST_F(OnnxFeatureTest, broken_model_evaluates_to_all_zeros) { add_expr("in1", "tensor(x[2]):[docid,5]"); add_expr("in2", "tensor(x[3]):[docid,10,31515]"); - add_onnx(OnnxModel("fragile", fragile_model).dry_run_on_setup(false)); + add_onnx(std::move(OnnxModel("fragile", fragile_model).dry_run_on_setup(false))); EXPECT_TRUE(try_compile(onnx_feature("fragile"))); MyIssues my_issues; EXPECT_EQ(my_issues.list.size(), 0); @@ -199,7 +199,7 @@ TEST_F(OnnxFeatureTest, broken_model_evaluates_to_all_zeros) { TEST_F(OnnxFeatureTest, broken_model_fails_with_dry_run) { add_expr("in1", "tensor(x[2]):[docid,5]"); add_expr("in2", "tensor(x[3]):[docid,10,31515]"); - add_onnx(OnnxModel("fragile", fragile_model).dry_run_on_setup(true)); + add_onnx(std::move(OnnxModel("fragile", fragile_model).dry_run_on_setup(true))); EXPECT_FALSE(try_compile(onnx_feature("fragile"))); } diff --git a/searchlib/src/vespa/searchlib/fef/onnx_model.cpp b/searchlib/src/vespa/searchlib/fef/onnx_model.cpp index bdb037db78d..2e7642887ae 100644 --- a/searchlib/src/vespa/searchlib/fef/onnx_model.cpp +++ b/searchlib/src/vespa/searchlib/fef/onnx_model.cpp @@ -15,6 +15,10 @@ OnnxModel::OnnxModel(const vespalib::string &name_in, { } +OnnxModel::OnnxModel(OnnxModel &&) noexcept = default; +OnnxModel & OnnxModel::operator =(OnnxModel &&) noexcept = default; +OnnxModel::~OnnxModel() = default; + OnnxModel & OnnxModel::input_feature(const vespalib::string &model_input_name, const vespalib::string &input_feature) { _input_features[model_input_name] = input_feature; @@ -58,6 +62,4 @@ OnnxModel::operator==(const OnnxModel &rhs) const { std::tie(rhs._name, rhs._file_path, rhs._input_features, rhs._output_names, rhs._dry_run_on_setup)); } -OnnxModel::~OnnxModel() = default; - } diff --git a/searchlib/src/vespa/searchlib/fef/onnx_model.h b/searchlib/src/vespa/searchlib/fef/onnx_model.h index f3fef9caa3e..fc5b434b22b 100644 --- a/searchlib/src/vespa/searchlib/fef/onnx_model.h +++ b/searchlib/src/vespa/searchlib/fef/onnx_model.h @@ -24,6 +24,10 @@ private: public: OnnxModel(const vespalib::string &name_in, const vespalib::string &file_path_in); + OnnxModel(OnnxModel &&) noexcept; + OnnxModel & operator=(OnnxModel &&) noexcept; + OnnxModel(const OnnxModel &) = delete; + OnnxModel & operator =(const OnnxModel &) = delete; ~OnnxModel(); const vespalib::string &name() const { return _name; } diff --git a/searchlib/src/vespa/searchlib/fef/test/indexenvironment.cpp b/searchlib/src/vespa/searchlib/fef/test/indexenvironment.cpp index daa9054dff2..0665f274e53 100644 --- a/searchlib/src/vespa/searchlib/fef/test/indexenvironment.cpp +++ b/searchlib/src/vespa/searchlib/fef/test/indexenvironment.cpp @@ -11,6 +11,7 @@ using vespalib::eval::ValueType; IndexEnvironment::IndexEnvironment() = default; IndexEnvironment::~IndexEnvironment() = default; +IndexEnvironment::Constant::~Constant() = default; const FieldInfo * IndexEnvironment::getField(uint32_t id) const @@ -21,10 +22,9 @@ IndexEnvironment::getField(uint32_t id) const const FieldInfo * IndexEnvironment::getFieldByName(const string &name) const { - for (std::vector::const_iterator it = _fields.begin(); - it != _fields.end(); ++it) { - if (it->name() == name) { - return &(*it); + for (const auto & field : _fields) { + if (field.name() == name) { + return &field; } } return nullptr; @@ -38,7 +38,7 @@ IndexEnvironment::getConstantValue(const vespalib::string &name) const if (it != _constants.end()) { return std::make_unique(it->second); } else { - return vespalib::eval::ConstantValue::UP(nullptr); + return {nullptr}; } } @@ -47,9 +47,7 @@ IndexEnvironment::addConstantValue(const vespalib::string &name, vespalib::eval::ValueType type, std::unique_ptr value) { - auto insertRes = _constants.emplace(name, - Constant(std::move(type), - std::move(value))); + auto insertRes = _constants.emplace(name, Constant(std::move(type), std::move(value))); assert(insertRes.second); // successful insert (void) insertRes; } @@ -81,9 +79,9 @@ IndexEnvironment::getOnnxModel(const vespalib::string &name) const } void -IndexEnvironment::addOnnxModel(const OnnxModel &model) +IndexEnvironment::addOnnxModel(OnnxModel model) { - _models.insert_or_assign(model.name(), model); + _models.insert_or_assign(model.name(), std::move(model)); } diff --git a/searchlib/src/vespa/searchlib/fef/test/indexenvironment.h b/searchlib/src/vespa/searchlib/fef/test/indexenvironment.h index f204dd06aed..cb14a7cdaea 100644 --- a/searchlib/src/vespa/searchlib/fef/test/indexenvironment.h +++ b/searchlib/src/vespa/searchlib/fef/test/indexenvironment.h @@ -24,27 +24,27 @@ public: vespalib::eval::ValueType _type; std::unique_ptr _value; Constant(vespalib::eval::ValueType type, - std::unique_ptr value) + std::unique_ptr value) : _type(std::move(type)), _value(std::move(value)) { } - Constant(Constant &&rhs) + Constant(Constant &&rhs) noexcept : _type(std::move(rhs._type)), _value(std::move(rhs._value)) { } const vespalib::eval::ValueType &type() const override { return _type; } const vespalib::eval::Value &value() const override { return *_value; } - ~Constant() { } + ~Constant() override; }; struct ConstantRef : vespalib::eval::ConstantValue { const Constant &_value; - ConstantRef(const Constant &value) + explicit ConstantRef(const Constant &value) : _value(value) { } const vespalib::eval::ValueType &type() const override { return _value.type(); } const vespalib::eval::Value &value() const override { return _value.value(); } - ~ConstantRef() { } + ~ConstantRef() override = default; }; using ConstantsMap = std::map; @@ -52,7 +52,9 @@ public: using ModelMap = std::map; IndexEnvironment(); - ~IndexEnvironment(); + IndexEnvironment(const IndexEnvironment &) = delete; + IndexEnvironment & operator=(const IndexEnvironment &) = delete; + ~IndexEnvironment() override; const Properties &getProperties() const override { return _properties; } uint32_t getNumFields() const override { return _fields.size(); } @@ -90,11 +92,7 @@ public: void addRankingExpression(const vespalib::string &name, const vespalib::string &value); const OnnxModel *getOnnxModel(const vespalib::string &name) const override; - void addOnnxModel(const OnnxModel &model); - -private: - IndexEnvironment(const IndexEnvironment &); // hide - IndexEnvironment & operator=(const IndexEnvironment &); // hide + void addOnnxModel(OnnxModel model); private: Properties _properties; -- cgit v1.2.3