From 67134e619715227e6cbe91f365213487ff1b1f34 Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Tue, 2 Jun 2020 11:46:16 +0000 Subject: use vector of UP as MultiSearch::Children * add helper class for constructing MultiSearch::Children (mostly for unit tests) * rewrite as needed to adapt --- .../src/tests/proton/matching/querynodes_test.cpp | 31 ++-- .../tests/queryeval/blueprint/blueprint_test.cpp | 30 ++-- searchlib/src/tests/queryeval/blueprint/mysearch.h | 22 +-- .../monitoring_search_iterator_test.cpp | 18 +- .../multibitvectoriterator_bench.cpp | 4 +- .../multibitvectoriterator_test.cpp | 186 ++++++++++----------- .../parallel_weak_and/parallel_weak_and_test.cpp | 15 +- searchlib/src/tests/queryeval/queryeval.cpp | 186 +++++++++------------ .../queryeval/simple_phrase/simple_phrase_test.cpp | 8 +- .../sparse_vector_benchmark_test.cpp | 48 +++--- .../queryeval/termwise_eval/termwise_eval_test.cpp | 130 ++++++++------ .../tests/queryeval/weak_and/wand_bench_setup.hpp | 6 +- .../weighted_set_term/weighted_set_term_test.cpp | 2 +- .../attribute/attribute_blueprint_factory.cpp | 11 +- .../src/vespa/searchlib/queryeval/CMakeLists.txt | 1 + .../src/vespa/searchlib/queryeval/andnotsearch.cpp | 21 +-- .../src/vespa/searchlib/queryeval/andnotsearch.h | 13 +- .../src/vespa/searchlib/queryeval/andsearch.cpp | 32 ++-- .../src/vespa/searchlib/queryeval/andsearch.h | 7 +- .../vespa/searchlib/queryeval/andsearchnostrict.h | 4 +- .../vespa/searchlib/queryeval/andsearchstrict.h | 4 +- .../src/vespa/searchlib/queryeval/blueprint.cpp | 5 +- .../src/vespa/searchlib/queryeval/blueprint.h | 3 +- .../searchlib/queryeval/children_iterators.cpp | 3 + .../vespa/searchlib/queryeval/children_iterators.h | 39 +++++ .../vespa/searchlib/queryeval/equiv_blueprint.cpp | 14 +- .../src/vespa/searchlib/queryeval/equivsearch.cpp | 14 +- .../src/vespa/searchlib/queryeval/equivsearch.h | 12 +- .../queryeval/intermediate_blueprints.cpp | 105 ++++++------ .../searchlib/queryeval/intermediate_blueprints.h | 16 +- .../searchlib/queryeval/multibitvectoriterator.cpp | 38 ++--- .../searchlib/queryeval/multibitvectoriterator.h | 2 +- .../src/vespa/searchlib/queryeval/multisearch.cpp | 15 +- .../src/vespa/searchlib/queryeval/multisearch.h | 13 +- .../src/vespa/searchlib/queryeval/nearsearch.cpp | 15 +- .../src/vespa/searchlib/queryeval/nearsearch.h | 6 +- .../src/vespa/searchlib/queryeval/orlikesearch.h | 4 +- .../src/vespa/searchlib/queryeval/orsearch.cpp | 28 ++-- searchlib/src/vespa/searchlib/queryeval/orsearch.h | 7 +- .../src/vespa/searchlib/queryeval/ranksearch.cpp | 10 +- .../src/vespa/searchlib/queryeval/ranksearch.h | 5 +- .../queryeval/simple_phrase_blueprint.cpp | 15 +- .../searchlib/queryeval/simple_phrase_search.cpp | 12 +- .../searchlib/queryeval/simple_phrase_search.h | 2 +- .../searchlib/queryeval/sourceblendersearch.cpp | 6 +- .../searchlib/queryeval/sourceblendersearch.h | 5 +- .../queryeval/termwise_blueprint_helper.cpp | 18 +- .../queryeval/termwise_blueprint_helper.h | 11 +- .../src/vespa/searchlib/queryeval/test/leafspec.h | 35 ++-- .../vespa/searchlib/queryeval/test/trackedsearch.h | 6 + .../src/vespa/searchlib/queryeval/test/wandspec.h | 6 +- .../vespa/searchlib/queryeval/wand/wand_parts.h | 1 + .../searchlib/queryeval/wand/weak_and_search.cpp | 14 +- .../searchlib/queryeval/wand/weak_and_search.h | 6 +- .../queryeval/weighted_set_term_search.cpp | 8 +- .../searchlib/queryeval/weighted_set_term_search.h | 8 +- searchlib/src/vespa/searchlib/test/initrange.h | 1 + .../searchlib/test/searchiteratorverifier.cpp | 24 +-- 58 files changed, 694 insertions(+), 617 deletions(-) create mode 100644 searchlib/src/vespa/searchlib/queryeval/children_iterators.cpp create mode 100644 searchlib/src/vespa/searchlib/queryeval/children_iterators.h diff --git a/searchcore/src/tests/proton/matching/querynodes_test.cpp b/searchcore/src/tests/proton/matching/querynodes_test.cpp index 896b2f7e07f..c1247b630a3 100644 --- a/searchcore/src/tests/proton/matching/querynodes_test.cpp +++ b/searchcore/src/tests/proton/matching/querynodes_test.cpp @@ -48,6 +48,7 @@ using search::query::QueryBuilder; using search::queryeval::AndNotSearch; using search::queryeval::AndSearch; using search::queryeval::Blueprint; +using search::queryeval::ChildrenIterators; using search::queryeval::ElementIteratorWrapper; using search::queryeval::ElementIterator; using search::queryeval::EmptySearch; @@ -104,12 +105,12 @@ public: explicit Create(bool strict = true) : _strict(strict) {} Create &add(SearchIterator *s) { - _children.push_back(s); + _children.emplace_back(s); return *this; } - operator SearchIterator *() const { - return SearchType::create(_children, _strict); + operator SearchIterator *() { + return SearchType::create(std::move(_children), _strict).release(); } }; typedef Create MyOr; @@ -143,9 +144,11 @@ public: return *this; } - operator SearchIterator *() const { + operator SearchIterator *() { return SourceBlenderSearch::create( - ISourceSelectorDummy::makeDummyIterator(), _children, _strict); + ISourceSelectorDummy::makeDummyIterator(), + _children, + _strict).release(); } }; @@ -245,20 +248,20 @@ SearchIterator *getLeaf(const string &fld, const string &tag) { template <> SearchIterator *getLeaf(const string &fld, const string &tag) { SimplePhraseSearch::Children children; - children.push_back(getTerm(phrase_term1, fld, tag)); - children.push_back(getTerm(phrase_term2, fld, tag)); + children.emplace_back(getTerm(phrase_term1, fld, tag)); + children.emplace_back(getTerm(phrase_term2, fld, tag)); static TermFieldMatchData tmd; TermFieldMatchDataArray tfmda; tfmda.add(&tmd).add(&tmd); vector eval_order(2); - return new SimplePhraseSearch(children, MatchData::UP(), tfmda, eval_order, tmd, true); + return new SimplePhraseSearch(std::move(children), MatchData::UP(), tfmda, eval_order, tmd, true); } template SearchIterator *getNearParent(SearchIterator *a, SearchIterator *b) { typename NearType::Children children; - children.push_back(a); - children.push_back(b); + children.emplace_back(a); + children.emplace_back(b); TermFieldMatchDataArray data; static TermFieldMatchData tmd; // we only check how many term/field combinations @@ -266,15 +269,15 @@ SearchIterator *getNearParent(SearchIterator *a, SearchIterator *b) { // two terms searching in (two index fields + two attribute fields) data.add(&tmd).add(&tmd).add(&tmd).add(&tmd) .add(&tmd).add(&tmd).add(&tmd).add(&tmd); - return new NearType(children, data, distance, true); + return new NearType(std::move(children), data, distance, true); } template SearchIterator *getSimpleParent(SearchIterator *a, SearchIterator *b) { typename SearchType::Children children; - children.push_back(a); - children.push_back(b); - return SearchType::create(children, true); + children.emplace_back(a); + children.emplace_back(b); + return SearchType::create(std::move(children), true).release(); } template diff --git a/searchlib/src/tests/queryeval/blueprint/blueprint_test.cpp b/searchlib/src/tests/queryeval/blueprint/blueprint_test.cpp index 6625a4a09ce..0672e51378e 100644 --- a/searchlib/src/tests/queryeval/blueprint/blueprint_test.cpp +++ b/searchlib/src/tests/queryeval/blueprint/blueprint_test.cpp @@ -39,11 +39,11 @@ public: return true; } - virtual SearchIterator::UP - createIntermediateSearch(const MultiSearch::Children &subSearches, + SearchIterator::UP + createIntermediateSearch(MultiSearch::Children subSearches, bool strict, MatchData &md) const override { - return SearchIterator::UP(new MySearch("or", subSearches, &md, strict)); + return SearchIterator::UP(new MySearch("or", std::move(subSearches), &md, strict)); } static MyOr& create() { return *(new MyOr()); } @@ -56,11 +56,11 @@ class OtherOr : public OrBlueprint { private: public: - virtual SearchIterator::UP - createIntermediateSearch(const MultiSearch::Children &subSearches, + SearchIterator::UP + createIntermediateSearch(MultiSearch::Children subSearches, bool strict, MatchData &md) const override { - return SearchIterator::UP(new MySearch("or", subSearches, &md, strict)); + return SearchIterator::UP(new MySearch("or", std::move(subSearches), &md, strict)); } static OtherOr& create() { return *(new OtherOr()); } @@ -86,11 +86,11 @@ public: return (i == 0); } - virtual SearchIterator::UP - createIntermediateSearch(const MultiSearch::Children &subSearches, + SearchIterator::UP + createIntermediateSearch(MultiSearch::Children subSearches, bool strict, MatchData &md) const override { - return SearchIterator::UP(new MySearch("and", subSearches, &md, strict)); + return SearchIterator::UP(new MySearch("and", std::move(subSearches), &md, strict)); } static MyAnd& create() { return *(new MyAnd()); } @@ -103,11 +103,11 @@ class OtherAnd : public AndBlueprint { private: public: - virtual SearchIterator::UP - createIntermediateSearch(const MultiSearch::Children &subSearches, + SearchIterator::UP + createIntermediateSearch(MultiSearch::Children subSearches, bool strict, MatchData &md) const override { - return SearchIterator::UP(new MySearch("and", subSearches, &md, strict)); + return SearchIterator::UP(new MySearch("and", std::move(subSearches), &md, strict)); } static OtherAnd& create() { return *(new OtherAnd()); } @@ -118,11 +118,11 @@ public: class OtherAndNot : public AndNotBlueprint { public: - virtual SearchIterator::UP - createIntermediateSearch(const MultiSearch::Children &subSearches, + SearchIterator::UP + createIntermediateSearch(MultiSearch::Children subSearches, bool strict, MatchData &md) const override { - return SearchIterator::UP(new MySearch("andnot", subSearches, &md, strict)); + return SearchIterator::UP(new MySearch("andnot", std::move(subSearches), &md, strict)); } static OtherAndNot& create() { return *(new OtherAndNot()); } diff --git a/searchlib/src/tests/queryeval/blueprint/mysearch.h b/searchlib/src/tests/queryeval/blueprint/mysearch.h index 1f2057a4a56..012e19f26f5 100644 --- a/searchlib/src/tests/queryeval/blueprint/mysearch.h +++ b/searchlib/src/tests/queryeval/blueprint/mysearch.h @@ -9,11 +9,9 @@ namespace search::queryeval { //----------------------------------------------------------------------------- -class MySearch : public SearchIterator +class MySearch : public MultiSearch { public: - typedef MultiSearch::Children Children; - typedef std::vector MyChildren; typedef search::fef::TermFieldMatchDataArray TFMDA; typedef search::fef::MatchData MatchData; @@ -21,7 +19,6 @@ private: vespalib::string _tag; bool _isLeaf; bool _isStrict; - MyChildren _children; TFMDA _match; MatchData *_md; @@ -33,21 +30,18 @@ protected: public: MySearch(const std::string &tag, bool leaf, bool strict) - : _tag(tag), _isLeaf(leaf), _isStrict(strict), _children(), + : _tag(tag), _isLeaf(leaf), _isStrict(strict), _match(), _md(0) {} MySearch(const std::string &tag, const TFMDA &tfmda, bool strict) - : _tag(tag), _isLeaf(true), _isStrict(strict), _children(), + : _tag(tag), _isLeaf(true), _isStrict(strict), _match(tfmda), _md(0) {} - MySearch(const std::string &tag, const Children &children, + MySearch(const std::string &tag, Children children, MatchData *md, bool strict) - : _tag(tag), _isLeaf(false), _isStrict(strict), _children(), - _match(), _md(md) { - for (size_t i(0); i < children.size(); i++) { - _children.emplace_back(children[i]); - } - } + : MultiSearch(std::move(children)), + _tag(tag), _isLeaf(false), _isStrict(strict), + _match(), _md(md) {} MySearch &add(SearchIterator *search) { _children.emplace_back(search); @@ -98,7 +92,7 @@ public: visit(visitor, "_tag", _tag); visit(visitor, "_isLeaf", _isLeaf); visit(visitor, "_isStrict", _isStrict); - visit(visitor, "_children", _children); + MultiSearch::visitMembers(visitor); visit(visitor, "_handles", _handles); } diff --git a/searchlib/src/tests/queryeval/monitoring_search_iterator/monitoring_search_iterator_test.cpp b/searchlib/src/tests/queryeval/monitoring_search_iterator/monitoring_search_iterator_test.cpp index e863cbe7a73..0e0840d9013 100644 --- a/searchlib/src/tests/queryeval/monitoring_search_iterator/monitoring_search_iterator_test.cpp +++ b/searchlib/src/tests/queryeval/monitoring_search_iterator/monitoring_search_iterator_test.cpp @@ -77,16 +77,18 @@ struct TreeFixture : _itr() { MultiSearch::Children children; - children.push_back(new MonitoringSearchIterator("child1", - SearchIterator::UP - (new SimpleSearch(SimpleResult().addHit(2).addHit(4).addHit(6))), - false)); - children.push_back(new MonitoringSearchIterator("child2", - SearchIterator::UP - (new SimpleSearch(SimpleResult().addHit(3).addHit(4).addHit(5))), + children.emplace_back( + new MonitoringSearchIterator("child1", + SearchIterator::UP + (new SimpleSearch(SimpleResult().addHit(2).addHit(4).addHit(6))), + false)); + children.emplace_back( + new MonitoringSearchIterator("child2", + SearchIterator::UP + (new SimpleSearch(SimpleResult().addHit(3).addHit(4).addHit(5))), false)); _itr.reset(new MonitoringSearchIterator("and", - SearchIterator::UP(AndSearch::create(children, true)), + SearchIterator::UP(AndSearch::create(std::move(children), true)), false)); _res.search(*_itr); } diff --git a/searchlib/src/tests/queryeval/multibitvectoriterator/multibitvectoriterator_bench.cpp b/searchlib/src/tests/queryeval/multibitvectoriterator/multibitvectoriterator_bench.cpp index 6e0baf3b2e8..e1d0e034a89 100644 --- a/searchlib/src/tests/queryeval/multibitvectoriterator/multibitvectoriterator_bench.cpp +++ b/searchlib/src/tests/queryeval/multibitvectoriterator/multibitvectoriterator_bench.cpp @@ -98,9 +98,9 @@ Test::testSearch(bool strict) TermFieldMatchData tfmd; MultiSearch::Children andd; for (size_t i(0); i < _bvs.size(); i++) { - andd.push_back(BitVectorIterator::create(_bvs[i].get(), tfmd, strict, false).release()); + andd.push_back(BitVectorIterator::create(_bvs[i].get(), tfmd, strict, false)); } - SearchIterator::UP s(T::create(andd, strict)); + SearchIterator::UP s = T::create(std::move(andd), strict); if (_optimize) { LOG(info, "Optimizing iterator"); s = MultiBitVectorIteratorBase::optimize(std::move(s)); diff --git a/searchlib/src/tests/queryeval/multibitvectoriterator/multibitvectoriterator_test.cpp b/searchlib/src/tests/queryeval/multibitvectoriterator/multibitvectoriterator_test.cpp index 296d06d0b7b..565c013a2fe 100644 --- a/searchlib/src/tests/queryeval/multibitvectoriterator/multibitvectoriterator_test.cpp +++ b/searchlib/src/tests/queryeval/multibitvectoriterator/multibitvectoriterator_test.cpp @@ -125,10 +125,10 @@ Test::testAndWith(bool invert) TermFieldMatchData tfmd; { MultiSearch::Children children; - children.push_back(createIter(0, invert, tfmd, false).release()); - children.push_back(createIter(1, invert, tfmd, false).release()); + children.push_back(createIter(0, invert, tfmd, false)); + children.push_back(createIter(1, invert, tfmd, false)); - SearchIterator::UP s(AndSearch::create(children, false)); + SearchIterator::UP s = AndSearch::create(std::move(children), false); s = MultiBitVectorIteratorBase::optimize(std::move(s)); s->initFullRange(); @@ -137,10 +137,10 @@ Test::testAndWith(bool invert) H lastHits2F = seekNoReset(*s, 130, _bvs[0]->size()); children.clear(); - children.push_back(createIter(0, invert, tfmd, false).release()); - children.push_back(createIter(1, invert, tfmd, false).release()); - children.push_back(createIter(2, invert, tfmd, false).release()); - s.reset(AndSearch::create(children, false)); + children.push_back(createIter(0, invert, tfmd, false)); + children.push_back(createIter(1, invert, tfmd, false)); + children.push_back(createIter(2, invert, tfmd, false)); + s = AndSearch::create(std::move(children), false); s = MultiBitVectorIteratorBase::optimize(std::move(s)); s->initFullRange(); H firstHits3 = seekNoReset(*s, 1, 130); @@ -197,12 +197,12 @@ Test::testBug7163266() MultiSearch::Children children; UnpackInfo unpackInfo; for (size_t i(0); i < 28; i++) { - children.push_back(new TrueSearch(tfmd[2])); + children.emplace_back(new TrueSearch(tfmd[2])); unpackInfo.add(i); } - children.push_back(createIter(0, false, tfmd[0], false).release()); - children.push_back(createIter(1, false, tfmd[1], false).release()); - SearchIterator::UP s(AndSearch::create(children, false, unpackInfo)); + children.push_back(createIter(0, false, tfmd[0], false)); + children.push_back(createIter(1, false, tfmd[1], false)); + SearchIterator::UP s = AndSearch::create(std::move(children), false, unpackInfo); const MultiSearch * ms = dynamic_cast(s.get()); EXPECT_TRUE(ms != nullptr); EXPECT_EQUAL(30u, ms->getChildren().size()); @@ -233,14 +233,14 @@ Test::testThatOptimizePreservesUnpack() _bvs[1]->setBit(1); _bvs[2]->setBit(1); MultiSearch::Children children; - children.push_back(createIter(0, false, tfmd[0], false).release()); - children.push_back(createIter(1, false, tfmd[1], false).release()); - children.push_back(new TrueSearch(tfmd[2])); - children.push_back(createIter(2, false, tfmd[3], false).release()); + children.push_back(createIter(0, false, tfmd[0], false)); + children.push_back(createIter(1, false, tfmd[1], false)); + children.emplace_back(new TrueSearch(tfmd[2])); + children.push_back(createIter(2, false, tfmd[3], false)); UnpackInfo unpackInfo; unpackInfo.add(1); unpackInfo.add(2); - SearchIterator::UP s(T::create(children, false, unpackInfo)); + SearchIterator::UP s = T::create(std::move(children), false, unpackInfo); s->initFullRange(); const MultiSearch * ms = dynamic_cast(s.get()); EXPECT_TRUE(ms != nullptr); @@ -291,10 +291,10 @@ Test::verifyUnpackOfOr(const UnpackInfo &unpackInfo) { TermFieldMatchData tfmdA[3]; MultiSearch::Children children; - children.push_back(createIter(0, false, tfmdA[0], false).release()); - children.push_back(createIter(1, false, tfmdA[1], false).release()); - children.push_back(createIter(2, false, tfmdA[2], false).release()); - SearchIterator::UP s(OrSearch::create(children, false, unpackInfo)); + children.push_back(createIter(0, false, tfmdA[0], false)); + children.push_back(createIter(1, false, tfmdA[1], false)); + children.push_back(createIter(2, false, tfmdA[2], false)); + SearchIterator::UP s = OrSearch::create(std::move(children), false, unpackInfo); verifyOrUnpack(*s, tfmdA); for (auto & tfmd : tfmdA) { @@ -353,23 +353,23 @@ Test::testSearch(bool strict, bool invert) uint32_t docIdLimit(_bvs[0]->size()); { MultiSearch::Children children; - children.push_back(createIter(0, invert, tfmd, strict).release()); - SearchIterator::UP s(T::create(children, strict)); + children.push_back(createIter(0, invert, tfmd, strict)); + SearchIterator::UP s = T::create(std::move(children), strict); searchAndCompare(std::move(s), docIdLimit); } { MultiSearch::Children children; - children.push_back(createIter(0, invert, tfmd, strict).release()); - children.push_back(createIter(1, invert, tfmd, strict).release()); - SearchIterator::UP s(T::create(children, strict)); + children.push_back(createIter(0, invert, tfmd, strict)); + children.push_back(createIter(1, invert, tfmd, strict)); + SearchIterator::UP s = T::create(std::move(children), strict); searchAndCompare(std::move(s), docIdLimit); } { MultiSearch::Children children; - children.push_back(createIter(0, invert, tfmd, strict).release()); - children.push_back(createIter(1, invert, tfmd, strict).release()); - children.push_back(createIter(2, invert, tfmd, strict).release()); - SearchIterator::UP s(T::create(children, strict)); + children.push_back(createIter(0, invert, tfmd, strict)); + children.push_back(createIter(1, invert, tfmd, strict)); + children.push_back(createIter(2, invert, tfmd, strict)); + SearchIterator::UP s = T::create(std::move(children), strict); searchAndCompare(std::move(s), docIdLimit); } } @@ -382,79 +382,79 @@ Test::testOptimizeCommon(bool isAnd, bool invert) { MultiSearch::Children children; - children.push_back(createIter(0, invert, tfmd, false).release()); + children.push_back(createIter(0, invert, tfmd, false)); - SearchIterator::UP s(T::create(children, false)); + SearchIterator::UP s = T::create(std::move(children), false); s = MultiBitVectorIteratorBase::optimize(std::move(s)); EXPECT_TRUE(dynamic_cast(s.get()) != nullptr); const MultiSearch & m(dynamic_cast(*s)); EXPECT_EQUAL(1u, m.getChildren().size()); - EXPECT_TRUE(dynamic_cast(m.getChildren()[0]) != nullptr); + EXPECT_TRUE(dynamic_cast(m.getChildren()[0].get()) != nullptr); } { MultiSearch::Children children; - children.push_back(createIter(0, invert, tfmd, false).release()); - children.push_back(new EmptySearch()); + children.push_back(createIter(0, invert, tfmd, false)); + children.emplace_back(new EmptySearch()); - SearchIterator::UP s(T::create(children, false)); + SearchIterator::UP s = T::create(std::move(children), false); s = MultiBitVectorIteratorBase::optimize(std::move(s)); EXPECT_TRUE(dynamic_cast(s.get()) != nullptr); const MultiSearch & m(dynamic_cast(*s)); EXPECT_EQUAL(2u, m.getChildren().size()); - EXPECT_TRUE(dynamic_cast(m.getChildren()[0]) != nullptr); - EXPECT_TRUE(dynamic_cast(m.getChildren()[1]) != nullptr); + EXPECT_TRUE(dynamic_cast(m.getChildren()[0].get()) != nullptr); + EXPECT_TRUE(dynamic_cast(m.getChildren()[1].get()) != nullptr); } { MultiSearch::Children children; - children.push_back(new EmptySearch()); - children.push_back(createIter(0, invert, tfmd, false).release()); + children.emplace_back(new EmptySearch()); + children.push_back(createIter(0, invert, tfmd, false)); - SearchIterator::UP s(T::create(children, false)); + SearchIterator::UP s = T::create(std::move(children), false); s = MultiBitVectorIteratorBase::optimize(std::move(s)); EXPECT_TRUE(dynamic_cast(s.get()) != nullptr); const MultiSearch & m(dynamic_cast(*s)); EXPECT_EQUAL(2u, m.getChildren().size()); - EXPECT_TRUE(dynamic_cast(m.getChildren()[0]) != nullptr); - EXPECT_TRUE(dynamic_cast(m.getChildren()[1]) != nullptr); + EXPECT_TRUE(dynamic_cast(m.getChildren()[0].get()) != nullptr); + EXPECT_TRUE(dynamic_cast(m.getChildren()[1].get()) != nullptr); } { MultiSearch::Children children; - children.push_back(new EmptySearch()); - children.push_back(createIter(0, invert, tfmd, false).release()); - children.push_back(createIter(1, invert, tfmd, false).release()); + children.emplace_back(new EmptySearch()); + children.push_back(createIter(0, invert, tfmd, false)); + children.push_back(createIter(1, invert, tfmd, false)); - SearchIterator::UP s(T::create(children, false)); + SearchIterator::UP s = T::create(std::move(children), false); s = MultiBitVectorIteratorBase::optimize(std::move(s)); EXPECT_TRUE(s); EXPECT_TRUE(dynamic_cast(s.get()) != nullptr); const MultiSearch & m(dynamic_cast(*s)); EXPECT_EQUAL(2u, m.getChildren().size()); - EXPECT_TRUE(dynamic_cast(m.getChildren()[0]) != nullptr); - EXPECT_TRUE(dynamic_cast(m.getChildren()[1]) != nullptr); + EXPECT_TRUE(dynamic_cast(m.getChildren()[0].get()) != nullptr); + EXPECT_TRUE(dynamic_cast(m.getChildren()[1].get()) != nullptr); EXPECT_TRUE(Trinary::False == m.getChildren()[1]->is_strict()); } { MultiSearch::Children children; - children.push_back(new EmptySearch()); - children.push_back(createIter(0, invert, tfmd, true).release()); - children.push_back(createIter(1, invert, tfmd, false).release()); + children.emplace_back(new EmptySearch()); + children.push_back(createIter(0, invert, tfmd, true)); + children.push_back(createIter(1, invert, tfmd, false)); - SearchIterator::UP s(T::create(children, false)); + SearchIterator::UP s = T::create(std::move(children), false); s = MultiBitVectorIteratorBase::optimize(std::move(s)); EXPECT_TRUE(s); EXPECT_TRUE(dynamic_cast(s.get()) != nullptr); const MultiSearch & m(dynamic_cast(*s)); EXPECT_EQUAL(2u, m.getChildren().size()); - EXPECT_TRUE(dynamic_cast(m.getChildren()[0]) != nullptr); - EXPECT_TRUE(dynamic_cast(m.getChildren()[1]) != nullptr); + EXPECT_TRUE(dynamic_cast(m.getChildren()[0].get()) != nullptr); + EXPECT_TRUE(dynamic_cast(m.getChildren()[1].get()) != nullptr); EXPECT_TRUE(Trinary::True == m.getChildren()[1]->is_strict()); } { MultiSearch::Children children; - children.push_back(createIter(0, invert, tfmd, false).release()); - children.push_back(createIter(1, invert, tfmd, false).release()); + children.push_back(createIter(0, invert, tfmd, false)); + children.push_back(createIter(1, invert, tfmd, false)); - SearchIterator::UP s(T::create(children, false)); + SearchIterator::UP s = T::create(std::move(children), false); s = MultiBitVectorIteratorBase::optimize(std::move(s)); SearchIterator::UP filter(s->andWith(createIter(2, invert, tfmd, false), 9)); @@ -465,9 +465,9 @@ Test::testOptimizeCommon(bool isAnd, bool invert) } children.clear(); - children.push_back(createIter(0, invert, tfmd, false).release()); - children.push_back(createIter(1, invert, tfmd, false).release()); - s.reset(T::create(children, true)); + children.push_back(createIter(0, invert, tfmd, false)); + children.push_back(createIter(1, invert, tfmd, false)); + s = T::create(std::move(children), true); s = MultiBitVectorIteratorBase::optimize(std::move(s)); filter = s->andWith(createIter(2, invert, tfmd, false), 9); @@ -487,10 +487,10 @@ Test::testOptimizeAndOr(bool invert) { MultiSearch::Children children; - children.push_back(createIter(0, invert, tfmd, false).release()); - children.push_back(createIter(1, invert, tfmd, false).release()); + children.push_back(createIter(0, invert, tfmd, false)); + children.push_back(createIter(1, invert, tfmd, false)); - SearchIterator::UP s(T::create(children, false)); + SearchIterator::UP s = T::create(std::move(children), false); s = MultiBitVectorIteratorBase::optimize(std::move(s)); EXPECT_TRUE(s); EXPECT_TRUE(dynamic_cast(s.get()) != nullptr); @@ -498,67 +498,67 @@ Test::testOptimizeAndOr(bool invert) } { MultiSearch::Children children; - children.push_back(createIter(0, invert, tfmd, false).release()); - children.push_back(new EmptySearch()); - children.push_back(createIter(1, invert, tfmd, false).release()); + children.push_back(createIter(0, invert, tfmd, false)); + children.emplace_back(new EmptySearch()); + children.push_back(createIter(1, invert, tfmd, false)); - SearchIterator::UP s(T::create(children, false)); + SearchIterator::UP s = T::create(std::move(children), false); s = MultiBitVectorIteratorBase::optimize(std::move(s)); EXPECT_TRUE(s); EXPECT_TRUE(dynamic_cast(s.get()) != nullptr); const MultiSearch & m(dynamic_cast(*s)); EXPECT_EQUAL(2u, m.getChildren().size()); - EXPECT_TRUE(dynamic_cast(m.getChildren()[0]) != nullptr); + EXPECT_TRUE(dynamic_cast(m.getChildren()[0].get()) != nullptr); EXPECT_TRUE(Trinary::False == m.getChildren()[0]->is_strict()); - EXPECT_TRUE(dynamic_cast(m.getChildren()[1]) != nullptr); + EXPECT_TRUE(dynamic_cast(m.getChildren()[1].get()) != nullptr); } { MultiSearch::Children children; - children.push_back(createIter(0, invert, tfmd, false).release()); - children.push_back(createIter(1, invert, tfmd, false).release()); - children.push_back(new EmptySearch()); + children.push_back(createIter(0, invert, tfmd, false)); + children.push_back(createIter(1, invert, tfmd, false)); + children.emplace_back(new EmptySearch()); - SearchIterator::UP s(T::create(children, false)); + SearchIterator::UP s = T::create(std::move(children), false); s = MultiBitVectorIteratorBase::optimize(std::move(s)); EXPECT_TRUE(s); EXPECT_TRUE(dynamic_cast(s.get()) != nullptr); const MultiSearch & m(dynamic_cast(*s)); EXPECT_EQUAL(2u, m.getChildren().size()); - EXPECT_TRUE(dynamic_cast(m.getChildren()[0]) != nullptr); + EXPECT_TRUE(dynamic_cast(m.getChildren()[0].get()) != nullptr); EXPECT_TRUE(Trinary::False == m.getChildren()[0]->is_strict()); - EXPECT_TRUE(dynamic_cast(m.getChildren()[1]) != nullptr); + EXPECT_TRUE(dynamic_cast(m.getChildren()[1].get()) != nullptr); } { MultiSearch::Children children; - children.push_back(createIter(0, invert, tfmd, true).release()); - children.push_back(new EmptySearch()); - children.push_back(createIter(1, invert, tfmd, false).release()); + children.push_back(createIter(0, invert, tfmd, true)); + children.emplace_back(new EmptySearch()); + children.push_back(createIter(1, invert, tfmd, false)); - SearchIterator::UP s(T::create(children, false)); + SearchIterator::UP s = T::create(std::move(children), false); s = MultiBitVectorIteratorBase::optimize(std::move(s)); EXPECT_TRUE(s); EXPECT_TRUE(dynamic_cast(s.get()) != nullptr); const MultiSearch & m(dynamic_cast(*s)); EXPECT_EQUAL(2u, m.getChildren().size()); - EXPECT_TRUE(dynamic_cast(m.getChildren()[0]) != nullptr); + EXPECT_TRUE(dynamic_cast(m.getChildren()[0].get()) != nullptr); EXPECT_TRUE(Trinary::True == m.getChildren()[0]->is_strict()); - EXPECT_TRUE(dynamic_cast(m.getChildren()[1]) != nullptr); + EXPECT_TRUE(dynamic_cast(m.getChildren()[1].get()) != nullptr); } { MultiSearch::Children children; - children.push_back(createIter(0, invert, tfmd, true).release()); - children.push_back(createIter(1, invert, tfmd, false).release()); - children.push_back(new EmptySearch()); + children.push_back(createIter(0, invert, tfmd, true)); + children.push_back(createIter(1, invert, tfmd, false)); + children.emplace_back(new EmptySearch()); - SearchIterator::UP s(T::create(children, false)); + SearchIterator::UP s = T::create(std::move(children), false); s = MultiBitVectorIteratorBase::optimize(std::move(s)); EXPECT_TRUE(s); EXPECT_TRUE(dynamic_cast(s.get()) != nullptr); const MultiSearch & m(dynamic_cast(*s)); EXPECT_EQUAL(2u, m.getChildren().size()); - EXPECT_TRUE(dynamic_cast(m.getChildren()[0]) != nullptr); + EXPECT_TRUE(dynamic_cast(m.getChildren()[0].get()) != nullptr); EXPECT_TRUE(Trinary::True == m.getChildren()[0]->is_strict()); - EXPECT_TRUE(dynamic_cast(m.getChildren()[1]) != nullptr); + EXPECT_TRUE(dynamic_cast(m.getChildren()[1].get()) != nullptr); } } @@ -569,9 +569,9 @@ Test::testEndGuard(bool invert) TermFieldMatchData tfmd; MultiSearch::Children children; - children.push_back(createIter(0, invert, tfmd, true).release()); - children.push_back(createIter(1, invert, tfmd, true).release()); - SearchIterator::UP s(T::create(children, false)); + children.push_back(createIter(0, invert, tfmd, true)); + children.push_back(createIter(1, invert, tfmd, true)); + SearchIterator::UP s = T::create(std::move(children), false); s = MultiBitVectorIteratorBase::optimize(std::move(s)); s->initFullRange(); EXPECT_TRUE(s); @@ -618,11 +618,11 @@ SearchIterator::UP Verifier::create(bool strict) const { MultiSearch::Children bvs; for (const auto & bv : _bvs) { - bvs.push_back(BitVectorIterator::create(bv.get(), getDocIdLimit(), _tfmd, strict, false).release()); + bvs.push_back(BitVectorIterator::create(bv.get(), getDocIdLimit(), _tfmd, strict, false)); } - SearchIterator::UP iter(_is_and ? AndSearch::create(bvs, strict) : OrSearch::create(bvs, strict)); + SearchIterator::UP iter(_is_and ? AndSearch::create(std::move(bvs), strict) : OrSearch::create(std::move(bvs), strict)); auto mbvit = MultiBitVectorIteratorBase::optimize(std::move(iter)); - EXPECT_TRUE((bvs.size() < 2) || (dynamic_cast(mbvit.get()) != nullptr)); + EXPECT_TRUE((_bvs.size() < 2) || (dynamic_cast(mbvit.get()) != nullptr)); EXPECT_EQUAL(strict, Trinary::True == mbvit->is_strict()); return mbvit; } diff --git a/searchlib/src/tests/queryeval/parallel_weak_and/parallel_weak_and_test.cpp b/searchlib/src/tests/queryeval/parallel_weak_and/parallel_weak_and_test.cpp index 7926a518317..9761b0da2d7 100644 --- a/searchlib/src/tests/queryeval/parallel_weak_and/parallel_weak_and_test.cpp +++ b/searchlib/src/tests/queryeval/parallel_weak_and/parallel_weak_and_test.cpp @@ -89,14 +89,17 @@ struct WandTestSpec : public WandSpec WandTestSpec(uint32_t scoresToTrack, uint32_t scoresAdjustFrequency = 1, score_t scoreThreshold = 0, double thresholdBoostFactor = 1); ~WandTestSpec(); - SearchIterator *create() { + SearchIterator::UP create() { MatchData::UP childrenMatchData = createMatchData(); MatchData *tmp = childrenMatchData.get(); - return new TrackedSearch("PWAND", getHistory(), ParallelWeakAndSearch::create(getTerms(tmp), - matchParams, - RankParams(rootMatchData, - std::move(childrenMatchData)), - true)); + return SearchIterator::UP( + new TrackedSearch("PWAND", getHistory(), + ParallelWeakAndSearch::create( + getTerms(tmp), + matchParams, + RankParams(rootMatchData, + std::move(childrenMatchData)), + true))); } }; diff --git a/searchlib/src/tests/queryeval/queryeval.cpp b/searchlib/src/tests/queryeval/queryeval.cpp index 29cdd6a4b84..48b91607ab1 100644 --- a/searchlib/src/tests/queryeval/queryeval.cpp +++ b/searchlib/src/tests/queryeval/queryeval.cpp @@ -56,11 +56,13 @@ SearchIterator *simple(const std::string &tag) { return &((new SimpleSearch(SimpleResult()))->tag(tag)); } -Collect search2(const std::string &t1, const std::string &t2) { - return Collect().add(simple(t1)).add(simple(t2)); +MultiSearch::Children search2(const std::string &t1, const std::string &t2) { + MultiSearch::Children children; + children.emplace_back(simple(t1)); + children.emplace_back(simple(t2)); + return children; } - class ISourceSelectorDummy : public ISourceSelector { static SourceStore _sourceStoreDummy; @@ -95,9 +97,9 @@ void testMultiSearch(SearchIterator & search) { TEST("test that OR.andWith is a NOOP") { TermFieldMatchData tfmd; MultiSearch::Children ch; - ch.push_back(new TrueSearch(tfmd)); - ch.push_back(new TrueSearch(tfmd)); - SearchIterator::UP search(OrSearch::create(ch, true)); + ch.emplace_back(new TrueSearch(tfmd)); + ch.emplace_back(new TrueSearch(tfmd)); + SearchIterator::UP search(OrSearch::create(std::move(ch), true)); auto filter = std::make_unique(tfmd); EXPECT_TRUE(search->andWith(std::move(filter), 1)); @@ -106,9 +108,9 @@ TEST("test that OR.andWith is a NOOP") { TEST("test that non-strict AND.andWith is a NOOP") { TermFieldMatchData tfmd; MultiSearch::Children ch; - ch.push_back(new TrueSearch(tfmd)); - ch.push_back(new TrueSearch(tfmd)); - SearchIterator::UP search(AndSearch::create(ch, false)); + ch.emplace_back(new TrueSearch(tfmd)); + ch.emplace_back(new TrueSearch(tfmd)); + SearchIterator::UP search(AndSearch::create(std::move(ch), false)); SearchIterator::UP filter = std::make_unique(tfmd); filter = search->andWith(std::move(filter), 8); EXPECT_TRUE(filter); @@ -116,10 +118,10 @@ TEST("test that non-strict AND.andWith is a NOOP") { TEST("test that strict AND.andWith steals filter and places it correctly based on estimate") { TermFieldMatchData tfmd; - MultiSearch::Children ch; - ch.push_back(new TrueSearch(tfmd)); - ch.push_back(new TrueSearch(tfmd)); - SearchIterator::UP search(AndSearch::create(ch, true)); + std::vector ch; + ch.emplace_back(new TrueSearch(tfmd)); + ch.emplace_back(new TrueSearch(tfmd)); + SearchIterator::UP search(AndSearch::create({ch[0], ch[1]}, true)); static_cast(*search).estimate(7); auto filter = std::make_unique(tfmd); SearchIterator * filterP = filter.get(); @@ -127,18 +129,18 @@ TEST("test that strict AND.andWith steals filter and places it correctly based o EXPECT_TRUE(nullptr == search->andWith(std::move(filter), 8).get()); const MultiSearch::Children & andChildren = static_cast(*search).getChildren(); EXPECT_EQUAL(3u, andChildren.size()); - EXPECT_EQUAL(ch[0], andChildren[0]); - EXPECT_EQUAL(filterP, andChildren[1]); - EXPECT_EQUAL(ch[1], andChildren[2]); + EXPECT_EQUAL(ch[0], andChildren[0].get()); + EXPECT_EQUAL(filterP, andChildren[1].get()); + EXPECT_EQUAL(ch[1], andChildren[2].get()); auto filter2 = std::make_unique(tfmd); SearchIterator * filter2P = filter2.get(); EXPECT_TRUE(nullptr == search->andWith(std::move(filter2), 6).get()); EXPECT_EQUAL(4u, andChildren.size()); - EXPECT_EQUAL(filter2P, andChildren[0]); - EXPECT_EQUAL(ch[0], andChildren[1]); - EXPECT_EQUAL(filterP, andChildren[2]); - EXPECT_EQUAL(ch[1], andChildren[3]); + EXPECT_EQUAL(filter2P, andChildren[0].get()); + EXPECT_EQUAL(ch[0], andChildren[1].get()); + EXPECT_EQUAL(filterP, andChildren[2].get()); + EXPECT_EQUAL(ch[1], andChildren[3].get()); } class NonStrictTrueSearch : public TrueSearch @@ -150,69 +152,45 @@ public: TEST("test that strict AND.andWith does not place non-strict iterator first") { TermFieldMatchData tfmd; - MultiSearch::Children ch; - ch.push_back(new TrueSearch(tfmd)); - ch.push_back(new TrueSearch(tfmd)); - SearchIterator::UP search(AndSearch::create(ch, true)); + std::vector ch; + ch.emplace_back(new TrueSearch(tfmd)); + ch.emplace_back(new TrueSearch(tfmd)); + SearchIterator::UP search(AndSearch::create({ch[0], ch[1]}, true)); static_cast(*search).estimate(7); auto filter = std::make_unique(tfmd); SearchIterator * filterP = filter.get(); EXPECT_TRUE(nullptr == search->andWith(std::move(filter), 6).get()); const MultiSearch::Children & andChildren = static_cast(*search).getChildren(); EXPECT_EQUAL(3u, andChildren.size()); - EXPECT_EQUAL(ch[0], andChildren[0]); - EXPECT_EQUAL(filterP, andChildren[1]); - EXPECT_EQUAL(ch[1], andChildren[2]); + EXPECT_EQUAL(ch[0], andChildren[0].get()); + EXPECT_EQUAL(filterP, andChildren[1].get()); + EXPECT_EQUAL(ch[1], andChildren[2].get()); } TEST("test that strict rank search forwards to its greedy first child") { TermFieldMatchData tfmd; - SearchIterator::UP search( - RankSearch::create( - Collect() - .add(AndSearch::create(search2("a", "b"), true)) - .add(new TrueSearch(tfmd)), - true) - ); + SearchIterator::UP search = RankSearch::create({ AndSearch::create(search2("a", "b"), true), new TrueSearch(tfmd) }, true); auto filter = std::make_unique(tfmd); EXPECT_TRUE(nullptr == search->andWith(std::move(filter), 8).get()); } TEST("test that non-strict rank search does NOT forward to its greedy first child") { TermFieldMatchData tfmd; - SearchIterator::UP search( - RankSearch::create( - Collect() - .add(AndSearch::create(search2("a", "b"), true)) - .add(new TrueSearch(tfmd)), - false) - ); + SearchIterator::UP search = RankSearch::create({ AndSearch::create(search2("a", "b"), true), new TrueSearch(tfmd) }, false); auto filter = std::make_unique(tfmd); EXPECT_TRUE(nullptr != search->andWith(std::move(filter), 8).get()); } TEST("test that strict andnot search forwards to its greedy first child") { TermFieldMatchData tfmd; - SearchIterator::UP search( - AndNotSearch::create( - Collect() - .add(AndSearch::create(search2("a", "b"), true)) - .add(new TrueSearch(tfmd)), - true) - ); + SearchIterator::UP search = AndNotSearch::create({ AndSearch::create(search2("a", "b"), true), new TrueSearch(tfmd) }, true); auto filter = std::make_unique(tfmd); EXPECT_TRUE(nullptr == search->andWith(std::move(filter), 8).get()); } TEST("test that non-strict andnot search does NOT forward to its greedy first child") { TermFieldMatchData tfmd; - SearchIterator::UP search( - AndNotSearch::create( - Collect() - .add(AndSearch::create(search2("a", "b"), true)) - .add(new TrueSearch(tfmd)), - false) - ); + SearchIterator::UP search = AndNotSearch::create({ AndSearch::create(search2("a", "b"), true), new TrueSearch(tfmd) }, false); auto filter = std::make_unique(tfmd); EXPECT_TRUE(nullptr != search->andWith(std::move(filter), 8).get()); } @@ -298,10 +276,10 @@ TEST("testOr") { { TermFieldMatchData tfmd; MultiSearch::Children ch; - ch.push_back(new TrueSearch(tfmd)); - ch.push_back(new TrueSearch(tfmd)); - ch.push_back(new TrueSearch(tfmd)); - SearchIterator::UP orSearch(OrSearch::create(ch, true)); + ch.emplace_back(new TrueSearch(tfmd)); + ch.emplace_back(new TrueSearch(tfmd)); + ch.emplace_back(new TrueSearch(tfmd)); + SearchIterator::UP orSearch(OrSearch::create(std::move(ch), true)); testMultiSearch(*orSearch); } } @@ -309,8 +287,8 @@ TEST("testOr") { class TestInsertRemoveSearch : public MultiSearch { public: - TestInsertRemoveSearch(const MultiSearch::Children & children) : - MultiSearch(children), + TestInsertRemoveSearch(ChildrenIterators children) : + MultiSearch(std::move(children)), _accumRemove(0), _accumInsert(0) { } @@ -327,31 +305,31 @@ struct MultiSearchRemoveTest { }; TEST("testMultiSearch") { - MultiSearch::Children children; - children.push_back(new EmptySearch()); - children.push_back(new EmptySearch()); - children.push_back(new EmptySearch()); - TestInsertRemoveSearch ms(children); + std::vector orig; + orig.emplace_back(new EmptySearch()); + orig.emplace_back(new EmptySearch()); + orig.emplace_back(new EmptySearch()); + TestInsertRemoveSearch ms({orig[0], orig[1], orig[2]}); EXPECT_EQUAL(3u, ms.getChildren().size()); - EXPECT_EQUAL(children[0], ms.getChildren()[0]); - EXPECT_EQUAL(children[1], ms.getChildren()[1]); - EXPECT_EQUAL(children[2], ms.getChildren()[2]); + EXPECT_EQUAL(orig[0], ms.getChildren()[0].get()); + EXPECT_EQUAL(orig[1], ms.getChildren()[1].get()); + EXPECT_EQUAL(orig[2], ms.getChildren()[2].get()); EXPECT_EQUAL(0u, ms._accumInsert); EXPECT_EQUAL(0u, ms._accumRemove); - EXPECT_EQUAL(children[1], MultiSearchRemoveTest::remove(ms, 1).get()); + EXPECT_EQUAL(orig[1], MultiSearchRemoveTest::remove(ms, 1).get()); EXPECT_EQUAL(2u, ms.getChildren().size()); - EXPECT_EQUAL(children[0], ms.getChildren()[0]); - EXPECT_EQUAL(children[2], ms.getChildren()[1]); + EXPECT_EQUAL(orig[0], ms.getChildren()[0].get()); + EXPECT_EQUAL(orig[2], ms.getChildren()[1].get()); EXPECT_EQUAL(0u, ms._accumInsert); EXPECT_EQUAL(1u, ms._accumRemove); - children.push_back(new EmptySearch()); - ms.insert(1, SearchIterator::UP(children.back())); + orig.emplace_back(new EmptySearch()); + ms.insert(1, SearchIterator::UP(orig.back())); EXPECT_EQUAL(3u, ms.getChildren().size()); - EXPECT_EQUAL(children[0], ms.getChildren()[0]); - EXPECT_EQUAL(children[3], ms.getChildren()[1]); - EXPECT_EQUAL(children[2], ms.getChildren()[2]); + EXPECT_EQUAL(orig[0], ms.getChildren()[0].get()); + EXPECT_EQUAL(orig[3], ms.getChildren()[1].get()); + EXPECT_EQUAL(orig[2], ms.getChildren()[2].get()); EXPECT_EQUAL(1u, ms._accumInsert); EXPECT_EQUAL(1u, ms._accumRemove); } @@ -627,23 +605,19 @@ TEST("testDump") { #ifdef __clang__ #pragma clang diagnostic pop #endif - SearchIterator::UP search( - AndSearch::create( - Collect() - .add(AndNotSearch::create(search2("+", "-"), true)) - .add(AndSearch::create(search2("and_a", "and_b"), true)) - .add(new BooleanMatchIteratorWrapper(SearchIterator::UP(simple("wrapped")), TermFieldMatchDataArray())) - .add(new NearSearch(search2("near_a", "near_b"), - TermFieldMatchDataArray(), - 5u, true)) - .add(new ONearSearch(search2("onear_a", "onear_b"), - TermFieldMatchDataArray(), 10, true)) - .add(OrSearch::create(search2("or_a", "or_b"), false)) - .add(RankSearch::create(search2("rank_a", "rank_b"),false)) - .add(SourceBlenderSearch::create(selector(), Collect() - .add(Source(simple("blend_a"), 2)) - .add(Source(simple("blend_b"), 4)), true)) - , true)); + + SearchIterator::UP search = AndSearch::create( { + AndNotSearch::create(search2("+", "-"), true), + AndSearch::create(search2("and_a", "and_b"), true), + new BooleanMatchIteratorWrapper(SearchIterator::UP(simple("wrapped")), TermFieldMatchDataArray()), + new NearSearch(search2("near_a", "near_b"), TermFieldMatchDataArray(), 5u, true), + new ONearSearch(search2("onear_a", "onear_b"), TermFieldMatchDataArray(), 10, true), + OrSearch::create(search2("or_a", "or_b"), false), + RankSearch::create(search2("rank_a", "rank_b"),false), + SourceBlenderSearch::create(selector(), Collect() + .add(Source(simple("blend_a"), 2)) + .add(Source(simple("blend_b"), 4)), + true) }, true); vespalib::string sas = search->asString(); EXPECT_TRUE(sas.size() > 50); vespalib::Slime slime; @@ -846,26 +820,26 @@ TEST("test InitRangeVerifier") { TEST("Test multisearch and andsearchstrict iterators adheres to initRange") { InitRangeVerifier ir; - ir.verify( AndSearch::create({ ir.createIterator(ir.getExpectedDocIds(), false).release(), - ir.createFullIterator().release() }, false)); + ir.verify( AndSearch::create({ ir.createIterator(ir.getExpectedDocIds(), false), + ir.createFullIterator() }, false)); - ir.verify( AndSearch::create({ ir.createIterator(ir.getExpectedDocIds(), true).release(), - ir.createFullIterator().release() }, true)); + ir.verify( AndSearch::create({ ir.createIterator(ir.getExpectedDocIds(), true), + ir.createFullIterator() }, true)); } TEST("Test andnotsearchstrict iterators adheres to initRange") { InitRangeVerifier ir; - TEST_DO(ir.verify( AndNotSearch::create({ir.createIterator(ir.getExpectedDocIds(), false).release(), - ir.createEmptyIterator().release() }, false))); - TEST_DO(ir.verify( AndNotSearch::create({ir.createIterator(ir.getExpectedDocIds(), true).release(), - ir.createEmptyIterator().release() }, true))); + TEST_DO(ir.verify( AndNotSearch::create({ir.createIterator(ir.getExpectedDocIds(), false), + ir.createEmptyIterator() }, false))); + TEST_DO(ir.verify( AndNotSearch::create({ir.createIterator(ir.getExpectedDocIds(), true), + ir.createEmptyIterator() }, true))); auto inverted = InitRangeVerifier::invert(ir.getExpectedDocIds(), ir.getDocIdLimit()); - TEST_DO(ir.verify( AndNotSearch::create({ir.createFullIterator().release(), - ir.createIterator(inverted, false).release() }, false))); - TEST_DO(ir.verify( AndNotSearch::create({ir.createFullIterator().release(), - ir.createIterator(inverted, false).release() }, true))); + TEST_DO(ir.verify( AndNotSearch::create({ir.createFullIterator(), + ir.createIterator(inverted, false) }, false))); + TEST_DO(ir.verify( AndNotSearch::create({ir.createFullIterator(), + ir.createIterator(inverted, false) }, true))); } diff --git a/searchlib/src/tests/queryeval/simple_phrase/simple_phrase_test.cpp b/searchlib/src/tests/queryeval/simple_phrase/simple_phrase_test.cpp index dfe2e2edbd9..d76f13afdca 100644 --- a/searchlib/src/tests/queryeval/simple_phrase/simple_phrase_test.cpp +++ b/searchlib/src/tests/queryeval/simple_phrase/simple_phrase_test.cpp @@ -170,10 +170,12 @@ public: } SimplePhraseSearch::Children children; for (size_t i = 0; i < _children.size(); ++i) { - children.push_back(_children[i]->createSearch(*_md, _strict).release()); + children.push_back(_children[i]->createSearch(*_md, _strict)); } - search = std::make_unique(children, MatchData::UP(), childMatch, _order, - *_md->resolveTermField(phrase_handle), _strict); + search = std::make_unique(std::move(children), + MatchData::UP(), childMatch, _order, + *_md->resolveTermField(phrase_handle), + _strict); } search->initFullRange(); return search.release(); diff --git a/searchlib/src/tests/queryeval/sparse_vector_benchmark/sparse_vector_benchmark_test.cpp b/searchlib/src/tests/queryeval/sparse_vector_benchmark/sparse_vector_benchmark_test.cpp index 1cf39183206..68290e105f1 100644 --- a/searchlib/src/tests/queryeval/sparse_vector_benchmark/sparse_vector_benchmark_test.cpp +++ b/searchlib/src/tests/queryeval/sparse_vector_benchmark/sparse_vector_benchmark_test.cpp @@ -139,19 +139,19 @@ constexpr vespalib::duration max_time = 1000s; struct ChildFactory { ChildFactory() {} virtual std::string name() const = 0; - virtual SearchIterator *createChild(uint32_t idx, uint32_t limit) const = 0; + virtual SearchIterator::UP createChild(uint32_t idx, uint32_t limit) const = 0; virtual ~ChildFactory() {} }; struct SparseVectorFactory { virtual std::string name() const = 0; - virtual SearchIterator *createSparseVector(ChildFactory &childFactory, uint32_t childCnt, uint32_t limit) const = 0; + virtual SearchIterator::UP createSparseVector(ChildFactory &childFactory, uint32_t childCnt, uint32_t limit) const = 0; virtual ~SparseVectorFactory() {} }; struct FilterStrategy { virtual std::string name() const = 0; - virtual SearchIterator *createRoot(SparseVectorFactory &vectorFactory, ChildFactory &childFactory, uint32_t childCnt, uint32_t limit) const = 0; + virtual SearchIterator::UP createRoot(SparseVectorFactory &vectorFactory, ChildFactory &childFactory, uint32_t childCnt, uint32_t limit) const = 0; virtual ~FilterStrategy() {} }; @@ -184,8 +184,8 @@ struct ModSearchFactory : ChildFactory { virtual std::string name() const override { return vespalib::make_string("ModSearch(%u)", bias); } - virtual SearchIterator *createChild(uint32_t idx, uint32_t limit) const override { - return new ModSearch(bias + idx, limit); + SearchIterator::UP createChild(uint32_t idx, uint32_t limit) const override { + return SearchIterator::UP(new ModSearch(bias + idx, limit)); } }; @@ -197,7 +197,7 @@ struct VespaWandFactory : SparseVectorFactory { virtual std::string name() const override { return vespalib::make_string("VespaWand(%u)", n); } - virtual SearchIterator *createSparseVector(ChildFactory &childFactory, uint32_t childCnt, uint32_t limit) const override { + SearchIterator::UP createSparseVector(ChildFactory &childFactory, uint32_t childCnt, uint32_t limit) const override { wand::Terms terms; for (size_t i = 0; i < childCnt; ++i) { terms.push_back(wand::Term(childFactory.createChild(i, limit), default_weight, limit / (i + 1))); @@ -212,12 +212,12 @@ struct RiseWandFactory : SparseVectorFactory { virtual std::string name() const override { return vespalib::make_string("RiseWand(%u)", n); } - virtual SearchIterator *createSparseVector(ChildFactory &childFactory, uint32_t childCnt, uint32_t limit) const override { + SearchIterator::UP createSparseVector(ChildFactory &childFactory, uint32_t childCnt, uint32_t limit) const override { wand::Terms terms; for (size_t i = 0; i < childCnt; ++i) { terms.push_back(wand::Term(childFactory.createChild(i, limit), default_weight, limit / (i + 1))); } - return new rise::TermFrequencyRiseWand(terms, n); + return SearchIterator::UP(new rise::TermFrequencyRiseWand(terms, n)); } }; @@ -226,11 +226,11 @@ struct WeightedSetFactory : SparseVectorFactory { virtual std::string name() const override { return vespalib::make_string("WeightedSet"); } - virtual SearchIterator *createSparseVector(ChildFactory &childFactory, uint32_t childCnt, uint32_t limit) const override { - std::vector terms; + SearchIterator::UP createSparseVector(ChildFactory &childFactory, uint32_t childCnt, uint32_t limit) const override { + std::vector terms; std::vector weights; for (size_t i = 0; i < childCnt; ++i) { - terms.push_back(childFactory.createChild(i, limit)); + terms.push_back(childFactory.createChild(i, limit).release()); weights.push_back(default_weight); } return WeightedSetTermSearch::create(terms, tfmd, weights, MatchData::UP(nullptr)); @@ -242,22 +242,22 @@ struct DotProductFactory : SparseVectorFactory { virtual std::string name() const override { return vespalib::make_string("DotProduct"); } - virtual SearchIterator *createSparseVector(ChildFactory &childFactory, uint32_t childCnt, uint32_t limit) const override { + SearchIterator::UP createSparseVector(ChildFactory &childFactory, uint32_t childCnt, uint32_t limit) const override { MatchDataLayout layout; std::vector handles; for (size_t i = 0; i < childCnt; ++i) { handles.push_back(layout.allocTermField(0)); } - std::vector terms; + std::vector terms; std::vector childMatch; std::vector weights; MatchData::UP md = layout.createMatchData(); for (size_t i = 0; i < childCnt; ++i) { - terms.push_back(childFactory.createChild(i, limit)); + terms.push_back(childFactory.createChild(i, limit).release()); childMatch.push_back(md->resolveTermField(handles[i])); weights.push_back(default_weight); } - return DotProductSearch::create(terms, tfmd, childMatch, weights, std::move(md)).release(); + return DotProductSearch::create(terms, tfmd, childMatch, weights, std::move(md)); } }; @@ -265,12 +265,12 @@ struct OrFactory : SparseVectorFactory { virtual std::string name() const override { return vespalib::make_string("Or"); } - virtual SearchIterator *createSparseVector(ChildFactory &childFactory, uint32_t childCnt, uint32_t limit) const override { + SearchIterator::UP createSparseVector(ChildFactory &childFactory, uint32_t childCnt, uint32_t limit) const override { OrSearch::Children children; for (size_t i = 0; i < childCnt; ++i) { children.push_back(childFactory.createChild(i, limit)); } - return OrSearch::create(children, true); + return OrSearch::create(std::move(children), true); } }; @@ -280,7 +280,7 @@ struct NoFilterStrategy : FilterStrategy { virtual std::string name() const override { return vespalib::make_string("NoFilter"); } - virtual SearchIterator *createRoot(SparseVectorFactory &vectorFactory, ChildFactory &childFactory, uint32_t childCnt, uint32_t limit) const override { + SearchIterator::UP createRoot(SparseVectorFactory &vectorFactory, ChildFactory &childFactory, uint32_t childCnt, uint32_t limit) const override { return vectorFactory.createSparseVector(childFactory, childCnt, limit); } }; @@ -289,11 +289,11 @@ struct PositiveFilterBeforeStrategy : FilterStrategy { virtual std::string name() const override { return vespalib::make_string("PositiveBefore"); } - virtual SearchIterator *createRoot(SparseVectorFactory &vectorFactory, ChildFactory &childFactory, uint32_t childCnt, uint32_t limit) const override { + SearchIterator::UP createRoot(SparseVectorFactory &vectorFactory, ChildFactory &childFactory, uint32_t childCnt, uint32_t limit) const override { AndSearch::Children children; - children.push_back(new ModSearch(2, limit)); // <- 50% hits (hardcoded) + children.emplace_back(new ModSearch(2, limit)); // <- 50% hits (hardcoded) children.push_back(vectorFactory.createSparseVector(childFactory, childCnt, limit)); - return AndSearch::create(children, true); + return AndSearch::create(std::move(children), true); } }; @@ -301,11 +301,11 @@ struct NegativeFilterAfterStrategy : FilterStrategy { virtual std::string name() const override { return vespalib::make_string("NegativeAfter"); } - virtual SearchIterator *createRoot(SparseVectorFactory &vectorFactory, ChildFactory &childFactory, uint32_t childCnt, uint32_t limit) const override { + SearchIterator::UP createRoot(SparseVectorFactory &vectorFactory, ChildFactory &childFactory, uint32_t childCnt, uint32_t limit) const override { AndNotSearch::Children children; children.push_back(vectorFactory.createSparseVector(childFactory, childCnt, limit)); - children.push_back(new ModSearch(2, limit)); // <- 50% hits (hardcoded) - return AndNotSearch::create(children, true); + children.emplace_back(new ModSearch(2, limit)); // <- 50% hits (hardcoded) + return AndNotSearch::create(std::move(children), true); } }; diff --git a/searchlib/src/tests/queryeval/termwise_eval/termwise_eval_test.cpp b/searchlib/src/tests/queryeval/termwise_eval/termwise_eval_test.cpp index 8d70daa4ca8..dee1bdb0b9a 100644 --- a/searchlib/src/tests/queryeval/termwise_eval/termwise_eval_test.cpp +++ b/searchlib/src/tests/queryeval/termwise_eval/termwise_eval_test.cpp @@ -120,32 +120,32 @@ SearchIterator *TERM(std::initializer_list hits, bool strict) { return new MyTerm(hits, strict); } -SearchIterator *ANDNOT(std::initializer_list children, bool strict) { - return AndNotSearch::create(children, strict); +SearchIterator::UP ANDNOT(ChildrenIterators children, bool strict) { + return AndNotSearch::create(std::move(children), strict); } -SearchIterator *AND(std::initializer_list children, bool strict) { - return AndSearch::create(children, strict); +SearchIterator::UP AND(ChildrenIterators children, bool strict) { + return AndSearch::create(std::move(children), strict); } -SearchIterator *ANDz(std::initializer_list children, bool strict) { - return AndSearch::create(children, strict, no_unpack()); +SearchIterator::UP ANDz(ChildrenIterators children, bool strict) { + return AndSearch::create(std::move(children), strict, no_unpack()); } -SearchIterator *ANDs(std::initializer_list children, bool strict) { - return AndSearch::create(children, strict, selective_unpack()); +SearchIterator::UP ANDs(ChildrenIterators children, bool strict) { + return AndSearch::create(std::move(children), strict, selective_unpack()); } -SearchIterator *OR(std::initializer_list children, bool strict) { - return OrSearch::create(children, strict); +SearchIterator::UP OR(ChildrenIterators children, bool strict) { + return OrSearch::create(std::move(children), strict); } -SearchIterator *ORz(std::initializer_list children, bool strict) { - return OrSearch::create(children, strict, no_unpack()); +SearchIterator::UP ORz(ChildrenIterators children, bool strict) { + return OrSearch::create(std::move(children), strict, no_unpack()); } -SearchIterator *ORs(std::initializer_list children, bool strict) { - return OrSearch::create(children, strict, selective_unpack()); +SearchIterator::UP ORs(ChildrenIterators children, bool strict) { + return OrSearch::create(std::move(children), strict, selective_unpack()); } //----------------------------------------------------------------------------- @@ -156,22 +156,22 @@ std::unique_ptr UP(T *t) { return std::unique_ptr(t); } //----------------------------------------------------------------------------- SearchIterator::UP make_search(bool strict) { - return UP(AND({OR({TERM({2,7}, true), - TERM({4,8}, true), - TERM({5,6,9}, true)}, true), - OR({TERM({1,4,7}, false), - TERM({2,5,8}, true), - TERM({3,6}, false)}, false), - OR({TERM({1,2,3}, false), - TERM({4,6}, false), - TERM({8,9}, false)}, false)}, strict)); + return AND({OR({ TERM({2,7}, true), + TERM({4,8}, true), + TERM({5,6,9}, true) }, true), + OR({ TERM({1,4,7}, false), + TERM({2,5,8}, true), + TERM({3,6}, false) }, false), + OR({ TERM({1,2,3}, false), + TERM({4,6}, false), + TERM({8,9}, false) }, false)}, strict); } SearchIterator::UP make_filter_search(bool strict) { - return UP(ANDNOT({TERM({1,2,3,4,5,6,7,8,9}, true), - TERM({1,9}, false), - TERM({3,7}, true), - TERM({5}, false)}, strict)); + return ANDNOT({ TERM({1,2,3,4,5,6,7,8,9}, true), + TERM({1,9}, false), + TERM({3,7}, true), + TERM({5}, false) }, strict); } void add_if_inside(uint32_t docid, uint32_t begin, uint32_t end, std::vector &expect) { @@ -262,7 +262,7 @@ TEST("require that termwise filter search produces appropriate results") { } TEST("require that termwise ANDNOT with single term works") { - TEST_DO(verify({2,3,4}, *make_termwise(UP(ANDNOT({TERM({1,2,3,4,5}, true)}, true)), true), 2, 5)); + TEST_DO(verify({2,3,4}, *make_termwise(ANDNOT({ TERM({1,2,3,4,5}, true) }, true), true), 2, 5)); } TEST("require that pseudo term is rewindable") { @@ -361,20 +361,20 @@ TEST("require that match data keeps track of the termwise limit") { //----------------------------------------------------------------------------- TEST("require that terwise test search string dump is detailed enough") { - EXPECT_EQUAL(make_termwise(UP(OR({TERM({1,2,3}, true), TERM({2,3}, true), TERM({3}, true)}, true)), true)->asString(), - make_termwise(UP(OR({TERM({1,2,3}, true), TERM({2,3}, true), TERM({3}, true)}, true)), true)->asString()); + EXPECT_EQUAL(make_termwise(OR({ TERM({1,2,3}, true), TERM({2,3}, true), TERM({3}, true) }, true), true)->asString(), + make_termwise(OR({ TERM({1,2,3}, true), TERM({2,3}, true), TERM({3}, true) }, true), true)->asString()); - EXPECT_NOT_EQUAL(make_termwise(UP(OR({TERM({1,2,3}, true), TERM({2,3}, true), TERM({3}, true)}, true)), true)->asString(), - make_termwise(UP(OR({TERM({1,2,3}, true), TERM({2,3}, false), TERM({3}, true)}, true)), true)->asString()); + EXPECT_NOT_EQUAL(make_termwise(OR({ TERM({1,2,3}, true), TERM({2,3}, true), TERM({3}, true) }, true), true)->asString(), + make_termwise(OR({ TERM({1,2,3}, true), TERM({2,3}, false), TERM({3}, true) }, true), true)->asString()); - EXPECT_NOT_EQUAL(make_termwise(UP(OR({TERM({1,2,3}, true), TERM({2,3}, true), TERM({3}, true)}, true)), true)->asString(), - make_termwise(UP(OR({TERM({1,2,3}, true), TERM({2,3}, true), TERM({3}, true)}, false)), true)->asString()); + EXPECT_NOT_EQUAL(make_termwise(OR({ TERM({1,2,3}, true), TERM({2,3}, true), TERM({3}, true) }, true), true)->asString(), + make_termwise(OR({ TERM({1,2,3}, true), TERM({2,3}, true), TERM({3}, true) }, false), true)->asString()); - EXPECT_NOT_EQUAL(make_termwise(UP(OR({TERM({1,2,3}, true), TERM({2,3}, true), TERM({3}, true)}, true)), true)->asString(), - make_termwise(UP(OR({TERM({1,2,3}, true), TERM({2,3}, true), TERM({3}, true)}, true)), false)->asString()); + EXPECT_NOT_EQUAL(make_termwise(OR({ TERM({1,2,3}, true), TERM({2,3}, true), TERM({3}, true) }, true), true)->asString(), + make_termwise(OR({ TERM({1,2,3}, true), TERM({2,3}, true), TERM({3}, true) }, true), false)->asString()); - EXPECT_NOT_EQUAL(make_termwise(UP(OR({TERM({1,2,3}, true), TERM({2,3}, true), TERM({3}, true)}, true)), true)->asString(), - make_termwise(UP(OR({TERM({1,2,3}, true), TERM({3}, true), TERM({2,3}, true)}, true)), true)->asString()); + EXPECT_NOT_EQUAL(make_termwise(OR({ TERM({1,2,3}, true), TERM({2,3}, true), TERM({3}, true) }, true), true)->asString(), + make_termwise(OR({ TERM({1,2,3}, true), TERM({3}, true), TERM({2,3}, true) }, true), true)->asString()); } //----------------------------------------------------------------------------- @@ -389,7 +389,7 @@ TEST("require that basic termwise evaluation works") { my_or.addChild(UP(new MyBlueprint({2}, true, 2))); for (bool strict: {true, false}) { EXPECT_EQUAL(my_or.createSearch(*md, strict)->asString(), - make_termwise(UP(OR({TERM({1}, strict), TERM({2}, strict)}, strict)), strict)->asString()); + make_termwise(OR({ TERM({1}, strict), TERM({2}, strict) }, strict), strict)->asString()); } } @@ -434,7 +434,9 @@ TEST("require that termwise evaluation can be multi-level, but not duplicated") my_or.addChild(std::move(child)); for (bool strict: {true, false}) { EXPECT_EQUAL(my_or.createSearch(*md, strict)->asString(), - make_termwise(UP(OR({TERM({1}, strict), ORz({TERM({2}, strict), TERM({3}, strict)}, strict)}, strict)), strict)->asString()); + make_termwise(OR({ TERM({1}, strict), + ORz({ TERM({2}, strict), TERM({3}, strict) }, strict) }, + strict), strict)->asString()); } } @@ -450,7 +452,7 @@ TEST("require that OR can be completely termwise") { my_or.addChild(UP(new MyBlueprint({2}, true, 2))); for (bool strict: {true, false}) { EXPECT_EQUAL(my_or.createSearch(*md, strict)->asString(), - make_termwise(UP(OR({TERM({1}, strict), TERM({2}, strict)}, strict)), strict)->asString()); + make_termwise(OR({ TERM({1}, strict), TERM({2}, strict) }, strict), strict)->asString()); } } @@ -465,7 +467,8 @@ TEST("require that OR can be partially termwise") { my_or.addChild(UP(new MyBlueprint({3}, true, 3))); for (bool strict: {true, false}) { EXPECT_EQUAL(my_or.createSearch(*md, strict)->asString(), - UP(ORs({make_termwise(UP(OR({TERM({1}, strict), TERM({3}, strict)}, strict)), strict).release(), TERM({2}, strict)}, strict))->asString()); + ORs({ make_termwise(OR({ TERM({1}, strict), TERM({3}, strict) }, strict), strict), + TERM({2}, strict) }, strict)->asString()); } } @@ -480,7 +483,9 @@ TEST("require that OR puts termwise subquery at the right place") { my_or.addChild(UP(new MyBlueprint({3}, true, 3))); for (bool strict: {true, false}) { EXPECT_EQUAL(my_or.createSearch(*md, strict)->asString(), - UP(ORs({TERM({1}, strict), make_termwise(UP(OR({TERM({2}, strict), TERM({3}, strict)}, strict)), strict).release()}, strict))->asString()); + ORs({ TERM({1}, strict), + make_termwise(OR({ TERM({2}, strict), TERM({3}, strict) }, strict), + strict) }, strict)->asString()); } } @@ -496,7 +501,10 @@ TEST("require that OR can use termwise eval also when having non-termwise childr my_or.addChild(UP(new MyBlueprint({3}, true, 3))); for (bool strict: {true, false}) { EXPECT_EQUAL(my_or.createSearch(*md, strict)->asString(), - UP(ORz({TERM({1}, strict), make_termwise(UP(OR({TERM({2}, strict), TERM({3}, strict)}, strict)), strict).release()}, strict))->asString()); + ORz({ TERM({1}, strict), + make_termwise(OR({ TERM({2}, strict), TERM({3}, strict) }, strict), + strict)}, + strict)->asString()); } } @@ -512,7 +520,7 @@ TEST("require that AND can be completely termwise") { my_and.addChild(UP(new MyBlueprint({2}, true, 2))); for (bool strict: {true, false}) { EXPECT_EQUAL(my_and.createSearch(*md, strict)->asString(), - make_termwise(UP(AND({TERM({1}, strict), TERM({2}, false)}, strict)), strict)->asString()); + make_termwise(AND({ TERM({1}, strict), TERM({2}, false) }, strict), strict)->asString()); } } @@ -527,7 +535,10 @@ TEST("require that AND can be partially termwise") { my_and.addChild(UP(new MyBlueprint({3}, true, 3))); for (bool strict: {true, false}) { EXPECT_EQUAL(my_and.createSearch(*md, strict)->asString(), - UP(ANDs({make_termwise(UP(AND({TERM({1}, strict), TERM({3}, false)}, strict)), strict).release(), TERM({2}, false)}, strict))->asString()); + ANDs({ make_termwise(AND({ TERM({1}, strict), TERM({3}, false) }, + strict), + strict), + TERM({2}, false) }, strict)->asString()); } } @@ -542,7 +553,9 @@ TEST("require that AND puts termwise subquery at the right place") { my_and.addChild(UP(new MyBlueprint({3}, true, 3))); for (bool strict: {true, false}) { EXPECT_EQUAL(my_and.createSearch(*md, strict)->asString(), - UP(ANDs({TERM({1}, strict), make_termwise(UP(AND({TERM({2}, false), TERM({3}, false)}, false)), false).release()}, strict))->asString()); + ANDs({ TERM({1}, strict), + make_termwise(AND({ TERM({2}, false), TERM({3}, false) }, false), + false) }, strict)->asString()); } } @@ -558,7 +571,9 @@ TEST("require that AND can use termwise eval also when having non-termwise child my_and.addChild(UP(new MyBlueprint({3}, true, 3))); for (bool strict: {true, false}) { EXPECT_EQUAL(my_and.createSearch(*md, strict)->asString(), - UP(ANDz({TERM({1}, strict), make_termwise(UP(AND({TERM({2}, false), TERM({3}, false)}, false)), false).release()}, strict))->asString()); + ANDz({ TERM({1}, strict), + make_termwise(AND({ TERM({2}, false), TERM({3}, false) }, false), + false) }, strict)->asString()); } } @@ -573,7 +588,8 @@ TEST("require that ANDNOT can be completely termwise") { my_andnot.addChild(UP(new MyBlueprint({2}, true, 2))); for (bool strict: {true, false}) { EXPECT_EQUAL(my_andnot.createSearch(*md, strict)->asString(), - make_termwise(UP(ANDNOT({TERM({1}, strict), TERM({2}, false)}, strict)), strict)->asString()); + make_termwise(ANDNOT({ TERM({1}, strict), TERM({2}, false) }, + strict), strict)->asString()); } } @@ -586,7 +602,9 @@ TEST("require that ANDNOT can be partially termwise") { my_andnot.addChild(UP(new MyBlueprint({3}, true, 3))); for (bool strict: {true, false}) { EXPECT_EQUAL(my_andnot.createSearch(*md, strict)->asString(), - UP(ANDNOT({TERM({1}, strict), make_termwise(UP(OR({TERM({2}, false), TERM({3}, false)}, false)), false).release()}, strict))->asString()); + ANDNOT({ TERM({1}, strict), + make_termwise(OR({ TERM({2}, false), TERM({3}, false) }, false), + false) }, strict)->asString()); } } @@ -600,7 +618,9 @@ TEST("require that ANDNOT can be partially termwise with first child being termw my_andnot.addChild(UP(new MyBlueprint({3}, true, 3))); for (bool strict: {true, false}) { EXPECT_EQUAL(my_andnot.createSearch(*md, strict)->asString(), - UP(ANDNOT({make_termwise(UP(ANDNOT({TERM({1}, strict), TERM({3}, false)}, strict)), strict).release(), TERM({2}, false)}, strict))->asString()); + ANDNOT({ make_termwise(ANDNOT({ TERM({1}, strict), TERM({3}, false) }, strict), + strict), + TERM({2}, false) }, strict)->asString()); } } @@ -613,13 +633,13 @@ TEST("require that termwise blueprint helper calculates unpack info correctly") my_or.addChild(UP(new MyBlueprint({3}, true, 3))); my_or.addChild(UP(new MyBlueprint({4}, true, 4))); // ranked my_or.addChild(UP(new MyBlueprint({5}, true, 5))); - MultiSearch::Children dummy_searches(5, nullptr); + MultiSearch::Children dummy_searches(5); UnpackInfo unpack; // non-termwise unpack info unpack.add(1); unpack.add(3); - TermwiseBlueprintHelper helper(my_or, dummy_searches, unpack); - EXPECT_EQUAL(helper.children.size(), 3u); - EXPECT_EQUAL(helper.termwise.size(), 2u); + TermwiseBlueprintHelper helper(my_or, std::move(dummy_searches), unpack); + EXPECT_EQUAL(helper.get_result().size(), 3u); + EXPECT_EQUAL(helper.get_termwise_children().size(), 2u); EXPECT_EQUAL(helper.first_termwise, 2u); EXPECT_TRUE(!helper.termwise_unpack.needUnpack(0)); EXPECT_TRUE(helper.termwise_unpack.needUnpack(1)); diff --git a/searchlib/src/tests/queryeval/weak_and/wand_bench_setup.hpp b/searchlib/src/tests/queryeval/weak_and/wand_bench_setup.hpp index 95e4cc8ea79..f5aafab87c3 100644 --- a/searchlib/src/tests/queryeval/weak_and/wand_bench_setup.hpp +++ b/searchlib/src/tests/queryeval/weak_and/wand_bench_setup.hpp @@ -179,9 +179,9 @@ struct FilterFactory : WandFactory { virtual std::string name() const override { return make_string("Filter (mod=%u) [%s]", n, factory.name().c_str()); } virtual SearchIterator::UP create(const wand::Terms &terms) override { AndNotSearch::Children children; - children.push_back(factory.create(terms).release()); - children.push_back(new ModSearch(stats, n, search::endDocId, n, NULL)); - return SearchIterator::UP(AndNotSearch::create(children, true)); + children.push_back(factory.create(terms)); + children.emplace_back(new ModSearch(stats, n, search::endDocId, n, NULL)); + return AndNotSearch::create(std::move(children), true); } }; diff --git a/searchlib/src/tests/queryeval/weighted_set_term/weighted_set_term_test.cpp b/searchlib/src/tests/queryeval/weighted_set_term/weighted_set_term_test.cpp index 85d2f3f4a37..8514a221230 100644 --- a/searchlib/src/tests/queryeval/weighted_set_term/weighted_set_term_test.cpp +++ b/searchlib/src/tests/queryeval/weighted_set_term/weighted_set_term_test.cpp @@ -123,7 +123,7 @@ struct MockFixture { mock = new MockSearch(initial); children.push_back(mock); weights.push_back(1); - search.reset(WeightedSetTermSearch::create(children, tfmd, weights, MatchData::UP(nullptr))); + search = WeightedSetTermSearch::create(children, tfmd, weights, MatchData::UP(nullptr)); } }; diff --git a/searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp b/searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp index a9a593b429d..74a23db8b95 100644 --- a/searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp +++ b/searchlib/src/vespa/searchlib/attribute/attribute_blueprint_factory.cpp @@ -169,7 +169,8 @@ AttributeFieldBlueprint::visitMembers(vespalib::ObjectVisitor &visitor) const template struct LocationPreFilterIterator : public OrLikeSearch { - LocationPreFilterIterator(const std::vector &children) : OrLikeSearch(children, NoUnpack()) {} + LocationPreFilterIterator(OrSearch::Children children) + : OrLikeSearch(std::move(children), NoUnpack()) {} void doUnpack(uint32_t) override {} }; @@ -215,14 +216,14 @@ public: SearchIterator::UP createLeafSearch(const TermFieldMatchDataArray &tfmda, bool strict) const override { - std::vector children; + OrSearch::Children children; for (auto it(_rangeSearches.begin()), mt(_rangeSearches.end()); it != mt; it++) { - children.push_back((*it)->createIterator(tfmda[0], strict).release()); + children.push_back((*it)->createIterator(tfmda[0], strict)); } if (strict) { - return std::make_unique>(children); + return std::make_unique>(std::move(children)); } else { - return std::make_unique>(children); + return std::make_unique>(std::move(children)); } } diff --git a/searchlib/src/vespa/searchlib/queryeval/CMakeLists.txt b/searchlib/src/vespa/searchlib/queryeval/CMakeLists.txt index c6ca41a2a2a..a080fbcd387 100644 --- a/searchlib/src/vespa/searchlib/queryeval/CMakeLists.txt +++ b/searchlib/src/vespa/searchlib/queryeval/CMakeLists.txt @@ -5,6 +5,7 @@ vespa_add_library(searchlib_queryeval OBJECT andsearch.cpp blueprint.cpp booleanmatchiteratorwrapper.cpp + children_iterators.cpp create_blueprint_visitor_helper.cpp document_weight_search_iterator.cpp dot_product_blueprint.cpp diff --git a/searchlib/src/vespa/searchlib/queryeval/andnotsearch.cpp b/searchlib/src/vespa/searchlib/queryeval/andnotsearch.cpp index 5646af9e677..61d6d2d9259 100644 --- a/searchlib/src/vespa/searchlib/queryeval/andnotsearch.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/andnotsearch.cpp @@ -51,7 +51,7 @@ public: * * @param children the search objects we are andnot'ing **/ - AndNotSearchStrict(const Children & children) : AndNotSearchStrictBase(children) + AndNotSearchStrict(Children children) : AndNotSearchStrictBase(std::move(children)) { } @@ -105,8 +105,8 @@ AndNotSearchStrict::internalSeek(uint32_t docid) } // namespace -OptimizedAndNotForBlackListing::OptimizedAndNotForBlackListing(const MultiSearch::Children & children) : - AndNotSearchStrictBase(children) +OptimizedAndNotForBlackListing::OptimizedAndNotForBlackListing(MultiSearch::Children children) : + AndNotSearchStrictBase(std::move(children)) { } @@ -131,23 +131,24 @@ void OptimizedAndNotForBlackListing::doUnpack(uint32_t docid) positive()->doUnpack(docid); } -SearchIterator * -AndNotSearch::create(const AndNotSearch::Children &children, bool strict) { +std::unique_ptr +AndNotSearch::create(ChildrenIterators children_in, bool strict) { + MultiSearch::Children children = std::move(children_in); if (strict) { - if ((children.size() == 2) && OptimizedAndNotForBlackListing::isBlackListIterator(children[1])) { - return new OptimizedAndNotForBlackListing(children); + if ((children.size() == 2) && OptimizedAndNotForBlackListing::isBlackListIterator(children[1].get())) { + return std::make_unique(std::move(children)); } else { - return new AndNotSearchStrict(children); + return std::make_unique(std::move(children)); } } else { - return new AndNotSearch(children); + return SearchIterator::UP(new AndNotSearch(std::move(children))); } } BitVector::UP AndNotSearch::get_hits(uint32_t begin_id) { const Children &children = getChildren(); - BitVector::UP result = children.front()->get_hits(begin_id); + BitVector::UP result = children[0]->get_hits(begin_id); result->notSelf(); result = TermwiseHelper::orChildren(std::move(result), children.begin()+1, children.end(), begin_id); result->notSelf(); diff --git a/searchlib/src/vespa/searchlib/queryeval/andnotsearch.h b/searchlib/src/vespa/searchlib/queryeval/andnotsearch.h index c29ef9407ad..53a9aad6cca 100644 --- a/searchlib/src/vespa/searchlib/queryeval/andnotsearch.h +++ b/searchlib/src/vespa/searchlib/queryeval/andnotsearch.h @@ -24,11 +24,10 @@ protected: * * @param children the search objects we are andnot'ing **/ - AndNotSearch(const Children & children) : MultiSearch(children) { } + AndNotSearch(MultiSearch::Children children) : MultiSearch(std::move(children)) { } public: - // Caller takes ownership of the returned SearchIterator. - static SearchIterator *create(const Children &children, bool strict); + static std::unique_ptr create(ChildrenIterators children, bool strict); std::unique_ptr get_hits(uint32_t begin_id) override; void or_hits_into(BitVector &result, uint32_t begin_id) override; @@ -43,7 +42,7 @@ private: class AndNotSearchStrictBase : public AndNotSearch { protected: - AndNotSearchStrictBase(const Children & children) : AndNotSearch(children) { } + AndNotSearchStrictBase(Children children) : AndNotSearch(std::move(children)) { } private: Trinary is_strict() const override { return Trinary::True; } UP andWith(UP filter, uint32_t estimate) override; @@ -61,7 +60,7 @@ private: //typedef FilterAttributeIteratorT BlackListIterator; typedef AttributeIteratorT BlackListIterator; public: - OptimizedAndNotForBlackListing(const MultiSearch::Children & children); + OptimizedAndNotForBlackListing(MultiSearch::Children children); static bool isBlackListIterator(const SearchIterator * iterator); uint32_t seekFast(uint32_t docid) { @@ -69,8 +68,8 @@ public: } void initRange(uint32_t beginid, uint32_t endid) override; private: - SearchIterator * positive() { return getChildren()[0]; } - BlackListIterator * blackList() { return static_cast(getChildren()[1]); } + SearchIterator * positive() { return getChildren()[0].get(); } + BlackListIterator * blackList() { return static_cast(getChildren()[1].get()); } template uint32_t internalSeek(uint32_t docid) { uint32_t curr(docid); diff --git a/searchlib/src/vespa/searchlib/queryeval/andsearch.cpp b/searchlib/src/vespa/searchlib/queryeval/andsearch.cpp index 3cbb30e5f89..229cd27afed 100644 --- a/searchlib/src/vespa/searchlib/queryeval/andsearch.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/andsearch.cpp @@ -48,8 +48,8 @@ AndSearch::doUnpack(uint32_t docid) } } -AndSearch::AndSearch(const Children & children) : - MultiSearch(children), +AndSearch::AndSearch(Children children) : + MultiSearch(std::move(children)), _estimate(std::numeric_limits::max()) { } @@ -99,31 +99,37 @@ private: } -AndSearch * -AndSearch::create(const MultiSearch::Children &children, bool strict) +std::unique_ptr +AndSearch::create(ChildrenIterators children, bool strict) { UnpackInfo unpackInfo; unpackInfo.forceAll(); - return create(children, strict, unpackInfo); + return create(std::move(children), strict, unpackInfo); } -AndSearch * -AndSearch::create(const MultiSearch::Children &children, bool strict, const UnpackInfo & unpackInfo) { +std::unique_ptr +AndSearch::create(ChildrenIterators children, bool strict, const UnpackInfo & unpackInfo) { if (strict) { if (unpackInfo.unpackAll()) { - return new AndSearchStrict(children, FullUnpack()); + using MyAnd = AndSearchStrict; + return std::make_unique(std::move(children), FullUnpack()); } else if(unpackInfo.empty()) { - return new AndSearchStrict(children, NoUnpack()); + using MyAnd = AndSearchStrict; + return std::make_unique(std::move(children), NoUnpack()); } else { - return new AndSearchStrict(children, SelectiveUnpack(unpackInfo)); + using MyAnd = AndSearchStrict; + return std::make_unique(std::move(children), SelectiveUnpack(unpackInfo)); } } else { if (unpackInfo.unpackAll()) { - return new AndSearchNoStrict(children, FullUnpack()); + using MyAnd = AndSearchNoStrict; + return std::make_unique(std::move(children), FullUnpack()); } else if (unpackInfo.empty()) { - return new AndSearchNoStrict(children, NoUnpack()); + using MyAnd = AndSearchNoStrict; + return std::make_unique(std::move(children), NoUnpack()); } else { - return new AndSearchNoStrict(children, SelectiveUnpack(unpackInfo)); + using MyAnd = AndSearchNoStrict; + return std::make_unique(std::move(children), SelectiveUnpack(unpackInfo)); } } } diff --git a/searchlib/src/vespa/searchlib/queryeval/andsearch.h b/searchlib/src/vespa/searchlib/queryeval/andsearch.h index 59c5769a1a2..b081951e826 100644 --- a/searchlib/src/vespa/searchlib/queryeval/andsearch.h +++ b/searchlib/src/vespa/searchlib/queryeval/andsearch.h @@ -13,9 +13,8 @@ namespace search::queryeval { class AndSearch : public MultiSearch { public: - // Caller takes ownership of the returned SearchIterator. - static AndSearch *create(const Children &children, bool strict, const UnpackInfo & unpackInfo); - static AndSearch *create(const Children &children, bool strict); + static std::unique_ptr create(ChildrenIterators children, bool strict, const UnpackInfo & unpackInfo); + static std::unique_ptr create(ChildrenIterators children, bool strict); std::unique_ptr get_hits(uint32_t begin_id) override; void or_hits_into(BitVector &result, uint32_t begin_id) override; @@ -24,7 +23,7 @@ public: AndSearch & estimate(uint32_t est) { _estimate = est; return *this; } uint32_t estimate() const { return _estimate; } protected: - AndSearch(const Children & children); + AndSearch(Children children); void doUnpack(uint32_t docid) override; UP andWith(UP filter, uint32_t estimate) override; UP offerFilterToChildren(UP filter, uint32_t estimate); diff --git a/searchlib/src/vespa/searchlib/queryeval/andsearchnostrict.h b/searchlib/src/vespa/searchlib/queryeval/andsearchnostrict.h index 1b1a5e77445..fac502f4c98 100644 --- a/searchlib/src/vespa/searchlib/queryeval/andsearchnostrict.h +++ b/searchlib/src/vespa/searchlib/queryeval/andsearchnostrict.h @@ -22,8 +22,8 @@ public: * @param children the search objects we are and'ing * ownership of the children is taken by the MultiSearch base class. **/ - AndSearchNoStrict(const Children & children, const Unpack & unpacker) : - AndSearch(children), + AndSearchNoStrict(Children children, const Unpack & unpacker) : + AndSearch(std::move(children)), _unpacker(unpacker) { } diff --git a/searchlib/src/vespa/searchlib/queryeval/andsearchstrict.h b/searchlib/src/vespa/searchlib/queryeval/andsearchstrict.h index 87be4248a0a..7ec179404b6 100644 --- a/searchlib/src/vespa/searchlib/queryeval/andsearchstrict.h +++ b/searchlib/src/vespa/searchlib/queryeval/andsearchstrict.h @@ -23,8 +23,8 @@ protected: Trinary is_strict() const override { return Trinary::True; } SearchIterator::UP andWith(SearchIterator::UP filter, uint32_t estimate) override; public: - AndSearchStrict(const MultiSearch::Children & children, const Unpack & unpacker) : - AndSearchNoStrict(children, unpacker) + AndSearchStrict(MultiSearch::Children children, const Unpack & unpacker) + : AndSearchNoStrict(std::move(children), unpacker) { } diff --git a/searchlib/src/vespa/searchlib/queryeval/blueprint.cpp b/searchlib/src/vespa/searchlib/queryeval/blueprint.cpp index 8881bc7da05..1a7a0c5eba2 100644 --- a/searchlib/src/vespa/searchlib/queryeval/blueprint.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/blueprint.cpp @@ -400,10 +400,9 @@ IntermediateBlueprint::createSearch(fef::MatchData &md, bool strict) const subSearches.reserve(_children.size()); for (size_t i = 0; i < _children.size(); ++i) { bool strictChild = (strict && inheritStrict(i)); - SearchIterator::UP search = _children[i]->createSearch(md, strictChild); - subSearches.push_back(search.release()); + subSearches.push_back(_children[i]->createSearch(md, strictChild)); } - return createIntermediateSearch(subSearches, strict, md); + return createIntermediateSearch(std::move(subSearches), strict, md); } IntermediateBlueprint::IntermediateBlueprint() = default; diff --git a/searchlib/src/vespa/searchlib/queryeval/blueprint.h b/searchlib/src/vespa/searchlib/queryeval/blueprint.h index 10a68e45d27..ef15736073e 100644 --- a/searchlib/src/vespa/searchlib/queryeval/blueprint.h +++ b/searchlib/src/vespa/searchlib/queryeval/blueprint.h @@ -6,6 +6,7 @@ #include "unpackinfo.h" #include "executeinfo.h" #include "global_filter.h" +#include "multisearch.h" #include namespace vespalib { class ObjectVisitor; } @@ -297,7 +298,7 @@ public: virtual void sort(std::vector &children) const = 0; virtual bool inheritStrict(size_t i) const = 0; virtual SearchIteratorUP - createIntermediateSearch(const std::vector &subSearches, + createIntermediateSearch(MultiSearch::Children subSearches, bool strict, fef::MatchData &md) const = 0; void visitMembers(vespalib::ObjectVisitor &visitor) const override; diff --git a/searchlib/src/vespa/searchlib/queryeval/children_iterators.cpp b/searchlib/src/vespa/searchlib/queryeval/children_iterators.cpp new file mode 100644 index 00000000000..3abbd6a1b81 --- /dev/null +++ b/searchlib/src/vespa/searchlib/queryeval/children_iterators.cpp @@ -0,0 +1,3 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "children_iterators.h" diff --git a/searchlib/src/vespa/searchlib/queryeval/children_iterators.h b/searchlib/src/vespa/searchlib/queryeval/children_iterators.h new file mode 100644 index 00000000000..aa147e0299b --- /dev/null +++ b/searchlib/src/vespa/searchlib/queryeval/children_iterators.h @@ -0,0 +1,39 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include "searchiterator.h" + +namespace search::queryeval { + +/** + * Convenience for constructing MultiSearch::Children + * holding them or passing ownership around. + **/ +class ChildrenIterators { + private: + std::vector _data; + public: + ChildrenIterators(std::vector data) + : _data(std::move(data)) {} + ChildrenIterators(ChildrenIterators && other) = default; + + // convenience constructors for unit tests: + template + ChildrenIterators(SearchIterator::UP a, Args&&... args) { + _data.reserve(1 + sizeof...(Args)); + _data.push_back(std::move(a)); + (_data.emplace_back(std::forward(args)), ...); + } + template + ChildrenIterators(SearchIterator *a, Args&&... args) { + _data.reserve(1 + sizeof...(Args)); + _data.emplace_back(a); + (_data.emplace_back(std::forward(args)), ...); + } + operator std::vector () && { + return std::move(_data); + } +}; + +} // namespace diff --git a/searchlib/src/vespa/searchlib/queryeval/equiv_blueprint.cpp b/searchlib/src/vespa/searchlib/queryeval/equiv_blueprint.cpp index 8a798e6cce3..6d044ca337d 100644 --- a/searchlib/src/vespa/searchlib/queryeval/equiv_blueprint.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/equiv_blueprint.cpp @@ -57,7 +57,8 @@ SearchIterator::UP EquivBlueprint::createLeafSearch(const fef::TermFieldMatchDataArray &outputs, bool strict) const { fef::MatchData::UP md = _layout.createMatchData(); - MultiSearch::Children children(_terms.size()); + MultiSearch::Children children; + children.reserve(_terms.size()); fef::TermMatchDataMerger::Inputs childMatch; vespalib::hash_map unpack_needs(outputs.size()); for (size_t i = 0; i < outputs.size(); ++i) { @@ -70,20 +71,21 @@ EquivBlueprint::createLeafSearch(const fef::TermFieldMatchDataArray &outputs, bo unpack_needs[child_term_field_match_data->getFieldId()].notify(*child_term_field_match_data); childMatch.emplace_back(child_term_field_match_data, _exactness[i]); } - children[i] = _terms[i]->createSearch(*md, strict).release(); + children.push_back(_terms[i]->createSearch(*md, strict)); } - return SearchIterator::UP(EquivSearch::create(children, std::move(md), childMatch, outputs, strict)); + return EquivSearch::create(std::move(children), std::move(md), childMatch, outputs, strict); } SearchIterator::UP EquivBlueprint::createFilterSearch(bool strict, FilterConstraint constraint) const { - MultiSearch::Children children(_terms.size()); + MultiSearch::Children children; + children.reserve(_terms.size()); for (size_t i = 0; i < _terms.size(); ++i) { - children[i] = _terms[i]->createFilterSearch(strict, constraint).release(); + children.push_back(_terms[i]->createFilterSearch(strict, constraint)); } UnpackInfo unpack_info; - return SearchIterator::UP(OrSearch::create(children, strict, unpack_info)); + return OrSearch::create(std::move(children), strict, unpack_info); } void diff --git a/searchlib/src/vespa/searchlib/queryeval/equivsearch.cpp b/searchlib/src/vespa/searchlib/queryeval/equivsearch.cpp index 593701fd14f..95af4da01b0 100644 --- a/searchlib/src/vespa/searchlib/queryeval/equivsearch.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/equivsearch.cpp @@ -21,19 +21,19 @@ public: * * @param children the search objects that should be equivalent **/ - EquivImpl(const MultiSearch::Children &children, + EquivImpl(MultiSearch::Children children, fef::MatchData::UP inputMatchData, const fef::TermMatchDataMerger::Inputs &inputs, const fef::TermFieldMatchDataArray &outputs); }; template -EquivImpl::EquivImpl(const MultiSearch::Children &children, +EquivImpl::EquivImpl(MultiSearch::Children children, fef::MatchData::UP inputMatchData, const fef::TermMatchDataMerger::Inputs &inputs, const fef::TermFieldMatchDataArray &outputs) - : OrLikeSearch(children, NoUnpack()), + : OrLikeSearch(std::move(children), NoUnpack()), _inputMatchData(std::move(inputMatchData)), _merger(inputs, outputs), _valid(outputs.valid()) @@ -50,17 +50,17 @@ EquivImpl::doUnpack(uint32_t docid) } } -SearchIterator * -EquivSearch::create(const Children &children, +SearchIterator::UP +EquivSearch::create(Children children, fef::MatchData::UP inputMatchData, const fef::TermMatchDataMerger::Inputs &inputs, const fef::TermFieldMatchDataArray &outputs, bool strict) { if (strict) { - return new EquivImpl(children, std::move(inputMatchData), inputs, outputs); + return std::make_unique>(std::move(children), std::move(inputMatchData), inputs, outputs); } else { - return new EquivImpl(children, std::move(inputMatchData), inputs, outputs); + return std::make_unique>(std::move(children), std::move(inputMatchData), inputs, outputs); } } diff --git a/searchlib/src/vespa/searchlib/queryeval/equivsearch.h b/searchlib/src/vespa/searchlib/queryeval/equivsearch.h index 252e17e610a..7dc7f90ee23 100644 --- a/searchlib/src/vespa/searchlib/queryeval/equivsearch.h +++ b/searchlib/src/vespa/searchlib/queryeval/equivsearch.h @@ -18,12 +18,12 @@ class EquivSearch : public SearchIterator public: typedef MultiSearch::Children Children; - // Caller takes ownership of the returned SearchIterator. - static SearchIterator *create(const Children &children, - fef::MatchData::UP inputMD, - const fef::TermMatchDataMerger::Inputs &inputs, - const fef::TermFieldMatchDataArray &outputs, - bool strict); + static SearchIterator::UP + create(Children children, + fef::MatchData::UP inputMD, + const fef::TermMatchDataMerger::Inputs &inputs, + const fef::TermFieldMatchDataArray &outputs, + bool strict); }; } diff --git a/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.cpp b/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.cpp index 14e53d6d868..fd6a399e92f 100644 --- a/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.cpp @@ -91,12 +91,12 @@ SearchIterator::UP createAndFilter(const IntermediateBlueprint &self, for (size_t i = 0; i < self.childCnt(); ++i) { bool child_strict = strict && (i == 0); auto search = self.getChild(i).createFilterSearch(child_strict, constraint); - sub_searches.push_back(search.release()); + sub_searches.push_back(std::move(search)); } UnpackInfo unpack_info; - AndSearch * search = AndSearch::create(sub_searches, strict, unpack_info); + auto search = AndSearch::create(std::move(sub_searches), strict, unpack_info); search->estimate(self.getState().estimate().estHits); - return SearchIterator::UP(search); + return search; } } // namespace search::queryeval:: @@ -166,23 +166,24 @@ AndNotBlueprint::inheritStrict(size_t i) const } SearchIterator::UP -AndNotBlueprint::createIntermediateSearch(const MultiSearch::Children &sub_searches, +AndNotBlueprint::createIntermediateSearch(MultiSearch::Children sub_searches, bool strict, search::fef::MatchData &md) const { UnpackInfo unpack_info(calculateUnpackInfo(md)); if (should_do_termwise_eval(unpack_info, md.get_termwise_limit())) { - TermwiseBlueprintHelper helper(*this, sub_searches, unpack_info); + TermwiseBlueprintHelper helper(*this, std::move(sub_searches), unpack_info); bool termwise_strict = (strict && inheritStrict(helper.first_termwise)); auto termwise_search = (helper.first_termwise == 0) - ? SearchIterator::UP(AndNotSearch::create(helper.termwise, termwise_strict)) - : SearchIterator::UP(OrSearch::create(helper.termwise, termwise_strict)); + ? AndNotSearch::create(helper.get_termwise_children(), termwise_strict) + : OrSearch::create(helper.get_termwise_children(), termwise_strict); helper.insert_termwise(std::move(termwise_search), termwise_strict); - if (helper.children.size() == 1) { - return SearchIterator::UP(helper.children.front()); + auto rearranged = helper.get_result(); + if (rearranged.size() == 1) { + return std::move(rearranged[0]); } - return SearchIterator::UP(AndNotSearch::create(helper.children, strict)); + return AndNotSearch::create(std::move(rearranged), strict); } - return SearchIterator::UP(AndNotSearch::create(sub_searches, strict)); + return AndNotSearch::create(std::move(sub_searches), strict); } namespace { @@ -207,9 +208,9 @@ AndNotBlueprint::createFilterSearch(bool strict, FilterConstraint constraint) co auto search = (i == 0) ? getChild(i).createFilterSearch(child_strict, constraint) : getChild(i).createFilterSearch(child_strict, invert(constraint)); - sub_searches.push_back(search.release()); + sub_searches.push_back(std::move(search)); } - return SearchIterator::UP(AndNotSearch::create(sub_searches, strict)); + return AndNotSearch::create(std::move(sub_searches), strict); } //----------------------------------------------------------------------------- @@ -265,26 +266,27 @@ AndBlueprint::inheritStrict(size_t i) const } SearchIterator::UP -AndBlueprint::createIntermediateSearch(const MultiSearch::Children &sub_searches, +AndBlueprint::createIntermediateSearch(MultiSearch::Children sub_searches, bool strict, search::fef::MatchData & md) const { UnpackInfo unpack_info(calculateUnpackInfo(md)); - AndSearch * search = 0; + std::unique_ptr search; if (should_do_termwise_eval(unpack_info, md.get_termwise_limit())) { - TermwiseBlueprintHelper helper(*this, sub_searches, unpack_info); + TermwiseBlueprintHelper helper(*this, std::move(sub_searches), unpack_info); bool termwise_strict = (strict && inheritStrict(helper.first_termwise)); - auto termwise_search = SearchIterator::UP(AndSearch::create(helper.termwise, termwise_strict)); + auto termwise_search = AndSearch::create(helper.get_termwise_children(), termwise_strict); helper.insert_termwise(std::move(termwise_search), termwise_strict); - if (helper.children.size() == 1) { - return SearchIterator::UP(helper.children.front()); + auto rearranged = helper.get_result(); + if (rearranged.size() == 1) { + return std::move(rearranged[0]); } else { - search = AndSearch::create(helper.children, strict, helper.termwise_unpack); + search = AndSearch::create(std::move(rearranged), strict, helper.termwise_unpack); } } else { - search = AndSearch::create(sub_searches, strict, unpack_info); + search = AndSearch::create(std::move(sub_searches), strict, unpack_info); } search->estimate(getState().estimate().estHits); - return SearchIterator::UP(search); + return search; } SearchIterator::UP @@ -353,21 +355,22 @@ OrBlueprint::inheritStrict(size_t) const } SearchIterator::UP -OrBlueprint::createIntermediateSearch(const MultiSearch::Children &sub_searches, +OrBlueprint::createIntermediateSearch(MultiSearch::Children sub_searches, bool strict, search::fef::MatchData & md) const { UnpackInfo unpack_info(calculateUnpackInfo(md)); if (should_do_termwise_eval(unpack_info, md.get_termwise_limit())) { - TermwiseBlueprintHelper helper(*this, sub_searches, unpack_info); + TermwiseBlueprintHelper helper(*this, std::move(sub_searches), unpack_info); bool termwise_strict = (strict && inheritStrict(helper.first_termwise)); - auto termwise_search = SearchIterator::UP(OrSearch::create(helper.termwise, termwise_strict)); + auto termwise_search = OrSearch::create(helper.get_termwise_children(), termwise_strict); helper.insert_termwise(std::move(termwise_search), termwise_strict); - if (helper.children.size() == 1) { - return SearchIterator::UP(helper.children.front()); + auto rearranged = helper.get_result(); + if (rearranged.size() == 1) { + return std::move(rearranged[0]); } - return SearchIterator::UP(OrSearch::create(helper.children, strict, helper.termwise_unpack)); + return OrSearch::create(std::move(rearranged), strict, helper.termwise_unpack); } - return SearchIterator::UP(OrSearch::create(sub_searches, strict, unpack_info)); + return OrSearch::create(std::move(sub_searches), strict, unpack_info); } SearchIterator::UP @@ -378,10 +381,10 @@ OrBlueprint::createFilterSearch(bool strict, FilterConstraint constraint) const for (size_t i = 0; i < childCnt(); ++i) { bool child_strict = strict && inheritStrict(i); auto search = getChild(i).createFilterSearch(child_strict, constraint); - sub_searches.push_back(search.release()); + sub_searches.push_back(std::move(search)); } UnpackInfo unpack_info; - return SearchIterator::UP(OrSearch::create(sub_searches, strict, unpack_info)); + return OrSearch::create(std::move(sub_searches), strict, unpack_info); } //----------------------------------------------------------------------------- @@ -423,18 +426,18 @@ WeakAndBlueprint::always_needs_unpack() const } SearchIterator::UP -WeakAndBlueprint::createIntermediateSearch(const MultiSearch::Children &sub_searches, +WeakAndBlueprint::createIntermediateSearch(MultiSearch::Children sub_searches, bool strict, search::fef::MatchData &) const { WeakAndSearch::Terms terms; assert(sub_searches.size() == childCnt()); assert(_weights.size() == childCnt()); for (size_t i = 0; i < sub_searches.size(); ++i) { - terms.push_back(wand::Term(sub_searches[i], + terms.push_back(wand::Term(sub_searches[i].release(), _weights[i], getChild(i).getState().estimate().estHits)); } - return SearchIterator::UP(WeakAndSearch::create(terms, _n, strict)); + return WeakAndSearch::create(terms, _n, strict); } //----------------------------------------------------------------------------- @@ -471,7 +474,7 @@ NearBlueprint::createSearch(fef::MatchData &md, bool strict) const } SearchIterator::UP -NearBlueprint::createIntermediateSearch(const MultiSearch::Children &sub_searches, +NearBlueprint::createIntermediateSearch(MultiSearch::Children sub_searches, bool strict, search::fef::MatchData &md) const { search::fef::TermFieldMatchDataArray tfmda; @@ -481,7 +484,7 @@ NearBlueprint::createIntermediateSearch(const MultiSearch::Children &sub_searche tfmda.add(cs.field(j).resolve(md)); } } - return SearchIterator::UP(new NearSearch(sub_searches, tfmda, _window, strict)); + return SearchIterator::UP(new NearSearch(std::move(sub_searches), tfmda, _window, strict)); } SearchIterator::UP @@ -529,7 +532,7 @@ ONearBlueprint::createSearch(fef::MatchData &md, bool strict) const } SearchIterator::UP -ONearBlueprint::createIntermediateSearch(const MultiSearch::Children &sub_searches, +ONearBlueprint::createIntermediateSearch(MultiSearch::Children sub_searches, bool strict, search::fef::MatchData &md) const { search::fef::TermFieldMatchDataArray tfmda; @@ -541,7 +544,7 @@ ONearBlueprint::createIntermediateSearch(const MultiSearch::Children &sub_search } // could sort sub_searches here // but then strictness inheritance would also need to be fixed - return SearchIterator::UP(new ONearSearch(sub_searches, tfmda, _window, strict)); + return SearchIterator::UP(new ONearSearch(std::move(sub_searches), tfmda, _window, strict)); } SearchIterator::UP @@ -604,27 +607,27 @@ RankBlueprint::inheritStrict(size_t i) const } SearchIterator::UP -RankBlueprint::createIntermediateSearch(const MultiSearch::Children &sub_searches, +RankBlueprint::createIntermediateSearch(MultiSearch::Children sub_searches, bool strict, search::fef::MatchData & md) const { UnpackInfo unpack_info(calculateUnpackInfo(md)); if (unpack_info.unpackAll()) { - return SearchIterator::UP(RankSearch::create(sub_searches, strict)); + return RankSearch::create(std::move(sub_searches), strict); } else { MultiSearch::Children require_unpack; require_unpack.reserve(sub_searches.size()); - require_unpack.push_back(sub_searches[0]); + require_unpack.push_back(std::move(sub_searches[0])); for (size_t i(1); i < sub_searches.size(); i++) { if (unpack_info.needUnpack(i)) { - require_unpack.push_back(sub_searches[i]); + require_unpack.push_back(std::move(sub_searches[i])); } else { - delete sub_searches[i]; + sub_searches[i].reset(); } } if (require_unpack.size() == 1) { - return SearchIterator::UP(require_unpack[0]); + return SearchIterator::UP(std::move(require_unpack[0])); } else { - return SearchIterator::UP(RankSearch::create(require_unpack, strict)); + return RankSearch::create(std::move(require_unpack), strict); } } } @@ -688,18 +691,18 @@ SourceBlenderBlueprint::findSource(uint32_t sourceId) const } SearchIterator::UP -SourceBlenderBlueprint::createIntermediateSearch(const MultiSearch::Children &sub_searches, +SourceBlenderBlueprint::createIntermediateSearch(MultiSearch::Children sub_searches, bool strict, search::fef::MatchData &) const { SourceBlenderSearch::Children children; assert(sub_searches.size() == childCnt()); for (size_t i = 0; i < sub_searches.size(); ++i) { - children.push_back(SourceBlenderSearch::Child(sub_searches[i], + children.push_back(SourceBlenderSearch::Child(sub_searches[i].release(), getChild(i).getSourceId())); assert(children.back().sourceId != 0xffffffff); } - return SearchIterator::UP(SourceBlenderSearch::create(_selector.createIterator(), - children, strict)); + return SourceBlenderSearch::create(_selector.createIterator(), + children, strict); } SearchIterator::UP @@ -711,10 +714,10 @@ SourceBlenderBlueprint::createFilterSearch(bool strict, FilterConstraint constra for (size_t i = 0; i < childCnt(); ++i) { bool child_strict = strict && inheritStrict(i); auto search = getChild(i).createFilterSearch(child_strict, constraint); - sub_searches.push_back(search.release()); + sub_searches.push_back(std::move(search)); } UnpackInfo unpack_info; - return SearchIterator::UP(OrSearch::create(sub_searches, strict, unpack_info)); + return OrSearch::create(std::move(sub_searches), strict, unpack_info); } else { return std::make_unique(); } diff --git a/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.h b/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.h index 65dced2ea6b..6bbe4562641 100644 --- a/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.h +++ b/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.h @@ -22,7 +22,7 @@ public: void sort(std::vector &children) const override; bool inheritStrict(size_t i) const override; SearchIterator::UP - createIntermediateSearch(const MultiSearch::Children &subSearches, + createIntermediateSearch(MultiSearch::Children subSearches, bool strict, fef::MatchData &md) const override; SearchIterator::UP createFilterSearch(bool strict, FilterConstraint constraint) const override; @@ -49,7 +49,7 @@ public: void sort(std::vector &children) const override; bool inheritStrict(size_t i) const override; SearchIterator::UP - createIntermediateSearch(const MultiSearch::Children &subSearches, + createIntermediateSearch(MultiSearch::Children subSearches, bool strict, fef::MatchData &md) const override; SearchIterator::UP createFilterSearch(bool strict, FilterConstraint constraint) const override; @@ -69,7 +69,7 @@ public: void sort(std::vector &children) const override; bool inheritStrict(size_t i) const override; SearchIterator::UP - createIntermediateSearch(const MultiSearch::Children &subSearches, + createIntermediateSearch(MultiSearch::Children subSearches, bool strict, fef::MatchData &md) const override; SearchIterator::UP createFilterSearch(bool strict, FilterConstraint constraint) const override; @@ -90,7 +90,7 @@ public: bool inheritStrict(size_t i) const override; bool always_needs_unpack() const override; SearchIterator::UP - createIntermediateSearch(const MultiSearch::Children &subSearches, + createIntermediateSearch(MultiSearch::Children subSearches, bool strict, fef::MatchData &md) const override; WeakAndBlueprint(uint32_t n) : _n(n) {} @@ -118,7 +118,7 @@ public: bool inheritStrict(size_t i) const override; SearchIteratorUP createSearch(fef::MatchData &md, bool strict) const override; SearchIterator::UP - createIntermediateSearch(const MultiSearch::Children &subSearches, + createIntermediateSearch(MultiSearch::Children subSearches, bool strict, fef::MatchData &md) const override; SearchIterator::UP createFilterSearch(bool strict, FilterConstraint constraint) const override; @@ -140,7 +140,7 @@ public: bool inheritStrict(size_t i) const override; SearchIteratorUP createSearch(fef::MatchData &md, bool strict) const override; SearchIterator::UP - createIntermediateSearch(const MultiSearch::Children &subSearches, + createIntermediateSearch(MultiSearch::Children subSearches, bool strict, fef::MatchData &md) const override; SearchIterator::UP createFilterSearch(bool strict, FilterConstraint constraint) const override; @@ -159,7 +159,7 @@ public: void sort(std::vector &children) const override; bool inheritStrict(size_t i) const override; SearchIterator::UP - createIntermediateSearch(const MultiSearch::Children &subSearches, + createIntermediateSearch(MultiSearch::Children subSearches, bool strict, fef::MatchData &md) const override; SearchIterator::UP createFilterSearch(bool strict, FilterConstraint constraint) const override; @@ -185,7 +185,7 @@ public: */ ssize_t findSource(uint32_t sourceId) const; SearchIterator::UP - createIntermediateSearch(const MultiSearch::Children &subSearches, + createIntermediateSearch(MultiSearch::Children subSearches, bool strict, fef::MatchData &md) const override; SearchIterator::UP createFilterSearch(bool strict, FilterConstraint constraint) const override; diff --git a/searchlib/src/vespa/searchlib/queryeval/multibitvectoriterator.cpp b/searchlib/src/vespa/searchlib/queryeval/multibitvectoriterator.cpp index e9e34b4f5ce..105d57b22b1 100644 --- a/searchlib/src/vespa/searchlib/queryeval/multibitvectoriterator.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/multibitvectoriterator.cpp @@ -21,7 +21,7 @@ template class MultiBitVectorIterator : public MultiBitVectorIteratorBase { public: - MultiBitVectorIterator(const Children & children) : MultiBitVectorIteratorBase(children) { } + MultiBitVectorIterator(Children children) : MultiBitVectorIteratorBase(std::move(children)) { } protected: void updateLastValue(uint32_t docId); void strictSeek(uint32_t docId); @@ -36,7 +36,7 @@ template class MultiBitVectorIteratorStrict : public MultiBitVectorIterator { public: - MultiBitVectorIteratorStrict(const MultiSearch::Children & children) : MultiBitVectorIterator(children) { } + MultiBitVectorIteratorStrict(MultiSearch::Children children) : MultiBitVectorIterator(std::move(children)) { } private: void doSeek(uint32_t docId) override { this->strictSeek(docId); } Trinary is_strict() const override { return Trinary::True; } @@ -112,7 +112,7 @@ typedef MultiBitVectorIteratorStrict OrBVIteratorStrict; bool hasAtLeast2Bitvectors(const MultiSearch::Children & children) { size_t count(0); - for (const SearchIterator * search : children) { + for (const auto & search : children) { if (search->isBitVector()) { count++; } @@ -133,16 +133,16 @@ bool canOptimize(const MultiSearch & s) { } -MultiBitVectorIteratorBase::MultiBitVectorIteratorBase(const Children & children) : - MultiSearch(children), +MultiBitVectorIteratorBase::MultiBitVectorIteratorBase(Children children) : + MultiSearch(std::move(children)), _numDocs(std::numeric_limits::max()), _lastValue(0), _lastMaxDocIdLimit(0), _bvs() { - _bvs.reserve(children.size()); - for (size_t i(0); i < children.size(); i++) { - const auto * bv = static_cast(children[i]); + _bvs.reserve(getChildren().size()); + for (size_t i(0); i < getChildren().size(); i++) { + const auto * bv = static_cast(getChildren()[i].get()); _bvs.emplace_back(reinterpret_cast(bv->getBitValues()), bv->isInverted()); _numDocs = std::min(_numDocs, bv->getDocIdLimit()); } @@ -178,8 +178,8 @@ MultiBitVectorIteratorBase::doUnpack(uint32_t docid) } else { auto &children = getChildren(); _unpackInfo.each([&children,docid](size_t i) { - static_cast(children[i])->unpack(docid); - }, children.size()); + static_cast(children[i].get())->unpack(docid); + }, children.size()); } } @@ -218,7 +218,7 @@ MultiBitVectorIteratorBase::optimizeMultiSearch(SearchIterator::UP parentIt) if ( ! strict && (bit->is_strict() == Trinary::True)) { strict = true; } - stolen.push_back(bit.release()); + stolen.push_back(std::move(bit)); } else { it++; } @@ -226,21 +226,21 @@ MultiBitVectorIteratorBase::optimizeMultiSearch(SearchIterator::UP parentIt) SearchIterator::UP next; if (parent.isAnd()) { if (strict) { - next = std::make_unique(stolen); + next = std::make_unique(std::move(stolen)); } else { - next = std::make_unique(stolen); + next = std::make_unique(std::move(stolen)); } } else if (parent.isOr()) { if (strict) { - next = std::make_unique(stolen); + next = std::make_unique(std::move(stolen)); } else { - next = std::make_unique(stolen); + next = std::make_unique(std::move(stolen)); } } else if (parent.isAndNot()) { if (strict) { - next = std::make_unique(stolen); + next = std::make_unique(std::move(stolen)); } else { - next = std::make_unique(stolen); + next = std::make_unique(std::move(stolen)); } } auto & nextM(static_cast(*next)); @@ -254,8 +254,8 @@ MultiBitVectorIteratorBase::optimizeMultiSearch(SearchIterator::UP parentIt) } } auto & toOptimize(const_cast(parent.getChildren())); - for (SearchIterator * & search : toOptimize) { - search = optimize(MultiSearch::UP(search)).release(); + for (auto & search : toOptimize) { + search = optimize(std::move(search)); } return parentIt; diff --git a/searchlib/src/vespa/searchlib/queryeval/multibitvectoriterator.h b/searchlib/src/vespa/searchlib/queryeval/multibitvectoriterator.h index cc40f834114..cde9ffcbfe5 100644 --- a/searchlib/src/vespa/searchlib/queryeval/multibitvectoriterator.h +++ b/searchlib/src/vespa/searchlib/queryeval/multibitvectoriterator.h @@ -20,7 +20,7 @@ public: */ static SearchIterator::UP optimize(SearchIterator::UP parent); protected: - MultiBitVectorIteratorBase(const Children & children); + MultiBitVectorIteratorBase(Children children); class MetaWord { public: MetaWord(const Word * words, bool inverted) : _words(words), _inverted(inverted) { } diff --git a/searchlib/src/vespa/searchlib/queryeval/multisearch.cpp b/searchlib/src/vespa/searchlib/queryeval/multisearch.cpp index b63a54785a4..51098f50b37 100644 --- a/searchlib/src/vespa/searchlib/queryeval/multisearch.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/multisearch.cpp @@ -10,7 +10,7 @@ void MultiSearch::insert(size_t index, SearchIterator::UP search) { assert(index <= _children.size()); - _children.insert(_children.begin()+index, search.release()); + _children.insert(_children.begin()+index, std::move(search)); onInsert(index); } @@ -18,7 +18,7 @@ SearchIterator::UP MultiSearch::remove(size_t index) { assert(index < _children.size()); - SearchIterator::UP search(_children[index]); + SearchIterator::UP search = std::move(_children[index]); _children.erase(_children.begin() + index); onRemove(index); return search; @@ -27,7 +27,7 @@ MultiSearch::remove(size_t index) void MultiSearch::doUnpack(uint32_t docid) { - for (SearchIterator *child: _children) { + for (auto &child: _children) { if (__builtin_expect(child->getDocId() < docid, false)) { child->doSeek(docid); } @@ -37,23 +37,20 @@ MultiSearch::doUnpack(uint32_t docid) } } -MultiSearch::MultiSearch(const Children & children) - : _children(children) +MultiSearch::MultiSearch(Children children) + : _children(std::move(children)) { } MultiSearch::~MultiSearch() { - for (SearchIterator * child : _children) { - delete child; - } } void MultiSearch::initRange(uint32_t beginid, uint32_t endid) { SearchIterator::initRange(beginid, endid); - for (SearchIterator * child : _children) { + for (auto & child : _children) { child->initRange(beginid, endid); } } diff --git a/searchlib/src/vespa/searchlib/queryeval/multisearch.h b/searchlib/src/vespa/searchlib/queryeval/multisearch.h index af96734b26a..bd916f7953b 100644 --- a/searchlib/src/vespa/searchlib/queryeval/multisearch.h +++ b/searchlib/src/vespa/searchlib/queryeval/multisearch.h @@ -3,6 +3,7 @@ #pragma once #include "searchiterator.h" +#include "children_iterators.h" struct MultiSearchRemoveTest; @@ -12,25 +13,26 @@ class MultiBitVectorIteratorBase; /** * A virtual intermediate class that serves as the basis for combining searches - * like and, or any or others that take a list of children. + * like AND, OR, RANK or others that take a list of children. **/ class MultiSearch : public SearchIterator { friend struct ::MultiSearchRemoveTest; friend class ::search::queryeval::MultiBitVectorIteratorBase; + friend class MySearch; public: /** - * Defines how to represent the children iterators. vespalib::Array usage - * generates faster and more compact code then using std::vector. + * Defines how to represent the children iterators. */ - typedef std::vector Children; + using Children = std::vector; + /** * Create a new Multi Search with the given children. * * @param children the search objects we are and'ing * this object takes ownership of the children. **/ - MultiSearch(const Children & children); + MultiSearch(Children children); virtual ~MultiSearch() override; const Children & getChildren() const { return _children; } virtual bool isAnd() const { return false; } @@ -40,6 +42,7 @@ public: virtual bool needUnpack(size_t index) const { (void) index; return true; } void initRange(uint32_t beginId, uint32_t endId) override; protected: + MultiSearch() {} void doUnpack(uint32_t docid) override; void visitMembers(vespalib::ObjectVisitor &visitor) const override; private: diff --git a/searchlib/src/vespa/searchlib/queryeval/nearsearch.cpp b/searchlib/src/vespa/searchlib/queryeval/nearsearch.cpp index 4ecd84fb7c3..f7b83de5f4b 100644 --- a/searchlib/src/vespa/searchlib/queryeval/nearsearch.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/nearsearch.cpp @@ -31,11 +31,11 @@ void setup_fields(uint32_t window, std::vector &matchers, const TermFieldMatc } // namespace search::queryeval:: -NearSearchBase::NearSearchBase(const Children & terms, +NearSearchBase::NearSearchBase(Children terms, const TermFieldMatchDataArray &data, uint32_t window, bool strict) - : AndSearch(terms), + : AndSearch(std::move(terms)), _data_size(data.size()), _window(window), _strict(strict) @@ -107,8 +107,7 @@ NearSearchBase::doSeek(uint32_t docId) const Children & terms(getChildren()); bool foundHit = true; for (uint32_t i = 0, len = terms.size(); i < len; ++i) { - SearchIterator *term = terms[i]; - if (!term->seek(docId)) { + if (! terms[i]->seek(docId)) { LOG(debug, "Term %d does not occur in document %d.", i, docId); foundHit = false; break; @@ -123,11 +122,11 @@ NearSearchBase::doSeek(uint32_t docId) } } -NearSearch::NearSearch(const Children & terms, +NearSearch::NearSearch(Children terms, const TermFieldMatchDataArray &data, uint32_t window, bool strict) - : NearSearchBase(terms, data, window, strict), + : NearSearchBase(std::move(terms), data, window, strict), _matchers() { setup_fields(window, _matchers, data); @@ -224,11 +223,11 @@ NearSearch::match(uint32_t docId) return false; } -ONearSearch::ONearSearch(const Children & terms, +ONearSearch::ONearSearch(Children terms, const TermFieldMatchDataArray &data, uint32_t window, bool strict) - : NearSearchBase(terms, data, window, strict), + : NearSearchBase(std::move(terms), data, window, strict), _matchers() { setup_fields(window, _matchers, data); diff --git a/searchlib/src/vespa/searchlib/queryeval/nearsearch.h b/searchlib/src/vespa/searchlib/queryeval/nearsearch.h index 27c862735b1..13bd1c53383 100644 --- a/searchlib/src/vespa/searchlib/queryeval/nearsearch.h +++ b/searchlib/src/vespa/searchlib/queryeval/nearsearch.h @@ -72,7 +72,7 @@ public: * @param window The size of the window in which all terms must occur. * @param strict Whether or not to skip to next matching document if seek fails. */ - NearSearchBase(const Children & terms, + NearSearchBase(Children terms, const TermFieldMatchDataArray &data, uint32_t window, bool strict); @@ -106,7 +106,7 @@ public: * @param window The size of the window in which all terms must occur. * @param strict Whether or not to skip to next matching document if seek fails. */ - NearSearch(const Children & terms, + NearSearch(Children terms, const TermFieldMatchDataArray &data, uint32_t window, bool strict = true); @@ -138,7 +138,7 @@ public: * @param window The size of the window in which all terms must occur. * @param strict Whether or not to skip to next matching document if seek fails. */ - ONearSearch(const Children & terms, + ONearSearch(Children terms, const TermFieldMatchDataArray &data, uint32_t window, bool strict = true); diff --git a/searchlib/src/vespa/searchlib/queryeval/orlikesearch.h b/searchlib/src/vespa/searchlib/queryeval/orlikesearch.h index 1dec50ffc97..57a2ff11997 100644 --- a/searchlib/src/vespa/searchlib/queryeval/orlikesearch.h +++ b/searchlib/src/vespa/searchlib/queryeval/orlikesearch.h @@ -46,8 +46,8 @@ public: * * @param children the search objects we are or'ing **/ - OrLikeSearch(const Children &children, const Unpack & unpacker) : - OrSearch(children), + OrLikeSearch(Children children, const Unpack & unpacker) + : OrSearch(std::move(children)), _unpacker(unpacker) { } private: diff --git a/searchlib/src/vespa/searchlib/queryeval/orsearch.cpp b/searchlib/src/vespa/searchlib/queryeval/orsearch.cpp index b5e0ff9c423..9d1dd252796 100644 --- a/searchlib/src/vespa/searchlib/queryeval/orsearch.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/orsearch.cpp @@ -78,30 +78,36 @@ OrSearch::or_hits_into(BitVector &result, uint32_t begin_id) TermwiseHelper::orChildren(result, getChildren().begin(), getChildren().end(), begin_id); } -SearchIterator * -OrSearch::create(const MultiSearch::Children &children, bool strict) { +SearchIterator::UP +OrSearch::create(ChildrenIterators children, bool strict) { UnpackInfo unpackInfo; unpackInfo.forceAll(); - return create(children, strict, unpackInfo); + return create(std::move(children), strict, unpackInfo); } -SearchIterator * -OrSearch::create(const MultiSearch::Children &children, bool strict, const UnpackInfo & unpackInfo) { +SearchIterator::UP +OrSearch::create(ChildrenIterators children, bool strict, const UnpackInfo & unpackInfo) { if (strict) { if (unpackInfo.unpackAll()) { - return new OrLikeSearch(children, FullUnpack()); + using MyOr = OrLikeSearch; + return std::make_unique(std::move(children), FullUnpack()); } else if(unpackInfo.empty()) { - return new OrLikeSearch(children, NoUnpack()); + using MyOr = OrLikeSearch; + return std::make_unique(std::move(children), NoUnpack()); } else { - return new OrLikeSearch(children, SelectiveUnpack(unpackInfo)); + using MyOr = OrLikeSearch; + return std::make_unique(std::move(children), SelectiveUnpack(unpackInfo)); } } else { if (unpackInfo.unpackAll()) { - return new OrLikeSearch(children, FullUnpack()); + using MyOr = OrLikeSearch; + return std::make_unique(std::move(children), FullUnpack()); } else if(unpackInfo.empty()) { - return new OrLikeSearch(children, NoUnpack()); + using MyOr = OrLikeSearch; + return std::make_unique(std::move(children), NoUnpack()); } else { - return new OrLikeSearch(children, SelectiveUnpack(unpackInfo)); + using MyOr = OrLikeSearch; + return std::make_unique(std::move(children), SelectiveUnpack(unpackInfo)); } } } diff --git a/searchlib/src/vespa/searchlib/queryeval/orsearch.h b/searchlib/src/vespa/searchlib/queryeval/orsearch.h index e3d74573db8..79da1b85b6f 100644 --- a/searchlib/src/vespa/searchlib/queryeval/orsearch.h +++ b/searchlib/src/vespa/searchlib/queryeval/orsearch.h @@ -15,16 +15,15 @@ class OrSearch : public MultiSearch public: typedef MultiSearch::Children Children; - // Caller takes ownership of the returned SearchIterator. - static SearchIterator *create(const Children &children, bool strict); - static SearchIterator *create(const Children &children, bool strict, const UnpackInfo & unpackInfo); + static SearchIterator::UP create(ChildrenIterators children, bool strict); + static SearchIterator::UP create(ChildrenIterators children, bool strict, const UnpackInfo & unpackInfo); std::unique_ptr get_hits(uint32_t begin_id) override; void or_hits_into(BitVector &result, uint32_t begin_id) override; void and_hits_into(BitVector &result, uint32_t begin_id) override; protected: - OrSearch(const Children & children) : MultiSearch(children) { } + OrSearch(Children children) : MultiSearch(std::move(children)) { } private: bool isOr() const override { return true; } diff --git a/searchlib/src/vespa/searchlib/queryeval/ranksearch.cpp b/searchlib/src/vespa/searchlib/queryeval/ranksearch.cpp index 2bcf2267b1d..a915442bf0c 100644 --- a/searchlib/src/vespa/searchlib/queryeval/ranksearch.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/ranksearch.cpp @@ -32,7 +32,7 @@ public: * * @param children the search objects we are rank'ing **/ - RankSearchStrict(const Children & children) : RankSearch(children) { } + RankSearchStrict(Children children) : RankSearch(std::move(children)) { } }; SearchIterator::UP @@ -49,12 +49,12 @@ RankSearchStrict::doSeek(uint32_t docid) } } // namespace -SearchIterator * -RankSearch::create(const RankSearch::Children &children, bool strict) { +SearchIterator::UP +RankSearch::create(ChildrenIterators children, bool strict) { if (strict) { - return new RankSearchStrict(children); + return UP(new RankSearchStrict(std::move(children))); } else { - return new RankSearch(children); + return UP(new RankSearch(std::move(children))); } } diff --git a/searchlib/src/vespa/searchlib/queryeval/ranksearch.h b/searchlib/src/vespa/searchlib/queryeval/ranksearch.h index 60efed3c694..443202f3e59 100644 --- a/searchlib/src/vespa/searchlib/queryeval/ranksearch.h +++ b/searchlib/src/vespa/searchlib/queryeval/ranksearch.h @@ -20,11 +20,10 @@ protected: * * @param children the search objects we are rank'ing **/ - RankSearch(const Children & children) : MultiSearch(children) { } + RankSearch(Children children) : MultiSearch(std::move(children)) { } public: - // Caller takes ownership of the returned SearchIterator. - static SearchIterator *create(const Children &children, bool strict); + static SearchIterator::UP create(ChildrenIterators children, bool strict); }; } diff --git a/searchlib/src/vespa/searchlib/queryeval/simple_phrase_blueprint.cpp b/searchlib/src/vespa/searchlib/queryeval/simple_phrase_blueprint.cpp index fc29c7ef45d..83d5a5e1739 100644 --- a/searchlib/src/vespa/searchlib/queryeval/simple_phrase_blueprint.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/simple_phrase_blueprint.cpp @@ -61,7 +61,8 @@ SimplePhraseBlueprint::createLeafSearch(const fef::TermFieldMatchDataArray &tfmd assert(tfmda.size() == 1); fef::MatchData::UP md = _layout.createMatchData(); fef::TermFieldMatchDataArray childMatch; - SimplePhraseSearch::Children children(_terms.size()); + SimplePhraseSearch::Children children; + children.reserve(_terms.size()); std::multimap order_map; for (size_t i = 0; i < _terms.size(); ++i) { const State &childState = _terms[i]->getState(); @@ -70,7 +71,7 @@ SimplePhraseBlueprint::createLeafSearch(const fef::TermFieldMatchDataArray &tfmd child_term_field_match_data->setNeedInterleavedFeatures(tfmda[0]->needs_interleaved_features()); child_term_field_match_data->setNeedNormalFeatures(true); childMatch.add(child_term_field_match_data); - children[i] = _terms[i]->createSearch(*md, strict).release(); + children.push_back(_terms[i]->createSearch(*md, strict)); order_map.insert(std::make_pair(childState.estimate().estHits, i)); } std::vector eval_order; @@ -78,7 +79,8 @@ SimplePhraseBlueprint::createLeafSearch(const fef::TermFieldMatchDataArray &tfmd eval_order.push_back(child.second); } - auto phrase = std::make_unique(children, std::move(md), childMatch, + auto phrase = std::make_unique(std::move(children), + std::move(md), childMatch, eval_order, *tfmda[0], strict); phrase->setDoom(& _doom); return phrase; @@ -88,13 +90,14 @@ SearchIterator::UP SimplePhraseBlueprint::createFilterSearch(bool strict, FilterConstraint constraint) const { if (constraint == FilterConstraint::UPPER_BOUND) { - MultiSearch::Children children(_terms.size()); + MultiSearch::Children children; + children.reserve(_terms.size()); for (size_t i = 0; i < _terms.size(); ++i) { bool child_strict = strict && (i == 0); - children[i] = _terms[i]->createFilterSearch(child_strict, constraint).release(); + children.push_back(_terms[i]->createFilterSearch(child_strict, constraint)); } UnpackInfo unpack_info; - return SearchIterator::UP(AndSearch::create(children, strict, unpack_info)); + return AndSearch::create(std::move(children), strict, unpack_info); } else { return std::make_unique(); } diff --git a/searchlib/src/vespa/searchlib/queryeval/simple_phrase_search.cpp b/searchlib/src/vespa/searchlib/queryeval/simple_phrase_search.cpp index df0dff06582..43a9ee4ab91 100644 --- a/searchlib/src/vespa/searchlib/queryeval/simple_phrase_search.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/simple_phrase_search.cpp @@ -157,23 +157,23 @@ SimplePhraseSearch::phraseSeek(uint32_t doc_id) { } -SimplePhraseSearch::SimplePhraseSearch(const Children &children, +SimplePhraseSearch::SimplePhraseSearch(Children children, fef::MatchData::UP md, const fef::TermFieldMatchDataArray &childMatch, vector eval_order, TermFieldMatchData &tmd, bool strict) - : AndSearch(children), + : AndSearch(std::move(children)), _md(std::move(md)), _childMatch(childMatch), _eval_order(std::move(eval_order)), _tmd(tmd), _doom(nullptr), _strict(strict), - _iterators(children.size()) + _iterators(getChildren().size()) { - assert(!children.empty()); - assert(children.size() == _childMatch.size()); - assert(children.size() == _eval_order.size()); + assert(getChildren().size() > 0); + assert(getChildren().size() == _childMatch.size()); + assert(getChildren().size() == _eval_order.size()); } void diff --git a/searchlib/src/vespa/searchlib/queryeval/simple_phrase_search.h b/searchlib/src/vespa/searchlib/queryeval/simple_phrase_search.h index d45e67ed4cb..5cec039e733 100644 --- a/searchlib/src/vespa/searchlib/queryeval/simple_phrase_search.h +++ b/searchlib/src/vespa/searchlib/queryeval/simple_phrase_search.h @@ -43,7 +43,7 @@ public: * terms. The term with fewest hits should be * evaluated first. **/ - SimplePhraseSearch(const Children &children, + SimplePhraseSearch(Children children, fef::MatchData::UP md, const fef::TermFieldMatchDataArray &childMatch, std::vector eval_order, diff --git a/searchlib/src/vespa/searchlib/queryeval/sourceblendersearch.cpp b/searchlib/src/vespa/searchlib/queryeval/sourceblendersearch.cpp index 3280e0ac2cc..282e3bef104 100644 --- a/searchlib/src/vespa/searchlib/queryeval/sourceblendersearch.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/sourceblendersearch.cpp @@ -158,14 +158,14 @@ SourceBlenderSearch::setChild(size_t index, SearchIterator::UP child) { _sources[_children[index]] = child.release(); } -SourceBlenderSearch * +SearchIterator::UP SourceBlenderSearch::create(std::unique_ptr sourceSelector, const Children &children, bool strict) { if (strict) { - return new SourceBlenderSearchStrict(std::move(sourceSelector), children); + return SearchIterator::UP(new SourceBlenderSearchStrict(std::move(sourceSelector), children)); } else { - return new SourceBlenderSearch(std::move(sourceSelector), children); + return SearchIterator::UP(new SourceBlenderSearch(std::move(sourceSelector), children)); } } diff --git a/searchlib/src/vespa/searchlib/queryeval/sourceblendersearch.h b/searchlib/src/vespa/searchlib/queryeval/sourceblendersearch.h index 9b4d8f58034..f209e0f7fd8 100644 --- a/searchlib/src/vespa/searchlib/queryeval/sourceblendersearch.h +++ b/searchlib/src/vespa/searchlib/queryeval/sourceblendersearch.h @@ -68,8 +68,9 @@ public: * @param strict whether this search is strict * (a strict search will locate its next hit when seeking fails) **/ - static SourceBlenderSearch * create(std::unique_ptr sourceSelector, - const Children &children, bool strict); + static SearchIterator::UP create(std::unique_ptr sourceSelector, + const Children &children, + bool strict); ~SourceBlenderSearch() override; size_t getNumChildren() const { return _children.size(); } SearchIterator::UP steal(size_t index) { diff --git a/searchlib/src/vespa/searchlib/queryeval/termwise_blueprint_helper.cpp b/searchlib/src/vespa/searchlib/queryeval/termwise_blueprint_helper.cpp index ae21fd93ba3..e26a1652441 100644 --- a/searchlib/src/vespa/searchlib/queryeval/termwise_blueprint_helper.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/termwise_blueprint_helper.cpp @@ -6,27 +6,27 @@ namespace search::queryeval { TermwiseBlueprintHelper::TermwiseBlueprintHelper(const IntermediateBlueprint &self, - const MultiSearch::Children &subSearches, + MultiSearch::Children subSearches, UnpackInfo &unpackInfo) - : children(), - termwise(), + : termwise_ch(), + other_ch(), first_termwise(subSearches.size()), termwise_unpack() { - children.reserve(subSearches.size()); - termwise.reserve(subSearches.size()); + other_ch.reserve(subSearches.size()); + termwise_ch.reserve(subSearches.size()); for (size_t i = 0; i < subSearches.size(); ++i) { bool need_unpack = unpackInfo.needUnpack(i); bool allow_termwise = self.getChild(i).getState().allow_termwise_eval(); if (need_unpack || !allow_termwise) { if (need_unpack) { - size_t index = (i < first_termwise) ? children.size() : (children.size() + 1); + size_t index = (i < first_termwise) ? other_ch.size() : (other_ch.size() + 1); termwise_unpack.add(index); } - children.push_back(subSearches[i]); + other_ch.push_back(std::move(subSearches[i])); } else { first_termwise = std::min(i, first_termwise); - termwise.push_back(subSearches[i]); + termwise_ch.push_back(std::move(subSearches[i])); } } } @@ -37,7 +37,7 @@ void TermwiseBlueprintHelper::insert_termwise(SearchIterator::UP search, bool strict) { auto termwise_search = make_termwise(std::move(search), strict); - children.insert(children.begin() + first_termwise, termwise_search.release()); + other_ch.insert(other_ch.begin() + first_termwise, std::move(termwise_search)); } } diff --git a/searchlib/src/vespa/searchlib/queryeval/termwise_blueprint_helper.h b/searchlib/src/vespa/searchlib/queryeval/termwise_blueprint_helper.h index e6b46cfb7d2..2917ce4d8c9 100644 --- a/searchlib/src/vespa/searchlib/queryeval/termwise_blueprint_helper.h +++ b/searchlib/src/vespa/searchlib/queryeval/termwise_blueprint_helper.h @@ -15,13 +15,18 @@ namespace search::queryeval { * termwise and non-termwise parts with each other. **/ struct TermwiseBlueprintHelper { - MultiSearch::Children children; - MultiSearch::Children termwise; +private: + MultiSearch::Children termwise_ch; + MultiSearch::Children other_ch; +public: size_t first_termwise; UnpackInfo termwise_unpack; + MultiSearch::Children get_termwise_children() { return std::move(termwise_ch); } + MultiSearch::Children get_result() { return std::move(other_ch); } + TermwiseBlueprintHelper(const IntermediateBlueprint &self, - const MultiSearch::Children &subSearches, UnpackInfo &unpackInfo); + MultiSearch::Children subSearches, UnpackInfo &unpackInfo); ~TermwiseBlueprintHelper(); void insert_termwise(SearchIterator::UP search, bool strict); diff --git a/searchlib/src/vespa/searchlib/queryeval/test/leafspec.h b/searchlib/src/vespa/searchlib/queryeval/test/leafspec.h index 47b5ed26b60..fa14941844e 100644 --- a/searchlib/src/vespa/searchlib/queryeval/test/leafspec.h +++ b/searchlib/src/vespa/searchlib/queryeval/test/leafspec.h @@ -18,7 +18,7 @@ struct LeafSpec int32_t weight; int32_t maxWeight; FakeResult result; - SearchIterator *search; + SearchIterator::UP search; LeafSpec(const std::string &n, int32_t w = 100) : name(n), weight(w), @@ -26,31 +26,36 @@ struct LeafSpec result(), search() {} + LeafSpec(LeafSpec && other) = default; ~LeafSpec() {} - LeafSpec &doc(uint32_t docid) { + LeafSpec && doc(uint32_t docid) && { result.doc(docid); - return *this; + return std::move(*this); } - LeafSpec &doc(uint32_t docid, int32_t w) { + LeafSpec && doc(uint32_t docid, int32_t w) && { result.doc(docid); result.weight(w); result.pos(0); maxWeight = std::max(maxWeight, w); - return *this; + return std::move(*this); } - LeafSpec &itr(SearchIterator *si) { - search = si; - return *this; + LeafSpec && itr(SearchIterator::UP si) && { + search = std::move(si); + return std::move(*this); } - SearchIterator *create(SearchHistory &hist, fef::TermFieldMatchData *tfmd) const { - if (search != nullptr) { - return new TrackedSearch(name, hist, search); + LeafSpec && itr(SearchIterator *si) && { + search.reset(si); + return std::move(*this); + } + SearchIterator::UP create(SearchHistory &hist, fef::TermFieldMatchData *tfmd) { + if (search) { + return SearchIterator::UP(new TrackedSearch(name, hist, std::move(search))); } else if (tfmd != nullptr) { - return new TrackedSearch(name, hist, result, *tfmd, - MinMaxPostingInfo(0, maxWeight)); + return SearchIterator::UP(new TrackedSearch(name, hist, result, *tfmd, + MinMaxPostingInfo(0, maxWeight))); } - return new TrackedSearch(name, hist, result, - MinMaxPostingInfo(0, maxWeight)); + return SearchIterator::UP(new TrackedSearch(name, hist, result, + MinMaxPostingInfo(0, maxWeight))); } }; diff --git a/searchlib/src/vespa/searchlib/queryeval/test/trackedsearch.h b/searchlib/src/vespa/searchlib/queryeval/test/trackedsearch.h index 6cb4c1a9dda..ae04d35e658 100644 --- a/searchlib/src/vespa/searchlib/queryeval/test/trackedsearch.h +++ b/searchlib/src/vespa/searchlib/queryeval/test/trackedsearch.h @@ -56,6 +56,12 @@ public: _search(new FakeSearch("", "", "", result, makeArray(tfmd))), _minMaxPostingInfo(new MinMaxPostingInfo(minMaxPostingInfo)) { setDocId(_search->getDocId()); } + + // wraps a generic search (typically wand) + TrackedSearch(const std::string &name, SearchHistory &hist, SearchIterator::UP search) + : _name(name), _history(hist), _matchData(), _search(std::move(search)), _minMaxPostingInfo() + { setDocId(_search->getDocId()); } + // wraps a generic search (typically wand) TrackedSearch(const std::string &name, SearchHistory &hist, SearchIterator *search) : _name(name), _history(hist), _matchData(), _search(search), _minMaxPostingInfo() diff --git a/searchlib/src/vespa/searchlib/queryeval/test/wandspec.h b/searchlib/src/vespa/searchlib/queryeval/test/wandspec.h index bf456c287d6..2fb2b3bc9e2 100644 --- a/searchlib/src/vespa/searchlib/queryeval/test/wandspec.h +++ b/searchlib/src/vespa/searchlib/queryeval/test/wandspec.h @@ -26,8 +26,8 @@ private: public: WandSpec() : _leafs(), _layout(), _handles(), _history() {} ~WandSpec() {} - WandSpec &leaf(const LeafSpec &l) { - _leafs.push_back(l); + WandSpec &leaf(LeafSpec && l) { + _leafs.emplace_back(std::move(l)); _handles.push_back(_layout.allocTermField(0)); return *this; } @@ -35,7 +35,7 @@ public: wand::Terms terms; for (size_t i = 0; i < _leafs.size(); ++i) { fef::TermFieldMatchData *tfmd = (matchData != NULL ? matchData->resolveTermField(_handles[i]) : NULL); - terms.push_back(wand::Term(_leafs[i].create(_history, tfmd), + terms.push_back(wand::Term(_leafs[i].create(_history, tfmd).release(), _leafs[i].weight, _leafs[i].result.inspect().size(), tfmd)); diff --git a/searchlib/src/vespa/searchlib/queryeval/wand/wand_parts.h b/searchlib/src/vespa/searchlib/queryeval/wand/wand_parts.h index bd60473e05d..6c4d9209da1 100644 --- a/searchlib/src/vespa/searchlib/queryeval/wand/wand_parts.h +++ b/searchlib/src/vespa/searchlib/queryeval/wand/wand_parts.h @@ -44,6 +44,7 @@ struct Term { : search(s), weight(w), estHits(e), matchData(tfmd) {} Term() : Term(nullptr, 0, 0, nullptr){} Term(SearchIterator *s, int32_t w, uint32_t e) : Term(s, w, e, nullptr) {} + Term(SearchIterator::UP s, int32_t w, uint32_t e) : Term(s.release(), w, e, nullptr) {} }; //----------------------------------------------------------------------------- diff --git a/searchlib/src/vespa/searchlib/queryeval/wand/weak_and_search.cpp b/searchlib/src/vespa/searchlib/queryeval/wand/weak_and_search.cpp index f290d3e19ea..d94dc6d8ae8 100644 --- a/searchlib/src/vespa/searchlib/queryeval/wand/weak_and_search.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/wand/weak_and_search.cpp @@ -106,27 +106,27 @@ WeakAndSearch::visitMembers(vespalib::ObjectVisitor &visitor) const //----------------------------------------------------------------------------- -SearchIterator * +SearchIterator::UP WeakAndSearch::createArrayWand(const Terms &terms, uint32_t n, bool strict) { if (strict) { - return new wand::WeakAndSearchLR(terms, n); + return SearchIterator::UP(new wand::WeakAndSearchLR(terms, n)); } else { - return new wand::WeakAndSearchLR(terms, n); + return SearchIterator::UP(new wand::WeakAndSearchLR(terms, n)); } } -SearchIterator * +SearchIterator::UP WeakAndSearch::createHeapWand(const Terms &terms, uint32_t n, bool strict) { if (strict) { - return new wand::WeakAndSearchLR(terms, n); + return SearchIterator::UP(new wand::WeakAndSearchLR(terms, n)); } else { - return new wand::WeakAndSearchLR(terms, n); + return SearchIterator::UP(new wand::WeakAndSearchLR(terms, n)); } } -SearchIterator * +SearchIterator::UP WeakAndSearch::create(const Terms &terms, uint32_t n, bool strict) { if (terms.size() < 128) { diff --git a/searchlib/src/vespa/searchlib/queryeval/wand/weak_and_search.h b/searchlib/src/vespa/searchlib/queryeval/wand/weak_and_search.h index 5b09d087873..e51b5ca102d 100644 --- a/searchlib/src/vespa/searchlib/queryeval/wand/weak_and_search.h +++ b/searchlib/src/vespa/searchlib/queryeval/wand/weak_and_search.h @@ -16,9 +16,9 @@ struct WeakAndSearch : SearchIterator { virtual const Terms &getTerms() const = 0; virtual uint32_t getN() const = 0; void visitMembers(vespalib::ObjectVisitor &visitor) const override; - static SearchIterator *createArrayWand(const Terms &terms, uint32_t n, bool strict); - static SearchIterator *createHeapWand(const Terms &terms, uint32_t n, bool strict); - static SearchIterator *create(const Terms &terms, uint32_t n, bool strict); + static SearchIterator::UP createArrayWand(const Terms &terms, uint32_t n, bool strict); + static SearchIterator::UP createHeapWand(const Terms &terms, uint32_t n, bool strict); + static SearchIterator::UP create(const Terms &terms, uint32_t n, bool strict); }; } // namespace queryeval diff --git a/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_search.cpp b/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_search.cpp index 2801f1c5e0c..71270c84c63 100644 --- a/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_search.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_search.cpp @@ -131,8 +131,8 @@ public: //----------------------------------------------------------------------------- -SearchIterator * -WeightedSetTermSearch::create(const std::vector &children, +SearchIterator::UP +WeightedSetTermSearch::create(const std::vector &children, TermFieldMatchData &tmd, const std::vector &weights, fef::MatchData::UP match_data) @@ -141,9 +141,9 @@ WeightedSetTermSearch::create(const std::vector &children, typedef WeightedSetTermSearchImpl HeapImpl; if (children.size() < 128) { - return new ArrayHeapImpl(tmd, weights, SearchIteratorPack(children, std::move(match_data))); + return SearchIterator::UP(new ArrayHeapImpl(tmd, weights, SearchIteratorPack(children, std::move(match_data)))); } - return new HeapImpl(tmd, weights, SearchIteratorPack(children, std::move(match_data))); + return SearchIterator::UP(new HeapImpl(tmd, weights, SearchIteratorPack(children, std::move(match_data)))); } //----------------------------------------------------------------------------- diff --git a/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_search.h b/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_search.h index 397ac0caf2e..7eba0907f6b 100644 --- a/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_search.h +++ b/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_search.h @@ -27,10 +27,10 @@ protected: WeightedSetTermSearch() {} public: - static SearchIterator* create(const std::vector &children, - search::fef::TermFieldMatchData &tmd, - const std::vector &weights, - fef::MatchData::UP match_data); + static SearchIterator::UP create(const std::vector &children, + search::fef::TermFieldMatchData &tmd, + const std::vector &weights, + fef::MatchData::UP match_data); static SearchIterator::UP create(search::fef::TermFieldMatchData &tmd, const std::vector &weights, diff --git a/searchlib/src/vespa/searchlib/test/initrange.h b/searchlib/src/vespa/searchlib/test/initrange.h index 9431740ac08..a143dfdb119 100644 --- a/searchlib/src/vespa/searchlib/test/initrange.h +++ b/searchlib/src/vespa/searchlib/test/initrange.h @@ -25,6 +25,7 @@ public: void verify(SearchIterator & iterator) const; /// Convenience that takes ownership of the pointer. void verify(SearchIterator * iterator) const; + void verify(SearchIterator::UP iterator) const { verify(*iterator); } private: void verify(SearchIterator & iterator, bool strict) const; void verify(SearchIterator & iterator, const Ranges & ranges, bool strict) const; diff --git a/searchlib/src/vespa/searchlib/test/searchiteratorverifier.cpp b/searchlib/src/vespa/searchlib/test/searchiteratorverifier.cpp index fce768ab0d2..ec53d6d9d00 100644 --- a/searchlib/src/vespa/searchlib/test/searchiteratorverifier.cpp +++ b/searchlib/src/vespa/searchlib/test/searchiteratorverifier.cpp @@ -170,9 +170,9 @@ void SearchIteratorVerifier::verifyAnd(bool strict) const { fef::TermFieldMatchData tfmd; MultiSearch::Children children; - children.emplace_back(create(strict).release()); - children.emplace_back(BitVectorIterator::create(_everyOddBitSet.get(), getDocIdLimit(), tfmd, false).release()); - SearchIterator::UP search(AndSearch::create(children, strict, UnpackInfo())); + children.push_back(create(strict)); + children.push_back(BitVectorIterator::create(_everyOddBitSet.get(), getDocIdLimit(), tfmd, false)); + auto search = AndSearch::create(std::move(children), strict, UnpackInfo()); TEST_DO(verify(*search, strict, _expectedAnd)); TEST_DO(verifyTermwise(std::move(search), strict, _expectedAnd)); } @@ -183,18 +183,18 @@ SearchIteratorVerifier::verifyAndNot(bool strict) const { { for (bool notStrictness : {false, true}) { MultiSearch::Children children; - children.emplace_back(create(strict).release()); - children.emplace_back(BitVectorIterator::create(_everyOddBitSet.get(), getDocIdLimit(), tfmd, notStrictness).release()); - SearchIterator::UP search(AndNotSearch::create(children, strict)); + children.push_back(create(strict)); + children.push_back(BitVectorIterator::create(_everyOddBitSet.get(), getDocIdLimit(), tfmd, notStrictness)); + auto search = AndNotSearch::create(std::move(children), strict); TEST_DO(verify(*search, strict, _expectedAndNotPositive)); TEST_DO(verifyTermwise(std::move(search), strict, _expectedAndNotPositive)); } } { MultiSearch::Children children; - children.emplace_back(BitVectorIterator::create(_everyOddBitSet.get(), getDocIdLimit(), tfmd, true).release()); - children.emplace_back(create(strict).release()); - SearchIterator::UP search(AndNotSearch::create(children, strict)); + children.push_back(BitVectorIterator::create(_everyOddBitSet.get(), getDocIdLimit(), tfmd, true)); + children.push_back(create(strict)); + auto search = AndNotSearch::create(std::move(children), strict); TEST_DO(verify(*search, strict, _expectedAndNotNegative)); TEST_DO(verifyTermwise(std::move(search), strict, _expectedAndNotNegative)); } @@ -205,9 +205,9 @@ void SearchIteratorVerifier::verifyOr(bool strict) const { fef::TermFieldMatchData tfmd; MultiSearch::Children children; - children.emplace_back(create(strict).release()); - children.emplace_back(BitVectorIterator::create(_everyOddBitSet.get(), getDocIdLimit(), tfmd, strict).release()); - SearchIterator::UP search(OrSearch::create(children, strict, UnpackInfo())); + children.push_back(create(strict)); + children.push_back(BitVectorIterator::create(_everyOddBitSet.get(), getDocIdLimit(), tfmd, strict)); + SearchIterator::UP search(OrSearch::create(std::move(children), strict, UnpackInfo())); TEST_DO(verify(*search, strict, _expectedOr)); TEST_DO(verifyTermwise(std::move(search), strict, _expectedOr)); } -- cgit v1.2.3 From 10044079375f308895ab8848a7b3d2d4a38f1441 Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Fri, 5 Jun 2020 12:32:25 +0000 Subject: add TODOs about further use of unique_ptr --- .../queryeval/sparse_vector_benchmark/sparse_vector_benchmark_test.cpp | 1 + .../src/vespa/searchlib/attribute/attribute_weighted_set_blueprint.cpp | 1 + searchlib/src/vespa/searchlib/queryeval/dot_product_blueprint.cpp | 1 + searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.cpp | 2 ++ .../src/vespa/searchlib/queryeval/wand/parallel_weak_and_blueprint.cpp | 1 + searchlib/src/vespa/searchlib/queryeval/wand/wand_parts.h | 1 + searchlib/src/vespa/searchlib/queryeval/weighted_set_term_blueprint.cpp | 1 + searchlib/src/vespa/searchlib/queryeval/weighted_set_term_search.h | 1 + 8 files changed, 9 insertions(+) diff --git a/searchlib/src/tests/queryeval/sparse_vector_benchmark/sparse_vector_benchmark_test.cpp b/searchlib/src/tests/queryeval/sparse_vector_benchmark/sparse_vector_benchmark_test.cpp index 68290e105f1..0de0815731b 100644 --- a/searchlib/src/tests/queryeval/sparse_vector_benchmark/sparse_vector_benchmark_test.cpp +++ b/searchlib/src/tests/queryeval/sparse_vector_benchmark/sparse_vector_benchmark_test.cpp @@ -230,6 +230,7 @@ struct WeightedSetFactory : SparseVectorFactory { std::vector terms; std::vector weights; for (size_t i = 0; i < childCnt; ++i) { + // TODO: pass ownership with unique_ptr terms.push_back(childFactory.createChild(i, limit).release()); weights.push_back(default_weight); } diff --git a/searchlib/src/vespa/searchlib/attribute/attribute_weighted_set_blueprint.cpp b/searchlib/src/vespa/searchlib/attribute/attribute_weighted_set_blueprint.cpp index af3fbca6943..121cb736471 100644 --- a/searchlib/src/vespa/searchlib/attribute/attribute_weighted_set_blueprint.cpp +++ b/searchlib/src/vespa/searchlib/attribute/attribute_weighted_set_blueprint.cpp @@ -162,6 +162,7 @@ AttributeWeightedSetBlueprint::createLeafSearch(const fef::TermFieldMatchDataArr auto child_tfmd = match_data->resolveTermField(handle); std::vector children(_contexts.size()); for (size_t i = 0; i < _contexts.size(); ++i) { + // TODO: pass ownership with unique_ptr children[i] = _contexts[i]->createIterator(child_tfmd, true).release(); } return queryeval::SearchIterator::UP(queryeval::WeightedSetTermSearch::create(children, tfmd, _weights, std::move(match_data))); diff --git a/searchlib/src/vespa/searchlib/queryeval/dot_product_blueprint.cpp b/searchlib/src/vespa/searchlib/queryeval/dot_product_blueprint.cpp index 741aec98f4f..454db4e820a 100644 --- a/searchlib/src/vespa/searchlib/queryeval/dot_product_blueprint.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/dot_product_blueprint.cpp @@ -59,6 +59,7 @@ DotProductBlueprint::createLeafSearch(const search::fef::TermFieldMatchDataArray const State &childState = _terms[i]->getState(); assert(childState.numFields() == 1); childMatch.push_back(childState.field(0).resolve(*md)); + // TODO: pass ownership with unique_ptr children[i] = _terms[i]->createSearch(*md, true).release(); } return DotProductSearch::create(children, *tfmda[0], childMatch, _weights, std::move(md)); diff --git a/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.cpp b/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.cpp index fd6a399e92f..3d3a703cd7b 100644 --- a/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.cpp @@ -433,6 +433,7 @@ WeakAndBlueprint::createIntermediateSearch(MultiSearch::Children sub_searches, assert(sub_searches.size() == childCnt()); assert(_weights.size() == childCnt()); for (size_t i = 0; i < sub_searches.size(); ++i) { + // TODO: pass ownership with unique_ptr terms.push_back(wand::Term(sub_searches[i].release(), _weights[i], getChild(i).getState().estimate().estHits)); @@ -697,6 +698,7 @@ SourceBlenderBlueprint::createIntermediateSearch(MultiSearch::Children sub_searc SourceBlenderSearch::Children children; assert(sub_searches.size() == childCnt()); for (size_t i = 0; i < sub_searches.size(); ++i) { + // TODO: pass ownership with unique_ptr children.push_back(SourceBlenderSearch::Child(sub_searches[i].release(), getChild(i).getSourceId())); assert(children.back().sourceId != 0xffffffff); diff --git a/searchlib/src/vespa/searchlib/queryeval/wand/parallel_weak_and_blueprint.cpp b/searchlib/src/vespa/searchlib/queryeval/wand/parallel_weak_and_blueprint.cpp index 7ad4a36f871..d4f16e2f91c 100644 --- a/searchlib/src/vespa/searchlib/queryeval/wand/parallel_weak_and_blueprint.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/wand/parallel_weak_and_blueprint.cpp @@ -86,6 +86,7 @@ ParallelWeakAndBlueprint::createLeafSearch(const search::fef::TermFieldMatchData for (size_t i = 0; i < _terms.size(); ++i) { const State &childState = _terms[i]->getState(); assert(childState.numFields() == 1); + // TODO: pass ownership with unique_ptr terms.push_back(wand::Term(_terms[i]->createSearch(*childrenMatchData, true).release(), _weights[i], childState.estimate().estHits, diff --git a/searchlib/src/vespa/searchlib/queryeval/wand/wand_parts.h b/searchlib/src/vespa/searchlib/queryeval/wand/wand_parts.h index 6c4d9209da1..071d6d99470 100644 --- a/searchlib/src/vespa/searchlib/queryeval/wand/wand_parts.h +++ b/searchlib/src/vespa/searchlib/queryeval/wand/wand_parts.h @@ -35,6 +35,7 @@ typedef std::vector AttrDictEntries; * Wrapper used to specify underlying terms during setup **/ struct Term { + // TODO: use unique_ptr for ownership SearchIterator *search; int32_t weight; uint32_t estHits; diff --git a/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_blueprint.cpp b/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_blueprint.cpp index 36378439c01..cea35d976f0 100644 --- a/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_blueprint.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_blueprint.cpp @@ -50,6 +50,7 @@ WeightedSetTermBlueprint::createLeafSearch(const fef::TermFieldMatchDataArray &t fef::MatchData::UP md = _layout.createMatchData(); std::vector children(_terms.size()); for (size_t i = 0; i < _terms.size(); ++i) { + // TODO: pass ownership with unique_ptr children[i] = _terms[i]->createSearch(*md, true).release(); } return SearchIterator::UP(WeightedSetTermSearch::create(children, *tfmda[0], _weights, std::move(md))); diff --git a/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_search.h b/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_search.h index 7eba0907f6b..ecc620a3adb 100644 --- a/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_search.h +++ b/searchlib/src/vespa/searchlib/queryeval/weighted_set_term_search.h @@ -27,6 +27,7 @@ protected: WeightedSetTermSearch() {} public: + // TODO: pass ownership with unique_ptr static SearchIterator::UP create(const std::vector &children, search::fef::TermFieldMatchData &tmd, const std::vector &weights, -- cgit v1.2.3 From fbc19529236528b262d80a068b456bf02e878e89 Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Fri, 5 Jun 2020 13:11:23 +0000 Subject: adjust to use std::make_unique --- .../src/vespa/searchlib/queryeval/sourceblendersearch.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/searchlib/src/vespa/searchlib/queryeval/sourceblendersearch.cpp b/searchlib/src/vespa/searchlib/queryeval/sourceblendersearch.cpp index 282e3bef104..b50e71d5d53 100644 --- a/searchlib/src/vespa/searchlib/queryeval/sourceblendersearch.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/sourceblendersearch.cpp @@ -10,6 +10,14 @@ namespace search::queryeval { EmptySearch SourceBlenderSearch::_emptySearch; +class SourceBlenderSearchNonStrict : public SourceBlenderSearch +{ +public: + SourceBlenderSearchNonStrict(std::unique_ptr sourceSelector, const Children &children) + : SourceBlenderSearch(std::move(sourceSelector), children) + {} +}; + class SourceBlenderSearchStrict : public SourceBlenderSearch { public: @@ -163,9 +171,9 @@ SourceBlenderSearch::create(std::unique_ptr sourceSele const Children &children, bool strict) { if (strict) { - return SearchIterator::UP(new SourceBlenderSearchStrict(std::move(sourceSelector), children)); + return std::make_unique(std::move(sourceSelector), children); } else { - return SearchIterator::UP(new SourceBlenderSearch(std::move(sourceSelector), children)); + return std::make_unique(std::move(sourceSelector), children); } } -- cgit v1.2.3