diff options
Diffstat (limited to 'streamingvisitors')
7 files changed, 48 insertions, 64 deletions
diff --git a/streamingvisitors/src/tests/hitcollector/hitcollector_test.cpp b/streamingvisitors/src/tests/hitcollector/hitcollector_test.cpp index adab7d57028..b0373ae635a 100644 --- a/streamingvisitors/src/tests/hitcollector/hitcollector_test.cpp +++ b/streamingvisitors/src/tests/hitcollector/hitcollector_test.cpp @@ -74,7 +74,6 @@ protected: void testFeatureSet(); DocumentType _docType; - std::vector<vsm::StorageDocument::UP> _backedHits; HitCollectorTest(); ~HitCollectorTest() override; @@ -85,7 +84,7 @@ HitCollectorTest::HitCollectorTest() { } -HitCollectorTest::~HitCollectorTest() {} +HitCollectorTest::~HitCollectorTest() = default; void HitCollectorTest::assertHit(SearchResult::RankType expRank, uint32_t hitNo, SearchResult & rs) @@ -109,11 +108,10 @@ void HitCollectorTest::addHit(HitCollector &hc, uint32_t docId, double score, const char *sortData, size_t sortDataSize) { auto doc = document::Document::make_without_repo(_docType, DocumentId("id:ns:testdoc::")); - auto sdoc = std::make_unique<StorageDocument>(std::move(doc), SharedFieldPathMap(), 0); + auto sdoc = std::make_shared<StorageDocument>(std::move(doc), SharedFieldPathMap(), 0); ASSERT_TRUE(sdoc->valid()); MatchData md(MatchData::params()); - hc.addHit(sdoc.get(), docId, md, score, sortData, sortDataSize); - _backedHits.push_back(std::move(sdoc)); + hc.addHit(std::move(sdoc), docId, md, score, sortData, sortDataSize); } TEST_F(HitCollectorTest, simple) diff --git a/streamingvisitors/src/tests/matching_elements_filler/matching_elements_filler_test.cpp b/streamingvisitors/src/tests/matching_elements_filler/matching_elements_filler_test.cpp index 4f57b5a0b92..28adb9669de 100644 --- a/streamingvisitors/src/tests/matching_elements_filler/matching_elements_filler_test.cpp +++ b/streamingvisitors/src/tests/matching_elements_filler/matching_elements_filler_test.cpp @@ -282,7 +282,6 @@ class MatchingElementsFillerTest : public ::testing::Test { Query _query; std::unique_ptr<MatchingElementsFiller> _matching_elements_filler; std::unique_ptr<MatchingElements> _matching_elements; - std::unique_ptr<StorageDocument> _sdoc; public: MatchingElementsFillerTest(); ~MatchingElementsFillerTest(); @@ -303,15 +302,14 @@ MatchingElementsFillerTest::MatchingElementsFillerTest() _search_result(), _query(), _matching_elements_filler(), - _matching_elements(), - _sdoc() + _matching_elements() { _env.field_paths = make_field_path_map(_doc_type); _search_result.addHit(1, "id::test::1", 0.0, nullptr, 0); - _sdoc = std::make_unique<StorageDocument>(_doc_type.make_test_doc(), _env.field_paths, _env.field_paths->size()); - EXPECT_TRUE(_sdoc->valid()); + auto sdoc = std::make_shared<StorageDocument>(_doc_type.make_test_doc(), _env.field_paths, _env.field_paths->size()); + EXPECT_TRUE(sdoc->valid()); MatchData md(MatchData::params()); - _hit_collector.addHit(_sdoc.get(), 1, md, 0.0, nullptr, 0); + _hit_collector.addHit(std::move(sdoc), 1, md, 0.0, nullptr, 0); } MatchingElementsFillerTest::~MatchingElementsFillerTest() = default; diff --git a/streamingvisitors/src/vespa/searchvisitor/hitcollector.cpp b/streamingvisitors/src/vespa/searchvisitor/hitcollector.cpp index 305e76477ca..75eccaef83c 100644 --- a/streamingvisitors/src/vespa/searchvisitor/hitcollector.cpp +++ b/streamingvisitors/src/vespa/searchvisitor/hitcollector.cpp @@ -17,11 +17,11 @@ using FefUtils = search::fef::Utils; namespace streaming { -HitCollector::Hit::Hit(const vsm::StorageDocument * doc, uint32_t docId, const MatchData & matchData, +HitCollector::Hit::Hit(vsm::StorageDocument::SP doc, uint32_t docId, const MatchData & matchData, double score, const void * sortData, size_t sortDataLen) : _docid(docId), _score(score), - _document(doc), + _document(std::move(doc)), _matchData(), _sortBlob(sortData, sortDataLen) { @@ -33,10 +33,10 @@ HitCollector::Hit::Hit(const vsm::StorageDocument * doc, uint32_t docId, const HitCollector::Hit::~Hit() = default; -HitCollector::HitCollector(size_t wantedHits, bool use_sort_blob) : - _hits(), - _heap(), - _use_sort_blob(use_sort_blob) +HitCollector::HitCollector(size_t wantedHits, bool use_sort_blob) + : _hits(), + _heap(), + _use_sort_blob(use_sort_blob) { _hits.reserve(16); _heap.reserve(wantedHits); @@ -56,16 +56,16 @@ HitCollector::getDocSum(const search::DocumentIdT & docId) const } bool -HitCollector::addHit(const vsm::StorageDocument * doc, uint32_t docId, const MatchData & data, double score) +HitCollector::addHit(vsm::StorageDocument::SP doc, uint32_t docId, const MatchData & data, double score) { - return addHit(Hit(doc, docId, data, score)); + return addHit(Hit(std::move(doc), docId, data, score)); } bool -HitCollector::addHit(const vsm::StorageDocument * doc, uint32_t docId, const MatchData & data, +HitCollector::addHit(vsm::StorageDocument::SP doc, uint32_t docId, const MatchData & data, double score, const void * sortData, size_t sortDataLen) { - return addHit(Hit(doc, docId, data, score, sortData, sortDataLen)); + return addHit(Hit(std::move(doc), docId, data, score, sortData, sortDataLen)); } bool diff --git a/streamingvisitors/src/vespa/searchvisitor/hitcollector.h b/streamingvisitors/src/vespa/searchvisitor/hitcollector.h index 0a3a3ce3062..3a05f9dca86 100644 --- a/streamingvisitors/src/vespa/searchvisitor/hitcollector.h +++ b/streamingvisitors/src/vespa/searchvisitor/hitcollector.h @@ -26,10 +26,10 @@ private: class Hit { public: - Hit(const vsm::StorageDocument * doc, uint32_t docId, const MatchData & matchData, + Hit(vsm::StorageDocument::SP doc, uint32_t docId, const MatchData & matchData, double score, const void * sortData, size_t sortDataLen); - Hit(const vsm::StorageDocument * doc, uint32_t docId, const MatchData & matchData, double score) - : Hit(doc, docId, matchData, score, nullptr, 0) + Hit(vsm::StorageDocument::SP doc, uint32_t docId, const MatchData & matchData, double score) + : Hit(std::move(doc), docId, matchData, score, nullptr, 0) { } ~Hit(); Hit(const Hit &) = delete; @@ -52,14 +52,14 @@ private: return (diff == 0) ? cmpDocId(b) : diff; } - void dropDocument() noexcept { _document = nullptr; } + void dropDocument() noexcept { _document.reset(); } private: uint32_t _docid; double _score; - const vsm::StorageDocument * _document; + vsm::StorageDocument::SP _document; std::vector<TermFieldMatchData> _matchData; - vespalib::string _sortBlob; + vespalib::string _sortBlob; }; using HitVector = std::vector<Hit>; using Lids = std::vector<uint32_t>; @@ -118,7 +118,7 @@ public: * @param data The match data for the hit. * @return true if the document was added to the heap **/ - bool addHit(const vsm::StorageDocument * doc, uint32_t docId, const MatchData & data, double score); + bool addHit(vsm::StorageDocument::SP doc, uint32_t docId, const MatchData & data, double score); /** * Adds a hit to this hit collector. @@ -126,13 +126,13 @@ public: * If you add a nullptr document you should not use getDocSum() or fillSearchResult(), * as these functions expect valid documents. * - * @param doc The document that is a hit. Must be kept alive on the outside. + * @param doc The document that is a hit. * @param data The match data for the hit. * @param sortData The buffer of the sortdata. * @param sortDataLen The length of the sortdata. * @return true if the document was added to the heap **/ - bool addHit(const vsm::StorageDocument * doc, uint32_t docId, const MatchData & data, + bool addHit(vsm::StorageDocument::SP doc, uint32_t docId, const MatchData & data, double score, const void * sortData, size_t sortDataLen); /** diff --git a/streamingvisitors/src/vespa/searchvisitor/searchvisitor.cpp b/streamingvisitors/src/vespa/searchvisitor/searchvisitor.cpp index 7bd637ae5f4..9a0b720f054 100644 --- a/streamingvisitors/src/vespa/searchvisitor/searchvisitor.cpp +++ b/streamingvisitors/src/vespa/searchvisitor/searchvisitor.cpp @@ -725,31 +725,32 @@ SearchVisitor::RankController::keepMatchedDocument() return (!(_rankProcessor->getRankScore() <= _rankSetup->getRankScoreDropLimit())); } -bool +void SearchVisitor::RankController::collectMatchedDocument(bool hasSorting, SearchVisitor & visitor, const std::vector<char> & tmpSortBuffer, - const StorageDocument * document) + StorageDocument::SP document) { - bool amongTheBest(false); uint32_t docId = _rankProcessor->getDocId(); if (!hasSorting) { - amongTheBest = _rankProcessor->getHitCollector().addHit(document, docId, _rankProcessor->getMatchData(), - _rankProcessor->getRankScore()); + bool amongTheBest = _rankProcessor->getHitCollector().addHit(std::move(document), docId, + _rankProcessor->getMatchData(), + _rankProcessor->getRankScore()); if (amongTheBest && _dumpFeatures) { - _dumpProcessor->getHitCollector().addHit(nullptr, docId, _dumpProcessor->getMatchData(), _dumpProcessor->getRankScore()); + _dumpProcessor->getHitCollector().addHit({}, docId, _dumpProcessor->getMatchData(), _dumpProcessor->getRankScore()); } } else { size_t pos = visitor.fillSortBuffer(); LOG(spam, "SortBlob is %ld bytes", pos); - amongTheBest = _rankProcessor->getHitCollector().addHit(document, docId, _rankProcessor->getMatchData(), - _rankProcessor->getRankScore(), &tmpSortBuffer[0], pos); + bool amongTheBest = _rankProcessor->getHitCollector().addHit(std::move(document), docId, + _rankProcessor->getMatchData(), + _rankProcessor->getRankScore(), + &tmpSortBuffer[0], pos); if (amongTheBest && _dumpFeatures) { - _dumpProcessor->getHitCollector().addHit(nullptr, docId, _dumpProcessor->getMatchData(), + _dumpProcessor->getHitCollector().addHit({}, docId, _dumpProcessor->getMatchData(), _dumpProcessor->getRankScore(), &tmpSortBuffer[0], pos); } } - return amongTheBest; } void @@ -1042,7 +1043,7 @@ SearchVisitor::handleDocuments(const document::BucketId&, DocEntryList & entries const document::DocumentType* defaultDocType = _docTypeMapping.getDefaultDocumentType(); assert(defaultDocType); for (const auto & entry : entries) { - auto document = std::make_unique<StorageDocument>(entry->releaseDocument(), _fieldPathMap, highestFieldNo); + auto document = std::make_shared<StorageDocument>(entry->releaseDocument(), _fieldPathMap, highestFieldNo); try { if (defaultDocType != nullptr @@ -1051,9 +1052,7 @@ SearchVisitor::handleDocuments(const document::BucketId&, DocEntryList & entries LOG(debug, "Skipping document of type '%s' when handling only documents of type '%s'", document->docDoc().getType().getName().c_str(), defaultDocType->getName().c_str()); } else { - if (handleDocument(*document)) { - _backingDocuments.push_back(std::move(document)); - } + handleDocument(document); } } catch (const std::exception & e) { LOG(warning, "Caught exception handling document '%s'. Exception='%s'", @@ -1062,10 +1061,10 @@ SearchVisitor::handleDocuments(const document::BucketId&, DocEntryList & entries } } -bool -SearchVisitor::handleDocument(StorageDocument & document) +void +SearchVisitor::handleDocument(StorageDocument::SP documentSP) { - bool needToKeepDocument(false); + StorageDocument & document = *documentSP; _syntheticFieldsController.onDocument(document); group(document.docDoc(), 0, true); if (match(document)) { @@ -1079,14 +1078,11 @@ SearchVisitor::handleDocument(StorageDocument & document) _rankAttribute.add(rp.getRankScore()); } if (_rankController.keepMatchedDocument()) { - bool amongTheBest = _rankController.collectMatchedDocument(!_sortList.empty(), *this, _tmpSortBuffer, &document); + _rankController.collectMatchedDocument(!_sortList.empty(), *this, _tmpSortBuffer, std::move(documentSP)); _syntheticFieldsController.onDocumentMatch(document, documentId); SingleDocumentStore single(document); _summaryGenerator.setDocsumCache(single); group(document.docDoc(), rp.getRankScore(), false); - if (amongTheBest) { - needToKeepDocument = true; - } } else { _hitsRejectedCount++; LOG(debug, "Do not keep document with id '%s' because rank score (%f) <= rank score drop limit (%f)", @@ -1095,7 +1091,6 @@ SearchVisitor::handleDocument(StorageDocument & document) } else { LOG(debug, "Did not match document with id '%s'", document.docDoc().getId().getScheme().toString().c_str()); } - return needToKeepDocument; } void @@ -1228,10 +1223,7 @@ SearchVisitor::completedVisitingInternal(HitCounter& hitCounter) } generateGroupingResults(); - generateDocumentSummaries(); - _backingDocuments.clear(); - documentSummary.sort(); LOG(debug, "Docsum count: %lu", documentSummary.getSummaryCount()); } diff --git a/streamingvisitors/src/vespa/searchvisitor/searchvisitor.h b/streamingvisitors/src/vespa/searchvisitor/searchvisitor.h index bb16655193f..d8d97830244 100644 --- a/streamingvisitors/src/vespa/searchvisitor/searchvisitor.h +++ b/streamingvisitors/src/vespa/searchvisitor/searchvisitor.h @@ -198,12 +198,11 @@ private: * @param visitor the search visitor. * @param tmpSortBuffer the sort buffer containing the sort data. * @param document the document to collect. Must be kept alive on the outside. - * @return true if the document was added to the heap **/ - bool collectMatchedDocument(bool hasSorting, + void collectMatchedDocument(bool hasSorting, SearchVisitor & visitor, const std::vector<char> & tmpSortBuffer, - const vsm::StorageDocument * document); + vsm::StorageDocument::SP document); /** * Callback function that is called when visiting is completed. * Perform second phase ranking and calculate summary features / rank features if asked for. @@ -323,9 +322,8 @@ private: /** * Process one document * @param document Document to process. - * @return true if the underlying buffer is needed later on, then it must be kept. */ - bool handleDocument(vsm::StorageDocument & document); + void handleDocument(vsm::StorageDocument::SP document); /** * Collect the given document for grouping. @@ -397,7 +395,6 @@ private: size_t _limit; }; using GroupingList = std::vector< GroupingEntry >; - using DocumentVector = std::vector<vsm::StorageDocument::UP>; class StreamingDocsumsState { using ResolveClassInfo = search::docsummary::IDocsumWriter::ResolveClassInfo; @@ -485,7 +482,6 @@ private: bool _shouldFillRankAttribute; SyntheticFieldsController _syntheticFieldsController; RankController _rankController; - DocumentVector _backingDocuments; vsm::StringFieldIdTMapT _fieldsUnion; void setupAttributeVector(const vsm::FieldPath &fieldPath); diff --git a/streamingvisitors/src/vespa/vsm/common/storagedocument.h b/streamingvisitors/src/vespa/vsm/common/storagedocument.h index fb75373200b..4812ab20c79 100644 --- a/streamingvisitors/src/vespa/vsm/common/storagedocument.h +++ b/streamingvisitors/src/vespa/vsm/common/storagedocument.h @@ -13,7 +13,7 @@ using SharedFieldPathMap = std::shared_ptr<FieldPathMapT>; class StorageDocument : public Document { public: - using UP = std::unique_ptr<StorageDocument>; + using SP = std::shared_ptr<StorageDocument>; class SubDocument { public: @@ -40,7 +40,7 @@ public: StorageDocument(document::Document::UP doc, const SharedFieldPathMap &fim, size_t fieldNoLimit); StorageDocument(const StorageDocument &) = delete; StorageDocument & operator = (const StorageDocument &) = delete; - ~StorageDocument(); + ~StorageDocument() override; const document::Document &docDoc() const { return *_doc; } bool valid() const { return _doc.get() != nullptr; } |