diff options
author | Geir Storli <geirst@verizonmedia.com> | 2019-05-21 14:19:30 +0000 |
---|---|---|
committer | Geir Storli <geirst@verizonmedia.com> | 2019-05-21 14:24:05 +0000 |
commit | 5680dcdce81febbbafcab483080048eab520a083 (patch) | |
tree | 24591942d340dc8a700dfab167490207ad794fda /searchcore | |
parent | e3d08bf6f8ee38c1242d87f09d97323c7e07dfdd (diff) |
Make it possible for rank features to request which match data that should be available in a TermFieldMatchData instance.
Diffstat (limited to 'searchcore')
9 files changed, 136 insertions, 32 deletions
diff --git a/searchcore/CMakeLists.txt b/searchcore/CMakeLists.txt index 593834d4f9a..ac6a9f305dd 100644 --- a/searchcore/CMakeLists.txt +++ b/searchcore/CMakeLists.txt @@ -114,6 +114,7 @@ vespa_define_module( src/tests/proton/matching src/tests/proton/matching/constant_value_repo src/tests/proton/matching/docid_range_scheduler + src/tests/proton/matching/handle_recorder src/tests/proton/matching/index_environment src/tests/proton/matching/match_loop_communicator src/tests/proton/matching/match_phase_limiter diff --git a/searchcore/src/tests/proton/matching/handle_recorder/CMakeLists.txt b/searchcore/src/tests/proton/matching/handle_recorder/CMakeLists.txt new file mode 100644 index 00000000000..a56d22ab154 --- /dev/null +++ b/searchcore/src/tests/proton/matching/handle_recorder/CMakeLists.txt @@ -0,0 +1,10 @@ +# Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +vespa_add_executable(searchcore_matching_handle_recorder_test_app TEST + SOURCES + handle_recorder_test.cpp + DEPENDS + searchcore_matching + gtest +) +vespa_add_test(NAME searchcore_matching_handle_recorder_test_app COMMAND searchcore_matching_handle_recorder_test_app) diff --git a/searchcore/src/tests/proton/matching/handle_recorder/handle_recorder_test.cpp b/searchcore/src/tests/proton/matching/handle_recorder/handle_recorder_test.cpp new file mode 100644 index 00000000000..60caffd82d6 --- /dev/null +++ b/searchcore/src/tests/proton/matching/handle_recorder/handle_recorder_test.cpp @@ -0,0 +1,54 @@ +// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include <vespa/searchcore/proton/matching/handlerecorder.h> +#include <vespa/vespalib/gtest/gtest.h> + +#include <vespa/log/log.h> + +LOG_SETUP("handle_recorder_test"); + +using search::fef::MatchDataDetails; +using search::fef::TermFieldHandle; +using namespace proton::matching; + +using HandleSet = HandleRecorder::HandleSet; + +void +register_normal_handle(TermFieldHandle handle) +{ + HandleRecorder::register_handle(handle, MatchDataDetails::Normal); +} + +void +register_cheap_handle(TermFieldHandle handle) +{ + HandleRecorder::register_handle(handle, MatchDataDetails::Cheap); +} + +TEST(HandleRecorderTest, can_record_both_normal_and_cheap_handles) +{ + HandleRecorder recorder; + { + HandleRecorder::Binder binder(recorder); + register_normal_handle(3); + register_cheap_handle(5); + register_normal_handle(7); + } + EXPECT_EQ(HandleSet({3, 7}), recorder.get_normal_handles()); + EXPECT_EQ(HandleSet({5}), recorder.get_cheap_handles()); + EXPECT_EQ("normal: [3,7], cheap: [5]", recorder.to_string()); +} + +TEST(HandleRecorderTest, the_same_handle_can_be_in_both_normal_and_cheap_set) +{ + HandleRecorder recorder; + { + HandleRecorder::Binder binder(recorder); + register_normal_handle(3); + register_cheap_handle(3); + } + EXPECT_EQ(HandleSet({3}), recorder.get_normal_handles()); + EXPECT_EQ(HandleSet({3}), recorder.get_cheap_handles()); +} + +GTEST_MAIN_RUN_ALL_TESTS() diff --git a/searchcore/src/vespa/searchcore/proton/matching/handlerecorder.cpp b/searchcore/src/vespa/searchcore/proton/matching/handlerecorder.cpp index 3573c7ec7b8..a4f3d519311 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/handlerecorder.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/handlerecorder.cpp @@ -2,9 +2,10 @@ #include "handlerecorder.h" #include <vespa/vespalib/stllike/asciistream.h> -#include <cassert> #include <algorithm> +#include <cassert> +using search::fef::MatchDataDetails; using search::fef::TermFieldHandle; namespace proton::matching { @@ -16,33 +17,47 @@ namespace proton::matching { #endif namespace { - __thread HandleRecorder * _T_recorder TLS_LINKAGE = NULL; + __thread HandleRecorder * _T_recorder TLS_LINKAGE = nullptr; __thread bool _T_assert_all_handles_are_registered = false; } HandleRecorder::HandleRecorder() : - _handles() + _normal_handles(), + _cheap_handles() { } +namespace { + vespalib::string -HandleRecorder::toString() const +handles_to_string(const HandleRecorder::HandleSet& handles) { vespalib::asciistream os; std::vector<TermFieldHandle> sorted; - for (TermFieldHandle handle : _handles) { + for (TermFieldHandle handle : handles) { sorted.push_back(handle); } std::sort(sorted.begin(), sorted.end()); if ( !sorted.empty() ) { os << sorted[0]; for (size_t i(1); i < sorted.size(); i++) { - os << ' ' << sorted[i]; + os << ',' << sorted[i]; } } return os.str(); } +} + +vespalib::string +HandleRecorder::to_string() const +{ + vespalib::asciistream os; + os << "normal: [" << handles_to_string(_normal_handles) << "], "; + os << "cheap: [" << handles_to_string(_cheap_handles) << "]"; + return os.str(); +} + HandleRecorder::Binder::Binder(HandleRecorder & recorder) { _T_recorder = & recorder; @@ -64,17 +79,33 @@ HandleRecorder::~HandleRecorder() HandleRecorder::Binder::~Binder() { - _T_recorder = NULL; + _T_recorder = nullptr; } -void HandleRecorder::registerHandle(TermFieldHandle handle) +void +HandleRecorder::register_handle(TermFieldHandle handle, + MatchDataDetails requested_details) { // There should be no registration of handles that is not recorded. // That will lead to issues later on. - if (_T_recorder != NULL) { - _T_recorder->add(handle); + if (_T_recorder != nullptr) { + _T_recorder->add(handle, requested_details); } else if (_T_assert_all_handles_are_registered) { - assert(_T_recorder != NULL); + assert(_T_recorder != nullptr); + } +} + +void +HandleRecorder::add(TermFieldHandle handle, + MatchDataDetails requested_details) + +{ + if (requested_details == MatchDataDetails::Normal) { + _normal_handles.insert(handle); + } else if (requested_details == MatchDataDetails::Cheap) { + _cheap_handles.insert(handle); + } else { + abort(); } } diff --git a/searchcore/src/vespa/searchcore/proton/matching/handlerecorder.h b/searchcore/src/vespa/searchcore/proton/matching/handlerecorder.h index 8a8a4de7baa..54a30a0ed3d 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/handlerecorder.h +++ b/searchcore/src/vespa/searchcore/proton/matching/handlerecorder.h @@ -3,11 +3,11 @@ #pragma once #include <vespa/searchlib/fef/handle.h> +#include <vespa/searchlib/fef/match_data_details.h> #include <vespa/vespalib/stllike/hash_set.h> #include <vespa/vespalib/util/noncopyable.hpp> -namespace proton { -namespace matching { +namespace proton::matching { /** * This is a recorder that will register all handles used by any features for a given query. @@ -20,7 +20,7 @@ namespace matching { class HandleRecorder { public: - typedef vespalib::hash_set<search::fef::TermFieldHandle> HandleSet; + using HandleSet = vespalib::hash_set<search::fef::TermFieldHandle>; class Binder : public vespalib::noncopyable { public: Binder(HandleRecorder & recorder); @@ -33,16 +33,17 @@ public: }; HandleRecorder(); ~HandleRecorder(); - const HandleSet & getHandles() const { return _handles; } - static void registerHandle(search::fef::TermFieldHandle handle); - vespalib::string toString() const; + const HandleSet& get_normal_handles() const { return _normal_handles; } + const HandleSet& get_cheap_handles() const { return _cheap_handles; } + static void register_handle(search::fef::TermFieldHandle handle, + search::fef::MatchDataDetails requested_details); + vespalib::string to_string() const; private: - void add(search::fef::TermFieldHandle handle) { - _handles.insert(handle); - } - HandleSet _handles; + void add(search::fef::TermFieldHandle handle, + search::fef::MatchDataDetails requested_details); + HandleSet _normal_handles; + HandleSet _cheap_handles; }; -} // namespace matching -} // namespace proton +} diff --git a/searchcore/src/vespa/searchcore/proton/matching/match_tools.cpp b/searchcore/src/vespa/searchcore/proton/matching/match_tools.cpp index 28d56b7e0a2..e6dab8cbea4 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/match_tools.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/match_tools.cpp @@ -38,6 +38,7 @@ bool contains_all(const HandleRecorder::HandleSet &old_set, } void tag_match_data(const HandleRecorder::HandleSet &handles, MatchData &match_data) { + // TODO: Move tagging to separate component (for testing) and tag normal and cheap. for (TermFieldHandle handle = 0; handle < match_data.getNumTermFields(); ++handle) { if (handles.find(handle) == handles.end()) { match_data.resolveTermField(handle)->tagAsNotNeeded(); @@ -81,12 +82,14 @@ MatchTools::setup(search::fef::RankProgram::UP rank_program, double termwise_lim _rank_program->setup(*_match_data, _queryEnv, _featureOverrides); } bool can_reuse_search = (_search && !_search_has_changed && - contains_all(_used_handles, recorder.getHandles())); + contains_all(_used_normal_handles, recorder.get_normal_handles()) && + contains_all(_used_cheap_handles, recorder.get_cheap_handles())); if (!can_reuse_search) { - tag_match_data(recorder.getHandles(), *_match_data); + tag_match_data(recorder.get_normal_handles(), *_match_data); _match_data->set_termwise_limit(termwise_limit); _search = _query.createSearch(*_match_data); - _used_handles = recorder.getHandles(); + _used_normal_handles = recorder.get_normal_handles(); + _used_cheap_handles = recorder.get_cheap_handles(); _search_has_changed = false; } } @@ -111,7 +114,8 @@ MatchTools::MatchTools(QueryLimiter & queryLimiter, _match_data(mdl.createMatchData()), _rank_program(), _search(), - _used_handles(), + _used_normal_handles(), + _used_cheap_handles(), _search_has_changed(false) { } diff --git a/searchcore/src/vespa/searchcore/proton/matching/match_tools.h b/searchcore/src/vespa/searchcore/proton/matching/match_tools.h index dfef51bfc5e..10770554187 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/match_tools.h +++ b/searchcore/src/vespa/searchcore/proton/matching/match_tools.h @@ -35,7 +35,8 @@ private: search::fef::MatchData::UP _match_data; search::fef::RankProgram::UP _rank_program; search::queryeval::SearchIterator::UP _search; - HandleRecorder::HandleSet _used_handles; + HandleRecorder::HandleSet _used_normal_handles; + HandleRecorder::HandleSet _used_cheap_handles; bool _search_has_changed; void setup(search::fef::RankProgram::UP, double termwise_limit = 1.0); public: diff --git a/searchcore/src/vespa/searchcore/proton/matching/querynodes.cpp b/searchcore/src/vespa/searchcore/proton/matching/querynodes.cpp index b0d45ee0684..6d810594aa7 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/querynodes.cpp +++ b/searchcore/src/vespa/searchcore/proton/matching/querynodes.cpp @@ -14,6 +14,7 @@ using search::fef::FieldInfo; using search::fef::FieldType; using search::fef::IIndexEnvironment; using search::fef::MatchData; +using search::fef::MatchDataDetails; using search::fef::MatchDataLayout; using search::fef::TermFieldHandle; using search::query::Node; @@ -115,10 +116,10 @@ ProtonTermData::lookupField(uint32_t fieldId) const } TermFieldHandle -ProtonTermData::FieldEntry::getHandle() const +ProtonTermData::FieldEntry::getHandle(MatchDataDetails requested_details) const { - TermFieldHandle handle(search::fef::SimpleTermFieldData::getHandle()); - HandleRecorder::registerHandle(handle); + TermFieldHandle handle(search::fef::SimpleTermFieldData::getHandle(requested_details)); + HandleRecorder::register_handle(handle, requested_details); return handle; } diff --git a/searchcore/src/vespa/searchcore/proton/matching/querynodes.h b/searchcore/src/vespa/searchcore/proton/matching/querynodes.h index a06f5287669..8cf65c1e67b 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/querynodes.h +++ b/searchcore/src/vespa/searchcore/proton/matching/querynodes.h @@ -40,7 +40,8 @@ public: return FieldSpec(field_name, getFieldId(), getHandle(), filter_field); } - search::fef::TermFieldHandle getHandle() const override; + using SimpleTermFieldData::getHandle; + search::fef::TermFieldHandle getHandle(search::fef::MatchDataDetails requested_details) const override; }; private: |