diff options
45 files changed, 344 insertions, 319 deletions
diff --git a/persistence/src/vespa/persistence/spi/clusterstate.cpp b/persistence/src/vespa/persistence/spi/clusterstate.cpp index f82e6165fb8..ad5039fade1 100644 --- a/persistence/src/vespa/persistence/spi/clusterstate.cpp +++ b/persistence/src/vespa/persistence/spi/clusterstate.cpp @@ -61,8 +61,8 @@ ClusterState::shouldBeReady(const Bucket& b) const { _distribution->getIdealNodes(lib::NodeType::STORAGE, *_state, b.getBucketId(), idealNodes, "uim", _distribution->getReadyCopies()); - for (uint32_t i=0, n=idealNodes.size(); i<n; ++i) { - if (idealNodes[i] == _nodeIndex) return Trinary::True; + for (uint16_t node : idealNodes) { + if (node == _nodeIndex) return Trinary::True; } return Trinary::False; } diff --git a/searchcore/src/apps/verify_ranksetup/verify_ranksetup.cpp b/searchcore/src/apps/verify_ranksetup/verify_ranksetup.cpp index dcfbc34653d..43d0b709a1b 100644 --- a/searchcore/src/apps/verify_ranksetup/verify_ranksetup.cpp +++ b/searchcore/src/apps/verify_ranksetup/verify_ranksetup.cpp @@ -18,6 +18,7 @@ #include <vespa/searchcore/config/config-onnx-models.h> #include <vespa/searchcore/proton/matching/indexenvironment.h> #include <vespa/searchcore/proton/matching/ranking_expressions.h> +#include <vespa/searchcore/proton/matching/onnx_models.h> #include <vespa/searchlib/features/setup.h> #include <vespa/searchlib/fef/fef.h> #include <vespa/searchlib/fef/test/plugin/setup.h> @@ -32,7 +33,7 @@ using config::ConfigRuntimeException; using config::ConfigSubscriber; using config::IConfigContext; using config::InvalidConfigException; -using proton::matching::IConstantValueRepo; +using proton::matching::IRankingAssetsRepo; using proton::matching::RankingExpressions; using proton::matching::OnnxModels; using vespa::config::search::AttributesConfig; @@ -91,7 +92,7 @@ make_models(const OnnxModelsConfig &modelsCfg, const VerifyRanksetupConfig &myCf entry.name.c_str(), entry.fileref.c_str())); } } - return OnnxModels(model_list); + return OnnxModels(std::move(model_list)); } class VerifyRankSetup @@ -100,9 +101,7 @@ private: std::vector<search::fef::Message> _messages; bool verify(const search::index::Schema &schema, const search::fef::Properties &props, - const IConstantValueRepo &repo, - const RankingExpressions &expressions, - const OnnxModels &models); + const IRankingAssetsRepo &repo); bool verifyConfig(const VerifyRanksetupConfig &myCfg, const RankProfilesConfig &rankCfg, @@ -119,14 +118,28 @@ public: bool verify(const std::string & configId); }; -struct DummyConstantValueRepo : IConstantValueRepo { +struct DummyRankingAssetsRepo : IRankingAssetsRepo { const RankingConstantsConfig &cfg; - DummyConstantValueRepo(const RankingConstantsConfig &cfg_in) : cfg(cfg_in) {} + RankingExpressions _expressions; + OnnxModels _onnxModels; + DummyRankingAssetsRepo(const RankingConstantsConfig &cfg_in, RankingExpressions expressions, OnnxModels onnxModels) + : cfg(cfg_in), + _expressions(std::move(expressions)), + _onnxModels(std::move(onnxModels)) + {} vespalib::eval::ConstantValue::UP getConstant(const vespalib::string &name) const override; + + vespalib::string getExpression(const vespalib::string & name) const override { + return _expressions.loadExpression(name); + } + + const search::fef::OnnxModel *getOnnxModel(const vespalib::string & name) const override { + return _onnxModels.getModel(name); + } }; vespalib::eval::ConstantValue::UP -DummyConstantValueRepo::getConstant(const vespalib::string &name) const { +DummyRankingAssetsRepo::getConstant(const vespalib::string &name) const { for (const auto &entry: cfg.constant) { if (entry.name == name) { try { @@ -137,7 +150,7 @@ DummyConstantValueRepo::getConstant(const vespalib::string &name) const { } } } - return vespalib::eval::ConstantValue::UP(nullptr); + return {}; } VerifyRankSetup::VerifyRankSetup() @@ -149,11 +162,9 @@ VerifyRankSetup::~VerifyRankSetup() = default; bool VerifyRankSetup::verify(const search::index::Schema &schema, const search::fef::Properties &props, - const IConstantValueRepo &repo, - const RankingExpressions &expressions, - const OnnxModels &models) + const IRankingAssetsRepo &repo) { - proton::matching::IndexEnvironment indexEnv(0, schema, props, repo, expressions, models); + proton::matching::IndexEnvironment indexEnv(0, schema, props, repo); search::fef::BlueprintFactory factory; search::features::setup_search_features(factory); search::fef::test::setup_fef_test_plugin(factory); @@ -192,9 +203,8 @@ VerifyRankSetup::verifyConfig(const VerifyRanksetupConfig &myCfg, search::index::Schema schema; search::index::SchemaBuilder::build(schemaCfg, schema); search::index::SchemaBuilder::build(attributeCfg, schema); - DummyConstantValueRepo repo(constantsCfg); - auto expressions = make_expressions(expressionsCfg, myCfg, _messages); - auto models = make_models(modelsCfg, myCfg, _messages); + DummyRankingAssetsRepo repo(constantsCfg, make_expressions(expressionsCfg, myCfg, _messages), + make_models(modelsCfg, myCfg, _messages)); for(size_t i = 0; i < rankCfg.rankprofile.size(); i++) { search::fef::Properties properties; const RankProfilesConfig::Rankprofile &profile = rankCfg.rankprofile[i]; @@ -202,7 +212,7 @@ VerifyRankSetup::verifyConfig(const VerifyRanksetupConfig &myCfg, properties.add(profile.fef.property[j].name, profile.fef.property[j].value); } - if (verify(schema, properties, repo, expressions, models)) { + if (verify(schema, properties, repo)) { _messages.emplace_back(search::fef::Level::INFO, fmt("rank profile '%s': pass", profile.name.c_str())); } else { diff --git a/searchcore/src/tests/proton/documentdb/configurer/configurer_test.cpp b/searchcore/src/tests/proton/documentdb/configurer/configurer_test.cpp index 6fff5998ee8..e9fb8be9600 100644 --- a/searchcore/src/tests/proton/documentdb/configurer/configurer_test.cpp +++ b/searchcore/src/tests/proton/documentdb/configurer/configurer_test.cpp @@ -152,7 +152,7 @@ struct Fixture vespalib::TestClock _clock; matching::QueryLimiter _queryLimiter; EmptyConstantValueFactory _constantValueFactory; - ConstantValueRepo _constantValueRepo; + RankingAssetsRepo _constantValueRepo; vespalib::ThreadStackExecutor _summaryExecutor; std::shared_ptr<PendingLidTrackerBase> _pendingLidsForCommit; ViewSet _views; diff --git a/searchcore/src/tests/proton/documentdb/documentdbconfig/documentdbconfig_test.cpp b/searchcore/src/tests/proton/documentdb/documentdbconfig/documentdbconfig_test.cpp index 47e9568e086..fd47cc9b264 100644 --- a/searchcore/src/tests/proton/documentdb/documentdbconfig/documentdbconfig_test.cpp +++ b/searchcore/src/tests/proton/documentdb/documentdbconfig/documentdbconfig_test.cpp @@ -8,7 +8,6 @@ #include <vespa/vespalib/testkit/testapp.h> #include <vespa/config-summary.h> #include <vespa/config-summarymap.h> -#include <vespa/document/config/documenttypes_config_fwd.h> #include <vespa/document/repo/configbuilder.h> #include <vespa/document/repo/documenttyperepo.h> #include <vespa/document/datatype/datatype.h> @@ -73,13 +72,13 @@ public: return *this; } MyConfigBuilder &addRankingExpression() { - auto expr_list = RankingExpressions().add("my_expr", "my_file"); - _builder.rankingExpressions(make_shared<RankingExpressions>(expr_list)); + _builder.rankingExpressions(make_shared<RankingExpressions>(std::move(RankingExpressions().add("my_expr", "my_file")))); return *this; } MyConfigBuilder &addOnnxModel() { - OnnxModels::Vector models = {{"my_model_name", "my_model_file"}}; - _builder.onnxModels(make_shared<OnnxModels>(models)); + OnnxModels::Vector models; + models.emplace_back("my_model_name", "my_model_file"); + _builder.onnxModels(make_shared<OnnxModels>(std::move(models))); return *this; } MyConfigBuilder &addImportedField() { @@ -175,7 +174,7 @@ struct DelayAttributeAspectFixture { Schema::SP schema; ConfigSP attrCfg; ConfigSP noAttrCfg; - DelayAttributeAspectFixture(bool hasDocField) + explicit DelayAttributeAspectFixture(bool hasDocField) : schema(make_shared<Schema>()), attrCfg(), noAttrCfg() diff --git a/searchcore/src/tests/proton/documentdb/fileconfigmanager/fileconfigmanager_test.cpp b/searchcore/src/tests/proton/documentdb/fileconfigmanager/fileconfigmanager_test.cpp index d5acd5d40bf..3f254558740 100644 --- a/searchcore/src/tests/proton/documentdb/fileconfigmanager/fileconfigmanager_test.cpp +++ b/searchcore/src/tests/proton/documentdb/fileconfigmanager/fileconfigmanager_test.cpp @@ -115,11 +115,13 @@ addConfigsThatAreNotSavedToDisk(const DocumentDBConfig &cfg) RankingConstants::Vector constants = {{"my_name", "my_type", "my_path"}}; builder.rankingConstants(std::make_shared<RankingConstants>(constants)); - auto expr_list = RankingExpressions().add("my_expr", "my_file"); - builder.rankingExpressions(std::make_shared<RankingExpressions>(expr_list)); + auto expr_list = std::make_shared<RankingExpressions>(); + expr_list->add("my_expr", "my_file"); + builder.rankingExpressions(expr_list); - OnnxModels::Vector models = {{"my_model_name", "my_model_file"}}; - builder.onnxModels(std::make_shared<OnnxModels>(models)); + OnnxModels::Vector models; + models.emplace_back("my_model_name", "my_model_file"); + builder.onnxModels(std::make_shared<OnnxModels>(std::move(models))); ImportedFieldsConfigBuilder importedFields; importedFields.attribute.resize(1); diff --git a/searchcore/src/tests/proton/matching/constant_value_repo/constant_value_repo_test.cpp b/searchcore/src/tests/proton/matching/constant_value_repo/constant_value_repo_test.cpp index 4c06feb6ae7..3c157221e60 100644 --- a/searchcore/src/tests/proton/matching/constant_value_repo/constant_value_repo_test.cpp +++ b/searchcore/src/tests/proton/matching/constant_value_repo/constant_value_repo_test.cpp @@ -2,7 +2,7 @@ #include <vespa/vespalib/testkit/test_kit.h> -#include <vespa/searchcore/proton/matching/constant_value_repo.h> +#include <vespa/searchcore/proton/matching/ranking_assets_repo.h> #include <vespa/eval/eval/value_cache/constant_value.h> using namespace proton::matching; @@ -14,9 +14,9 @@ private: ValueType _type; public: - DoubleConstantValue(double value_) : _value(value_), _type(ValueType::double_type()) {} - virtual const ValueType &type() const override { return _type; } - virtual const Value &value() const override { return _value; } + explicit DoubleConstantValue(double value_) : _value(value_), _type(ValueType::double_type()) {} + const ValueType &type() const override { return _type; } + const Value &value() const override { return _value; } }; class MyConstantValueFactory : public ConstantValueFactory { @@ -30,7 +30,7 @@ public: void add(const vespalib::string &path, const vespalib::string &type, double value) { _map.insert(std::make_pair(std::make_pair(path, type), value)); } - virtual ConstantValue::UP create(const vespalib::string &path, const vespalib::string &type) const override { + ConstantValue::UP create(const vespalib::string &path, const vespalib::string &type) const override { auto itr = _map.find(std::make_pair(path, type)); if (itr != _map.end()) { return std::make_unique<DoubleConstantValue>(itr->second); @@ -41,14 +41,16 @@ public: struct Fixture { MyConstantValueFactory factory; - ConstantValueRepo repo; + RankingAssetsRepo repo; Fixture() : factory(), repo(factory) { factory.add("path_1", "double", 3); factory.add("path_2", "double", 5); - repo.reconfigure(RankingConstants({{"foo", "double", "path_1"}, - {"bar", "double", "path_3"}})); + RankingConstants::Vector constants; + constants.emplace_back("foo", "double", "path_1"); + constants.emplace_back("bar", "double", "path_3"); + repo.reconfigure(std::make_shared<RankingConstants>(constants),{}, {}); } }; @@ -69,8 +71,10 @@ TEST_F("require that non-existing constant value in factory returns bad constant TEST_F("require that reconfigure replaces existing constant values in repo", Fixture) { - f.repo.reconfigure(RankingConstants({{"bar", "double", "path_3"}, - {"baz", "double", "path_2"}})); + RankingConstants::Vector constants; + constants.emplace_back("bar", "double", "path_3"); + constants.emplace_back("baz", "double", "path_2"); + f.repo.reconfigure(std::make_shared<RankingConstants>(constants), {}, {}); f.factory.add("path_3", "double", 7); EXPECT_TRUE(f.repo.getConstant("foo").get() == nullptr); EXPECT_EQUAL(7, f.repo.getConstant("bar")->value().as_double()); diff --git a/searchcore/src/tests/proton/matching/index_environment/index_environment_test.cpp b/searchcore/src/tests/proton/matching/index_environment/index_environment_test.cpp index f6c10f7ddb5..bd7c3a4e8fd 100644 --- a/searchcore/src/tests/proton/matching/index_environment/index_environment_test.cpp +++ b/searchcore/src/tests/proton/matching/index_environment/index_environment_test.cpp @@ -4,6 +4,7 @@ #include <vespa/eval/eval/value_cache/constant_value.h> #include <vespa/searchcore/proton/matching/indexenvironment.h> #include <vespa/searchcore/proton/matching/ranking_expressions.h> +#include <vespa/searchcore/proton/matching/onnx_models.h> using namespace proton::matching; using search::fef::FieldInfo; @@ -33,17 +34,34 @@ RankingExpressions make_expressions() { OnnxModels make_models() { OnnxModels::Vector list; - list.emplace_back(OnnxModel("model1", "path1").input_feature("input1","feature1").output_name("output1", "out1")); + list.emplace_back(std::move(OnnxModel("model1", "path1").input_feature("input1","feature1").output_name("output1", "out1"))); list.emplace_back(OnnxModel("model2", "path2")); - return OnnxModels(list); + return {std::move(list)}; } -struct MyConstantValueRepo : public IConstantValueRepo { - virtual ConstantValue::UP getConstant(const vespalib::string &) const override { - return ConstantValue::UP(); +struct MyRankingAssetsRepo : public IRankingAssetsRepo { + RankingExpressions _expressions; + OnnxModels _onnxModels; + MyRankingAssetsRepo(RankingExpressions expressions, OnnxModels onnxModels) + : _expressions(std::move(expressions)), + _onnxModels(std::move(onnxModels)) + {} + ~MyRankingAssetsRepo() override; + ConstantValue::UP getConstant(const vespalib::string &) const override { + return {}; + } + + vespalib::string getExpression(const vespalib::string & name) const override { + return _expressions.loadExpression(name); + } + + const OnnxModel *getOnnxModel(const vespalib::string & name) const override { + return _onnxModels.getModel(name); } }; +MyRankingAssetsRepo::~MyRankingAssetsRepo() = default; + Schema::UP buildSchema() { @@ -60,19 +78,19 @@ buildEmptySchema() } struct Fixture { - MyConstantValueRepo repo; + MyRankingAssetsRepo repo; Schema::UP schema; IndexEnvironment env; - Fixture(Schema::UP schema_) - : repo(), + explicit Fixture(Schema::UP schema_) + : repo(make_expressions(), make_models()), schema(std::move(schema_)), - env(7, *schema, Properties(), repo, make_expressions(), make_models()) + env(7, *schema, Properties(), repo) { } const FieldInfo *assertField(size_t idx, const vespalib::string &name, DataType dataType, - CollectionType collectionType) { + CollectionType collectionType) const { const FieldInfo *field = env.getField(idx); ASSERT_TRUE(field != nullptr); EXPECT_EQUAL(field, env.getFieldByName(name)); @@ -85,7 +103,7 @@ struct Fixture { void assertHiddenAttributeField(size_t idx, const vespalib::string &name, DataType dataType, - CollectionType collectionType) { + CollectionType collectionType) const { const FieldInfo *field = assertField(idx, name, dataType, collectionType); EXPECT_FALSE(field->hasAttribute()); EXPECT_TRUE(field->type() == FieldType::HIDDEN_ATTRIBUTE); @@ -94,7 +112,7 @@ struct Fixture { void assertAttributeField(size_t idx, const vespalib::string &name, DataType dataType, - CollectionType collectionType) { + CollectionType collectionType) const { const FieldInfo *field = assertField(idx, name, dataType, collectionType); EXPECT_TRUE(field->hasAttribute()); EXPECT_TRUE(field->type() == FieldType::ATTRIBUTE); diff --git a/searchcore/src/tests/proton/matching/matching_test.cpp b/searchcore/src/tests/proton/matching/matching_test.cpp index dbf4200a24a..b94ef025aa6 100644 --- a/searchcore/src/tests/proton/matching/matching_test.cpp +++ b/searchcore/src/tests/proton/matching/matching_test.cpp @@ -3,7 +3,7 @@ #include <vespa/searchcore/proton/bucketdb/bucket_db_owner.h> #include <vespa/searchcore/proton/documentmetastore/documentmetastore.h> #include <vespa/searchcore/proton/matching/fakesearchcontext.h> -#include <vespa/searchcore/proton/matching/i_constant_value_repo.h> +#include <vespa/searchcore/proton/matching/i_ranking_assets_repo.h> #include <vespa/searchcore/proton/matching/match_context.h> #include <vespa/searchcore/proton/matching/match_params.h> #include <vespa/searchcore/proton/matching/match_tools.h> @@ -107,10 +107,18 @@ vespalib::string make_same_element_stack_dump(const vespalib::string &a1_term, c const uint32_t NUM_DOCS = 1000; -struct EmptyConstantValueRepo : public proton::matching::IConstantValueRepo { +struct EmptyRankingAssetsRepo : public proton::matching::IRankingAssetsRepo { vespalib::eval::ConstantValue::UP getConstant(const vespalib::string &) const override { return {}; } + + vespalib::string getExpression(const vespalib::string &) const override { + return {}; + } + + const OnnxModel *getOnnxModel(const vespalib::string &) const override { + return nullptr; + } }; //----------------------------------------------------------------------------- @@ -125,7 +133,7 @@ struct MyWorld { MatchingStats matchingStats; vespalib::TestClock clock; QueryLimiter queryLimiter; - EmptyConstantValueRepo constantValueRepo; + EmptyRankingAssetsRepo constantValueRepo; MyWorld(); ~MyWorld(); @@ -347,7 +355,7 @@ struct MyWorld { } Matcher::SP createMatcher() { - return std::make_shared<Matcher>(schema, config, clock.clock(), queryLimiter, constantValueRepo, RankingExpressions(), OnnxModels(), 0); + return std::make_shared<Matcher>(schema, config, clock.clock(), queryLimiter, constantValueRepo, 0); } struct MySearchHandler : ISearchHandler { diff --git a/searchcore/src/tests/proton/verify_ranksetup/verify_ranksetup_test.cpp b/searchcore/src/tests/proton/verify_ranksetup/verify_ranksetup_test.cpp index 6f4420695c4..33a52f70f2b 100644 --- a/searchcore/src/tests/proton/verify_ranksetup/verify_ranksetup_test.cpp +++ b/searchcore/src/tests/proton/verify_ranksetup/verify_ranksetup_test.cpp @@ -27,9 +27,9 @@ using vespalib::make_string_short::fmt; struct Writer { FILE *file; - Writer(const std::string &file_name) { + explicit Writer(const std::string &file_name) { file = fopen(file_name.c_str(), "w"); - ASSERT_TRUE(file != 0); + ASSERT_TRUE(file != nullptr); } void fmt(const char *format, ...) const #ifdef __GNUC__ @@ -63,7 +63,7 @@ struct Attribute { ~Attribute(); }; -Attribute::~Attribute() {} +Attribute::~Attribute() = default; struct Setup { std::map<std::string,std::pair<std::string,std::string> > indexes; @@ -75,8 +75,8 @@ struct Setup { std::map<std::string,OnnxModel> onnx_models; Setup(); ~Setup(); - void add_onnx_model(const OnnxModel &model) { - onnx_models.insert_or_assign(model.name(), model); + void add_onnx_model(OnnxModel model) { + onnx_models.insert_or_assign(model.name(), std::move(model)); } void index(const std::string &name, schema::DataType data_type, schema::CollectionType collection_type) @@ -141,7 +141,7 @@ struct Setup { } void write_indexschema(const Writer &out) { out.fmt("indexfield[%zu]\n", indexes.size()); - std::map<std::string,std::pair<std::string,std::string> >::const_iterator pos = indexes.begin(); + auto pos = indexes.begin(); for (size_t i = 0; pos != indexes.end(); ++pos, ++i) { out.fmt("indexfield[%zu].name \"%s\"\n", i, pos->first.c_str()); out.fmt("indexfield[%zu].datatype %s\n", i, pos->second.first.c_str()); @@ -151,7 +151,7 @@ struct Setup { void write_rank_profiles(const Writer &out) { out.fmt("rankprofile[%zu]\n", extra_profiles.size() + 1); out.fmt("rankprofile[0].name \"default\"\n"); - std::map<std::string,std::string>::const_iterator pos = properties.begin(); + auto pos = properties.begin(); for (size_t i = 0; pos != properties.end(); ++pos, ++i) { out.fmt("rankprofile[0].fef.property[%zu]\n", properties.size()); out.fmt("rankprofile[0].fef.property[%zu].name \"%s\"\n", i, pos->first.c_str()); @@ -258,7 +258,7 @@ Setup::Setup() { verify_dir(); } -Setup::~Setup() {} +Setup::~Setup() = default; //----------------------------------------------------------------------------- @@ -279,15 +279,15 @@ struct SimpleSetup : Setup { struct OnnxSetup : Setup { OnnxSetup() : Setup() { add_onnx_model(OnnxModel("simple", TEST_PATH("../../../../../eval/src/tests/tensor/onnx_wrapper/simple.onnx"))); - add_onnx_model(OnnxModel("mapped", TEST_PATH("../../../../../eval/src/tests/tensor/onnx_wrapper/simple.onnx")) + add_onnx_model(std::move(OnnxModel("mapped", TEST_PATH("../../../../../eval/src/tests/tensor/onnx_wrapper/simple.onnx")) .input_feature("query_tensor", "rankingExpression(qt)") .input_feature("attribute_tensor", "rankingExpression(at)") .input_feature("bias_tensor", "rankingExpression(bt)") - .output_name("output", "result")); - add_onnx_model(OnnxModel("fragile", TEST_PATH("../../../../../searchlib/src/tests/features/onnx_feature/fragile.onnx")) - .dry_run_on_setup(true)); - add_onnx_model(OnnxModel("unfragile", TEST_PATH("../../../../../searchlib/src/tests/features/onnx_feature/fragile.onnx")) - .dry_run_on_setup(false)); + .output_name("output", "result"))); + add_onnx_model(std::move(OnnxModel("fragile", TEST_PATH("../../../../../searchlib/src/tests/features/onnx_feature/fragile.onnx")) + .dry_run_on_setup(true))); + add_onnx_model(std::move(OnnxModel("unfragile", TEST_PATH("../../../../../searchlib/src/tests/features/onnx_feature/fragile.onnx")) + .dry_run_on_setup(false))); } }; diff --git a/searchcore/src/vespa/searchcore/proton/bucketdb/bucketstate.cpp b/searchcore/src/vespa/searchcore/proton/bucketdb/bucketstate.cpp index e041f9991fe..4c25657ab43 100644 --- a/searchcore/src/vespa/searchcore/proton/bucketdb/bucketstate.cpp +++ b/searchcore/src/vespa/searchcore/proton/bucketdb/bucketstate.cpp @@ -146,8 +146,6 @@ BucketState::operator+=(const BucketState &rhs) { for (uint32_t i = 0; i < COUNTS; ++i) { _docCount[i] += rhs._docCount[i]; - } - for (uint32_t i = 0; i < COUNTS; ++i) { _docSizes[i] += rhs._docSizes[i]; } @@ -167,14 +165,10 @@ BucketState::operator-=(const BucketState &rhs) { for (uint32_t i = 0; i < COUNTS; ++i) { assert(_docCount[i] >= rhs._docCount[i]); - } - for (uint32_t i = 0; i < COUNTS; ++i) { assert(_docSizes[i] >= rhs._docSizes[i]); } for (uint32_t i = 0; i < COUNTS; ++i) { _docCount[i] -= rhs._docCount[i]; - } - for (uint32_t i = 0; i < COUNTS; ++i) { _docSizes[i] -= rhs._docSizes[i]; } switch (_checksumType) { @@ -210,11 +204,9 @@ BucketState::operator storage::spi::BucketInfo() const using BucketInfo = storage::spi::BucketInfo; - return BucketInfo(getChecksum(), - documentCount, docSizes, - entryCount, entrySizes, - notReady > 0 ? BucketInfo::NOT_READY : BucketInfo::READY, - _active ? BucketInfo::ACTIVE : BucketInfo::NOT_ACTIVE); + return {getChecksum(), documentCount, uint32_t(docSizes), entryCount, uint32_t(entrySizes), + notReady > 0 ? BucketInfo::NOT_READY : BucketInfo::READY, + _active ? BucketInfo::ACTIVE : BucketInfo::NOT_ACTIVE}; } } diff --git a/searchcore/src/vespa/searchcore/proton/matching/CMakeLists.txt b/searchcore/src/vespa/searchcore/proton/matching/CMakeLists.txt index 1c203dd1284..597312843c9 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/CMakeLists.txt +++ b/searchcore/src/vespa/searchcore/proton/matching/CMakeLists.txt @@ -3,7 +3,7 @@ vespa_add_library(searchcore_matching STATIC SOURCES attribute_limiter.cpp blueprintbuilder.cpp - constant_value_repo.cpp + ranking_assets_repo.cpp docid_range_scheduler.cpp docsum_matcher.cpp document_scorer.cpp diff --git a/searchcore/src/vespa/searchcore/proton/matching/constant_value_repo.cpp b/searchcore/src/vespa/searchcore/proton/matching/constant_value_repo.cpp deleted file mode 100644 index 05448d2c83b..00000000000 --- a/searchcore/src/vespa/searchcore/proton/matching/constant_value_repo.cpp +++ /dev/null @@ -1,31 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#include "constant_value_repo.h" - -using vespalib::eval::ConstantValue; - -namespace proton::matching { - -ConstantValueRepo::ConstantValueRepo(const ConstantValueFactory &factory) - : _factory(factory), - _constants() -{ -} - -void -ConstantValueRepo::reconfigure(const RankingConstants &constants) -{ - _constants = constants; -} - -ConstantValue::UP -ConstantValueRepo::getConstant(const vespalib::string &name) const -{ - const RankingConstants::Constant *constant = _constants.getConstant(name); - if (constant != nullptr) { - return _factory.create(constant->filePath, constant->type); - } - return ConstantValue::UP(nullptr); -} - -} diff --git a/searchcore/src/vespa/searchcore/proton/matching/constant_value_repo.h b/searchcore/src/vespa/searchcore/proton/matching/constant_value_repo.h deleted file mode 100644 index 6bf91616019..00000000000 --- a/searchcore/src/vespa/searchcore/proton/matching/constant_value_repo.h +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#pragma once - -#include "i_constant_value_repo.h" -#include "ranking_constants.h" -#include <vespa/eval/eval/value_cache/constant_value.h> - -namespace proton::matching { - -/** - * Class that provides access to a configured set of rank constant values. - * - * This class maps the symbolic name used by rank features to the file path (where the constant is stored) and its type, - * and uses a factory to instantiate the actual constant values. - */ -class ConstantValueRepo : public IConstantValueRepo { -private: - using ConstantValueFactory = vespalib::eval::ConstantValueFactory; - - const ConstantValueFactory &_factory; - RankingConstants _constants; - -public: - ConstantValueRepo(const ConstantValueFactory &factory); - void reconfigure(const RankingConstants &constants); - virtual vespalib::eval::ConstantValue::UP getConstant(const vespalib::string &name) const override; -}; - -} diff --git a/searchcore/src/vespa/searchcore/proton/matching/i_constant_value_repo.h b/searchcore/src/vespa/searchcore/proton/matching/i_ranking_assets_repo.h index 3bc2dc642d3..a684f45b9fa 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/i_constant_value_repo.h +++ b/searchcore/src/vespa/searchcore/proton/matching/i_ranking_assets_repo.h @@ -4,15 +4,18 @@ #include <vespa/eval/eval/value_cache/constant_value.h> +namespace search::fef { class OnnxModel; } namespace proton::matching { /** * Interface for retrieving a named constant rank value to be used by features in the rank framework. * If the given value is not found a nullptr should be returned. */ -struct IConstantValueRepo { +struct IRankingAssetsRepo { virtual vespalib::eval::ConstantValue::UP getConstant(const vespalib::string &name) const = 0; - virtual ~IConstantValueRepo() {} + virtual vespalib::string getExpression(const vespalib::string &name) const = 0; + virtual const search::fef::OnnxModel *getOnnxModel(const vespalib::string &name) const = 0; + virtual ~IRankingAssetsRepo() = default; }; } diff --git a/searchcore/src/vespa/searchcore/proton/matching/indexenvironment.cpp b/searchcore/src/vespa/searchcore/proton/matching/indexenvironment.cpp index f13137d182c..1883aa1fbdd 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/indexenvironment.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/indexenvironment.cpp @@ -63,18 +63,14 @@ IndexEnvironment::insertField(const search::fef::FieldInfo &field) IndexEnvironment::IndexEnvironment(uint32_t distributionKey, const search::index::Schema &schema, - const search::fef::Properties &props, - const IConstantValueRepo &constantValueRepo, - RankingExpressions rankingExpressions, - OnnxModels onnxModels) + search::fef::Properties props, + const IRankingAssetsRepo &constantValueRepo) : _tableManager(), - _properties(props), + _properties(std::move(props)), _fieldNames(), _fields(), _motivation(UNKNOWN), _constantValueRepo(constantValueRepo), - _rankingExpressions(std::move(rankingExpressions)), - _onnxModels(std::move(onnxModels)), _distributionKey(distributionKey) { _tableManager.addFactory(std::make_shared<search::fef::FunctionTableFactory>(256)); @@ -133,18 +129,6 @@ IndexEnvironment::hintFieldAccess(uint32_t ) const { } void IndexEnvironment::hintAttributeAccess(const string &) const { } -vespalib::string -IndexEnvironment::getRankingExpression(const vespalib::string &name) const -{ - return _rankingExpressions.loadExpression(name); -} - -const search::fef::OnnxModel * -IndexEnvironment::getOnnxModel(const vespalib::string &name) const -{ - return _onnxModels.getModel(name); -} - IndexEnvironment::~IndexEnvironment() = default; } diff --git a/searchcore/src/vespa/searchcore/proton/matching/indexenvironment.h b/searchcore/src/vespa/searchcore/proton/matching/indexenvironment.h index ee1b5f2e938..5c2fcbceeb4 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/indexenvironment.h +++ b/searchcore/src/vespa/searchcore/proton/matching/indexenvironment.h @@ -2,9 +2,7 @@ #pragma once -#include "onnx_models.h" -#include "ranking_expressions.h" -#include "i_constant_value_repo.h" +#include "i_ranking_assets_repo.h" #include <vespa/searchlib/fef/fieldinfo.h> #include <vespa/searchlib/fef/iindexenvironment.h> #include <vespa/searchlib/fef/properties.h> @@ -26,9 +24,7 @@ private: FieldNameMap _fieldNames; std::vector<search::fef::FieldInfo> _fields; mutable FeatureMotivation _motivation; - const IConstantValueRepo &_constantValueRepo; - RankingExpressions _rankingExpressions; - OnnxModels _onnxModels; + const IRankingAssetsRepo &_constantValueRepo; uint32_t _distributionKey; @@ -53,10 +49,10 @@ public: **/ IndexEnvironment(uint32_t distributionKey, const search::index::Schema &schema, - const search::fef::Properties &props, - const IConstantValueRepo &constantValueRepo, - RankingExpressions rankingExpressions, - OnnxModels onnxModels); + search::fef::Properties props, + const IRankingAssetsRepo &constantValueRepo); + ~IndexEnvironment() override; + const search::fef::Properties &getProperties() const override; uint32_t getNumFields() const override; @@ -72,10 +68,13 @@ public: vespalib::eval::ConstantValue::UP getConstantValue(const vespalib::string &name) const override { return _constantValueRepo.getConstant(name); } - vespalib::string getRankingExpression(const vespalib::string &name) const override; + vespalib::string getRankingExpression(const vespalib::string &name) const override { + return _constantValueRepo.getExpression(name); + } - const search::fef::OnnxModel *getOnnxModel(const vespalib::string &name) const override; - ~IndexEnvironment() override; + const search::fef::OnnxModel *getOnnxModel(const vespalib::string &name) const override { + return _constantValueRepo.getOnnxModel(name); + } }; } diff --git a/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp b/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp index 108397e6e5a..763d2a9a84b 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/matcher.cpp @@ -50,14 +50,14 @@ struct StupidMetaStore : search::IDocumentMetaStore { bool getGid(DocId, GlobalId &) const override { return false; } bool getGidEvenIfMoved(DocId, GlobalId &) const override { return false; } bool getLid(const GlobalId &, DocId &) const override { return false; } - DocumentMetaData getMetaData(const GlobalId &) const override { return DocumentMetaData(); } + DocumentMetaData getMetaData(const GlobalId &) const override { return {}; } void getMetaData(const BucketId &, DocumentMetaData::Vector &) const override { } DocId getCommittedDocIdLimit() const override { return 1; } DocId getNumUsedLids() const override { return 0; } DocId getNumActiveLids() const override { return 0; } uint64_t getCurrentGeneration() const override { return 0; } - LidUsageStats getLidUsageStats() const override { return LidUsageStats(); } - Blueprint::UP createWhiteListBlueprint() const override { return Blueprint::UP(); } + LidUsageStats getLidUsageStats() const override { return {}; } + Blueprint::UP createWhiteListBlueprint() const override { return {}; } void foreach(const search::IGidToLidMapperVisitor &) const override { } }; @@ -100,10 +100,9 @@ handleGroupingSession(SessionManager &sessionMgr, GroupingContext & groupingCont } // namespace proton::matching::<unnamed> -Matcher::Matcher(const search::index::Schema &schema, const Properties &props, const vespalib::Clock &clock, - QueryLimiter &queryLimiter, const IConstantValueRepo &constantValueRepo, - RankingExpressions rankingExpressions, OnnxModels onnxModels, uint32_t distributionKey) - : _indexEnv(distributionKey, schema, props, constantValueRepo, std::move(rankingExpressions), std::move(onnxModels)), +Matcher::Matcher(const search::index::Schema &schema, Properties props, const vespalib::Clock &clock, + QueryLimiter &queryLimiter, const IRankingAssetsRepo &constantValueRepo, uint32_t distributionKey) + : _indexEnv(distributionKey, schema, std::move(props), constantValueRepo), _blueprintFactory(), _rankSetup(), _viewResolver(ViewResolver::createFromSchema(schema)), @@ -386,9 +385,4 @@ Matcher::canProduceSummaryFeatures() const { return ! _rankSetup->getSummaryFeatures().empty(); } -double -Matcher::get_termwise_limit() const { - return _rankSetup->get_termwise_limit(); -} - } diff --git a/searchcore/src/vespa/searchcore/proton/matching/matcher.h b/searchcore/src/vespa/searchcore/proton/matching/matcher.h index 4ef57d69d47..a0356731587 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/matcher.h +++ b/searchcore/src/vespa/searchcore/proton/matching/matcher.h @@ -2,7 +2,7 @@ #pragma once -#include "i_constant_value_repo.h" +#include "i_ranking_assets_repo.h" #include "indexenvironment.h" #include "matching_stats.h" #include "search_session.h" @@ -69,10 +69,7 @@ private: size_t computeNumThreadsPerSearch(search::queryeval::Blueprint::HitEstimate hits, const Properties & rankProperties) const; public: - /** - * Convenience typedefs. - */ - typedef std::shared_ptr<Matcher> SP; + using SP = std::shared_ptr<Matcher>; Matcher(const Matcher &) = delete; @@ -87,11 +84,9 @@ public: * @param props ranking configuration * @param clock used for timeout handling **/ - Matcher(const search::index::Schema &schema, const Properties &props, + Matcher(const search::index::Schema &schema, Properties props, const vespalib::Clock &clock, QueryLimiter &queryLimiter, - const IConstantValueRepo &constantValueRepo, - RankingExpressions rankingExpressions, OnnxModels onnxModels, - uint32_t distributionKey); + const IRankingAssetsRepo &constantValueRepo, uint32_t distributionKey); const search::fef::IIndexEnvironment &get_index_env() const { return _indexEnv; } @@ -179,7 +174,6 @@ public: * @return true if this rankprofile has summary-features enabled **/ bool canProduceSummaryFeatures() const; - double get_termwise_limit() const; }; } diff --git a/searchcore/src/vespa/searchcore/proton/matching/onnx_models.cpp b/searchcore/src/vespa/searchcore/proton/matching/onnx_models.cpp index 01cdd904389..399e4ab0ad0 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/onnx_models.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/onnx_models.cpp @@ -1,22 +1,19 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "onnx_models.h" -#include <assert.h> +#include <cassert> namespace proton::matching { -OnnxModels::OnnxModels() - : _models() -{ -} - +OnnxModels::OnnxModels() = default; +OnnxModels::OnnxModels(OnnxModels &&) noexcept = default; OnnxModels::~OnnxModels() = default; -OnnxModels::OnnxModels(const Vector &models) +OnnxModels::OnnxModels(Vector models) : _models() { - for (const auto &model: models) { - _models.emplace(model.name(), model); + for (auto &model: models) { + _models.emplace(model.name(), std::move(model)); } } diff --git a/searchcore/src/vespa/searchcore/proton/matching/onnx_models.h b/searchcore/src/vespa/searchcore/proton/matching/onnx_models.h index 4907c42128b..c0582600973 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/onnx_models.h +++ b/searchcore/src/vespa/searchcore/proton/matching/onnx_models.h @@ -27,7 +27,11 @@ private: public: using SP = std::shared_ptr<OnnxModels>; OnnxModels(); - OnnxModels(const Vector &models); + OnnxModels(Vector models); + OnnxModels(OnnxModels &&) noexcept; + OnnxModels & operator=(OnnxModels &&) = delete; + OnnxModels(const OnnxModels &) = delete; + OnnxModels & operator =(const OnnxModels &) = delete; ~OnnxModels(); bool operator==(const OnnxModels &rhs) const; const Model *getModel(const vespalib::string &name) const; diff --git a/searchcore/src/vespa/searchcore/proton/matching/ranking_assets_repo.cpp b/searchcore/src/vespa/searchcore/proton/matching/ranking_assets_repo.cpp new file mode 100644 index 00000000000..783a34389d8 --- /dev/null +++ b/searchcore/src/vespa/searchcore/proton/matching/ranking_assets_repo.cpp @@ -0,0 +1,46 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "ranking_assets_repo.h" + +using vespalib::eval::ConstantValue; + +namespace proton::matching { + +RankingAssetsRepo::RankingAssetsRepo(const ConstantValueFactory &factory) + : _factory(factory), + _constants() +{ +} + +RankingAssetsRepo::~RankingAssetsRepo() = default; + +void +RankingAssetsRepo::reconfigure(RankingConstants::SP constants, RankingExpressions::SP expressions, OnnxModels::SP models) +{ + _constants = std::move(constants); + _rankingExpressions = std::move(expressions); + _onnxModels = std::move(models); +} + +ConstantValue::UP +RankingAssetsRepo::getConstant(const vespalib::string &name) const +{ + if ( ! _constants) return {}; + const RankingConstants::Constant *constant = _constants->getConstant(name); + if (constant != nullptr) { + return _factory.create(constant->filePath, constant->type); + } + return {}; +} + +vespalib::string +RankingAssetsRepo::getExpression(const vespalib::string &name) const { + return _rankingExpressions ? _rankingExpressions->loadExpression(name) : ""; +} + +const search::fef::OnnxModel * +RankingAssetsRepo::getOnnxModel(const vespalib::string &name) const { + return _onnxModels ? _onnxModels->getModel(name) : nullptr; +} + +} diff --git a/searchcore/src/vespa/searchcore/proton/matching/ranking_assets_repo.h b/searchcore/src/vespa/searchcore/proton/matching/ranking_assets_repo.h new file mode 100644 index 00000000000..c22493b7c1c --- /dev/null +++ b/searchcore/src/vespa/searchcore/proton/matching/ranking_assets_repo.h @@ -0,0 +1,37 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include "i_ranking_assets_repo.h" +#include "ranking_constants.h" +#include "onnx_models.h" +#include "ranking_expressions.h" +#include <vespa/eval/eval/value_cache/constant_value.h> + +namespace proton::matching { + +/** + * Class that provides access to a configured set of rank constant values. + * + * This class maps the symbolic name used by rank features to the file path (where the constant is stored) and its type, + * and uses a factory to instantiate the actual constant values. + */ +class RankingAssetsRepo : public IRankingAssetsRepo { +private: + using ConstantValueFactory = vespalib::eval::ConstantValueFactory; + + const ConstantValueFactory &_factory; + RankingConstants::SP _constants; + RankingExpressions::SP _rankingExpressions; + OnnxModels::SP _onnxModels; + +public: + explicit RankingAssetsRepo(const ConstantValueFactory &factory); + ~RankingAssetsRepo() override; + void reconfigure(RankingConstants::SP constants, RankingExpressions::SP expressions, OnnxModels::SP models); + vespalib::eval::ConstantValue::UP getConstant(const vespalib::string &name) const override; + vespalib::string getExpression(const vespalib::string &name) const override; + const search::fef::OnnxModel *getOnnxModel(const vespalib::string &name) const override; +}; + +} diff --git a/searchcore/src/vespa/searchcore/proton/matching/ranking_constants.cpp b/searchcore/src/vespa/searchcore/proton/matching/ranking_constants.cpp index fbf3020611a..02f705ba4ea 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/ranking_constants.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/ranking_constants.cpp @@ -13,7 +13,7 @@ RankingConstants::Constant::Constant(const vespalib::string &name_in, { } -RankingConstants::Constant::~Constant() {} +RankingConstants::Constant::~Constant() = default; bool RankingConstants::Constant::operator==(const Constant &rhs) const @@ -28,7 +28,8 @@ RankingConstants::RankingConstants() { } -RankingConstants::~RankingConstants() {} +RankingConstants::~RankingConstants() = default; +RankingConstants::RankingConstants(RankingConstants &&) noexcept = default; RankingConstants::RankingConstants(const Vector &constants) : _constants() diff --git a/searchcore/src/vespa/searchcore/proton/matching/ranking_constants.h b/searchcore/src/vespa/searchcore/proton/matching/ranking_constants.h index bae04c66d1e..c527c6e5571 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/ranking_constants.h +++ b/searchcore/src/vespa/searchcore/proton/matching/ranking_constants.h @@ -35,7 +35,11 @@ private: public: using SP = std::shared_ptr<RankingConstants>; RankingConstants(); - RankingConstants(const Vector &constants); + RankingConstants(RankingConstants &&) noexcept; + RankingConstants & operator =(RankingConstants &&) = delete; + RankingConstants(const RankingConstants &) = delete; + RankingConstants & operator =(const RankingConstants &) = delete; + explicit RankingConstants(const Vector &constants); ~RankingConstants(); bool operator==(const RankingConstants &rhs) const; const Constant *getConstant(const vespalib::string &name) const; diff --git a/searchcore/src/vespa/searchcore/proton/matching/ranking_expressions.cpp b/searchcore/src/vespa/searchcore/proton/matching/ranking_expressions.cpp index 3de6fb0cfbf..98b1e3ea653 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/ranking_expressions.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/ranking_expressions.cpp @@ -24,8 +24,7 @@ vespalib::string extract_data(vespalib::Input &input) { } // unnamed RankingExpressions::RankingExpressions() = default; -RankingExpressions::RankingExpressions(RankingExpressions &&rhs) = default; -RankingExpressions::RankingExpressions(const RankingExpressions &rhs) = default; +RankingExpressions::RankingExpressions(RankingExpressions &&rhs) noexcept = default; RankingExpressions::~RankingExpressions() = default; RankingExpressions & diff --git a/searchcore/src/vespa/searchcore/proton/matching/ranking_expressions.h b/searchcore/src/vespa/searchcore/proton/matching/ranking_expressions.h index a8a7a358008..c228f2b92ca 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/ranking_expressions.h +++ b/searchcore/src/vespa/searchcore/proton/matching/ranking_expressions.h @@ -19,11 +19,13 @@ private: std::map<vespalib::string,vespalib::string> _expressions; public: + using SP = std::shared_ptr<RankingExpressions>; RankingExpressions(); - RankingExpressions(RankingExpressions &&rhs); - RankingExpressions(const RankingExpressions &rhs); + RankingExpressions(RankingExpressions &&rhs) noexcept; + RankingExpressions & operator=(RankingExpressions &&rhs) = delete; + RankingExpressions(const RankingExpressions &rhs) = delete; + RankingExpressions & operator=(const RankingExpressions &rhs) = delete; ~RankingExpressions(); - using SP = std::shared_ptr<RankingExpressions>; bool operator==(const RankingExpressions &rhs) const { return _expressions == rhs._expressions; } diff --git a/searchcore/src/vespa/searchcore/proton/server/buckethandler.h b/searchcore/src/vespa/searchcore/proton/server/buckethandler.h index 79c6e929a51..a58a869ca2c 100644 --- a/searchcore/src/vespa/searchcore/proton/server/buckethandler.h +++ b/searchcore/src/vespa/searchcore/proton/server/buckethandler.h @@ -45,7 +45,7 @@ public: * * @param executor The executor in which to run all tasks. */ - BucketHandler(vespalib::Executor &executor); + explicit BucketHandler(vespalib::Executor &executor); ~BucketHandler() override; void setReadyBucketHandler(documentmetastore::IBucketHandler &ready); diff --git a/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.cpp b/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.cpp index 415000c3f9b..8be55433fdd 100644 --- a/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.cpp @@ -54,7 +54,7 @@ blockedDueToClusterState(const std::shared_ptr<IBucketStateCalculator> &calc) } -BucketMoveJob::BucketMoveJob(const std::shared_ptr<IBucketStateCalculator> &calc, +BucketMoveJob::BucketMoveJob(std::shared_ptr<IBucketStateCalculator> calc, RetainGuard dbRetainer, IDocumentMoveHandler &moveHandler, IBucketModifiedHandler &modifiedHandler, @@ -75,7 +75,7 @@ BucketMoveJob::BucketMoveJob(const std::shared_ptr<IBucketStateCalculator> &calc IBucketStateChangedHandler(), IDiskMemUsageListener(), std::enable_shared_from_this<BucketMoveJob>(), - _calc(calc), + _calc(std::move(calc)), _dbRetainer(std::move(dbRetainer)), _moveHandler(moveHandler), _modifiedHandler(modifiedHandler), @@ -115,7 +115,7 @@ BucketMoveJob::~BucketMoveJob() } std::shared_ptr<BucketMoveJob> -BucketMoveJob::create(const std::shared_ptr<IBucketStateCalculator> &calc, +BucketMoveJob::create(std::shared_ptr<IBucketStateCalculator> calc, RetainGuard dbRetainer, IDocumentMoveHandler &moveHandler, IBucketModifiedHandler &modifiedHandler, @@ -132,7 +132,7 @@ BucketMoveJob::create(const std::shared_ptr<IBucketStateCalculator> &calc, document::BucketSpace bucketSpace) { return std::shared_ptr<BucketMoveJob>( - new BucketMoveJob(calc, std::move(dbRetainer), moveHandler, modifiedHandler, master, bucketExecutor, ready, notReady, + new BucketMoveJob(std::move(calc), std::move(dbRetainer), moveHandler, modifiedHandler, master, bucketExecutor, ready, notReady, bucketCreateNotifier, clusterStateChangedNotifier, bucketStateChangedNotifier, diskMemUsageNotifier, blockableConfig, docTypeName, bucketSpace), [&master](auto job) { @@ -302,7 +302,7 @@ BucketMoveJob::considerBucket(const bucketdb::Guard & guard, BucketId bucket) { void BucketMoveJob::reconsiderBucket(const bucketdb::Guard & guard, BucketId bucket) { assert( ! _bucketsInFlight.contains(bucket)); - auto [mustMove, wantReady] = needMove(bucket, guard->get(bucket)); + auto [mustMove, wantReady] = needMove(bucket, BucketStateWrapper(guard->get(bucket))); if (mustMove) { _buckets2Move[bucket] = wantReady; } else { @@ -324,7 +324,7 @@ BucketMoveJob::computeBuckets2Move(const bucketdb::Guard & guard) BucketMoveJob::BucketMoveSet toMove; BucketId::List buckets = guard->getBuckets(); for (BucketId bucketId : buckets) { - auto [mustMove, wantReady] = needMove(bucketId, guard->get(bucketId)); + auto [mustMove, wantReady] = needMove(bucketId, BucketStateWrapper(guard->get(bucketId))); if (mustMove) { toMove[bucketId] = wantReady; } diff --git a/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.h b/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.h index 9885b581a24..12c205e8011 100644 --- a/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.h +++ b/searchcore/src/vespa/searchcore/proton/server/bucketmovejob.h @@ -83,13 +83,13 @@ private: const bucketdb::BucketState & _state; public: - BucketStateWrapper(const bucketdb::BucketState & state) noexcept : _state(state) {} + explicit BucketStateWrapper(const bucketdb::BucketState & state) noexcept : _state(state) {} bool isActive() const noexcept { return _state.isActive(); } bool hasReadyBucketDocs() const noexcept { return _state.getReadyCount() != 0; } bool hasNotReadyBucketDocs() const noexcept { return _state.getNotReadyCount() != 0; } }; - BucketMoveJob(const std::shared_ptr<IBucketStateCalculator> &calc, + BucketMoveJob(std::shared_ptr<IBucketStateCalculator> calc, vespalib::RetainGuard dbRetainer, IDocumentMoveHandler &moveHandler, IBucketModifiedHandler &modifiedHandler, @@ -124,7 +124,7 @@ private: class StartMove; public: static std::shared_ptr<BucketMoveJob> - create(const std::shared_ptr<IBucketStateCalculator> &calc, + create(std::shared_ptr<IBucketStateCalculator> calc, vespalib::RetainGuard dbRetainer, IDocumentMoveHandler &moveHandler, IBucketModifiedHandler &modifiedHandler, diff --git a/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp b/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp index 783282b71c1..51c02c818b2 100644 --- a/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/documentdb.cpp @@ -74,7 +74,7 @@ constexpr uint32_t indexing_thread_stack_size = 128_Ki; index::IndexConfig makeIndexConfig(const ProtonConfig::Index & cfg) { - return index::IndexConfig(WarmupConfig(vespalib::from_s(cfg.warmup.time), cfg.warmup.unpack), cfg.maxflushed, cfg.cache.size); + return {WarmupConfig(vespalib::from_s(cfg.warmup.time), cfg.warmup.unpack), size_t(cfg.maxflushed), size_t(cfg.cache.size)}; } ReplayThrottlingPolicy @@ -92,7 +92,7 @@ make_replay_throttling_policy(const ProtonConfig::ReplayThrottlingPolicy& cfg) { class MetricsUpdateHook : public metrics::UpdateHook { DocumentDB &_db; public: - MetricsUpdateHook(DocumentDB &s) + explicit MetricsUpdateHook(DocumentDB &s) : metrics::UpdateHook("documentdb-hook"), _db(s) {} @@ -106,7 +106,7 @@ private: const DocumentDB& _doc_db; public: - DocumentDBResourceUsageProvider(const DocumentDB& doc_db) noexcept + explicit DocumentDBResourceUsageProvider(const DocumentDB& doc_db) noexcept : _doc_db(doc_db) {} TransientResourceUsage get_transient_resource_usage() const override { diff --git a/searchcore/src/vespa/searchcore/proton/server/documentdbconfigmanager.cpp b/searchcore/src/vespa/searchcore/proton/server/documentdbconfigmanager.cpp index 1aadc845744..4e6988034ad 100644 --- a/searchcore/src/vespa/searchcore/proton/server/documentdbconfigmanager.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/documentdbconfigmanager.cpp @@ -212,7 +212,7 @@ deriveConfig(const ProtonConfig::Summary & summary, const HwInfo & hwInfo) { .setMaxBucketSpread(log.maxbucketspread).setMinFileSizeFactor(log.minfilesizefactor) .compactCompression(deriveCompression(log.compact.compression)) .setFileConfig(fileConfig).disableCrcOnRead(chunk.skipcrconread); - return LogDocumentStore::Config(config, logConfig); + return {config, logConfig}; } search::LogDocumentStore::Config buildStoreConfig(const ProtonConfig & proton, const HwInfo & hwInfo) { @@ -380,7 +380,7 @@ DocumentDBConfigManager::update(FNET_Transport & transport, const ConfigSnapshot OnnxModels::configure(rc, models.back()); } } - newOnnxModels = std::make_shared<OnnxModels>(models); + newOnnxModels = std::make_shared<OnnxModels>(std::move(models)); } if (snapshot.isChanged<IndexschemaConfig>(_configId, currentGeneration)) { newIndexschemaConfig = snapshot.getConfig<IndexschemaConfig>(_configId); diff --git a/searchcore/src/vespa/searchcore/proton/server/maintenance_jobs_injector.cpp b/searchcore/src/vespa/searchcore/proton/server/maintenance_jobs_injector.cpp index 49b301da26e..cd001997c1c 100644 --- a/searchcore/src/vespa/searchcore/proton/server/maintenance_jobs_injector.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/maintenance_jobs_injector.cpp @@ -38,8 +38,8 @@ injectLidSpaceCompactionJobs(MaintenanceController &controller, auto job = lidspace::CompactionJob::create(config.getLidSpaceCompactionConfig(), controller.retainDB(), std::move(lidHandler), opStorer, controller.masterThread(), bucketExecutor, diskMemUsageNotifier,config.getBlockableJobConfig(), - clusterStateChangedNotifier, (calc ? calc->nodeRetired() : false), bucketSpace); - controller.registerJobInMasterThread(trackJob(tracker, std::move(job))); + clusterStateChangedNotifier, calc && calc->nodeRetired(), bucketSpace); + controller.registerJobInMasterThread(trackJob(std::move(tracker), std::move(job))); } } @@ -54,11 +54,11 @@ injectBucketMoveJob(MaintenanceController &controller, IBucketModifiedHandler &bucketModifiedHandler, IClusterStateChangedNotifier &clusterStateChangedNotifier, IBucketStateChangedNotifier &bucketStateChangedNotifier, - const std::shared_ptr<IBucketStateCalculator> &calc, + std::shared_ptr<IBucketStateCalculator> calc, DocumentDBJobTrackers &jobTrackers, IDiskMemUsageNotifier &diskMemUsageNotifier) { - auto bmj = BucketMoveJob::create(calc, controller.retainDB(), moveHandler, bucketModifiedHandler, controller.masterThread(), + auto bmj = BucketMoveJob::create(std::move(calc), controller.retainDB(), moveHandler, bucketModifiedHandler, controller.masterThread(), bucketExecutor, controller.getReadySubDB(), controller.getNotReadySubDB(), bucketCreateNotifier, clusterStateChangedNotifier, bucketStateChangedNotifier, diskMemUsageNotifier, config.getBlockableJobConfig(), docTypeName, bucketSpace); @@ -118,7 +118,8 @@ MaintenanceJobsInjector::injectJobs(MaintenanceController &controller, calc, jobTrackers, diskMemUsageNotifier); controller.registerJobInMasterThread( - std::make_unique<SampleAttributeUsageJob>(readyAttributeManager, notReadyAttributeManager, + std::make_unique<SampleAttributeUsageJob>(std::move(readyAttributeManager), + std::move(notReadyAttributeManager), attributeUsageFilter, docTypeName, config.getAttributeUsageSampleInterval())); } diff --git a/searchcore/src/vespa/searchcore/proton/server/matchers.cpp b/searchcore/src/vespa/searchcore/proton/server/matchers.cpp index 809f2e0a744..acaa355ce19 100644 --- a/searchcore/src/vespa/searchcore/proton/server/matchers.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/matchers.cpp @@ -18,10 +18,10 @@ using namespace vespalib::make_string_short; Matchers::Matchers(const vespalib::Clock &clock, matching::QueryLimiter &queryLimiter, - const matching::IConstantValueRepo &constantValueRepo) + const matching::IRankingAssetsRepo &constantValueRepo) : _rpmap(), _fallback(std::make_shared<Matcher>(search::index::Schema(), search::fef::Properties(), clock, queryLimiter, - constantValueRepo, RankingExpressions(), OnnxModels(), -1)), + constantValueRepo, -1)), _default() { } @@ -69,14 +69,4 @@ Matchers::lookup(const vespalib::string &name) const return found->second; } -vespalib::string -Matchers::listMatchers() const { - vespalib::string matchers; - for (const auto & entry : _rpmap) { - matchers += entry.first; - matchers += ' '; - } - return matchers; -} - } // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/server/matchers.h b/searchcore/src/vespa/searchcore/proton/server/matchers.h index fe467c1f565..e10390f4d64 100644 --- a/searchcore/src/vespa/searchcore/proton/server/matchers.h +++ b/searchcore/src/vespa/searchcore/proton/server/matchers.h @@ -12,7 +12,7 @@ namespace proton { namespace matching { class Matcher; class QueryLimiter; - struct IConstantValueRepo; + struct IRankingAssetsRepo; } class Matchers { @@ -22,11 +22,10 @@ private: std::shared_ptr<matching::Matcher> _fallback; std::shared_ptr<matching::Matcher> _default; public: - typedef std::shared_ptr<Matchers> SP; - typedef std::unique_ptr<Matchers> UP; + using SP = std::shared_ptr<Matchers>; Matchers(const vespalib::Clock &clock, matching::QueryLimiter &queryLimiter, - const matching::IConstantValueRepo &constantValueRepo); + const matching::IRankingAssetsRepo &constantValueRepo); Matchers(const Matchers &) = delete; Matchers & operator =(const Matchers &) = delete; ~Matchers(); @@ -34,8 +33,6 @@ public: matching::MatchingStats getStats() const; matching::MatchingStats getStats(const vespalib::string &name) const; std::shared_ptr<matching::Matcher> lookup(const vespalib::string &name) const; - vespalib::string listMatchers() const; - uint32_t numMatchers() const { return _rpmap.size(); } }; } // namespace proton diff --git a/searchcore/src/vespa/searchcore/proton/server/sample_attribute_usage_job.cpp b/searchcore/src/vespa/searchcore/proton/server/sample_attribute_usage_job.cpp index e21e0366c4c..299eb4ac36b 100644 --- a/searchcore/src/vespa/searchcore/proton/server/sample_attribute_usage_job.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/sample_attribute_usage_job.cpp @@ -16,8 +16,8 @@ SampleAttributeUsageJob(IAttributeManagerSP readyAttributeManager, const vespalib::string &docTypeName, vespalib::duration interval) : IMaintenanceJob("sample_attribute_usage." + docTypeName, vespalib::duration::zero(), interval), - _readyAttributeManager(readyAttributeManager), - _notReadyAttributeManager(notReadyAttributeManager), + _readyAttributeManager(std::move(readyAttributeManager)), + _notReadyAttributeManager(std::move(notReadyAttributeManager)), _attributeUsageFilter(attributeUsageFilter) { } diff --git a/searchcore/src/vespa/searchcore/proton/server/searchable_doc_subdb_configurer.cpp b/searchcore/src/vespa/searchcore/proton/server/searchable_doc_subdb_configurer.cpp index a182c4efb5c..390ed9a0d7a 100644 --- a/searchcore/src/vespa/searchcore/proton/server/searchable_doc_subdb_configurer.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/searchable_doc_subdb_configurer.cpp @@ -89,7 +89,7 @@ SearchableDocSubDBConfigurer(const ISummaryManager::SP &summaryMgr, SearchViewHolder &searchView, FeedViewHolder &feedView, matching::QueryLimiter &queryLimiter, - matching::ConstantValueRepo &constantValueRepo, + matching::RankingAssetsRepo &constantValueRepo, const vespalib::Clock &clock, const vespalib::string &subDbName, uint32_t distributionKey) : @@ -105,13 +105,11 @@ SearchableDocSubDBConfigurer(const ISummaryManager::SP &summaryMgr, SearchableDocSubDBConfigurer::~SearchableDocSubDBConfigurer() = default; -Matchers::UP +std::shared_ptr<Matchers> SearchableDocSubDBConfigurer::createMatchers(const Schema::SP &schema, - const RankProfilesConfig &cfg, - const RankingExpressions &rankingExpressions, - const OnnxModels &onnxModels) + const RankProfilesConfig &cfg) { - auto newMatchers = std::make_unique<Matchers>(_clock, _queryLimiter, _constantValueRepo); + auto newMatchers = std::make_shared<Matchers>(_clock, _queryLimiter, _constantValueRepo); for (const auto &profile : cfg.rankprofile) { vespalib::string name = profile.name; search::fef::Properties properties; @@ -119,9 +117,9 @@ SearchableDocSubDBConfigurer::createMatchers(const Schema::SP &schema, properties.add(property.name, property.value); } // schema instance only used during call. - auto profptr = std::make_shared<Matcher>(*schema, properties, _clock, _queryLimiter, _constantValueRepo, - rankingExpressions, onnxModels, _distributionKey); - newMatchers->add(name, profptr); + auto profptr = std::make_shared<Matcher>(*schema, std::move(properties), _clock, _queryLimiter, + _constantValueRepo, _distributionKey); + newMatchers->add(name, std::move(profptr)); } return newMatchers; } @@ -185,11 +183,10 @@ SearchableDocSubDBConfigurer::reconfigure(const DocumentDBConfig &newConfig, SearchView::SP searchView = _searchView.get(); Matchers::SP matchers = searchView->getMatchers(); if (params.shouldMatchersChange()) { - _constantValueRepo.reconfigure(newConfig.getRankingConstants()); - Matchers::SP newMatchers = createMatchers(newConfig.getSchemaSP(), - newConfig.getRankProfilesConfig(), - newConfig.getRankingExpressions(), - newConfig.getOnnxModels()); + _constantValueRepo.reconfigure(newConfig.getRankingConstantsSP(), + newConfig.getRankingExpressionsSP(), + newConfig.getOnnxModelsSP()); + Matchers::SP newMatchers = createMatchers(newConfig.getSchemaSP(), newConfig.getRankProfilesConfig()); matchers = newMatchers; shouldMatchViewChange = true; } diff --git a/searchcore/src/vespa/searchcore/proton/server/searchable_doc_subdb_configurer.h b/searchcore/src/vespa/searchcore/proton/server/searchable_doc_subdb_configurer.h index b19ebb1546e..29ab94f0708 100644 --- a/searchcore/src/vespa/searchcore/proton/server/searchable_doc_subdb_configurer.h +++ b/searchcore/src/vespa/searchcore/proton/server/searchable_doc_subdb_configurer.h @@ -10,7 +10,7 @@ #include <vespa/searchcore/proton/attribute/i_attribute_writer.h> #include <vespa/searchcore/proton/docsummary/summarymanager.h> #include <vespa/searchcore/proton/index/i_index_writer.h> -#include <vespa/searchcore/proton/matching/constant_value_repo.h> +#include <vespa/searchcore/proton/matching/ranking_assets_repo.h> #include <vespa/searchcore/proton/reprocessing/i_reprocessing_initializer.h> #include <vespa/searchsummary/config/config-juniperrc.h> #include <vespa/searchcommon/common/schema.h> @@ -43,7 +43,7 @@ private: SearchViewHolder &_searchView; FeedViewHolder &_feedView; matching::QueryLimiter &_queryLimiter; - matching::ConstantValueRepo &_constantValueRepo; + matching::RankingAssetsRepo &_constantValueRepo; const vespalib::Clock &_clock; vespalib::string _subDbName; uint32_t _distributionKey; @@ -69,16 +69,14 @@ public: SearchViewHolder &searchView, FeedViewHolder &feedView, matching::QueryLimiter &queryLimiter, - matching::ConstantValueRepo &constantValueRepo, + matching::RankingAssetsRepo &constantValueRepo, const vespalib::Clock &clock, const vespalib::string &subDbName, uint32_t distributionKey); ~SearchableDocSubDBConfigurer(); - Matchers::UP createMatchers(const search::index::Schema::SP &schema, - const vespa::config::search::RankProfilesConfig &cfg, - const proton::matching::RankingExpressions &rankingExpressions, - const proton::matching::OnnxModels &onnxModels); + Matchers::SP createMatchers(const search::index::Schema::SP &schema, + const vespa::config::search::RankProfilesConfig &cfg); void reconfigureIndexSearchable(); diff --git a/searchcore/src/vespa/searchcore/proton/server/searchabledocsubdb.cpp b/searchcore/src/vespa/searchcore/proton/server/searchabledocsubdb.cpp index ca731d1e379..209a78cb00c 100644 --- a/searchcore/src/vespa/searchcore/proton/server/searchabledocsubdb.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/searchabledocsubdb.cpp @@ -194,9 +194,10 @@ SearchableDocSubDB::initViews(const DocumentDBConfig &configSnapshot, const Sess AttributeManager::SP attrMgr = getAndResetInitAttributeManager(); const Schema::SP &schema = configSnapshot.getSchemaSP(); const IIndexManager::SP &indexMgr = getIndexManager(); - _constantValueRepo.reconfigure(configSnapshot.getRankingConstants()); - Matchers::SP matchers = _configurer.createMatchers(schema, configSnapshot.getRankProfilesConfig(), - configSnapshot.getRankingExpressions(), configSnapshot.getOnnxModels()); + _constantValueRepo.reconfigure(configSnapshot.getRankingConstantsSP(), + configSnapshot.getRankingExpressionsSP(), + configSnapshot.getOnnxModelsSP()); + Matchers::SP matchers = _configurer.createMatchers(schema, configSnapshot.getRankProfilesConfig()); auto matchView = std::make_shared<MatchView>(std::move(matchers), indexMgr->getSearchable(), attrMgr, sessionManager, _metaStoreCtx, _docIdLimit); _rSearchView.set(SearchView::create( diff --git a/searchcore/src/vespa/searchcore/proton/server/searchabledocsubdb.h b/searchcore/src/vespa/searchcore/proton/server/searchabledocsubdb.h index 08b34a3fbe6..7a7408dc0d6 100644 --- a/searchcore/src/vespa/searchcore/proton/server/searchabledocsubdb.h +++ b/searchcore/src/vespa/searchcore/proton/server/searchabledocsubdb.h @@ -16,7 +16,7 @@ #include <vespa/searchcore/proton/documentmetastore/documentmetastorecontext.h> #include <vespa/searchcore/proton/index/i_index_writer.h> #include <vespa/searchcore/proton/index/indexmanager.h> -#include <vespa/searchcore/proton/matching/constant_value_repo.h> +#include <vespa/searchcore/proton/matching/ranking_assets_repo.h> #include <vespa/searchcorespi/index/iindexmanager.h> #include <vespa/vespalib/util/blockingthreadstackexecutor.h> #include <vespa/vespalib/util/varholder.h> @@ -69,7 +69,7 @@ private: vespalib::VarHolder<SearchableFeedView::SP> _rFeedView; vespalib::eval::ConstantTensorLoader _tensorLoader; vespalib::eval::ConstantValueCache _constantValueCache; - matching::ConstantValueRepo _constantValueRepo; + matching::RankingAssetsRepo _constantValueRepo; SearchableDocSubDBConfigurer _configurer; vespalib::Executor &_warmupExecutor; std::shared_ptr<GidToLidChangeHandler> _realGidToLidChangeHandler; 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<OnnxBlueprint>("onnx")); factory.addPrototype(std::make_shared<OnnxBlueprint>("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<float>(a[2]):[10,20]"); add_expr("my_second_input", "tensor<float>(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<float>(d0[2])").add({{"d0",0}},15).add({{"d0",1}},30); auto expect_sub = TensorSpec("tensor<float>(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<float>(x[2]):[docid,5]"); add_expr("in2", "tensor<float>(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<float>(d0[2]):[2,15]")); EXPECT_EQ(get(3), TensorSpec::from_expr("tensor<float>(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<float>(x[2]):[docid,5]"); add_expr("in2", "tensor<float>(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<float>(x[2]):[docid,5]"); add_expr("in2", "tensor<float>(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<FieldInfo>::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<ConstantRef>(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<vespalib::eval::Value> 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<vespalib::eval::Value> _value; Constant(vespalib::eval::ValueType type, - std::unique_ptr<vespalib::eval::Value> value) + std::unique_ptr<vespalib::eval::Value> 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<vespalib::string, Constant>; @@ -52,7 +52,9 @@ public: using ModelMap = std::map<vespalib::string, OnnxModel>; 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; diff --git a/vespalib/src/vespa/vespalib/util/varholder.h b/vespalib/src/vespa/vespalib/util/varholder.h index d92c00e0081..c9bd9f1641a 100644 --- a/vespalib/src/vespa/vespalib/util/varholder.h +++ b/vespalib/src/vespa/vespalib/util/varholder.h @@ -13,10 +13,10 @@ class VarHolder mutable std::mutex _lock; public: VarHolder() : _v(), _lock() {} - explicit VarHolder(const T &v) : _v(v), _lock() {} + explicit VarHolder(T v) : _v(std::move(v)), _lock() {} VarHolder(const VarHolder &) = delete; VarHolder & operator = (const VarHolder &) = delete; - ~VarHolder() {} + ~VarHolder(); void set(const T &v) { T old; @@ -35,4 +35,7 @@ public: } }; +template <typename T> +VarHolder<T>::~VarHolder() = default; + } |