From 0a0670c449c7da08727e767c2000a08f1ec15563 Mon Sep 17 00:00:00 2001 From: Tor Egge Date: Mon, 27 Mar 2017 11:27:02 +0000 Subject: Refactor parameters for predicate attributes. --- .../src/vespa/searchcommon/attribute/config.cpp | 10 ++---- .../src/vespa/searchcommon/attribute/config.h | 21 +++--------- .../attribute/persistent_predicate_params.h | 37 ++++++++++++++++++++++ .../searchcommon/attribute/predicate_params.h | 32 +++++++++++++++++++ .../attribute_manager/attribute_manager_test.cpp | 9 ++++-- .../proton/attribute/attributemanager.cpp | 7 ++-- .../vespa/searchlib/attribute/configconverter.cpp | 8 +++-- .../searchlib/attribute/predicate_attribute.cpp | 8 ++--- 8 files changed, 96 insertions(+), 36 deletions(-) create mode 100644 searchcommon/src/vespa/searchcommon/attribute/persistent_predicate_params.h create mode 100644 searchcommon/src/vespa/searchcommon/attribute/predicate_params.h diff --git a/searchcommon/src/vespa/searchcommon/attribute/config.cpp b/searchcommon/src/vespa/searchcommon/attribute/config.cpp index 4d1cd540033..e7a4c28b27a 100644 --- a/searchcommon/src/vespa/searchcommon/attribute/config.cpp +++ b/searchcommon/src/vespa/searchcommon/attribute/config.cpp @@ -18,10 +18,7 @@ Config::Config() : _fastAccess(false), _growStrategy(), _compactionStrategy(), - _arity(8), - _lower_bound(LLONG_MIN), - _upper_bound(LLONG_MAX), - _dense_posting_list_threshold(0.4), + _predicateParams(), _tensorType(vespalib::eval::ValueType::error_type()) { } @@ -40,10 +37,7 @@ Config::Config(BasicType bt, _fastAccess(false), _growStrategy(), _compactionStrategy(), - _arity(8), - _lower_bound(LLONG_MIN), - _upper_bound(LLONG_MAX), - _dense_posting_list_threshold(0.4), + _predicateParams(), _tensorType(vespalib::eval::ValueType::error_type()) { } diff --git a/searchcommon/src/vespa/searchcommon/attribute/config.h b/searchcommon/src/vespa/searchcommon/attribute/config.h index 7a5919620bf..be2a1d2777d 100644 --- a/searchcommon/src/vespa/searchcommon/attribute/config.h +++ b/searchcommon/src/vespa/searchcommon/attribute/config.h @@ -7,6 +7,7 @@ #include #include #include +#include "predicate_params.h" namespace search { namespace attribute { @@ -25,10 +26,7 @@ public: CollectionType collectionType() const { return _type; } bool fastSearch() const { return _fastSearch; } bool huge() const { return _huge; } - uint32_t arity() const { return _arity; } - int64_t lower_bound() const { return _lower_bound; } - int64_t upper_bound() const { return _upper_bound; } - double dense_posting_list_threshold() const { return _dense_posting_list_threshold; } + const PredicateParams &predicateParams() const { return _predicateParams; } vespalib::eval::ValueType tensorType() const { return _tensorType; } /** @@ -67,10 +65,7 @@ public: const CompactionStrategy &getCompactionStrategy() const { return _compactionStrategy; } void setHuge(bool v) { _huge = v; } void setFastSearch(bool v) { _fastSearch = v; } - void setArity(uint32_t v) { _arity = v; } - void setBounds(int64_t lower, int64_t upper) { _lower_bound = lower; - _upper_bound = upper; } - void setDensePostingListThreshold(double v) { _dense_posting_list_threshold = v; } + void setPredicateParams(const PredicateParams &v) { _predicateParams = v; } void setTensorType(const vespalib::eval::ValueType &tensorType_in) { _tensorType = tensorType_in; } @@ -124,10 +119,7 @@ public: _fastAccess == b._fastAccess && _growStrategy == b._growStrategy && _compactionStrategy == b._compactionStrategy && - _arity == b._arity && - _lower_bound == b._lower_bound && - _upper_bound == b._upper_bound && - _dense_posting_list_threshold == b._dense_posting_list_threshold && + _predicateParams == b._predicateParams && (_basicType.type() != BasicType::Type::TENSOR || _tensorType == b._tensorType); } @@ -143,10 +135,7 @@ private: bool _fastAccess; GrowStrategy _growStrategy; CompactionStrategy _compactionStrategy; - uint32_t _arity; - int64_t _lower_bound; - int64_t _upper_bound; - double _dense_posting_list_threshold; + PredicateParams _predicateParams; vespalib::eval::ValueType _tensorType; }; } // namespace attribute diff --git a/searchcommon/src/vespa/searchcommon/attribute/persistent_predicate_params.h b/searchcommon/src/vespa/searchcommon/attribute/persistent_predicate_params.h new file mode 100644 index 00000000000..1aed2dad72c --- /dev/null +++ b/searchcommon/src/vespa/searchcommon/attribute/persistent_predicate_params.h @@ -0,0 +1,37 @@ +// Copyright 2017 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +namespace search { +namespace attribute { + +/* + * Persistent parameters for predicate attributes. + */ +class PersistentPredicateParams { + uint32_t _arity; + int64_t _lower_bound; + int64_t _upper_bound; + +public: + PersistentPredicateParams() + : _arity(8), + _lower_bound(std::numeric_limits::min()), + _upper_bound(std::numeric_limits::max()) + { + } + uint32_t arity() const { return _arity; } + int64_t lower_bound() const { return _lower_bound; } + int64_t upper_bound() const { return _upper_bound; } + void setArity(uint32_t v) { _arity = v; } + void setBounds(int64_t lower, int64_t upper) { _lower_bound = lower; _upper_bound = upper; } + + bool operator==(const PersistentPredicateParams &rhs) const { + return ((_arity == rhs._arity) && + (_lower_bound == rhs._lower_bound) && + (_upper_bound == rhs._upper_bound)); + } +}; + +} // namespace attribute +} // namespace search diff --git a/searchcommon/src/vespa/searchcommon/attribute/predicate_params.h b/searchcommon/src/vespa/searchcommon/attribute/predicate_params.h new file mode 100644 index 00000000000..8e8e0605b17 --- /dev/null +++ b/searchcommon/src/vespa/searchcommon/attribute/predicate_params.h @@ -0,0 +1,32 @@ +// Copyright 2017 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include "persistent_predicate_params.h" + +namespace search { +namespace attribute { + +/* + * Parameters for predicate attributes. + */ +class PredicateParams : public PersistentPredicateParams +{ + double _dense_posting_list_threshold; +public: + PredicateParams() + : PersistentPredicateParams(), + _dense_posting_list_threshold(0.4) + { + } + + double dense_posting_list_threshold() const { return _dense_posting_list_threshold; } + void setDensePostingListThreshold(double v) { _dense_posting_list_threshold = v; } + bool operator==(const PredicateParams &rhs) const { + return (PersistentPredicateParams::operator==(rhs) && + (_dense_posting_list_threshold == rhs._dense_posting_list_threshold)); + } +}; + +} // namespace attribute +} // namespace search diff --git a/searchcore/src/tests/proton/attribute/attribute_manager/attribute_manager_test.cpp b/searchcore/src/tests/proton/attribute/attribute_manager/attribute_manager_test.cpp index 50d79b42910..7232f997b0e 100644 --- a/searchcore/src/tests/proton/attribute/attribute_manager/attribute_manager_test.cpp +++ b/searchcore/src/tests/proton/attribute/attribute_manager/attribute_manager_test.cpp @@ -761,9 +761,14 @@ TEST_F("require that attribute vector of wrong type is dropped", BaseFixture) AVConfig dense_tensor(BasicType::TENSOR); dense_tensor.setTensorType(ValueType::from_spec("tensor(x[10])")); AVConfig predicate(BasicType::PREDICATE); - predicate.setArity(2); + using PredicateParams = search::attribute::PredicateParams; + PredicateParams predicateParams; + predicateParams.setArity(2); + predicate.setPredicateParams(predicateParams); AVConfig predicate2(BasicType::PREDICATE); - predicate2.setArity(4); + PredicateParams predicateParams2; + predicateParams2.setArity(4); + predicate2.setPredicateParams(predicateParams2); auto am1(std::make_shared (test_dir, "test.subdb", TuneFileAttributes(), diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp b/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp index 2b519806248..fa4f9258e36 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp +++ b/searchcore/src/vespa/searchcore/proton/attribute/attributemanager.cpp @@ -46,9 +46,10 @@ bool matchingTypes(const AttributeVector::SP &av, const search::attribute::Confi } } if (newConfig.basicType().type() == BasicType::PREDICATE) { - if ((oldConfig.arity() != newConfig.arity()) || - (oldConfig.lower_bound() != newConfig.lower_bound()) || - (oldConfig.upper_bound() != newConfig.upper_bound())) { + using Params = search::attribute::PersistentPredicateParams; + const Params &oldParams = oldConfig.predicateParams(); + const Params &newParams = newConfig.predicateParams(); + if (!(oldParams == newParams)) { return false; } } diff --git a/searchlib/src/vespa/searchlib/attribute/configconverter.cpp b/searchlib/src/vespa/searchlib/attribute/configconverter.cpp index 8c911a78d43..9e3c6156dbf 100644 --- a/searchlib/src/vespa/searchlib/attribute/configconverter.cpp +++ b/searchlib/src/vespa/searchlib/attribute/configconverter.cpp @@ -65,15 +65,17 @@ ConfigConverter::convert(const AttributesConfig::Attribute & cfg) cType.removeIfZero(cfg.removeifzero); cType.createIfNonExistant(cfg.createifnonexistent); Config retval(bType, cType); + PredicateParams predicateParams; retval.setFastSearch(cfg.fastsearch); retval.setHuge(cfg.huge); retval.setEnableBitVectors(cfg.enablebitvectors); retval.setEnableOnlyBitVector(cfg.enableonlybitvector); retval.setIsFilter(cfg.enableonlybitvector); retval.setFastAccess(cfg.fastaccess); - retval.setArity(cfg.arity); - retval.setBounds(cfg.lowerbound, cfg.upperbound); - retval.setDensePostingListThreshold(cfg.densepostinglistthreshold); + predicateParams.setArity(cfg.arity); + predicateParams.setBounds(cfg.lowerbound, cfg.upperbound); + predicateParams.setDensePostingListThreshold(cfg.densepostinglistthreshold); + retval.setPredicateParams(predicateParams); if (retval.basicType().type() == BasicType::Type::TENSOR) { if (!cfg.tensortype.empty()) { retval.setTensorType(ValueType::from_spec(cfg.tensortype)); diff --git a/searchlib/src/vespa/searchlib/attribute/predicate_attribute.cpp b/searchlib/src/vespa/searchlib/attribute/predicate_attribute.cpp index 3d3bbdcae43..2342c374643 100644 --- a/searchlib/src/vespa/searchlib/attribute/predicate_attribute.cpp +++ b/searchlib/src/vespa/searchlib/attribute/predicate_attribute.cpp @@ -55,7 +55,7 @@ int64_t adjustUpperBound(int32_t arity, int64_t upper_bound) { } SimpleIndexConfig createSimpleIndexConfig(const search::attribute::Config &config) { - return SimpleIndexConfig(config.dense_posting_list_threshold(), config.getGrowStrategy()); + return SimpleIndexConfig(config.predicateParams().dense_posting_list_threshold(), config.getGrowStrategy()); } } // namespace @@ -66,9 +66,9 @@ PredicateAttribute::PredicateAttribute(const vespalib::string &base_file_name, _base_file_name(base_file_name), _limit_provider(*this), _index(new PredicateIndex(getGenerationHandler(), getGenerationHolder(), - _limit_provider, createSimpleIndexConfig(config), config.arity())), - _lower_bound(adjustLowerBound(config.arity(), config.lower_bound())), - _upper_bound(adjustUpperBound(config.arity(), config.upper_bound())), + _limit_provider, createSimpleIndexConfig(config), config.predicateParams().arity())), + _lower_bound(adjustLowerBound(config.predicateParams().arity(), config.predicateParams().lower_bound())), + _upper_bound(adjustUpperBound(config.predicateParams().arity(), config.predicateParams().upper_bound())), _min_feature(config.getGrowStrategy(), getGenerationHolder()), _interval_range_vector(config.getGrowStrategy(), getGenerationHolder()), _max_interval_range(1) -- cgit v1.2.3