diff options
Diffstat (limited to 'searchlib')
33 files changed, 610 insertions, 412 deletions
diff --git a/searchlib/abi-spec.json b/searchlib/abi-spec.json index 0b1cb7a103c..2d67abb0e04 100644 --- a/searchlib/abi-spec.json +++ b/searchlib/abi-spec.json @@ -1728,4 +1728,4 @@ ], "fields" : [ ] } -} +}
\ No newline at end of file diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/RankingExpression.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/RankingExpression.java index a7b45feb043..c0dfc0e1d29 100755 --- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/RankingExpression.java +++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/RankingExpression.java @@ -257,6 +257,9 @@ public class RankingExpression implements Serializable { * @return a list of named rank properties required to implement this expression */ public Map<String, String> getRankProperties(SerializationContext context) { + if ("".equals(name)) { + return Map.of(); + } Deque<String> path = new LinkedList<>(); String serializedRoot = root.toString(new StringBuilder(), context, path, null).toString(); Map<String, String> serializedExpressions = context.serializedFunctions(); diff --git a/searchlib/src/test/java/com/yahoo/searchlib/rankingexpression/RankingExpressionTestCase.java b/searchlib/src/test/java/com/yahoo/searchlib/rankingexpression/RankingExpressionTestCase.java index 8af77ec1cdd..8d60f893c7c 100755 --- a/searchlib/src/test/java/com/yahoo/searchlib/rankingexpression/RankingExpressionTestCase.java +++ b/searchlib/src/test/java/com/yahoo/searchlib/rankingexpression/RankingExpressionTestCase.java @@ -364,7 +364,7 @@ public class RankingExpressionTestCase { private void assertSerialization(String expectedSerialization, String expressionString) { String serializedExpression; try { - RankingExpression expression = new RankingExpression(expressionString); + RankingExpression expression = new RankingExpression("secondphase", expressionString); // No functions -> expect one rank property serializedExpression = expression.getRankProperties(new SerializationContext()).values().iterator().next(); assertEquals(expectedSerialization, serializedExpression); @@ -376,7 +376,7 @@ public class RankingExpressionTestCase { try { // No functions -> output should be parseable to a ranking expression // (but not the same one due to primitivization) - RankingExpression reparsedExpression = new RankingExpression(serializedExpression); + RankingExpression reparsedExpression = new RankingExpression("secondphase", serializedExpression); // Serializing the primitivized expression should yield the same expression again String reserializedExpression = reparsedExpression.getRankProperties(new SerializationContext()).values().iterator().next(); @@ -399,7 +399,7 @@ public class RankingExpressionTestCase { if (print) System.out.println("Parsing expression '" + expressionString + "':"); - RankingExpression expression = new RankingExpression(expressionString); + RankingExpression expression = new RankingExpression("secondphase", expressionString); Map<String, String> rankProperties = expression.getRankProperties(new SerializationContext(functions, Optional.empty())); if (print) { diff --git a/searchlib/src/tests/attribute/bitvector/bitvector_test.cpp b/searchlib/src/tests/attribute/bitvector/bitvector_test.cpp index 63c0b784018..dfea4901180 100644 --- a/searchlib/src/tests/attribute/bitvector/bitvector_test.cpp +++ b/searchlib/src/tests/attribute/bitvector/bitvector_test.cpp @@ -43,9 +43,8 @@ struct BitVectorTest { using AttributePtr = AttributeVector::SP; - BitVectorTest() { } - - ~BitVectorTest() { } + BitVectorTest(); + ~BitVectorTest(); template <typename VectorType> VectorType & as(AttributePtr &v); @@ -102,6 +101,9 @@ struct BitVectorTest test(BasicType bt, CollectionType ct, const vespalib::string &pref); }; +BitVectorTest::BitVectorTest() = default; +BitVectorTest::~BitVectorTest() = default; + template <typename VectorType> VectorType & @@ -427,16 +429,14 @@ BitVectorTest::test(BasicType bt, CollectionType ct, const vespalib::string &pre SearchContextPtr sc = getSearch<VectorType>(tv, true); checkSearch(v, std::move(sc), 2, 1022, 205, !fastSearch && !filter, true); - sc = getSearch<VectorType>(tv, false); + sc = getSearch<VectorType>(tv, filter); checkSearch(v, std::move(sc), 2, 1022, 205, !filter, true); const search::IDocumentWeightAttribute *dwa = v->asDocumentWeightAttribute(); if (dwa != nullptr) { - search::IDocumentWeightAttribute::LookupResult lres = - dwa->lookup(getSearchStr<VectorType>(), dwa->get_dictionary_snapshot()); + auto lres = dwa->lookup(getSearchStr<VectorType>(), dwa->get_dictionary_snapshot()); using DWSI = search::queryeval::DocumentWeightSearchIterator; - using SI = search::queryeval::SearchIterator; TermFieldMatchData md; - SI::UP dwsi(new DWSI(md, *dwa, lres)); + auto dwsi = std::make_unique<DWSI>(md, *dwa, lres); if (!filter) { TEST_DO(checkSearch(v, std::move(dwsi), md, 2, 1022, 205, !filter, true)); } else { @@ -445,13 +445,13 @@ BitVectorTest::test(BasicType bt, CollectionType ct, const vespalib::string &pre } } populate(tv, 2, 973, false); - sc = getSearch<VectorType>(tv, true); + sc = getSearch<VectorType>(tv, filter); checkSearch(v, std::move(sc), 977, 1022, 10, !filter, true); populate(tv, 2, 973, true); sc = getSearch<VectorType>(tv, true); checkSearch(v, std::move(sc), 2, 1022, 205, !fastSearch && !filter, true); addDocs(v, 15000); - sc = getSearch<VectorType>(tv, true); + sc = getSearch<VectorType>(tv, filter); checkSearch(v, std::move(sc), 2, 1022, 205, !filter, true); populateAll(tv, 10, 15000, true); sc = getSearch<VectorType>(tv, true); diff --git a/searchlib/src/tests/attribute/posting_store/posting_store_test.cpp b/searchlib/src/tests/attribute/posting_store/posting_store_test.cpp index 61d94e120f5..af61301c06f 100644 --- a/searchlib/src/tests/attribute/posting_store/posting_store_test.cpp +++ b/searchlib/src/tests/attribute/posting_store/posting_store_test.cpp @@ -96,7 +96,7 @@ protected: return sequence; } - void populate(uint32_t sequence_length); + void populate(std::vector<uint32_t> sequence_lengths, std::optional<std::function<void()>> clear_callback = std::nullopt); EntryRef get_posting_ref(int key); void test_compact_btree_nodes(uint32_t sequence_length); void test_compact_sequence(uint32_t sequence_length); @@ -120,21 +120,28 @@ PostingStoreTest::~PostingStoreTest() } void -PostingStoreTest::populate(uint32_t sequence_length) +PostingStoreTest::populate(std::vector<uint32_t> sequence_lengths, std::optional<std::function<void()>> clear_callback) { auto& store = _store; auto& dictionary = _value_store.get_dictionary(); std::vector<EntryRef> refs; - for (int i = 0; i < 9000; ++i) { - refs.emplace_back(add_sequence(i + 6, i + 6 + sequence_length)); + for (auto sequence_length : sequence_lengths) { + for (int i = 0; i < 9000; ++i) { + refs.emplace_back(add_sequence(i + 6, i + 6 + sequence_length)); + } } - dictionary.update_posting_list(_value_store.insert(1), _value_store.get_comparator(), [this, sequence_length](EntryRef) { return add_sequence(4, 4 + sequence_length); }); - dictionary.update_posting_list(_value_store.insert(2), _value_store.get_comparator(), [this, sequence_length](EntryRef) { return add_sequence(5, 5 + sequence_length); }); - for (int i = 9000; i < 11000; ++i) { - refs.emplace_back(add_sequence(i + 6, i + 6 + sequence_length)); + dictionary.update_posting_list(_value_store.insert(1), _value_store.get_comparator(), [this, sequence_length = sequence_lengths.front()](EntryRef) { return add_sequence(4, 4 + sequence_length); }); + dictionary.update_posting_list(_value_store.insert(2), _value_store.get_comparator(), [this, sequence_length = sequence_lengths.front()](EntryRef) { return add_sequence(5, 5 + sequence_length); }); + for (auto sequence_length : sequence_lengths) { + for (int i = 9000; i < 11000; ++i) { + refs.emplace_back(add_sequence(i + 6, i + 6 + sequence_length)); + } } for (auto& ref : refs) { store.clear(ref); + if (clear_callback.has_value()) { + clear_callback.value()(); + } } inc_generation(); } @@ -150,7 +157,7 @@ PostingStoreTest::get_posting_ref(int key) void PostingStoreTest::test_compact_sequence(uint32_t sequence_length) { - populate(sequence_length); + populate({sequence_length}); auto &store = _store; EntryRef old_ref1 = get_posting_ref(1); EntryRef old_ref2 = get_posting_ref(2); @@ -183,7 +190,7 @@ PostingStoreTest::test_compact_sequence(uint32_t sequence_length) void PostingStoreTest::test_compact_btree_nodes(uint32_t sequence_length) { - populate(sequence_length); + populate({sequence_length}); auto &store = _store; EntryRef old_ref1 = get_posting_ref(1); EntryRef old_ref2 = get_posting_ref(2); @@ -246,6 +253,40 @@ TEST_P(PostingStoreTest, require_that_bitvectors_are_compacted) test_compact_sequence(huge_sequence_length); } +namespace { + +/* + * Check if compaction of btree nodes or short arrays is suppressed due to + * dead ratio being too low for the sum of both data stores. + */ +bool compaction_is_suppressed(MyPostingStore& store, CompactionStrategy& compaction_strategy) +{ + store.update_stat(compaction_strategy); + auto& compaction_spec = store.get_compaction_spec(); + auto memory_usage = store.getMemoryUsage(); + return ((compaction_strategy.should_compact_memory(memory_usage.btrees) && + !compaction_spec.btree_nodes()) || + (compaction_strategy.should_compact_memory(memory_usage.short_arrays) && + !compaction_spec.store())); +} + +} + +TEST_P(PostingStoreTest, require_that_compaction_is_suppressed) +{ + CompactionStrategy compaction_strategy(0.05, 0.2); + bool suppressed_compaction = false; + auto clear_callback = [this, &compaction_strategy, &suppressed_compaction]() mutable + { + inc_generation(); + if (compaction_is_suppressed(_store, compaction_strategy)) { + suppressed_compaction = true; + } + }; + populate({ 4, 10}, clear_callback); + EXPECT_TRUE(suppressed_compaction); +} + } GTEST_MAIN_RUN_ALL_TESTS() diff --git a/searchlib/src/tests/fef/properties/properties_test.cpp b/searchlib/src/tests/fef/properties/properties_test.cpp index 5e18c41b40a..c8073739b3e 100644 --- a/searchlib/src/tests/fef/properties/properties_test.cpp +++ b/searchlib/src/tests/fef/properties/properties_test.cpp @@ -10,9 +10,8 @@ using namespace search::fef::indexproperties; struct CopyVisitor : public IPropertiesVisitor { Properties &dst; - CopyVisitor(Properties &p) : dst(p) {} - virtual void visitProperty(const Property::Value &key, - const Property &values) override + explicit CopyVisitor(Properties &p) noexcept : dst(p) {} + void visitProperty(const Property::Value &key, const Property &values) override { for (uint32_t i = 0; i < values.size(); ++i) { dst.add(key, values.getAt(i)); @@ -590,5 +589,41 @@ TEST("test query feature type properties") EXPECT_EQUAL("", type::QueryFeature::lookup(p, "bar")); } +TEST("test integer lookup") +{ + EXPECT_EQUAL(matching::NumThreadsPerSearch::NAME, vespalib::string("vespa.matching.numthreadspersearch")); + EXPECT_EQUAL(matching::NumThreadsPerSearch::DEFAULT_VALUE, std::numeric_limits<uint32_t>::max()); + { + Properties p; + p.add("vespa.matching.numthreadspersearch", "50"); + EXPECT_EQUAL(matching::NumThreadsPerSearch::lookup(p), 50u); + } + { + Properties p; + p.add("vespa.matching.numthreadspersearch", "50 "); + EXPECT_EQUAL(matching::NumThreadsPerSearch::lookup(p), 50u); + } + { + Properties p; + p.add("vespa.matching.numthreadspersearch", " 50"); + EXPECT_EQUAL(matching::NumThreadsPerSearch::lookup(p), 50u); + } + { + Properties p; + p.add("vespa.matching.numthreadspersearch", " "); + EXPECT_EQUAL(matching::NumThreadsPerSearch::lookup(p), matching::NumThreadsPerSearch::DEFAULT_VALUE); + } + { + Properties p; + p.add("vespa.matching.numthreadspersearch", "50x"); + EXPECT_EQUAL(matching::NumThreadsPerSearch::lookup(p), 50u); + } + { + Properties p; + p.add("vespa.matching.numthreadspersearch", "x"); + EXPECT_EQUAL(matching::NumThreadsPerSearch::lookup(p), matching::NumThreadsPerSearch::DEFAULT_VALUE); + } +} + TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchlib/src/tests/query/querybuilder_test.cpp b/searchlib/src/tests/query/querybuilder_test.cpp index 606d6a2474a..40af60d4a04 100644 --- a/searchlib/src/tests/query/querybuilder_test.cpp +++ b/searchlib/src/tests/query/querybuilder_test.cpp @@ -257,7 +257,7 @@ void checkQueryTreeTypes(Node *node) { EXPECT_TRUE(checkTerm(loc_term, location, view[10], id[10], weight[10])); auto* wand = as_node<WeakAnd>(and_node->getChildren()[4]); - EXPECT_EQUAL(123u, wand->getMinHits()); + EXPECT_EQUAL(123u, wand->getTargetNumHits()); EXPECT_EQUAL(2u, wand->getChildren().size()); string_term = as_node<StringTerm>(wand->getChildren()[0]); EXPECT_TRUE(checkTerm(string_term, str[4], view[4], id[4], weight[4])); diff --git a/searchlib/src/tests/queryeval/blueprint/blueprint_test.cpp b/searchlib/src/tests/queryeval/blueprint/blueprint_test.cpp index 1d908bed568..372b76fa9af 100644 --- a/searchlib/src/tests/queryeval/blueprint/blueprint_test.cpp +++ b/searchlib/src/tests/queryeval/blueprint/blueprint_test.cpp @@ -81,7 +81,7 @@ public: } FieldSpecBaseList exposeFields() const override { - return FieldSpecBaseList(); + return {}; } bool inheritStrict(size_t i) const override { @@ -140,7 +140,7 @@ struct MyTerm : SimpleLeafBlueprint { setEstimate(HitEstimate(hitEstimate, false)); } SearchIterator::UP createLeafSearch(const search::fef::TermFieldMatchDataArray &, bool) const override { - return SearchIterator::UP(); + return {}; } SearchIteratorUP createFilterSearch(bool strict, FilterConstraint constraint) const override { return create_default_filter(strict, constraint); @@ -311,13 +311,13 @@ TEST("testHitEstimateCalculation") TEST("testHitEstimatePropagation") { - MyLeaf *leaf1 = new MyLeaf(); + auto *leaf1 = new MyLeaf(); leaf1->estimate(10); - MyLeaf *leaf2 = new MyLeaf(); + auto *leaf2 = new MyLeaf(); leaf2->estimate(20); - MyLeaf *leaf3 = new MyLeaf(); + auto *leaf3 = new MyLeaf(); leaf3->estimate(30); MyOr *parent = new MyOr(); @@ -346,7 +346,7 @@ TEST("testHitEstimatePropagation") leaf3->estimate(25); EXPECT_EQUAL(20u, root->getState().estimate().estHits); parent->addChild(std::move(tmp)); - EXPECT_TRUE(tmp.get() == 0); + EXPECT_FALSE(tmp); EXPECT_EQUAL(25u, root->getState().estimate().estHits); } @@ -572,7 +572,7 @@ TEST_F("testSearchCreation", Fixture) .addField(3, 3).create()); SearchIterator::UP leafsearch = f.create(*l); - MySearch *lw = new MySearch("leaf", true, true); + auto *lw = new MySearch("leaf", true, true); lw->addHandle(1).addHandle(2).addHandle(3); SearchIterator::UP wantleaf(lw); @@ -584,11 +584,11 @@ TEST_F("testSearchCreation", Fixture) .add(MyLeafSpec(2).addField(2, 2).create())); SearchIterator::UP andsearch = f.create(*a); - MySearch *l1 = new MySearch("leaf", true, true); - MySearch *l2 = new MySearch("leaf", true, false); + auto *l1 = new MySearch("leaf", true, true); + auto *l2 = new MySearch("leaf", true, false); l1->addHandle(1); l2->addHandle(2); - MySearch *aw = new MySearch("and", false, true); + auto *aw = new MySearch("and", false, true); aw->add(l1); aw->add(l2); SearchIterator::UP wanted(aw); @@ -600,11 +600,11 @@ TEST_F("testSearchCreation", Fixture) .add(MyLeafSpec(2).addField(2, 22).create())); SearchIterator::UP orsearch = f.create(*o); - MySearch *l1 = new MySearch("leaf", true, true); - MySearch *l2 = new MySearch("leaf", true, true); + auto *l1 = new MySearch("leaf", true, true); + auto *l2 = new MySearch("leaf", true, true); l1->addHandle(11); l2->addHandle(22); - MySearch *ow = new MySearch("or", false, true); + auto *ow = new MySearch("or", false, true); ow->add(l1); ow->add(l2); SearchIterator::UP wanted(ow); @@ -619,7 +619,7 @@ TEST("testBlueprintMakeNew") .add(MyLeafSpec(2).addField(2, 22).create())); orig->setSourceId(42); MyOr *myOr = dynamic_cast<MyOr*>(orig.get()); - ASSERT_TRUE(myOr != 0); + ASSERT_TRUE(myOr != nullptr); EXPECT_EQUAL(42u, orig->getSourceId()); EXPECT_EQUAL(2u, orig->getState().numFields()); } diff --git a/searchlib/src/tests/queryeval/blueprint/intermediate_blueprints_test.cpp b/searchlib/src/tests/queryeval/blueprint/intermediate_blueprints_test.cpp index a663944938c..51e22dbcf2c 100644 --- a/searchlib/src/tests/queryeval/blueprint/intermediate_blueprints_test.cpp +++ b/searchlib/src/tests/queryeval/blueprint/intermediate_blueprints_test.cpp @@ -8,7 +8,6 @@ #include <vespa/searchlib/queryeval/leaf_blueprints.h> #include <vespa/searchlib/queryeval/equiv_blueprint.h> #include <vespa/searchlib/queryeval/multisearch.h> -#include <vespa/searchlib/queryeval/andnotsearch.h> #include <vespa/searchlib/queryeval/wand/weak_and_search.h> #include <vespa/searchlib/queryeval/fake_requestcontext.h> #include <vespa/searchlib/test/diskindex/testdiskindex.h> @@ -74,13 +73,13 @@ TEST("test AndNot Blueprint") { std::vector<Blueprint::HitEstimate> est; EXPECT_EQUAL(true, b.combine(est).empty); EXPECT_EQUAL(0u, b.combine(est).estHits); - est.push_back(Blueprint::HitEstimate(10, false)); + est.emplace_back(10, false); EXPECT_EQUAL(false, b.combine(est).empty); EXPECT_EQUAL(10u, b.combine(est).estHits); - est.push_back(Blueprint::HitEstimate(20, false)); + est.emplace_back(20, false); EXPECT_EQUAL(false, b.combine(est).empty); EXPECT_EQUAL(10u, b.combine(est).estHits); - est.push_back(Blueprint::HitEstimate(5, false)); + est.emplace_back(5, false); EXPECT_EQUAL(false, b.combine(est).empty); EXPECT_EQUAL(10u, b.combine(est).estHits); } @@ -120,12 +119,14 @@ TEST("test And propagates updated histestimate") { bp.addChild(ap(MyLeafSpec(20).create<RememberExecuteInfo>()->setSourceId(2))); bp.addChild(ap(MyLeafSpec(200).create<RememberExecuteInfo>()->setSourceId(2))); bp.addChild(ap(MyLeafSpec(2000).create<RememberExecuteInfo>()->setSourceId(2))); - bp.optimize_self(); + bp.optimize_self(Blueprint::OptimizePass::FIRST); + bp.optimize_self(Blueprint::OptimizePass::SECOND); + bp.optimize_self(Blueprint::OptimizePass::LAST); bp.setDocIdLimit(5000); bp.fetchPostings(ExecuteInfo::TRUE); EXPECT_EQUAL(3u, bp.childCnt()); for (uint32_t i = 0; i < bp.childCnt(); i++) { - const RememberExecuteInfo & child = dynamic_cast<const RememberExecuteInfo &>(bp.getChild(i)); + const auto & child = dynamic_cast<const RememberExecuteInfo &>(bp.getChild(i)); EXPECT_EQUAL((i == 0), child.executeInfo.isStrict()); } EXPECT_EQUAL(1.0f, dynamic_cast<const RememberExecuteInfo &>(bp.getChild(0)).executeInfo.hitRate()); @@ -133,22 +134,45 @@ TEST("test And propagates updated histestimate") { EXPECT_EQUAL(1.0f/(250*25), dynamic_cast<const RememberExecuteInfo &>(bp.getChild(2)).executeInfo.hitRate()); } +TEST("test Or propagates updated histestimate") { + OrBlueprint bp; + bp.setSourceId(2); + bp.addChild(ap(MyLeafSpec(5000).create<RememberExecuteInfo>()->setSourceId(2))); + bp.addChild(ap(MyLeafSpec(2000).create<RememberExecuteInfo>()->setSourceId(2))); + bp.addChild(ap(MyLeafSpec(800).create<RememberExecuteInfo>()->setSourceId(2))); + bp.addChild(ap(MyLeafSpec(20).create<RememberExecuteInfo>()->setSourceId(2))); + bp.optimize_self(Blueprint::OptimizePass::FIRST); + bp.optimize_self(Blueprint::OptimizePass::SECOND); + bp.optimize_self(Blueprint::OptimizePass::LAST); + bp.setDocIdLimit(5000); + bp.fetchPostings(ExecuteInfo::TRUE); + EXPECT_EQUAL(4u, bp.childCnt()); + for (uint32_t i = 0; i < bp.childCnt(); i++) { + const auto & child = dynamic_cast<const RememberExecuteInfo &>(bp.getChild(i)); + EXPECT_TRUE(child.executeInfo.isStrict()); + } + EXPECT_EQUAL(1.0f, dynamic_cast<const RememberExecuteInfo &>(bp.getChild(0)).executeInfo.hitRate()); + EXPECT_EQUAL(1.0f, dynamic_cast<const RememberExecuteInfo &>(bp.getChild(1)).executeInfo.hitRate()); + EXPECT_EQUAL(3.0f/5.0f, dynamic_cast<const RememberExecuteInfo &>(bp.getChild(2)).executeInfo.hitRate()); + EXPECT_EQUAL(3.0f*42.0f/(5.0f*50.0f), dynamic_cast<const RememberExecuteInfo &>(bp.getChild(3)).executeInfo.hitRate()); +} + TEST("test And Blueprint") { AndBlueprint b; { // combine std::vector<Blueprint::HitEstimate> est; EXPECT_EQUAL(true, b.combine(est).empty); EXPECT_EQUAL(0u, b.combine(est).estHits); - est.push_back(Blueprint::HitEstimate(10, false)); + est.emplace_back(10, false); EXPECT_EQUAL(false, b.combine(est).empty); EXPECT_EQUAL(10u, b.combine(est).estHits); - est.push_back(Blueprint::HitEstimate(20, false)); + est.emplace_back(20, false); EXPECT_EQUAL(false, b.combine(est).empty); EXPECT_EQUAL(10u, b.combine(est).estHits); - est.push_back(Blueprint::HitEstimate(5, false)); + est.emplace_back(5, false); EXPECT_EQUAL(false, b.combine(est).empty); EXPECT_EQUAL(5u, b.combine(est).estHits); - est.push_back(Blueprint::HitEstimate(0, true)); + est.emplace_back(0, true); EXPECT_EQUAL(true, b.combine(est).empty); EXPECT_EQUAL(0u, b.combine(est).estHits); } @@ -187,16 +211,16 @@ TEST("test Or Blueprint") { std::vector<Blueprint::HitEstimate> est; EXPECT_EQUAL(true, b.combine(est).empty); EXPECT_EQUAL(0u, b.combine(est).estHits); - est.push_back(Blueprint::HitEstimate(10, false)); + est.emplace_back(10, false); EXPECT_EQUAL(false, b.combine(est).empty); EXPECT_EQUAL(10u, b.combine(est).estHits); - est.push_back(Blueprint::HitEstimate(20, false)); + est.emplace_back(20, false); EXPECT_EQUAL(false, b.combine(est).empty); EXPECT_EQUAL(20u, b.combine(est).estHits); - est.push_back(Blueprint::HitEstimate(5, false)); + est.emplace_back(5, false); EXPECT_EQUAL(false, b.combine(est).empty); EXPECT_EQUAL(20u, b.combine(est).estHits); - est.push_back(Blueprint::HitEstimate(0, true)); + est.emplace_back(0, true); EXPECT_EQUAL(false, b.combine(est).empty); EXPECT_EQUAL(20u, b.combine(est).estHits); } @@ -259,16 +283,16 @@ TEST("test Near Blueprint") { std::vector<Blueprint::HitEstimate> est; EXPECT_EQUAL(true, b.combine(est).empty); EXPECT_EQUAL(0u, b.combine(est).estHits); - est.push_back(Blueprint::HitEstimate(10, false)); + est.emplace_back(10, false); EXPECT_EQUAL(false, b.combine(est).empty); EXPECT_EQUAL(10u, b.combine(est).estHits); - est.push_back(Blueprint::HitEstimate(20, false)); + est.emplace_back(20, false); EXPECT_EQUAL(false, b.combine(est).empty); EXPECT_EQUAL(10u, b.combine(est).estHits); - est.push_back(Blueprint::HitEstimate(5, false)); + est.emplace_back(5, false); EXPECT_EQUAL(false, b.combine(est).empty); EXPECT_EQUAL(5u, b.combine(est).estHits); - est.push_back(Blueprint::HitEstimate(0, true)); + est.emplace_back(0, true); EXPECT_EQUAL(true, b.combine(est).empty); EXPECT_EQUAL(0u, b.combine(est).estHits); } @@ -300,16 +324,16 @@ TEST("test ONear Blueprint") { std::vector<Blueprint::HitEstimate> est; EXPECT_EQUAL(true, b.combine(est).empty); EXPECT_EQUAL(0u, b.combine(est).estHits); - est.push_back(Blueprint::HitEstimate(10, false)); + est.emplace_back(10, false); EXPECT_EQUAL(false, b.combine(est).empty); EXPECT_EQUAL(10u, b.combine(est).estHits); - est.push_back(Blueprint::HitEstimate(20, false)); + est.emplace_back(20, false); EXPECT_EQUAL(false, b.combine(est).empty); EXPECT_EQUAL(10u, b.combine(est).estHits); - est.push_back(Blueprint::HitEstimate(5, false)); + est.emplace_back(5, false); EXPECT_EQUAL(false, b.combine(est).empty); EXPECT_EQUAL(5u, b.combine(est).estHits); - est.push_back(Blueprint::HitEstimate(0, true)); + est.emplace_back(0, true); EXPECT_EQUAL(true, b.combine(est).empty); EXPECT_EQUAL(0u, b.combine(est).estHits); } @@ -341,16 +365,16 @@ TEST("test Rank Blueprint") { std::vector<Blueprint::HitEstimate> est; EXPECT_EQUAL(true, b.combine(est).empty); EXPECT_EQUAL(0u, b.combine(est).estHits); - est.push_back(Blueprint::HitEstimate(10, false)); + est.emplace_back(10, false); EXPECT_EQUAL(false, b.combine(est).empty); EXPECT_EQUAL(10u, b.combine(est).estHits); - est.push_back(Blueprint::HitEstimate(20, false)); + est.emplace_back(20, false); EXPECT_EQUAL(false, b.combine(est).empty); EXPECT_EQUAL(10u, b.combine(est).estHits); - est.push_back(Blueprint::HitEstimate(5, false)); + est.emplace_back(5, false); EXPECT_EQUAL(false, b.combine(est).empty); EXPECT_EQUAL(10u, b.combine(est).estHits); - est.push_back(Blueprint::HitEstimate(0, true)); + est.emplace_back(0, true); EXPECT_EQUAL(false, b.combine(est).empty); EXPECT_EQUAL(10u, b.combine(est).estHits); } @@ -391,16 +415,16 @@ TEST("test SourceBlender Blueprint") { std::vector<Blueprint::HitEstimate> est; EXPECT_EQUAL(true, b.combine(est).empty); EXPECT_EQUAL(0u, b.combine(est).estHits); - est.push_back(Blueprint::HitEstimate(10, false)); + est.emplace_back(10, false); EXPECT_EQUAL(false, b.combine(est).empty); EXPECT_EQUAL(10u, b.combine(est).estHits); - est.push_back(Blueprint::HitEstimate(20, false)); + est.emplace_back(20, false); EXPECT_EQUAL(false, b.combine(est).empty); EXPECT_EQUAL(20u, b.combine(est).estHits); - est.push_back(Blueprint::HitEstimate(5, false)); + est.emplace_back(5, false); EXPECT_EQUAL(false, b.combine(est).empty); EXPECT_EQUAL(20u, b.combine(est).estHits); - est.push_back(Blueprint::HitEstimate(0, true)); + est.emplace_back(0, true); EXPECT_EQUAL(false, b.combine(est).empty); EXPECT_EQUAL(20u, b.combine(est).estHits); } @@ -453,60 +477,60 @@ TEST("test SourceBlender below AND optimization") { auto selector_1 = std::make_unique<InvalidSelector>(); // the one auto selector_2 = std::make_unique<InvalidSelector>(); // not the one //------------------------------------------------------------------------- - AndBlueprint *top = new AndBlueprint(); + auto *top = new AndBlueprint(); Blueprint::UP top_bp(top); top->addChild(ap(MyLeafSpec(2).create())); top->addChild(ap(MyLeafSpec(1).create())); top->addChild(ap(MyLeafSpec(3).create())); { - SourceBlenderBlueprint *blender = new SourceBlenderBlueprint(*selector_1); + auto *blender = new SourceBlenderBlueprint(*selector_1); blender->addChild(ap(MyLeafSpec(200).create()->setSourceId(2))); blender->addChild(ap(MyLeafSpec(100).create()->setSourceId(1))); blender->addChild(ap(MyLeafSpec(300).create()->setSourceId(3))); top->addChild(ap(blender)); } { - SourceBlenderBlueprint *blender = new SourceBlenderBlueprint(*selector_1); + auto *blender = new SourceBlenderBlueprint(*selector_1); blender->addChild(ap(MyLeafSpec(20).create()->setSourceId(2))); blender->addChild(ap(MyLeafSpec(10).create()->setSourceId(1))); blender->addChild(ap(MyLeafSpec(30).create()->setSourceId(3))); top->addChild(ap(blender)); } { - SourceBlenderBlueprint *blender = new SourceBlenderBlueprint(*selector_2); + auto *blender = new SourceBlenderBlueprint(*selector_2); blender->addChild(ap(MyLeafSpec(10).create()->setSourceId(1))); blender->addChild(ap(MyLeafSpec(20).create()->setSourceId(2))); top->addChild(ap(blender)); } { - SourceBlenderBlueprint *blender = new SourceBlenderBlueprint(*selector_1); + auto *blender = new SourceBlenderBlueprint(*selector_1); blender->addChild(ap(MyLeafSpec(2000).create()->setSourceId(2))); blender->addChild(ap(MyLeafSpec(1000).create()->setSourceId(1))); top->addChild(ap(blender)); } //------------------------------------------------------------------------- - AndBlueprint *expect = new AndBlueprint(); + auto *expect = new AndBlueprint(); Blueprint::UP expect_bp(expect); expect->addChild(ap(MyLeafSpec(1).create())); expect->addChild(ap(MyLeafSpec(2).create())); expect->addChild(ap(MyLeafSpec(3).create())); { - SourceBlenderBlueprint *blender = new SourceBlenderBlueprint(*selector_2); + auto *blender = new SourceBlenderBlueprint(*selector_2); blender->addChild(ap(MyLeafSpec(10).create()->setSourceId(1))); blender->addChild(ap(MyLeafSpec(20).create()->setSourceId(2))); expect->addChild(ap(blender)); } { - SourceBlenderBlueprint *blender(new SourceBlenderBlueprint(*selector_1)); + auto *blender(new SourceBlenderBlueprint(*selector_1)); { - AndBlueprint *sub_and = new AndBlueprint(); + auto *sub_and = new AndBlueprint(); sub_and->setSourceId(3); sub_and->addChild(ap(MyLeafSpec(30).create()->setSourceId(3))); sub_and->addChild(ap(MyLeafSpec(300).create()->setSourceId(3))); blender->addChild(ap(sub_and)); } { - AndBlueprint *sub_and = new AndBlueprint(); + auto *sub_and = new AndBlueprint(); sub_and->setSourceId(2); sub_and->addChild(ap(MyLeafSpec(20).create()->setSourceId(2))); sub_and->addChild(ap(MyLeafSpec(200).create()->setSourceId(2))); @@ -514,7 +538,7 @@ TEST("test SourceBlender below AND optimization") { blender->addChild(ap(sub_and)); } { - AndBlueprint *sub_and = new AndBlueprint(); + auto *sub_and = new AndBlueprint(); sub_and->setSourceId(1); sub_and->addChild(ap(MyLeafSpec(10).create()->setSourceId(1))); sub_and->addChild(ap(MyLeafSpec(100).create()->setSourceId(1))); @@ -535,51 +559,51 @@ TEST("test SourceBlender below OR optimization") { auto selector_1 = std::make_unique<InvalidSelector>(); // the one auto selector_2 = std::make_unique<InvalidSelector>(); // not the one //------------------------------------------------------------------------- - OrBlueprint *top = new OrBlueprint(); + auto *top = new OrBlueprint(); Blueprint::UP top_up(top); top->addChild(ap(MyLeafSpec(2).create())); top->addChild(ap(MyLeafSpec(1).create())); top->addChild(ap(MyLeafSpec(3).create())); { - SourceBlenderBlueprint *blender = new SourceBlenderBlueprint(*selector_1); + auto *blender = new SourceBlenderBlueprint(*selector_1); blender->addChild(ap(MyLeafSpec(200).create()->setSourceId(2))); blender->addChild(ap(MyLeafSpec(100).create()->setSourceId(1))); blender->addChild(ap(MyLeafSpec(300).create()->setSourceId(3))); top->addChild(ap(blender)); } { - SourceBlenderBlueprint *blender = new SourceBlenderBlueprint(*selector_1); + auto *blender = new SourceBlenderBlueprint(*selector_1); blender->addChild(ap(MyLeafSpec(20).create()->setSourceId(2))); blender->addChild(ap(MyLeafSpec(10).create()->setSourceId(1))); blender->addChild(ap(MyLeafSpec(30).create()->setSourceId(3))); top->addChild(ap(blender)); } { - SourceBlenderBlueprint *blender = new SourceBlenderBlueprint(*selector_2); + auto *blender = new SourceBlenderBlueprint(*selector_2); blender->addChild(ap(MyLeafSpec(10).create()->setSourceId(1))); blender->addChild(ap(MyLeafSpec(20).create()->setSourceId(2))); top->addChild(ap(blender)); } { - SourceBlenderBlueprint *blender = new SourceBlenderBlueprint(*selector_1); + auto *blender = new SourceBlenderBlueprint(*selector_1); blender->addChild(ap(MyLeafSpec(2000).create()->setSourceId(2))); blender->addChild(ap(MyLeafSpec(1000).create()->setSourceId(1))); top->addChild(ap(blender)); } //------------------------------------------------------------------------- - OrBlueprint *expect = new OrBlueprint(); + auto *expect = new OrBlueprint(); Blueprint::UP expect_up(expect); { - SourceBlenderBlueprint *blender(new SourceBlenderBlueprint(*selector_1)); + auto *blender(new SourceBlenderBlueprint(*selector_1)); { - OrBlueprint *sub_and = new OrBlueprint(); + auto *sub_and = new OrBlueprint(); sub_and->setSourceId(3); sub_and->addChild(ap(MyLeafSpec(300).create()->setSourceId(3))); sub_and->addChild(ap(MyLeafSpec(30).create()->setSourceId(3))); blender->addChild(ap(sub_and)); } { - OrBlueprint *sub_and = new OrBlueprint(); + auto *sub_and = new OrBlueprint(); sub_and->setSourceId(2); sub_and->addChild(ap(MyLeafSpec(2000).create()->setSourceId(2))); sub_and->addChild(ap(MyLeafSpec(200).create()->setSourceId(2))); @@ -587,7 +611,7 @@ TEST("test SourceBlender below OR optimization") { blender->addChild(ap(sub_and)); } { - OrBlueprint *sub_and = new OrBlueprint(); + auto *sub_and = new OrBlueprint(); sub_and->setSourceId(1); sub_and->addChild(ap(MyLeafSpec(1000).create()->setSourceId(1))); sub_and->addChild(ap(MyLeafSpec(100).create()->setSourceId(1))); @@ -597,7 +621,7 @@ TEST("test SourceBlender below OR optimization") { expect->addChild(ap(blender)); } { - SourceBlenderBlueprint *blender = new SourceBlenderBlueprint(*selector_2); + auto *blender = new SourceBlenderBlueprint(*selector_2); blender->addChild(ap(MyLeafSpec(10).create()->setSourceId(1))); blender->addChild(ap(MyLeafSpec(20).create()->setSourceId(2))); expect->addChild(ap(blender)); @@ -617,10 +641,10 @@ TEST("test SourceBlender below AND_NOT optimization") { auto selector_1 = std::make_unique<InvalidSelector>(); // the one auto selector_2 = std::make_unique<InvalidSelector>(); // not the one //------------------------------------------------------------------------- - AndNotBlueprint *top = new AndNotBlueprint(); + auto *top = new AndNotBlueprint(); Blueprint::UP top_up(top); { - SourceBlenderBlueprint *blender = new SourceBlenderBlueprint(*selector_1); + auto *blender = new SourceBlenderBlueprint(*selector_1); blender->addChild(ap(MyLeafSpec(42).create()->setSourceId(1))); top->addChild(ap(blender)); } @@ -628,50 +652,50 @@ TEST("test SourceBlender below AND_NOT optimization") { top->addChild(ap(MyLeafSpec(1).create())); top->addChild(ap(MyLeafSpec(3).create())); { - SourceBlenderBlueprint *blender = new SourceBlenderBlueprint(*selector_1); + auto *blender = new SourceBlenderBlueprint(*selector_1); blender->addChild(ap(MyLeafSpec(200).create()->setSourceId(2))); blender->addChild(ap(MyLeafSpec(100).create()->setSourceId(1))); blender->addChild(ap(MyLeafSpec(300).create()->setSourceId(3))); top->addChild(ap(blender)); } { - SourceBlenderBlueprint *blender = new SourceBlenderBlueprint(*selector_1); + auto *blender = new SourceBlenderBlueprint(*selector_1); blender->addChild(ap(MyLeafSpec(20).create()->setSourceId(2))); blender->addChild(ap(MyLeafSpec(10).create()->setSourceId(1))); blender->addChild(ap(MyLeafSpec(30).create()->setSourceId(3))); top->addChild(ap(blender)); } { - SourceBlenderBlueprint *blender = new SourceBlenderBlueprint(*selector_2); + auto *blender = new SourceBlenderBlueprint(*selector_2); blender->addChild(ap(MyLeafSpec(10).create()->setSourceId(1))); blender->addChild(ap(MyLeafSpec(20).create()->setSourceId(2))); top->addChild(ap(blender)); } { - SourceBlenderBlueprint *blender = new SourceBlenderBlueprint(*selector_1); + auto *blender = new SourceBlenderBlueprint(*selector_1); blender->addChild(ap(MyLeafSpec(2000).create()->setSourceId(2))); blender->addChild(ap(MyLeafSpec(1000).create()->setSourceId(1))); top->addChild(ap(blender)); } //------------------------------------------------------------------------- - AndNotBlueprint *expect = new AndNotBlueprint(); + auto *expect = new AndNotBlueprint(); Blueprint::UP expect_up(expect); { - SourceBlenderBlueprint *blender = new SourceBlenderBlueprint(*selector_1); + auto *blender = new SourceBlenderBlueprint(*selector_1); blender->addChild(ap(MyLeafSpec(42).create()->setSourceId(1))); expect->addChild(ap(blender)); } { - SourceBlenderBlueprint *blender(new SourceBlenderBlueprint(*selector_1)); + auto *blender(new SourceBlenderBlueprint(*selector_1)); { - OrBlueprint *sub_and = new OrBlueprint(); + auto *sub_and = new OrBlueprint(); sub_and->setSourceId(3); sub_and->addChild(ap(MyLeafSpec(300).create()->setSourceId(3))); sub_and->addChild(ap(MyLeafSpec(30).create()->setSourceId(3))); blender->addChild(ap(sub_and)); } { - OrBlueprint *sub_and = new OrBlueprint(); + auto *sub_and = new OrBlueprint(); sub_and->setSourceId(2); sub_and->addChild(ap(MyLeafSpec(2000).create()->setSourceId(2))); sub_and->addChild(ap(MyLeafSpec(200).create()->setSourceId(2))); @@ -679,7 +703,7 @@ TEST("test SourceBlender below AND_NOT optimization") { blender->addChild(ap(sub_and)); } { - OrBlueprint *sub_and = new OrBlueprint(); + auto *sub_and = new OrBlueprint(); sub_and->setSourceId(1); sub_and->addChild(ap(MyLeafSpec(1000).create()->setSourceId(1))); sub_and->addChild(ap(MyLeafSpec(100).create()->setSourceId(1))); @@ -689,7 +713,7 @@ TEST("test SourceBlender below AND_NOT optimization") { expect->addChild(ap(blender)); } { - SourceBlenderBlueprint *blender = new SourceBlenderBlueprint(*selector_2); + auto *blender = new SourceBlenderBlueprint(*selector_2); blender->addChild(ap(MyLeafSpec(10).create()->setSourceId(1))); blender->addChild(ap(MyLeafSpec(20).create()->setSourceId(2))); expect->addChild(ap(blender)); @@ -709,10 +733,10 @@ TEST("test SourceBlender below RANK optimization") { auto selector_1 = std::make_unique<InvalidSelector>(); // the one auto selector_2 = std::make_unique<InvalidSelector>(); // not the one //------------------------------------------------------------------------- - RankBlueprint *top = new RankBlueprint(); + auto *top = new RankBlueprint(); Blueprint::UP top_up(top); { - SourceBlenderBlueprint *blender = new SourceBlenderBlueprint(*selector_1); + auto *blender = new SourceBlenderBlueprint(*selector_1); blender->addChild(ap(MyLeafSpec(42).create()->setSourceId(1))); top->addChild(ap(blender)); } @@ -720,36 +744,36 @@ TEST("test SourceBlender below RANK optimization") { top->addChild(ap(MyLeafSpec(1).create())); top->addChild(ap(MyLeafSpec(3).create())); { - SourceBlenderBlueprint *blender = new SourceBlenderBlueprint(*selector_1); + auto *blender = new SourceBlenderBlueprint(*selector_1); blender->addChild(ap(MyLeafSpec(200).create()->setSourceId(2))); blender->addChild(ap(MyLeafSpec(100).create()->setSourceId(1))); blender->addChild(ap(MyLeafSpec(300).create()->setSourceId(3))); top->addChild(ap(blender)); } { - SourceBlenderBlueprint *blender = new SourceBlenderBlueprint(*selector_1); + auto *blender = new SourceBlenderBlueprint(*selector_1); blender->addChild(ap(MyLeafSpec(20).create()->setSourceId(2))); blender->addChild(ap(MyLeafSpec(10).create()->setSourceId(1))); blender->addChild(ap(MyLeafSpec(30).create()->setSourceId(3))); top->addChild(ap(blender)); } { - SourceBlenderBlueprint *blender = new SourceBlenderBlueprint(*selector_2); + auto *blender = new SourceBlenderBlueprint(*selector_2); blender->addChild(ap(MyLeafSpec(10).create()->setSourceId(1))); blender->addChild(ap(MyLeafSpec(20).create()->setSourceId(2))); top->addChild(ap(blender)); } { - SourceBlenderBlueprint *blender = new SourceBlenderBlueprint(*selector_1); + auto *blender = new SourceBlenderBlueprint(*selector_1); blender->addChild(ap(MyLeafSpec(2000).create()->setSourceId(2))); blender->addChild(ap(MyLeafSpec(1000).create()->setSourceId(1))); top->addChild(ap(blender)); } //------------------------------------------------------------------------- - RankBlueprint *expect = new RankBlueprint(); + auto *expect = new RankBlueprint(); Blueprint::UP expect_up(expect); { - SourceBlenderBlueprint *blender = new SourceBlenderBlueprint(*selector_1); + auto *blender = new SourceBlenderBlueprint(*selector_1); blender->addChild(ap(MyLeafSpec(42).create()->setSourceId(1))); expect->addChild(ap(blender)); } @@ -757,22 +781,22 @@ TEST("test SourceBlender below RANK optimization") { expect->addChild(ap(MyLeafSpec(1).create())); expect->addChild(ap(MyLeafSpec(3).create())); { - SourceBlenderBlueprint *blender = new SourceBlenderBlueprint(*selector_2); + auto *blender = new SourceBlenderBlueprint(*selector_2); blender->addChild(ap(MyLeafSpec(10).create()->setSourceId(1))); blender->addChild(ap(MyLeafSpec(20).create()->setSourceId(2))); expect->addChild(ap(blender)); } { - SourceBlenderBlueprint *blender(new SourceBlenderBlueprint(*selector_1)); + auto *blender(new SourceBlenderBlueprint(*selector_1)); { - OrBlueprint *sub_and = new OrBlueprint(); + auto *sub_and = new OrBlueprint(); sub_and->setSourceId(3); sub_and->addChild(ap(MyLeafSpec(300).create()->setSourceId(3))); sub_and->addChild(ap(MyLeafSpec(30).create()->setSourceId(3))); blender->addChild(ap(sub_and)); } { - OrBlueprint *sub_and = new OrBlueprint(); + auto *sub_and = new OrBlueprint(); sub_and->setSourceId(2); sub_and->addChild(ap(MyLeafSpec(2000).create()->setSourceId(2))); sub_and->addChild(ap(MyLeafSpec(200).create()->setSourceId(2))); @@ -780,7 +804,7 @@ TEST("test SourceBlender below RANK optimization") { blender->addChild(ap(sub_and)); } { - OrBlueprint *sub_and = new OrBlueprint(); + auto *sub_and = new OrBlueprint(); sub_and->setSourceId(1); sub_and->addChild(ap(MyLeafSpec(1000).create()->setSourceId(1))); sub_and->addChild(ap(MyLeafSpec(100).create()->setSourceId(1))); @@ -826,7 +850,7 @@ TEST("test empty root node optimization and safeness") { addChild(ap(MyLeafSpec(0, true).create())). addChild(ap(MyLeafSpec(0, true).create())))); //------------------------------------------------------------------------- - Blueprint::UP expect_up(new EmptyBlueprint()); + auto expect_up = std::make_unique<EmptyBlueprint>(); //------------------------------------------------------------------------- top1_up = Blueprint::optimize(std::move(top1_up)); top2_up = Blueprint::optimize(std::move(top2_up)); @@ -851,7 +875,7 @@ TEST("and with one empty child is optimized away") { top = Blueprint::optimize(std::move(top)); Blueprint::UP expect_up(ap((new SourceBlenderBlueprint(*selector))-> addChild(ap(MyLeafSpec(10).create())). - addChild(ap(new EmptyBlueprint())))); + addChild(std::make_unique<EmptyBlueprint>()))); EXPECT_EQUAL(expect_up->asString(), top->asString()); } @@ -964,11 +988,11 @@ TEST("require that replaced blueprints retain source id") { //------------------------------------------------------------------------- // replace empty root with empty search Blueprint::UP top1_up(ap(MyLeafSpec(0, true).create()->setSourceId(13))); - Blueprint::UP expect1_up(new EmptyBlueprint()); + auto expect1_up = std::make_unique<EmptyBlueprint>(); expect1_up->setSourceId(13); //------------------------------------------------------------------------- // replace self with single child - Blueprint::UP top2_up(ap(static_cast<AndBlueprint&>((new AndBlueprint())->setSourceId(42)). + Blueprint::UP top2_up(ap(dynamic_cast<AndBlueprint&>((new AndBlueprint())->setSourceId(42)). addChild(ap(MyLeafSpec(30).create()->setSourceId(55))))); Blueprint::UP expect2_up(ap(MyLeafSpec(30).create()->setSourceId(42))); //------------------------------------------------------------------------- @@ -1017,16 +1041,16 @@ TEST("test WeakAnd Blueprint") { std::vector<Blueprint::HitEstimate> est; EXPECT_EQUAL(true, b.combine(est).empty); EXPECT_EQUAL(0u, b.combine(est).estHits); - est.push_back(Blueprint::HitEstimate(10, false)); + est.emplace_back(10, false); EXPECT_EQUAL(false, b.combine(est).empty); EXPECT_EQUAL(10u, b.combine(est).estHits); - est.push_back(Blueprint::HitEstimate(20, false)); + est.emplace_back(20, false); EXPECT_EQUAL(false, b.combine(est).empty); EXPECT_EQUAL(20u, b.combine(est).estHits); - est.push_back(Blueprint::HitEstimate(5, false)); + est.emplace_back(5, false); EXPECT_EQUAL(false, b.combine(est).empty); EXPECT_EQUAL(20u, b.combine(est).estHits); - est.push_back(Blueprint::HitEstimate(0, true)); + est.emplace_back(0, true); EXPECT_EQUAL(false, b.combine(est).empty); EXPECT_EQUAL(20u, b.combine(est).estHits); } @@ -1057,14 +1081,14 @@ TEST("test WeakAnd Blueprint") { { WeakAndBlueprint wa(456); MatchData::UP md = MatchData::makeTestInstance(100, 10); - wa.addTerm(Blueprint::UP(new FakeBlueprint(field, x)), 120); - wa.addTerm(Blueprint::UP(new FakeBlueprint(field, z)), 140); - wa.addTerm(Blueprint::UP(new FakeBlueprint(field, y)), 130); + wa.addTerm(std::make_unique<FakeBlueprint>(field, x), 120); + wa.addTerm(std::make_unique<FakeBlueprint>(field, z), 140); + wa.addTerm(std::make_unique<FakeBlueprint>(field, y), 130); { wa.fetchPostings(ExecuteInfo::TRUE); SearchIterator::UP search = wa.createSearch(*md, true); - EXPECT_TRUE(dynamic_cast<WeakAndSearch*>(search.get()) != 0); - WeakAndSearch &s = dynamic_cast<WeakAndSearch&>(*search); + EXPECT_TRUE(dynamic_cast<WeakAndSearch*>(search.get()) != nullptr); + auto &s = dynamic_cast<WeakAndSearch&>(*search); EXPECT_EQUAL(456u, s.getN()); ASSERT_EQUAL(3u, s.getTerms().size()); EXPECT_GREATER(s.get_max_score(0), 0.0); @@ -1085,7 +1109,7 @@ TEST("test WeakAnd Blueprint") { { wa.fetchPostings(ExecuteInfo::FALSE); SearchIterator::UP search = wa.createSearch(*md, false); - EXPECT_TRUE(dynamic_cast<WeakAndSearch*>(search.get()) != 0); + EXPECT_TRUE(dynamic_cast<WeakAndSearch*>(search.get()) != nullptr); EXPECT_TRUE(search->seek(1)); EXPECT_TRUE(search->seek(2)); EXPECT_FALSE(search->seek(3)); @@ -1195,7 +1219,7 @@ namespace { SimpleStringTerm makeTerm(const std::string & term) { - return SimpleStringTerm(term, "field", 0, search::query::Weight(0)); + return {term, "field", 0, search::query::Weight(0)}; } } @@ -1231,7 +1255,7 @@ TEST("require that children does not optimize when parents refuse them to") { SearchIterator::UP search = top_up->createSearch(*md, true); EXPECT_EQUAL("search::queryeval::EquivImpl<true>", search->getClassName()); { - const MultiSearch & e = dynamic_cast<const MultiSearch &>(*search); + const auto & e = dynamic_cast<const MultiSearch &>(*search); EXPECT_EQUAL("search::BitVectorIteratorStrictT<false>", e.getChildren()[0]->getClassName()); EXPECT_EQUAL("search::diskindex::ZcRareWordPosOccIterator<true, false>", e.getChildren()[1]->getClassName()); EXPECT_EQUAL("search::diskindex::ZcRareWordPosOccIterator<true, false>", e.getChildren()[2]->getClassName()); @@ -1241,7 +1265,7 @@ TEST("require that children does not optimize when parents refuse them to") { search = top_up->createSearch(*md, true); EXPECT_EQUAL("search::queryeval::EquivImpl<true>", search->getClassName()); { - const MultiSearch & e = dynamic_cast<const MultiSearch &>(*search); + const auto & e = dynamic_cast<const MultiSearch &>(*search); EXPECT_EQUAL("search::BitVectorIteratorStrictT<false>", e.getChildren()[0]->getClassName()); EXPECT_EQUAL("search::diskindex::ZcRareWordPosOccIterator<true, false>", e.getChildren()[1]->getClassName()); EXPECT_EQUAL("search::diskindex::ZcRareWordPosOccIterator<true, false>", e.getChildren()[2]->getClassName()); @@ -1269,7 +1293,7 @@ TEST("require_that_unpack_optimization_is_not_overruled_by_equiv") { SearchIterator::UP search = top_up->createSearch(*md, true); EXPECT_EQUAL("search::queryeval::EquivImpl<true>", search->getClassName()); { - const MultiSearch & e = dynamic_cast<const MultiSearch &>(*search); + const auto & e = dynamic_cast<const MultiSearch &>(*search); EXPECT_EQUAL("search::queryeval::OrLikeSearch<true, search::queryeval::(anonymous namespace)::FullUnpack>", e.getChildren()[0]->getClassName()); } @@ -1278,7 +1302,7 @@ TEST("require_that_unpack_optimization_is_not_overruled_by_equiv") { search = top_up->createSearch(*md, true); EXPECT_EQUAL("search::queryeval::EquivImpl<true>", search->getClassName()); { - const MultiSearch & e = dynamic_cast<const MultiSearch &>(*search); + const auto & e = dynamic_cast<const MultiSearch &>(*search); EXPECT_EQUAL("search::queryeval::OrLikeSearch<true, search::queryeval::(anonymous namespace)::SelectiveUnpack>", e.getChildren()[0]->getClassName()); } @@ -1288,7 +1312,7 @@ TEST("require_that_unpack_optimization_is_not_overruled_by_equiv") { search = top_up->createSearch(*md, true); EXPECT_EQUAL("search::queryeval::EquivImpl<true>", search->getClassName()); { - const MultiSearch & e = dynamic_cast<const MultiSearch &>(*search); + const auto & e = dynamic_cast<const MultiSearch &>(*search); EXPECT_EQUAL("search::queryeval::OrLikeSearch<true, search::queryeval::NoUnpack>", e.getChildren()[0]->getClassName()); } @@ -1344,7 +1368,7 @@ TEST("require that children of onear are not optimized") { TEST("require that ANDNOT without children is optimized to empty search") { Blueprint::UP top_up(new AndNotBlueprint()); - Blueprint::UP expect_up(new EmptyBlueprint()); + auto expect_up = std::make_unique<EmptyBlueprint>(); top_up = Blueprint::optimize(std::move(top_up)); EXPECT_EQUAL(expect_up->asString(), top_up->asString()); } @@ -1395,22 +1419,57 @@ TEST("require that highest cost tier sorts last for AND") { EXPECT_EQUAL(expect_up->asString(), top_up->asString()); } -TEST("require that intermediate cost tier is minimum cost tier of children") { - Blueprint::UP bp1( - ap((new AndBlueprint())-> - addChild(ap(MyLeafSpec(10).cost_tier(1).create())). - addChild(ap(MyLeafSpec(20).cost_tier(2).create())). - addChild(ap(MyLeafSpec(30).cost_tier(3).create())))); - Blueprint::UP bp2( - ap((new AndBlueprint())-> - addChild(ap(MyLeafSpec(10).cost_tier(3).create())). - addChild(ap(MyLeafSpec(20).cost_tier(2).create())). - addChild(ap(MyLeafSpec(30).cost_tier(2).create())))); - EXPECT_EQUAL(bp1->getState().cost_tier(), 1u); - EXPECT_EQUAL(bp2->getState().cost_tier(), 2u); +template<typename BP> +void +verifyCostTierInheritance(uint8_t expected, uint8_t expected_reverse) { + auto bp1 = std::make_unique<BP>(); + bp1->addChild(ap(MyLeafSpec(10).cost_tier(1).create())). + addChild(ap(MyLeafSpec(20).cost_tier(2).create())). + addChild(ap(MyLeafSpec(30).cost_tier(3).create())); + auto bp2 = std::make_unique<BP>(); + bp2->addChild(ap(MyLeafSpec(10).cost_tier(3).create())). + addChild(ap(MyLeafSpec(20).cost_tier(2).create())). + addChild(ap(MyLeafSpec(30).cost_tier(1).create())); + EXPECT_EQUAL(bp1->getState().cost_tier(), expected); + EXPECT_EQUAL(bp2->getState().cost_tier(), expected_reverse); +} + +TEST("require that AND cost tier is minimum cost tier of children") { + verifyCostTierInheritance<AndBlueprint>(1, 1); +} + +TEST("require that OR cost tier is maximum cost tier of children") { + verifyCostTierInheritance<OrBlueprint>(3, 3); +} + +TEST("require that Rank cost tier is first childs cost tier") { + verifyCostTierInheritance<RankBlueprint>(1, 3); +} + +TEST("require that AndNot cost tier is first childs cost tier") { + verifyCostTierInheritance<AndNotBlueprint>(1, 3); +} + +struct MySourceBlender { + InvalidSelector selector; + SourceBlenderBlueprint sb; + MySourceBlender() : selector(), sb(selector) {} + IntermediateBlueprint & + addChild(Blueprint::UP child) { + return sb.addChild(std::move(child)); + } + const Blueprint::State &getState() const { + return sb.getState(); + } + +}; + +TEST("require that SourceBlender cost tier is maximum cost tier of children") { + verifyCostTierInheritance<MySourceBlender>(3, 3); } -void verify_or_est(const std::vector<Blueprint::HitEstimate> &child_estimates, Blueprint::HitEstimate expect) { +void +verify_or_est(const std::vector<Blueprint::HitEstimate> &child_estimates, Blueprint::HitEstimate expect) { OrBlueprint my_or; my_or.setDocIdLimit(32); auto my_est = my_or.combine(child_estimates); diff --git a/searchlib/src/tests/queryeval/filter_search/filter_search_test.cpp b/searchlib/src/tests/queryeval/filter_search/filter_search_test.cpp index 71033ed7d06..5933122d7a2 100644 --- a/searchlib/src/tests/queryeval/filter_search/filter_search_test.cpp +++ b/searchlib/src/tests/queryeval/filter_search/filter_search_test.cpp @@ -47,7 +47,7 @@ concept ChildCollector = requires(T a, std::unique_ptr<Blueprint> bp) { // inherit Blueprint to capture the default filter factory struct DefaultBlueprint : Blueprint { - void optimize(Blueprint* &) override { abort(); } + void optimize(Blueprint* &, OptimizePass) override { abort(); } const State &getState() const override { abort(); } void fetchPostings(const ExecuteInfo &) override { abort(); } void freeze() override { abort(); } diff --git a/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.hpp b/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.hpp index e4047b57341..cc12b1f7825 100644 --- a/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.hpp +++ b/searchlib/src/vespa/searchlib/attribute/postinglistsearchcontext.hpp @@ -204,7 +204,7 @@ createPostingIterator(fef::TermFieldMatchData *matchData, bool strict) } } // returning nullptr will trigger fallback to filter iterator - return SearchIterator::UP(); + return {}; } diff --git a/searchlib/src/vespa/searchlib/attribute/postingstore.cpp b/searchlib/src/vespa/searchlib/attribute/postingstore.cpp index 8bee8b3f7f7..2e9e5470b5f 100644 --- a/searchlib/src/vespa/searchlib/attribute/postingstore.cpp +++ b/searchlib/src/vespa/searchlib/attribute/postingstore.cpp @@ -44,8 +44,8 @@ PostingStoreBase2::resizeBitVectors(uint32_t newSize, uint32_t newCapacity) newSize = newCapacity; if (newSize == _bvSize && newCapacity == _bvCapacity) return false; - _minBvDocFreq = std::max(newSize >> 6, 64u); - _maxBvDocFreq = std::max(newSize >> 5, 128u); + _minBvDocFreq = std::max(newSize >> 7, 64u); + _maxBvDocFreq = std::max(newSize >> 6, 128u); if (_bvs.empty()) { _bvSize = newSize; _bvCapacity = newCapacity; @@ -613,9 +613,13 @@ PostingStore<DataT>::update_stat(const CompactionStrategy& compaction_strategy) vespalib::MemoryUsage usage; auto btree_nodes_memory_usage = _allocator.getMemoryUsage(); auto store_memory_usage = _store.getMemoryUsage(); - _compaction_spec = PostingStoreCompactionSpec(compaction_strategy.should_compact_memory(btree_nodes_memory_usage), compaction_strategy.should_compact_memory(store_memory_usage)); usage.merge(btree_nodes_memory_usage); usage.merge(store_memory_usage); + if (compaction_strategy.should_compact_memory(usage)) { + _compaction_spec = PostingStoreCompactionSpec(compaction_strategy.should_compact_memory(btree_nodes_memory_usage), compaction_strategy.should_compact_memory(store_memory_usage)); + } else { + _compaction_spec = PostingStoreCompactionSpec(); + } uint64_t bvExtraBytes = _bvExtraBytes; usage.incUsedBytes(bvExtraBytes); usage.incAllocatedBytes(bvExtraBytes); diff --git a/searchlib/src/vespa/searchlib/attribute/postingstore.h b/searchlib/src/vespa/searchlib/attribute/postingstore.h index e3fb88c81f1..bd19bbd3675 100644 --- a/searchlib/src/vespa/searchlib/attribute/postingstore.h +++ b/searchlib/src/vespa/searchlib/attribute/postingstore.h @@ -57,6 +57,9 @@ public: virtual ~PostingStoreBase2(); bool resizeBitVectors(uint32_t newSize, uint32_t newCapacity); virtual bool removeSparseBitVectors() = 0; + + // Only used by unit test. + const PostingStoreCompactionSpec& get_compaction_spec() const noexcept { return _compaction_spec; } }; template <typename DataT> diff --git a/searchlib/src/vespa/searchlib/expression/expressiontree.h b/searchlib/src/vespa/searchlib/expression/expressiontree.h index 52c075f3e29..127de66b6dc 100644 --- a/searchlib/src/vespa/searchlib/expression/expressiontree.h +++ b/searchlib/src/vespa/searchlib/expression/expressiontree.h @@ -50,11 +50,11 @@ public: }; ExpressionTree() noexcept; - ExpressionTree(const ExpressionNode & root); - ExpressionTree(ExpressionNode::UP root); + explicit ExpressionTree(const ExpressionNode & root); + explicit ExpressionTree(ExpressionNode::UP root); ExpressionTree(const ExpressionTree & rhs); ExpressionTree(ExpressionTree &&) noexcept = default; - ~ExpressionTree(); + ~ExpressionTree() override; ExpressionTree & operator = (ExpressionNode::UP rhs); ExpressionTree & operator = (const ExpressionTree & rhs); ExpressionTree & operator = (ExpressionTree &&) noexcept = default; diff --git a/searchlib/src/vespa/searchlib/fef/indexproperties.cpp b/searchlib/src/vespa/searchlib/fef/indexproperties.cpp index 9b111c4bd5d..9c986d0bc63 100644 --- a/searchlib/src/vespa/searchlib/fef/indexproperties.cpp +++ b/searchlib/src/vespa/searchlib/fef/indexproperties.cpp @@ -4,6 +4,7 @@ #include "properties.h" #include <vespa/vespalib/locale/c.h> #include <limits> +#include <charconv> namespace search::fef::indexproperties { @@ -49,10 +50,15 @@ uint32_t lookupUint32(const Properties &props, const vespalib::string &name, uint32_t defaultValue) { Property p = props.lookup(name); + uint32_t value(defaultValue); if (p.found()) { - return atoi(p.get().c_str()); + const auto & valS = p.get(); + const char * start = valS.c_str(); + const char * end = start + valS.size(); + while ((start != end) && isspace(start[0])) { start++; } + std::from_chars(start, end, value); } - return defaultValue; + return value; } bool @@ -173,11 +179,6 @@ namespace onsummary { namespace temporary { -const vespalib::string EnableNestedMultivalueGrouping::NAME("vespa.temporary.enable_nested_multivalue_grouping"); -bool EnableNestedMultivalueGrouping::check(const Properties &props) { - return lookupBool(props, NAME, false); -} - } namespace mutate { @@ -454,6 +455,12 @@ FuzzyAlgorithm::lookup(const Properties& props, vespalib::FuzzyMatchingAlgorithm return vespalib::fuzzy_matching_algorithm_from_string(value, default_value); } +const vespalib::string AlwaysMarkPhraseExpensive::NAME("vespa.matching.always_mark_phrase_expensive"); +const bool AlwaysMarkPhraseExpensive::DEFAULT_VALUE(false); +bool AlwaysMarkPhraseExpensive::check(const Properties &props, bool fallback) { + return lookupBool(props, NAME, fallback); +} + } // namespace matching namespace softtimeout { diff --git a/searchlib/src/vespa/searchlib/fef/indexproperties.h b/searchlib/src/vespa/searchlib/fef/indexproperties.h index c528c4366d6..1921f52276f 100644 --- a/searchlib/src/vespa/searchlib/fef/indexproperties.h +++ b/searchlib/src/vespa/searchlib/fef/indexproperties.h @@ -176,11 +176,8 @@ namespace mutate { }; } +// Add temporary flags used for safe rollout of new features here namespace temporary { - struct EnableNestedMultivalueGrouping { - static const vespalib::string NAME; - static bool check(const Properties &props); - }; } namespace mutate::on_match { @@ -339,6 +336,17 @@ namespace matching { static vespalib::FuzzyMatchingAlgorithm lookup(const Properties& props); static vespalib::FuzzyMatchingAlgorithm lookup(const Properties& props, vespalib::FuzzyMatchingAlgorithm default_value); }; + + /** + * When enabled, the unpacking part of the phrase iterator will be tagged as expensive + * under all intermediate iterators, not only AND. + **/ + struct AlwaysMarkPhraseExpensive { + static const vespalib::string NAME; + static const bool DEFAULT_VALUE; + static bool check(const Properties &props) { return check(props, DEFAULT_VALUE); } + static bool check(const Properties &props, bool fallback); + }; } namespace softtimeout { diff --git a/searchlib/src/vespa/searchlib/fef/properties.cpp b/searchlib/src/vespa/searchlib/fef/properties.cpp index bd4795fcc5a..6134807fa60 100644 --- a/searchlib/src/vespa/searchlib/fef/properties.cpp +++ b/searchlib/src/vespa/searchlib/fef/properties.cpp @@ -25,7 +25,7 @@ uint32_t Properties::rawHash(const void *buf, uint32_t len) noexcept { uint32_t res = 0; - unsigned const char *pt = (unsigned const char *) buf; + auto *pt = (unsigned const char *) buf; unsigned const char *end = pt + len; while (pt < end) { res = (res << 7) + (res >> 25) + *pt++; @@ -52,7 +52,7 @@ Properties::add(vespalib::stringref key, vespalib::stringref value) { if (!key.empty()) { Value & v = _data[key]; - v.push_back(value); + v.emplace_back(value); ++_numValues; } return *this; @@ -162,20 +162,20 @@ Property Properties::lookup(vespalib::stringref key) const noexcept { if (key.empty()) { - return Property(); + return {}; } auto node = _data.find(key); if (node == _data.end()) { - return Property(); + return {}; } - return Property(node->second); + return {node->second}; } Property Properties::lookup(vespalib::stringref namespace1, vespalib::stringref key) const noexcept { if (namespace1.empty() || key.empty()) { - return Property(); + return {}; } vespalib::string fullKey(namespace1); fullKey.append('.').append(key); @@ -187,7 +187,7 @@ Property Properties::lookup(vespalib::stringref namespace1, vespalib::stringref key) const noexcept { if (namespace1.empty() || namespace2.empty() || key.empty()) { - return Property(); + return {}; } vespalib::string fullKey(namespace1); fullKey.append('.').append(namespace2).append('.').append(key); @@ -200,7 +200,7 @@ Property Properties::lookup(vespalib::stringref namespace1, vespalib::stringref key) const noexcept { if (namespace1.empty() || namespace2.empty() || namespace3.empty() || key.empty()) { - return Property(); + return {}; } vespalib::string fullKey(namespace1); fullKey.append('.').append(namespace2).append('.').append(namespace3).append('.').append(key); diff --git a/searchlib/src/vespa/searchlib/fef/ranksetup.cpp b/searchlib/src/vespa/searchlib/fef/ranksetup.cpp index 33e7dbda04b..d6b0b900516 100644 --- a/searchlib/src/vespa/searchlib/fef/ranksetup.cpp +++ b/searchlib/src/vespa/searchlib/fef/ranksetup.cpp @@ -60,6 +60,7 @@ RankSetup::RankSetup(const BlueprintFactory &factory, const IIndexEnvironment &i _compiled(false), _compileError(false), _degradationAscendingOrder(false), + _always_mark_phrase_expensive(false), _diversityAttribute(), _diversityMinGroups(1), _diversityCutoffFactor(10.0), @@ -74,8 +75,7 @@ RankSetup::RankSetup(const BlueprintFactory &factory, const IIndexEnvironment &i _mutateOnFirstPhase(), _mutateOnSecondPhase(), _mutateOnSummary(), - _mutateAllowQueryOverride(false), - _enableNestedMultivalueGrouping(false) + _mutateAllowQueryOverride(false) { } RankSetup::~RankSetup() = default; @@ -134,7 +134,7 @@ RankSetup::configure() _mutateOnSummary._attribute = mutate::on_summary::Attribute::lookup(_indexEnv.getProperties()); _mutateOnSummary._operation = mutate::on_summary::Operation::lookup(_indexEnv.getProperties()); _mutateAllowQueryOverride = mutate::AllowQueryOverride::check(_indexEnv.getProperties()); - _enableNestedMultivalueGrouping = temporary::EnableNestedMultivalueGrouping::check(_indexEnv.getProperties()); + _always_mark_phrase_expensive = matching::AlwaysMarkPhraseExpensive::check(_indexEnv.getProperties()); } void diff --git a/searchlib/src/vespa/searchlib/fef/ranksetup.h b/searchlib/src/vespa/searchlib/fef/ranksetup.h index 6f4651939ad..d744b38cc6e 100644 --- a/searchlib/src/vespa/searchlib/fef/ranksetup.h +++ b/searchlib/src/vespa/searchlib/fef/ranksetup.h @@ -32,7 +32,7 @@ public: : _attribute(attribute), _operation(operation) {} - bool enabled() const { return !_attribute.empty() && !_operation.empty(); } + bool enabled() const noexcept { return !_attribute.empty() && !_operation.empty(); } vespalib::string _attribute; vespalib::string _operation; }; @@ -69,6 +69,7 @@ private: bool _compiled; bool _compileError; bool _degradationAscendingOrder; + bool _always_mark_phrase_expensive; vespalib::string _diversityAttribute; uint32_t _diversityMinGroups; double _diversityCutoffFactor; @@ -84,7 +85,6 @@ private: MutateOperation _mutateOnSecondPhase; MutateOperation _mutateOnSummary; bool _mutateAllowQueryOverride; - bool _enableNestedMultivalueGrouping; void compileAndCheckForErrors(BlueprintResolver &bp); public: @@ -221,6 +221,7 @@ public: bool isDegradationOrderAscending() const { return _degradationAscendingOrder; } + bool always_mark_phrase_expensive() const noexcept { return _always_mark_phrase_expensive; } /** get number of hits to collect during graceful degradation in match phase */ uint32_t getDegradationMaxHits() const { return _degradationMaxHits; @@ -458,7 +459,6 @@ public: const MutateOperation & getMutateOnSummary() const { return _mutateOnSummary; } bool allowMutateQueryOverride() const { return _mutateAllowQueryOverride; } - bool enableNestedMultivalueGrouping() const { return _enableNestedMultivalueGrouping; } }; } diff --git a/searchlib/src/vespa/searchlib/parsequery/stackdumpiterator.cpp b/searchlib/src/vespa/searchlib/parsequery/stackdumpiterator.cpp index d1a2c26a75d..ac54ea205be 100644 --- a/searchlib/src/vespa/searchlib/parsequery/stackdumpiterator.cpp +++ b/searchlib/src/vespa/searchlib/parsequery/stackdumpiterator.cpp @@ -1,35 +1,15 @@ // Copyright Vespa.ai. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "stackdumpiterator.h" +#include <vespa/searchlib/query/tree/predicate_query_term.h> #include <vespa/vespalib/util/compress.h> #include <vespa/vespalib/objects/nbo.h> +#include <cassert> using search::query::PredicateQueryTerm; namespace search { -namespace { - -uint64_t -readUint64(const char *&p) -{ - uint64_t value; - memcpy(&value, p, sizeof(value)); - p += sizeof(value); - return vespalib::nbo::n2h(value); -} - -double -read_double(const char *&p) -{ - double value; - memcpy(&value, p, sizeof(value)); - p += sizeof(value); - return vespalib::nbo::n2h(value); -} - -} - SimpleQueryStackDumpIterator::SimpleQueryStackDumpIterator(vespalib::stringref buf) : _buf(buf.begin()), _bufEnd(buf.end()), @@ -57,8 +37,7 @@ SimpleQueryStackDumpIterator::~SimpleQueryStackDumpIterator() = default; vespalib::stringref SimpleQueryStackDumpIterator::read_stringref(const char *&p) { - uint64_t len; - p += vespalib::compress::Integer::decompressPositive(len, p); + uint64_t len = readCompressedPositiveInt(p); if ((p + len) > _bufEnd) throw false; vespalib::stringref result(p, len); p += len; @@ -68,12 +47,40 @@ SimpleQueryStackDumpIterator::read_stringref(const char *&p) uint64_t SimpleQueryStackDumpIterator::readCompressedPositiveInt(const char *&p) { + if (p > _bufEnd || !vespalib::compress::Integer::check_decompress_space(p, _bufEnd - p)) { + throw false; + } uint64_t tmp; p += vespalib::compress::Integer::decompressPositive(tmp, p); - if (p > _bufEnd) throw false; + assert(p <= _bufEnd); return tmp; } +int64_t +SimpleQueryStackDumpIterator::readCompressedInt(const char *&p) +{ + if (p > _bufEnd || !vespalib::compress::Integer::check_decompress_positive_space(p, _bufEnd - p)) { + throw false; + } + int64_t tmp; + p += vespalib::compress::Integer::decompress(tmp, p); + assert(p <= _bufEnd); + return tmp; +} + +template <typename T> +T +SimpleQueryStackDumpIterator::read_value(const char *&p) +{ + T value; + if (p + sizeof(value) > _bufEnd) { + throw false; + } + memcpy(&value, p, sizeof(value)); + p += sizeof(value); + return vespalib::nbo::n2h(value); +} + bool SimpleQueryStackDumpIterator::next() { try { @@ -97,11 +104,8 @@ bool SimpleQueryStackDumpIterator::readNext() { _currType = ParseItem::GetType(typefield); if (ParseItem::GetFeature_Weight(typefield)) { - int64_t tmpLong; - if (p >= _bufEnd) return false; - p += vespalib::compress::Integer::decompress(tmpLong, p); + int64_t tmpLong = readCompressedInt(p); _currWeight.setPercent(tmpLong); - if (p > _bufEnd) return false; } else { _currWeight.setPercent(100); } @@ -154,13 +158,8 @@ bool SimpleQueryStackDumpIterator::readNext() { _currArity = 0; break; case ParseItem::ITEM_PURE_WEIGHTED_LONG: - { - if (p + sizeof(int64_t) > _bufEnd) { - return false; - } - _curr_integer_term = readUint64(p); - _currArity = 0; - } + _curr_integer_term = read_value<int64_t>(p); + _currArity = 0; break; case ParseItem::ITEM_WORD_ALTERNATIVES: _curr_index_name = read_stringref(p); @@ -180,20 +179,20 @@ bool SimpleQueryStackDumpIterator::readNext() { _currArity = 0; break; case ParseItem::ITEM_PREDICATE_QUERY: - if ( ! readPredicate(p)) return false; + readPredicate(p); break; case ParseItem::ITEM_WEIGHTED_SET: case ParseItem::ITEM_DOT_PRODUCT: case ParseItem::ITEM_WAND: case ParseItem::ITEM_PHRASE: - if (!readComplexTerm(p)) return false; + readComplexTerm(p); break; case ParseItem::ITEM_NEAREST_NEIGHBOR: - if ( ! readNN(p)) return false; + readNN(p); break; case ParseItem::ITEM_FUZZY: - if (!readFuzzy(p)) return false; + readFuzzy(p); break; case ParseItem::ITEM_TRUE: case ParseItem::ITEM_FALSE: @@ -210,7 +209,7 @@ bool SimpleQueryStackDumpIterator::readNext() { return (p <= _bufEnd); } -bool +void SimpleQueryStackDumpIterator::readPredicate(const char *&p) { _curr_index_name = read_stringref(p); _predicate_query_term = std::make_unique<PredicateQueryTerm>(); @@ -219,22 +218,19 @@ SimpleQueryStackDumpIterator::readPredicate(const char *&p) { for (size_t i = 0; i < count; ++i) { vespalib::stringref key = read_stringref(p); vespalib::stringref value = read_stringref(p); - if (p + sizeof(uint64_t) > _bufEnd) return false; - uint64_t sub_queries = readUint64(p); + uint64_t sub_queries = read_value<uint64_t>(p); _predicate_query_term->addFeature(key, value, sub_queries); } count = readCompressedPositiveInt(p); for (size_t i = 0; i < count; ++i) { vespalib::stringref key = read_stringref(p); - if (p + 2*sizeof(uint64_t) > _bufEnd) return false; - uint64_t value = readUint64(p); - uint64_t sub_queries = readUint64(p); + uint64_t value = read_value<uint64_t>(p); + uint64_t sub_queries = read_value<uint64_t>(p); _predicate_query_term->addRangeFeature(key, value, sub_queries); } - return true; } -bool +void SimpleQueryStackDumpIterator::readNN(const char *& p) { _curr_index_name = read_stringref(p); _curr_term = read_stringref(p); // query_tensor_name @@ -244,34 +240,35 @@ SimpleQueryStackDumpIterator::readNN(const char *& p) { // XXX: remove later when QRS doesn't send this extra flag _extraIntArg2 &= ~0x40; // QRS always sends this now: - if ((p + sizeof(double))> _bufEnd) return false; - _extraDoubleArg4 = read_double(p); // distance threshold + _extraDoubleArg4 = read_value<double>(p); // distance threshold _currArity = 0; - return true; } -bool +void SimpleQueryStackDumpIterator::readComplexTerm(const char *& p) { _currArity = readCompressedPositiveInt(p); _curr_index_name = read_stringref(p); if (_currType == ParseItem::ITEM_WAND) { _extraIntArg1 = readCompressedPositiveInt(p); // targetNumHits - if ((p + 2*sizeof(double))> _bufEnd) return false; - _extraDoubleArg4 = read_double(p); // scoreThreshold - _extraDoubleArg5 = read_double(p); // thresholdBoostFactor + _extraDoubleArg4 = read_value<double>(p); // scoreThreshold + _extraDoubleArg5 = read_value<double>(p); // thresholdBoostFactor } _curr_term = vespalib::stringref(); - return true; } -bool +void SimpleQueryStackDumpIterator::readFuzzy(const char *&p) { _curr_index_name = read_stringref(p); _curr_term = read_stringref(p); // fuzzy term _extraIntArg1 = readCompressedPositiveInt(p); // maxEditDistance _extraIntArg2 = readCompressedPositiveInt(p); // prefixLength _currArity = 0; - return true; +} + +std::unique_ptr<query::PredicateQueryTerm> +SimpleQueryStackDumpIterator::getPredicateQueryTerm() +{ + return std::move(_predicate_query_term); } } diff --git a/searchlib/src/vespa/searchlib/parsequery/stackdumpiterator.h b/searchlib/src/vespa/searchlib/parsequery/stackdumpiterator.h index dece4ecc0b6..add26ac7938 100644 --- a/searchlib/src/vespa/searchlib/parsequery/stackdumpiterator.h +++ b/searchlib/src/vespa/searchlib/parsequery/stackdumpiterator.h @@ -3,8 +3,10 @@ #pragma once #include "parse.h" -#include <vespa/searchlib/query/tree/predicate_query_term.h> #include <vespa/vespalib/stllike/string.h> +#include <memory> + +namespace search::query { class PredicateQueryTerm; } namespace search { /** @@ -45,14 +47,17 @@ private: double _extraDoubleArg4; double _extraDoubleArg5; /** The predicate query specification */ - query::PredicateQueryTerm::UP _predicate_query_term; + std::unique_ptr<query::PredicateQueryTerm> _predicate_query_term; VESPA_DLL_LOCAL vespalib::stringref read_stringref(const char *&p); VESPA_DLL_LOCAL uint64_t readCompressedPositiveInt(const char *&p); - VESPA_DLL_LOCAL bool readPredicate(const char *&p); - VESPA_DLL_LOCAL bool readNN(const char *&p); - VESPA_DLL_LOCAL bool readComplexTerm(const char *& p); - VESPA_DLL_LOCAL bool readFuzzy(const char *&p); + VESPA_DLL_LOCAL int64_t readCompressedInt(const char *&p); + template <typename T> + VESPA_DLL_LOCAL T read_value(const char*& p); + VESPA_DLL_LOCAL void readPredicate(const char *&p); + VESPA_DLL_LOCAL void readNN(const char *&p); + VESPA_DLL_LOCAL void readComplexTerm(const char *& p); + VESPA_DLL_LOCAL void readFuzzy(const char *&p); VESPA_DLL_LOCAL bool readNext(); public: /** @@ -118,7 +123,7 @@ public: uint32_t getFuzzyMaxEditDistance() const { return _extraIntArg1; } uint32_t getFuzzyPrefixLength() const { return _extraIntArg2; } - query::PredicateQueryTerm::UP getPredicateQueryTerm() { return std::move(_predicate_query_term); } + std::unique_ptr<query::PredicateQueryTerm> getPredicateQueryTerm(); vespalib::stringref getIndexName() const { return _curr_index_name; } vespalib::stringref getTerm() const { return _curr_term; } diff --git a/searchlib/src/vespa/searchlib/query/tree/intermediatenodes.h b/searchlib/src/vespa/searchlib/query/tree/intermediatenodes.h index a644387a0df..871c258494f 100644 --- a/searchlib/src/vespa/searchlib/query/tree/intermediatenodes.h +++ b/searchlib/src/vespa/searchlib/query/tree/intermediatenodes.h @@ -30,14 +30,14 @@ public: //----------------------------------------------------------------------------- class WeakAnd : public QueryNodeMixin<WeakAnd, Intermediate> { - uint32_t _minHits; + uint32_t _targetNumHits; vespalib::string _view; public: virtual ~WeakAnd() = 0; - WeakAnd(uint32_t minHits, const vespalib::string & view) : _minHits(minHits), _view(view) {} + WeakAnd(uint32_t targetNumHits, const vespalib::string & view) : _targetNumHits(targetNumHits), _view(view) {} - uint32_t getMinHits() const { return _minHits; } + uint32_t getTargetNumHits() const { return _targetNumHits; } const vespalib::string & getView() const { return _view; } }; diff --git a/searchlib/src/vespa/searchlib/query/tree/querybuilder.h b/searchlib/src/vespa/searchlib/query/tree/querybuilder.h index 157bc13016d..4f9632e8df9 100644 --- a/searchlib/src/vespa/searchlib/query/tree/querybuilder.h +++ b/searchlib/src/vespa/searchlib/query/tree/querybuilder.h @@ -117,8 +117,8 @@ template <class NodeTypes> typename NodeTypes::Or *createOr() { return new typename NodeTypes::Or; } template <class NodeTypes> -typename NodeTypes::WeakAnd *createWeakAnd(uint32_t minHits, vespalib::stringref view) { - return new typename NodeTypes::WeakAnd(minHits, view); +typename NodeTypes::WeakAnd *createWeakAnd(uint32_t targetNumHits, vespalib::stringref view) { + return new typename NodeTypes::WeakAnd(targetNumHits, view); } template <class NodeTypes> typename NodeTypes::Equiv *createEquiv(int32_t id, Weight weight) { @@ -259,8 +259,8 @@ public: typename NodeTypes::Or &addOr(int child_count) { return addIntermediate(createOr<NodeTypes>(), child_count); } - typename NodeTypes::WeakAnd &addWeakAnd(int child_count, uint32_t minHits, stringref view) { - return addIntermediate(createWeakAnd<NodeTypes>(minHits, view), child_count); + typename NodeTypes::WeakAnd &addWeakAnd(int child_count, uint32_t targetNumHits, stringref view) { + return addIntermediate(createWeakAnd<NodeTypes>(targetNumHits, view), child_count); } typename NodeTypes::Equiv &addEquiv(int child_count, int32_t id, Weight weight) { return addIntermediate(createEquiv<NodeTypes>(id, weight), child_count); diff --git a/searchlib/src/vespa/searchlib/query/tree/queryreplicator.h b/searchlib/src/vespa/searchlib/query/tree/queryreplicator.h index bf77d3f77ea..b7cce0f15b1 100644 --- a/searchlib/src/vespa/searchlib/query/tree/queryreplicator.h +++ b/searchlib/src/vespa/searchlib/query/tree/queryreplicator.h @@ -43,7 +43,7 @@ private: } void visit(WeakAnd &node) override { - _builder.addWeakAnd(node.getChildren().size(), node.getMinHits(), node.getView()); + _builder.addWeakAnd(node.getChildren().size(), node.getTargetNumHits(), node.getView()); visitNodes(node.getChildren()); } diff --git a/searchlib/src/vespa/searchlib/query/tree/simplequery.h b/searchlib/src/vespa/searchlib/query/tree/simplequery.h index b2b84402593..51af979a230 100644 --- a/searchlib/src/vespa/searchlib/query/tree/simplequery.h +++ b/searchlib/src/vespa/searchlib/query/tree/simplequery.h @@ -38,8 +38,8 @@ struct SimpleOr : Or ~SimpleOr() override; }; struct SimpleWeakAnd : WeakAnd { - SimpleWeakAnd(uint32_t minHits, vespalib::stringref view) : - WeakAnd(minHits, view) + SimpleWeakAnd(uint32_t targetNumHits, vespalib::stringref view) : + WeakAnd(targetNumHits, view) {} }; struct SimpleEquiv : Equiv { diff --git a/searchlib/src/vespa/searchlib/query/tree/stackdumpcreator.cpp b/searchlib/src/vespa/searchlib/query/tree/stackdumpcreator.cpp index 3a0deff7697..465263afeb4 100644 --- a/searchlib/src/vespa/searchlib/query/tree/stackdumpcreator.cpp +++ b/searchlib/src/vespa/searchlib/query/tree/stackdumpcreator.cpp @@ -9,6 +9,7 @@ #include <vespa/vespalib/util/size_literals.h> #include <vespa/searchlib/parsequery/parse.h> #include <vespa/searchlib/util/rawbuf.h> +#include <cassert> using vespalib::string; using std::vector; @@ -50,6 +51,13 @@ class QueryNodeConverter : public QueryVisitor { _buf.append(&i, sizeof(uint8_t)); } + void append_type_and_features(ParseItem::ItemType type, uint8_t item_features) { + assert(static_cast<uint32_t>(type) < 31u); + uint8_t type_and_features = (static_cast<uint8_t>(type) | item_features); + _buf.preAlloc(sizeof(uint8_t)); + _buf.append(&type_and_features, sizeof(uint8_t)); + } + void appendDouble(double i) { _buf.preAlloc(sizeof(double)); double nboVal = vespalib::nbo::n2h(i); @@ -61,7 +69,7 @@ class QueryNodeConverter : public QueryVisitor { template <typename V> void appendPredicateQueryTermVector(const V& v); - void createComplexIntermediate(const Term &node, const std::vector<Node *> & children, size_t type) { + void createComplexIntermediate(const Term &node, const std::vector<Node *> & children, ParseItem::ItemType type, uint8_t features) { uint8_t flags = 0; if (!node.isRanked()) { flags |= ParseItem::IFLAG_NORANK; @@ -70,11 +78,11 @@ class QueryNodeConverter : public QueryVisitor { flags |= ParseItem::IFLAG_NOPOSITIONDATA; } if (flags != 0) { - type |= ParseItem::IF_FLAGS; + features |= ParseItem::IF_FLAGS; } - appendByte(type); + append_type_and_features(type, features); appendCompressedNumber(node.getWeight().percent()); - if (type & ParseItem::IF_FLAGS) { + if ((features & ParseItem::IF_FLAGS) != 0) { appendByte(flags); } appendCompressedPositiveNumber(children.size()); @@ -82,30 +90,30 @@ class QueryNodeConverter : public QueryVisitor { visitNodes(children); } - void createIntermediate(const Intermediate &node, size_t type) { - appendByte(type); + void createIntermediate(const Intermediate &node, ParseItem::ItemType type) { + append_type_and_features(type, 0); appendCompressedPositiveNumber(node.getChildren().size()); visitNodes(node.getChildren()); } - void createIntermediate(const Intermediate &node, size_t type, size_t distance) { - appendByte(type); + void createIntermediate(const Intermediate &node, ParseItem::ItemType type, size_t distance) { + append_type_and_features(type, 0); appendCompressedPositiveNumber(node.getChildren().size()); appendCompressedPositiveNumber(distance); visitNodes(node.getChildren()); } - void createIntermediate(const Intermediate &node, size_t type, const vespalib::string & view) { - appendByte(type); + void createIntermediate(const Intermediate &node, ParseItem::ItemType type, const vespalib::string & view) { + append_type_and_features(type, 0); appendCompressedPositiveNumber(node.getChildren().size()); appendString(view); visitNodes(node.getChildren()); } - void createIntermediate(const Intermediate &node, size_t type, size_t distance, const vespalib::string & view) { - appendByte(type); + void createIntermediateX(const Intermediate &node, ParseItem::ItemType type, size_t target_num_hits, const vespalib::string & view) { + append_type_and_features(type, 0); appendCompressedPositiveNumber(node.getChildren().size()); - appendCompressedPositiveNumber(distance); + appendCompressedPositiveNumber(target_num_hits); appendString(view); visitNodes(node.getChildren()); } @@ -131,7 +139,7 @@ class QueryNodeConverter : public QueryVisitor { } void visit(WeakAnd &node) override { - createIntermediate(node, ParseItem::ITEM_WEAK_AND, node.getMinHits(), node.getView()); + createIntermediateX(node, ParseItem::ITEM_WEAK_AND, node.getTargetNumHits(), node.getView()); } void visit(Equiv &node) override { @@ -143,11 +151,11 @@ class QueryNodeConverter : public QueryVisitor { } void visit(Phrase &node) override { - createComplexIntermediate(node, node.getChildren(), (static_cast<uint8_t>(ParseItem::ITEM_PHRASE) | static_cast<uint8_t>(ParseItem::IF_WEIGHT))); + createComplexIntermediate(node, node.getChildren(), ParseItem::ITEM_PHRASE, static_cast<uint8_t>(ParseItem::IF_WEIGHT)); } template <typename NODE> - void createWeightedSet(NODE &node, uint8_t typefield) { + void createWeightedSet(NODE &node, ParseItem::ItemType type, uint8_t features) { uint8_t flags = 0; if (!node.isRanked()) { flags |= ParseItem::IFLAG_NORANK; @@ -158,11 +166,11 @@ class QueryNodeConverter : public QueryVisitor { flags |= ParseItem::IFLAG_NOPOSITIONDATA; } if (flags != 0) { - typefield |= ParseItem::IF_FLAGS; + features |= ParseItem::IF_FLAGS; } - appendByte(typefield); + append_type_and_features(type, features); appendCompressedNumber(node.getWeight().percent()); - if (typefield & ParseItem::IF_FLAGS) { + if ((features & ParseItem::IF_FLAGS) != 0) { appendByte(flags); } appendCompressedPositiveNumber(node.getNumTerms()); @@ -172,25 +180,24 @@ class QueryNodeConverter : public QueryVisitor { void createMultiTermNodes(const MultiTerm & mt) { for (size_t i = 0; i < mt.getNumTerms(); ++i) { auto term = mt.getAsString(i); - uint8_t typeField = static_cast<uint8_t>(ParseItem::ITEM_PURE_WEIGHTED_STRING) | static_cast<uint8_t>(ParseItem::IF_WEIGHT); - appendByte(typeField); + append_type_and_features(ParseItem::ITEM_PURE_WEIGHTED_STRING, static_cast<uint8_t>(ParseItem::IF_WEIGHT)); appendCompressedNumber(term.second.percent()); appendString(term.first); } } void visit(WeightedSetTerm &node) override { - createWeightedSet(node, static_cast<uint8_t>(ParseItem::ITEM_WEIGHTED_SET) | static_cast<uint8_t>(ParseItem::IF_WEIGHT)); + createWeightedSet(node, ParseItem::ITEM_WEIGHTED_SET, static_cast<uint8_t>(ParseItem::IF_WEIGHT)); createMultiTermNodes(node); } void visit(DotProduct &node) override { - createWeightedSet(node, static_cast<uint8_t>(ParseItem::ITEM_DOT_PRODUCT) | static_cast<uint8_t>(ParseItem::IF_WEIGHT)); + createWeightedSet(node, ParseItem::ITEM_DOT_PRODUCT, static_cast<uint8_t>(ParseItem::IF_WEIGHT)); createMultiTermNodes(node); } void visit(WandTerm &node) override { - createWeightedSet(node, static_cast<uint8_t>(ParseItem::ITEM_WAND) | static_cast<uint8_t>(ParseItem::IF_WEIGHT)); + createWeightedSet(node, ParseItem::ITEM_WAND, static_cast<uint8_t>(ParseItem::IF_WEIGHT)); appendCompressedPositiveNumber(node.getTargetNumHits()); appendDouble(node.getScoreThreshold()); appendDouble(node.getThresholdBoostFactor()); @@ -203,8 +210,8 @@ class QueryNodeConverter : public QueryVisitor { template <typename T> void appendTerm(const TermBase<T> &node); - void createTermNode(const TermNode &node, size_t type) { - uint8_t typefield = type | ParseItem::IF_WEIGHT | ParseItem::IF_UNIQUEID; + void createTermNode(const TermNode &node, ParseItem::ItemType type) { + uint8_t features = ParseItem::IF_WEIGHT | ParseItem::IF_UNIQUEID; uint8_t flags = 0; if (!node.isRanked()) { flags |= ParseItem::IFLAG_NORANK; @@ -213,19 +220,19 @@ class QueryNodeConverter : public QueryVisitor { flags |= ParseItem::IFLAG_NOPOSITIONDATA; } if (flags != 0) { - typefield |= ParseItem::IF_FLAGS; + features |= ParseItem::IF_FLAGS; } - appendByte(typefield); + append_type_and_features(type, features); appendCompressedNumber(node.getWeight().percent()); appendCompressedPositiveNumber(node.getId()); - if (typefield & ParseItem::IF_FLAGS) { + if ((features & ParseItem::IF_FLAGS) != 0) { appendByte(flags); } appendString(node.getView()); } template <class Term> - void createTerm(const Term &node, size_t type) { + void createTerm(const Term &node, ParseItem::ItemType type) { createTermNode(node, type); appendTerm(node); } @@ -239,11 +246,11 @@ class QueryNodeConverter : public QueryVisitor { } void visit(TrueQueryNode &) override { - appendByte(ParseItem::ITEM_TRUE); + append_type_and_features(ParseItem::ITEM_TRUE, 0); } void visit(FalseQueryNode &) override { - appendByte(ParseItem::ITEM_FALSE); + append_type_and_features(ParseItem::ITEM_FALSE, 0); } void visit(PrefixTerm &node) override { diff --git a/searchlib/src/vespa/searchlib/queryeval/blueprint.cpp b/searchlib/src/vespa/searchlib/queryeval/blueprint.cpp index e006a95ae5f..5eaa2dc40ab 100644 --- a/searchlib/src/vespa/searchlib/queryeval/blueprint.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/blueprint.cpp @@ -117,7 +117,7 @@ Blueprint::State::State(FieldSpecBaseList fields_in) noexcept Blueprint::State::~State() = default; Blueprint::Blueprint() noexcept - : _parent(0), + : _parent(nullptr), _sourceId(0xffffffff), _docid_limit(0), _frozen(false) @@ -129,19 +129,21 @@ Blueprint::~Blueprint() = default; Blueprint::UP Blueprint::optimize(Blueprint::UP bp) { Blueprint *root = bp.release(); - root->optimize(root); + root->optimize(root, OptimizePass::FIRST); + root->optimize(root, OptimizePass::SECOND); + root->optimize(root, OptimizePass::LAST); return Blueprint::UP(root); } void -Blueprint::optimize_self() +Blueprint::optimize_self(OptimizePass) { } Blueprint::UP Blueprint::get_replacement() { - return Blueprint::UP(); + return {}; } void @@ -163,8 +165,8 @@ std::unique_ptr<MatchingElementsSearch> Blueprint::create_matching_elements_search(const MatchingElementsFields &fields) const { (void) fields; - return std::unique_ptr<MatchingElementsSearch>(); -}; + return {}; +} namespace { @@ -196,7 +198,7 @@ template <typename Op> std::unique_ptr<SearchIterator> create_op_filter(const Blueprint::Children &children, bool strict, Blueprint::FilterConstraint constraint) { - REQUIRE(children.size() > 0); + REQUIRE( ! children.empty()); MultiSearch::Children list; std::unique_ptr<SearchIterator> spare; list.reserve(children.size()); @@ -261,7 +263,7 @@ Blueprint::create_atmost_or_filter(const Children &children, bool strict, Bluepr std::unique_ptr<SearchIterator> Blueprint::create_andnot_filter(const Children &children, bool strict, Blueprint::FilterConstraint constraint) { - REQUIRE(children.size() > 0); + REQUIRE( ! children.empty() ); MultiSearch::Children list; list.reserve(children.size()); { @@ -291,7 +293,7 @@ Blueprint::create_andnot_filter(const Children &children, bool strict, Blueprint std::unique_ptr<SearchIterator> Blueprint::create_first_child_filter(const Children &children, bool strict, Blueprint::FilterConstraint constraint) { - REQUIRE(children.size() > 0); + REQUIRE(!children.empty()); return children[0]->createFilterSearch(strict, constraint); } @@ -434,7 +436,7 @@ IntermediateBlueprint::infer_allow_termwise_eval() const } } return true; -}; +} bool IntermediateBlueprint::infer_want_global_filter() const @@ -539,19 +541,23 @@ IntermediateBlueprint::should_do_termwise_eval(const UnpackInfo &unpack, double } void -IntermediateBlueprint::optimize(Blueprint* &self) +IntermediateBlueprint::optimize(Blueprint* &self, OptimizePass pass) { assert(self == this); if (should_optimize_children()) { for (auto &child : _children) { auto *child_ptr = child.release(); - child_ptr->optimize(child_ptr); + child_ptr->optimize(child_ptr, pass); child.reset(child_ptr); } } - optimize_self(); - sort(_children); - maybe_eliminate_self(self, get_replacement()); + optimize_self(pass); + if (pass == OptimizePass::LAST) { + sort(_children); + } + if (pass == OptimizePass::FIRST) { + maybe_eliminate_self(self, get_replacement()); + } } void @@ -640,11 +646,7 @@ namespace { bool areAnyParentsEquiv(const Blueprint * node) { - return (node == nullptr) - ? false - : node->isEquiv() - ? true - : areAnyParentsEquiv(node->getParent()); + return (node != nullptr) && (node->isEquiv() || areAnyParentsEquiv(node->getParent())); } bool @@ -729,11 +731,13 @@ LeafBlueprint::getRange(vespalib::string &, vespalib::string &) const { } void -LeafBlueprint::optimize(Blueprint* &self) +LeafBlueprint::optimize(Blueprint* &self, OptimizePass pass) { assert(self == this); - optimize_self(); - maybe_eliminate_self(self, get_replacement()); + optimize_self(pass); + if (pass == OptimizePass::FIRST) { + maybe_eliminate_self(self, get_replacement()); + } } void @@ -767,7 +771,7 @@ LeafBlueprint::set_tree_size(uint32_t value) void visit(vespalib::ObjectVisitor &self, const vespalib::string &name, const search::queryeval::Blueprint *obj) { - if (obj != 0) { + if (obj != nullptr) { self.openStruct(name, obj->getClassName()); obj->visitMembers(self); self.closeStruct(); diff --git a/searchlib/src/vespa/searchlib/queryeval/blueprint.h b/searchlib/src/vespa/searchlib/queryeval/blueprint.h index c9ebab7ae9d..cd0e8f2af40 100644 --- a/searchlib/src/vespa/searchlib/queryeval/blueprint.h +++ b/searchlib/src/vespa/searchlib/queryeval/blueprint.h @@ -45,6 +45,8 @@ public: using Children = std::vector<Blueprint::UP>; using SearchIteratorUP = std::unique_ptr<SearchIterator>; + enum class OptimizePass { FIRST, SECOND, LAST }; + struct HitEstimate { uint32_t estHits; bool empty; @@ -79,8 +81,8 @@ public: static constexpr uint8_t COST_TIER_MAX = 255; State() noexcept; - State(FieldSpecBase field) noexcept; - State(FieldSpecBaseList fields_in) noexcept; + explicit State(FieldSpecBase field) noexcept; + explicit State(FieldSpecBaseList fields_in) noexcept; State(const State &rhs) = delete; State(State &&rhs) noexcept = default; State &operator=(const State &rhs) = delete; @@ -105,7 +107,7 @@ public: _estimateHits = est.estHits; _estimateEmpty = est.empty; } - HitEstimate estimate() const noexcept { return HitEstimate(_estimateHits, _estimateEmpty); } + HitEstimate estimate() const noexcept { return {_estimateHits, _estimateEmpty}; } double hit_ratio(uint32_t docid_limit) const noexcept { uint32_t total_hits = _estimateHits; uint32_t total_docs = std::max(total_hits, docid_limit); @@ -206,8 +208,8 @@ public: uint32_t get_docid_limit() const noexcept { return _docid_limit; } static Blueprint::UP optimize(Blueprint::UP bp); - virtual void optimize(Blueprint* &self) = 0; - virtual void optimize_self(); + virtual void optimize(Blueprint* &self, OptimizePass pass) = 0; + virtual void optimize_self(OptimizePass pass); virtual Blueprint::UP get_replacement(); virtual bool should_optimize_children() const { return true; } @@ -298,7 +300,7 @@ class IntermediateBlueprint : public blueprint::StateCache private: Children _children; HitEstimate calculateEstimate() const; - uint8_t calculate_cost_tier() const; + virtual uint8_t calculate_cost_tier() const; uint32_t calculate_tree_size() const; bool infer_allow_termwise_eval() const; bool infer_want_global_filter() const; @@ -326,7 +328,7 @@ public: void setDocIdLimit(uint32_t limit) noexcept final; - void optimize(Blueprint* &self) final; + void optimize(Blueprint* &self, OptimizePass pass) final; void set_global_filter(const GlobalFilter &global_filter, double estimated_hit_ratio) override; IndexList find(const IPredicate & check) const; @@ -361,7 +363,7 @@ class LeafBlueprint : public Blueprint private: State _state; protected: - void optimize(Blueprint* &self) final; + void optimize(Blueprint* &self, OptimizePass pass) final; void setEstimate(HitEstimate est) { _state.estimate(est); notifyChange(); @@ -374,7 +376,7 @@ protected: void set_want_global_filter(bool value); void set_tree_size(uint32_t value); - LeafBlueprint(bool allow_termwise_eval) noexcept + explicit LeafBlueprint(bool allow_termwise_eval) noexcept : _state() { _state.allow_termwise_eval(allow_termwise_eval); diff --git a/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.cpp b/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.cpp index 6e0bc695fe3..c0439df1c1b 100644 --- a/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.cpp @@ -103,25 +103,27 @@ AndNotBlueprint::exposeFields() const } void -AndNotBlueprint::optimize_self() +AndNotBlueprint::optimize_self(OptimizePass pass) { if (childCnt() == 0) { return; } - if (getChild(0).isAndNot()) { - auto *child = static_cast<AndNotBlueprint *>(&getChild(0)); - while (child->childCnt() > 1) { - addChild(child->removeChild(1)); + if (pass == OptimizePass::FIRST) { + if (getChild(0).isAndNot()) { + auto *child = static_cast<AndNotBlueprint *>(&getChild(0)); + while (child->childCnt() > 1) { + addChild(child->removeChild(1)); + } + insertChild(1, child->removeChild(0)); + removeChild(0); } - insertChild(1, child->removeChild(0)); - removeChild(0); - } - for (size_t i = 1; i < childCnt(); ++i) { - if (getChild(i).getState().estimate().empty) { - removeChild(i--); + for (size_t i = 1; i < childCnt(); ++i) { + if (getChild(i).getState().estimate().empty) { + removeChild(i--); + } } } - if ( !(getParent() && getParent()->isAndNot()) ) { + if (pass == OptimizePass::LAST) { optimize_source_blenders<OrBlueprint>(*this, 1); } } @@ -191,18 +193,20 @@ AndBlueprint::exposeFields() const } void -AndBlueprint::optimize_self() -{ - for (size_t i = 0; i < childCnt(); ++i) { - if (getChild(i).isAnd()) { - auto *child = static_cast<AndBlueprint *>(&getChild(i)); - while (child->childCnt() > 0) { - addChild(child->removeChild(0)); +AndBlueprint::optimize_self(OptimizePass pass) +{ + if (pass == OptimizePass::FIRST) { + for (size_t i = 0; i < childCnt(); ++i) { + if (getChild(i).isAnd()) { + auto *child = static_cast<AndBlueprint *>(&getChild(i)); + while (child->childCnt() > 0) { + addChild(child->removeChild(0)); + } + removeChild(i--); } - removeChild(i--); } } - if ( !(getParent() && getParent()->isAnd()) ) { + if (pass == OptimizePass::LAST) { optimize_source_blenders<AndBlueprint>(*this, 0); } } @@ -263,6 +267,17 @@ AndBlueprint::computeNextHitRate(const Blueprint & child, double hitRate) const return hitRate * child.hit_ratio(); } +double +OrBlueprint::computeNextHitRate(const Blueprint & child, double hitRate) const { + // Avoid dropping hitRate to zero when meeting a conservatively high hitrate in a child. + // Happens at least when using non fast-search attributes, and with AND nodes. + constexpr double MIN_INVERSE_HIT_RATIO = 0.10; + double inverse_child_hit_ratio = 1.0 - child.hit_ratio(); + return (inverse_child_hit_ratio > MIN_INVERSE_HIT_RATIO) + ? hitRate * inverse_child_hit_ratio + : hitRate; +} + //----------------------------------------------------------------------------- OrBlueprint::~OrBlueprint() = default; @@ -280,20 +295,22 @@ OrBlueprint::exposeFields() const } void -OrBlueprint::optimize_self() -{ - for (size_t i = 0; (childCnt() > 1) && (i < childCnt()); ++i) { - if (getChild(i).isOr()) { - auto *child = static_cast<OrBlueprint *>(&getChild(i)); - while (child->childCnt() > 0) { - addChild(child->removeChild(0)); +OrBlueprint::optimize_self(OptimizePass pass) +{ + if (pass == OptimizePass::FIRST) { + for (size_t i = 0; (childCnt() > 1) && (i < childCnt()); ++i) { + if (getChild(i).isOr()) { + auto *child = static_cast<OrBlueprint *>(&getChild(i)); + while (child->childCnt() > 0) { + addChild(child->removeChild(0)); + } + removeChild(i--); + } else if (getChild(i).getState().estimate().empty) { + removeChild(i--); } - removeChild(i--); - } else if (getChild(i).getState().estimate().empty) { - removeChild(i--); } } - if ( !(getParent() && getParent()->isOr()) ) { + if (pass == OptimizePass::LAST) { optimize_source_blenders<OrBlueprint>(*this, 0); } } @@ -344,6 +361,16 @@ OrBlueprint::createFilterSearch(bool strict, FilterConstraint constraint) const return create_or_filter(get_children(), strict, constraint); } +uint8_t +OrBlueprint::calculate_cost_tier() const +{ + uint8_t cost_tier = State::COST_TIER_NORMAL; + for (const Blueprint::UP &child : get_children()) { + cost_tier = std::max(cost_tier, child->getState().cost_tier()); + } + return cost_tier; +} + //----------------------------------------------------------------------------- WeakAndBlueprint::~WeakAndBlueprint() = default; @@ -531,14 +558,18 @@ RankBlueprint::exposeFields() const } void -RankBlueprint::optimize_self() +RankBlueprint::optimize_self(OptimizePass pass) { - for (size_t i = 1; i < childCnt(); ++i) { - if (getChild(i).getState().estimate().empty) { - removeChild(i--); + if (pass == OptimizePass::FIRST) { + for (size_t i = 1; i < childCnt(); ++i) { + if (getChild(i).getState().estimate().empty) { + removeChild(i--); + } } } - optimize_source_blenders<OrBlueprint>(*this, 1); + if (pass == OptimizePass::LAST) { + optimize_source_blenders<OrBlueprint>(*this, 1); + } } Blueprint::UP @@ -596,7 +627,7 @@ RankBlueprint::createFilterSearch(bool strict, FilterConstraint constraint) cons //----------------------------------------------------------------------------- -SourceBlenderBlueprint::SourceBlenderBlueprint(const ISourceSelector &selector) +SourceBlenderBlueprint::SourceBlenderBlueprint(const ISourceSelector &selector) noexcept : _selector(selector) { } @@ -626,27 +657,6 @@ SourceBlenderBlueprint::inheritStrict(size_t) const return true; } -class FindSource : public Blueprint::IPredicate -{ -public: - explicit FindSource(uint32_t sourceId) noexcept : _sourceId(sourceId) { } - bool check(const Blueprint & bp) const override { return bp.getSourceId() == _sourceId; } -private: - uint32_t _sourceId; -}; - -ssize_t -SourceBlenderBlueprint::findSource(uint32_t sourceId) const -{ - ssize_t index(-1); - FindSource fs(sourceId); - IndexList list = find(fs); - if ( ! list.empty()) { - index = list.front(); - } - return index; -} - SearchIterator::UP SourceBlenderBlueprint::createIntermediateSearch(MultiSearch::Children sub_searches, bool strict, search::fef::MatchData &) const @@ -673,6 +683,16 @@ SourceBlenderBlueprint::isCompatibleWith(const SourceBlenderBlueprint &other) co return (&_selector == &other._selector); } +uint8_t +SourceBlenderBlueprint::calculate_cost_tier() const +{ + uint8_t cost_tier = State::COST_TIER_NORMAL; + for (const Blueprint::UP &child : get_children()) { + cost_tier = std::max(cost_tier, child->getState().cost_tier()); + } + return cost_tier; +} + //----------------------------------------------------------------------------- } diff --git a/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.h b/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.h index b8ca46f7549..75dc47272af 100644 --- a/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.h +++ b/searchlib/src/vespa/searchlib/queryeval/intermediate_blueprints.h @@ -17,7 +17,7 @@ public: bool supports_termwise_children() const override { return true; } HitEstimate combine(const std::vector<HitEstimate> &data) const override; FieldSpecBaseList exposeFields() const override; - void optimize_self() override; + void optimize_self(OptimizePass pass) override; bool isAndNot() const override { return true; } Blueprint::UP get_replacement() override; void sort(Children &children) const override; @@ -28,6 +28,9 @@ public: SearchIterator::UP createFilterSearch(bool strict, FilterConstraint constraint) const override; private: + uint8_t calculate_cost_tier() const override { + return (childCnt() > 0) ? get_children()[0]->getState().cost_tier() : State::COST_TIER_NORMAL; + } bool isPositive(size_t index) const override { return index == 0; } }; @@ -40,7 +43,7 @@ public: bool supports_termwise_children() const override { return true; } HitEstimate combine(const std::vector<HitEstimate> &data) const override; FieldSpecBaseList exposeFields() const override; - void optimize_self() override; + void optimize_self(OptimizePass pass) override; bool isAnd() const override { return true; } Blueprint::UP get_replacement() override; void sort(Children &children) const override; @@ -64,7 +67,7 @@ public: bool supports_termwise_children() const override { return true; } HitEstimate combine(const std::vector<HitEstimate> &data) const override; FieldSpecBaseList exposeFields() const override; - void optimize_self() override; + void optimize_self(OptimizePass pass) override; bool isOr() const override { return true; } Blueprint::UP get_replacement() override; void sort(Children &children) const override; @@ -74,6 +77,9 @@ public: bool strict, fef::MatchData &md) const override; SearchIterator::UP createFilterSearch(bool strict, FilterConstraint constraint) const override; +private: + double computeNextHitRate(const Blueprint & child, double hitRate) const override; + uint8_t calculate_cost_tier() const override; }; //----------------------------------------------------------------------------- @@ -95,8 +101,8 @@ public: bool strict, fef::MatchData &md) const override; SearchIterator::UP createFilterSearch(bool strict, FilterConstraint constraint) const override; - WeakAndBlueprint(uint32_t n) : _n(n) {} - ~WeakAndBlueprint(); + explicit WeakAndBlueprint(uint32_t n) noexcept : _n(n) {} + ~WeakAndBlueprint() override; void addTerm(Blueprint::UP bp, uint32_t weight) { addChild(std::move(bp)); _weights.push_back(weight); @@ -124,7 +130,7 @@ public: bool strict, fef::MatchData &md) const override; SearchIterator::UP createFilterSearch(bool strict, FilterConstraint constraint) const override; - NearBlueprint(uint32_t window) : _window(window) {} + explicit NearBlueprint(uint32_t window) noexcept : _window(window) {} }; //----------------------------------------------------------------------------- @@ -146,7 +152,7 @@ public: bool strict, fef::MatchData &md) const override; SearchIterator::UP createFilterSearch(bool strict, FilterConstraint constraint) const override; - ONearBlueprint(uint32_t window) : _window(window) {} + explicit ONearBlueprint(uint32_t window) noexcept : _window(window) {} }; //----------------------------------------------------------------------------- @@ -156,7 +162,7 @@ class RankBlueprint final : public IntermediateBlueprint public: HitEstimate combine(const std::vector<HitEstimate> &data) const override; FieldSpecBaseList exposeFields() const override; - void optimize_self() override; + void optimize_self(OptimizePass pass) override; Blueprint::UP get_replacement() override; void sort(Children &children) const override; bool inheritStrict(size_t i) const override; @@ -166,6 +172,9 @@ public: bool strict, fef::MatchData &md) const override; SearchIterator::UP createFilterSearch(bool strict, FilterConstraint constraint) const override; + uint8_t calculate_cost_tier() const override { + return (childCnt() > 0) ? get_children()[0]->getState().cost_tier() : State::COST_TIER_NORMAL; + } }; //----------------------------------------------------------------------------- @@ -176,18 +185,12 @@ private: const ISourceSelector &_selector; public: - SourceBlenderBlueprint(const ISourceSelector &selector); + explicit SourceBlenderBlueprint(const ISourceSelector &selector) noexcept; ~SourceBlenderBlueprint() override; HitEstimate combine(const std::vector<HitEstimate> &data) const override; FieldSpecBaseList exposeFields() const override; void sort(Children &children) const override; bool inheritStrict(size_t i) const override; - /** - * Will return the index matching the given sourceId. - * @param sourceId The sourceid to find. - * @return The index to the child representing the sourceId. -1 if not found. - */ - ssize_t findSource(uint32_t sourceId) const; SearchIterator::UP createIntermediateSearch(MultiSearch::Children subSearches, bool strict, fef::MatchData &md) const override; @@ -197,6 +200,7 @@ public: /** check if this blueprint has the same source selector as the other */ bool isCompatibleWith(const SourceBlenderBlueprint &other) const; bool isSourceBlender() const override { return true; } + uint8_t calculate_cost_tier() const override; }; } diff --git a/searchlib/src/vespa/searchlib/queryeval/same_element_blueprint.cpp b/searchlib/src/vespa/searchlib/queryeval/same_element_blueprint.cpp index 6b2faac847e..5c795679d48 100644 --- a/searchlib/src/vespa/searchlib/queryeval/same_element_blueprint.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/same_element_blueprint.cpp @@ -45,12 +45,14 @@ SameElementBlueprint::addTerm(Blueprint::UP term) } void -SameElementBlueprint::optimize_self() +SameElementBlueprint::optimize_self(OptimizePass pass) { - std::sort(_terms.begin(), _terms.end(), - [](const auto &a, const auto &b) { - return (a->getState().estimate() < b->getState().estimate()); - }); + if (pass == OptimizePass::LAST) { + std::sort(_terms.begin(), _terms.end(), + [](const auto &a, const auto &b) { + return (a->getState().estimate() < b->getState().estimate()); + }); + } } void diff --git a/searchlib/src/vespa/searchlib/queryeval/same_element_blueprint.h b/searchlib/src/vespa/searchlib/queryeval/same_element_blueprint.h index 240d064c5cc..cc6331375cf 100644 --- a/searchlib/src/vespa/searchlib/queryeval/same_element_blueprint.h +++ b/searchlib/src/vespa/searchlib/queryeval/same_element_blueprint.h @@ -34,7 +34,7 @@ public: // used by create visitor void addTerm(Blueprint::UP term); - void optimize_self() override; + void optimize_self(OptimizePass pass) override; void fetchPostings(const ExecuteInfo &execInfo) override; std::unique_ptr<SameElementSearch> create_same_element_search(search::fef::TermFieldMatchData& tfmd, bool strict) const; diff --git a/searchlib/src/vespa/searchlib/queryeval/simple_phrase_blueprint.cpp b/searchlib/src/vespa/searchlib/queryeval/simple_phrase_blueprint.cpp index 8de16189c86..3cb69f00240 100644 --- a/searchlib/src/vespa/searchlib/queryeval/simple_phrase_blueprint.cpp +++ b/searchlib/src/vespa/searchlib/queryeval/simple_phrase_blueprint.cpp @@ -2,11 +2,8 @@ #include "simple_phrase_blueprint.h" #include "simple_phrase_search.h" -#include "emptysearch.h" #include "field_spec.hpp" -#include <vespa/searchlib/fef/termfieldmatchdata.h> #include <vespa/vespalib/objects/visit.hpp> -#include <algorithm> #include <map> namespace search::queryeval { @@ -28,7 +25,7 @@ SimplePhraseBlueprint::~SimplePhraseBlueprint() = default; FieldSpec SimplePhraseBlueprint::getNextChildField(const FieldSpec &outer) { - return FieldSpec(outer.getName(), outer.getFieldId(), _layout.allocTermField(outer.getFieldId()), false); + return {outer.getName(), outer.getFieldId(), _layout.allocTermField(outer.getFieldId()), false}; } void |