diff options
7 files changed, 119 insertions, 187 deletions
diff --git a/searchlib/src/tests/fef/featureoverride/featureoverride.cpp b/searchlib/src/tests/fef/featureoverride/featureoverride.cpp index 5ae0efa9291..80389064a8f 100644 --- a/searchlib/src/tests/fef/featureoverride/featureoverride.cpp +++ b/searchlib/src/tests/fef/featureoverride/featureoverride.cpp @@ -26,11 +26,7 @@ struct Fixture MatchData::UP md; Fixture() : mdl(), stash(), executors(), md() {} Fixture &add(FeatureExecutor *executor, size_t outCnt) { - executor->inputs_done(); - for (uint32_t outIdx = 0; outIdx < outCnt; ++outIdx) { - executor->bindOutput(mdl.allocFeature()); - } - executor->outputs_done(); + executor->bind_outputs(stash.create_array<NumberOrObject>(outCnt)); executors.push_back(executor); return *this; } @@ -93,7 +89,6 @@ TEST_F("test decorator - non-existing override", Fixture) TEST_F("test decorator - transitive override", Fixture) { - FeatureExecutor::SharedInputs inputs; FeatureExecutor *fe = &f.createValueExecutor(); vespalib::Stash &stash = f.stash; fe = &stash.create<FeatureOverrider>(*fe, 1, 50.0); @@ -101,11 +96,12 @@ TEST_F("test decorator - transitive override", Fixture) EXPECT_EQUAL(fe->outputs().size(), 3u); FeatureExecutor *fe2 = &stash.create<DoubleExecutor>(3); - fe2->bind_shared_inputs(inputs); - fe2->addInput(fe->outputs()[0]); - fe2->addInput(fe->outputs()[1]); - fe2->addInput(fe->outputs()[2]); fe2 = &stash.create<FeatureOverrider>(*fe2, 2, 10.0); + auto inputs = stash.create_array<const NumberOrObject *>(3); + inputs[0] = fe->outputs().get_raw(0); + inputs[1] = fe->outputs().get_raw(1); + inputs[2] = fe->outputs().get_raw(2); + fe2->bind_inputs(inputs); f.add(fe2, 3).run(); EXPECT_EQUAL(fe2->outputs().size(), 3u); diff --git a/searchlib/src/vespa/searchlib/fef/featureexecutor.cpp b/searchlib/src/vespa/searchlib/fef/featureexecutor.cpp index 13be241b821..316885b9e37 100644 --- a/searchlib/src/vespa/searchlib/fef/featureexecutor.cpp +++ b/searchlib/src/vespa/searchlib/fef/featureexecutor.cpp @@ -19,15 +19,37 @@ FeatureExecutor::isPure() } void +FeatureExecutor::handle_bind_inputs(vespalib::ConstArrayRef<const NumberOrObject *>) +{ +} + +void +FeatureExecutor::handle_bind_outputs(vespalib::ArrayRef<NumberOrObject>) +{ +} + +void FeatureExecutor::handle_bind_match_data(MatchData &) { } void +FeatureExecutor::bind_inputs(vespalib::ConstArrayRef<const NumberOrObject *> inputs) +{ + _inputs.bind(inputs); + handle_bind_inputs(inputs); +} + +void +FeatureExecutor::bind_outputs(vespalib::ArrayRef<NumberOrObject> outputs) +{ + _outputs.bind(outputs); + handle_bind_outputs(outputs); +} + +void FeatureExecutor::bind_match_data(MatchData &md) { - _inputs.bind(md); - _outputs.bind(md); handle_bind_match_data(md); } diff --git a/searchlib/src/vespa/searchlib/fef/featureexecutor.h b/searchlib/src/vespa/searchlib/fef/featureexecutor.h index 7623a3de718..98f83cf13c7 100644 --- a/searchlib/src/vespa/searchlib/fef/featureexecutor.h +++ b/searchlib/src/vespa/searchlib/fef/featureexecutor.h @@ -8,6 +8,7 @@ #include "matchdata.h" #include <cassert> #include <memory> +#include <vespa/vespalib/util/array.h> namespace search { namespace fef { @@ -21,95 +22,50 @@ namespace fef { class FeatureExecutor { public: - class SharedInputs { - std::vector<FeatureHandle> _inputs; - public: - SharedInputs() : _inputs() {} - void add(FeatureHandle handle) { _inputs.push_back(handle); } - size_t size() const { return _inputs.size(); } - FeatureHandle operator[](size_t idx) const { return _inputs[idx]; } - }; - class Inputs { - SharedInputs *_inputs; - uint32_t _offset; - uint32_t _size; - const MatchData *_md; + vespalib::ConstArrayRef<const NumberOrObject *> _inputs; public: - Inputs() : _inputs(nullptr), _offset(0), _size(0), _md(nullptr) {} - void bind(const MatchData &md) { _md = &md; } + Inputs() : _inputs(nullptr, 0) {} + void bind(vespalib::ConstArrayRef<const NumberOrObject *> inputs) { _inputs = inputs; } feature_t get_number(size_t idx) const { - return *_md->resolveFeature((*this)[idx]); + return _inputs[idx]->as_number; } vespalib::eval::Value::CREF get_object(size_t idx) const { - return *_md->resolve_object_feature((*this)[idx]); + return _inputs[idx]->as_object; } const NumberOrObject *get_raw(size_t idx) const { - return _md->resolve_raw((*this)[idx]); - } - void bind(SharedInputs &inputs) { - _inputs = &inputs; - _offset = _inputs->size(); - _size = 0; - } - void add(FeatureHandle handle) { - assert(_inputs != nullptr); - assert(_inputs->size() == (_offset + _size)); - _inputs->add(handle); - ++_size; - } - bool empty() const { return (_size == 0); } - size_t size() const { return _size; } - FeatureHandle operator[](size_t idx) const { - assert(idx < _size); - return (*_inputs)[_offset + idx]; + return _inputs[idx]; } + size_t size() const { return _inputs.size(); } }; class Outputs { - FeatureHandle _begin; - FeatureHandle _end; - MatchData *_md; + vespalib::ArrayRef<NumberOrObject> _outputs; public: - Outputs() : _begin(IllegalHandle), _end(IllegalHandle), _md(nullptr) {} - void bind(MatchData &md) { _md = &md; } + Outputs() : _outputs(nullptr, 0) {} + void bind(vespalib::ArrayRef<NumberOrObject> outputs) { _outputs = outputs; } void set_number(size_t idx, feature_t value) { - *_md->resolveFeature((*this)[idx]) = value; + _outputs[idx].as_number = value; } void set_object(size_t idx, vespalib::eval::Value::CREF value) { - *_md->resolve_object_feature((*this)[idx]) = value; + _outputs[idx].as_object = value; } feature_t *get_number_ptr(size_t idx) { - return _md->resolveFeature((*this)[idx]); + return &_outputs[idx].as_number; } - vespalib::eval::Value::CREF *get_object_ptr(size_t idx) { - return _md->resolve_object_feature((*this)[idx]); + vespalib::eval::Value::CREF *get_object_ptr(size_t idx) { + return &_outputs[idx].as_object; } feature_t get_number(size_t idx) const { - return *_md->resolveFeature((*this)[idx]); + return _outputs[idx].as_number; } vespalib::eval::Value::CREF get_object(size_t idx) const { - return *_md->resolve_object_feature((*this)[idx]); + return _outputs[idx].as_object; } const NumberOrObject *get_raw(size_t idx) const { - return _md->resolve_raw((*this)[idx]); - } - void add(FeatureHandle handle) { - if (_begin == IllegalHandle) { - _begin = handle; - _end = (_begin + 1); - } else if (handle == _end) { - ++_end; - } else { - assert(handle == _end); - } - } - bool empty() const { return (_end == _begin); } - size_t size() const { return (_end - _begin); } - FeatureHandle operator[](size_t idx) const { - assert(idx < (_end - _begin)); - return (_begin + idx); + return &_outputs[idx]; } + size_t size() const { return _outputs.size(); } }; private: @@ -120,6 +76,8 @@ private: Outputs _outputs; protected: + virtual void handle_bind_inputs(vespalib::ConstArrayRef<const NumberOrObject *> inputs); + virtual void handle_bind_outputs(vespalib::ArrayRef<NumberOrObject> outputs); virtual void handle_bind_match_data(MatchData &md); public: @@ -129,60 +87,13 @@ public: **/ FeatureExecutor(); - /** - * Bind shared external storage to this feature executor. The - * shared storage will be used to store the handle of feature - * inputs. This function must be called before starting to add - * inputs. - * - * @param shared_inputs shared store for input feature handles - **/ - void bind_shared_inputs(SharedInputs &shared_inputs) { _inputs.bind(shared_inputs); } - - // Bind inputs and outputs directly to the underlying match data - // to be able to hide the fact that input and output values are - // stored in a match data object from the executor itself. + // bind order per executor: inputs, outputs, match_data + void bind_inputs(vespalib::ConstArrayRef<const NumberOrObject *> inputs); + void bind_outputs(vespalib::ArrayRef<NumberOrObject> outputs); void bind_match_data(MatchData &md); - /** - * Add an input to this feature executor. All inputs must be added - * before this object is added to the feature execution manager. - * - * @param handle the feature handle of the input to add - **/ - void addInput(FeatureHandle handle) { _inputs.add(handle); } - virtual void inputs_done() {} // needed for feature decorators - - /** - * Access the input features for this executor. Use {@link - * MatchData#resolveFeature} to resolve these handles. - * - * @return const view of input features - **/ const Inputs &inputs() const { return _inputs; } - - /** - * Assign a feature handle to the next unbound output feature. - * This method will be invoked by the @ref FeatureExecutionManager - * when new feature executors are added. It may also be used for - * testing, but should not be invoked directly from application - * code. Note that this method must be invoked exactly the number - * of times indicated by the @ref getNumOutputs method. - * - * @param handle feature handle to be assigned to the next unbound - * output feature. - **/ - void bindOutput(FeatureHandle handle) { _outputs.add(handle); } - virtual void outputs_done() {} // needed for feature decorators - - /** - * Access the output features for this executor. Use {@link - * MatchData#resolveFeature} to resolve these handles. - * - * @return const view of output features - **/ const Outputs &outputs() const { return _outputs; } - Outputs &outputs() { return _outputs; } /** diff --git a/searchlib/src/vespa/searchlib/fef/featureoverrider.cpp b/searchlib/src/vespa/searchlib/fef/featureoverrider.cpp index 8ef0cf8d1ff..a4e0f58b590 100644 --- a/searchlib/src/vespa/searchlib/fef/featureoverrider.cpp +++ b/searchlib/src/vespa/searchlib/fef/featureoverrider.cpp @@ -6,29 +6,23 @@ namespace search { namespace fef { -FeatureOverrider::FeatureOverrider(FeatureExecutor &executor, uint32_t outputIdx, feature_t value) - : _executor(executor), - _outputIdx(outputIdx), - _value(value) +void +FeatureOverrider::handle_bind_inputs(vespalib::ConstArrayRef<const NumberOrObject *> inputs) { + _executor.bind_inputs(inputs); } void -FeatureOverrider::inputs_done() +FeatureOverrider::handle_bind_outputs(vespalib::ArrayRef<NumberOrObject> outputs) { - for (uint32_t i = 0; i < inputs().size(); ++i) { - _executor.addInput(inputs()[i]); - } - _executor.inputs_done(); + _executor.bind_outputs(outputs); } -void -FeatureOverrider::outputs_done() +FeatureOverrider::FeatureOverrider(FeatureExecutor &executor, uint32_t outputIdx, feature_t value) + : _executor(executor), + _outputIdx(outputIdx), + _value(value) { - for (uint32_t i = 0; i < outputs().size(); ++i) { - _executor.bindOutput(outputs()[i]); - } - _executor.outputs_done(); } bool diff --git a/searchlib/src/vespa/searchlib/fef/featureoverrider.h b/searchlib/src/vespa/searchlib/fef/featureoverrider.h index 1c3f8004018..2d942914fcc 100644 --- a/searchlib/src/vespa/searchlib/fef/featureoverrider.h +++ b/searchlib/src/vespa/searchlib/fef/featureoverrider.h @@ -25,6 +25,9 @@ private: feature_t _value; virtual void handle_bind_match_data(MatchData &md) override; + virtual void handle_bind_inputs(vespalib::ConstArrayRef<const NumberOrObject *> inputs) override; + virtual void handle_bind_outputs(vespalib::ArrayRef<NumberOrObject> outputs) override; + public: /** * Create a feature overrider that will override the given output @@ -35,8 +38,6 @@ public: * @param value what value to override with **/ FeatureOverrider(FeatureExecutor &executor, uint32_t outputIdx, feature_t value); - void inputs_done() override; - void outputs_done() override; bool isPure() override; void execute(uint32_t docId) override; }; diff --git a/searchlib/src/vespa/searchlib/fef/rank_program.cpp b/searchlib/src/vespa/searchlib/fef/rank_program.cpp index cce33cb26ed..b61d1a32664 100644 --- a/searchlib/src/vespa/searchlib/fef/rank_program.cpp +++ b/searchlib/src/vespa/searchlib/fef/rank_program.cpp @@ -6,6 +6,7 @@ LOG_SETUP(".fef.rank_program"); #include "rank_program.h" #include "featureoverrider.h" #include <algorithm> +#include <set> namespace search { namespace fef { @@ -56,22 +57,7 @@ std::vector<Override> prepare_overrides(const BlueprintResolver::FeatureMap &fea } struct UnboxingExecutor : FeatureExecutor { - using MappedValues = std::map<const NumberOrObject *, const NumberOrObject *>; - MappedValues &unbox_map; - UnboxingExecutor(SharedInputs &shared_inputs, - FeatureHandle old_feature, - FeatureHandle new_feature, - MappedValues &unbox_map_in) - : unbox_map(unbox_map_in) - { - bind_shared_inputs(shared_inputs); - addInput(old_feature); - bindOutput(new_feature); - } bool isPure() override { return true; } - void handle_bind_match_data(MatchData &) override { - unbox_map[inputs().get_raw(0)] = outputs().get_raw(0); - } void execute(uint32_t) override { outputs().set_number(0, inputs().get_object(0).get().as_double()); } @@ -79,37 +65,59 @@ struct UnboxingExecutor : FeatureExecutor { } // namespace search::fef::<unnamed> +size_t +RankProgram::count_features() const +{ + size_t cnt = 0; + const auto &specs = _resolver->getExecutorSpecs(); + for (const auto &entry: specs) { + cnt += entry.output_types.size(); // normal outputs + } + for (const auto &seed_entry: _resolver->getSeedMap()) { + auto seed = seed_entry.second; + if (specs[seed.executor].output_types[seed.output]) { + ++cnt; // unboxed seeds + } + } + return cnt; +} + void -RankProgram::add_unboxing_executors(MatchDataLayout &my_mdl) +RankProgram::add_unboxing_executors(vespalib::ArrayRef<NumberOrObject> features, size_t feature_offset, size_t total_features) { const auto &specs = _resolver->getExecutorSpecs(); for (const auto &seed_entry: _resolver->getSeedMap()) { auto seed = seed_entry.second; if (specs[seed.executor].output_types[seed.output]) { - FeatureHandle old_handle = _executors[seed.executor]->outputs()[seed.output]; - FeatureHandle new_handle = my_mdl.allocFeature(false); - _executors.emplace_back(&_stash.create<UnboxingExecutor>(_shared_inputs, old_handle, new_handle, _unboxed_seeds)); + vespalib::ArrayRef<const NumberOrObject *> inputs = _stash.create_array<const NumberOrObject *>(1); + inputs[0] = _executors[seed.executor]->outputs().get_raw(seed.output); + vespalib::ArrayRef<NumberOrObject> outputs(&features[feature_offset++], 1); + _unboxed_seeds[inputs[0]] = &outputs[0]; + _executors.emplace_back(&_stash.create<UnboxingExecutor>()); + _executors.back()->bind_inputs(inputs); + _executors.back()->bind_outputs(outputs); + _executors.back()->bind_match_data(*_match_data); } } + assert(feature_offset == total_features); } void RankProgram::compile() { - MatchData &md = match_data(); - std::vector<bool> is_calculated(md.getNumFeatures(), false); + std::set<const NumberOrObject *> is_calculated; for (size_t i = 0; i < _executors.size(); ++i) { FeatureExecutor &executor = *_executors[i]; bool is_const = executor.isPure(); const auto &inputs = executor.inputs(); for (size_t in_idx = 0; is_const && (in_idx < inputs.size()); ++in_idx) { - is_const &= is_calculated[inputs[in_idx]]; + is_const &= (is_calculated.count(inputs.get_raw(in_idx)) > 0); } if (is_const) { executor.execute(1); const auto &outputs = executor.outputs(); for (size_t out_idx = 0; out_idx < outputs.size(); ++out_idx) { - is_calculated[outputs[out_idx]] = true; + is_calculated.insert(outputs.get_raw(out_idx)); } } else { _program.push_back(&executor); @@ -141,7 +149,6 @@ RankProgram::resolve(const BlueprintResolver::FeatureMap &features, bool unbox_s RankProgram::RankProgram(BlueprintResolver::SP resolver) : _resolver(resolver), - _shared_inputs(), _program(), _stash(), _executors(), @@ -155,38 +162,37 @@ RankProgram::setup(const MatchDataLayout &mdl_in, const Properties &featureOverrides) { assert(_executors.empty()); - MatchDataLayout my_mdl(mdl_in); + _match_data = mdl_in.createMatchData(); std::vector<Override> overrides = prepare_overrides(_resolver->getFeatureMap(), featureOverrides); auto override = overrides.begin(); auto override_end = overrides.end(); + size_t feature_offset = 0; + size_t total_features = count_features(); + vespalib::ArrayRef<NumberOrObject> features = _stash.create_array<NumberOrObject>(total_features); const auto &specs = _resolver->getExecutorSpecs(); _executors.reserve(specs.size()); for (uint32_t i = 0; i < specs.size(); ++i) { + size_t num_inputs = specs[i].inputs.size(); + vespalib::ArrayRef<const NumberOrObject *> inputs = _stash.create_array<const NumberOrObject *>(num_inputs); + for (size_t input_idx = 0; input_idx < num_inputs; ++input_idx) { + auto ref = specs[i].inputs[input_idx]; + inputs[input_idx] = _executors[ref.executor]->outputs().get_raw(ref.output); + } + size_t num_outputs = specs[i].output_types.size(); + vespalib::ArrayRef<NumberOrObject> outputs(&features[feature_offset], num_outputs); + feature_offset += num_outputs; FeatureExecutor *executor = &(specs[i].blueprint->createExecutor(queryEnv, _stash)); - assert(executor); - executor->bind_shared_inputs(_shared_inputs); for (; (override < override_end) && (override->ref.executor == i); ++override) { FeatureExecutor *tmp = executor; executor = &(_stash.create<FeatureOverrider>(*tmp, override->ref.output, override->value)); - executor->bind_shared_inputs(_shared_inputs); - } - for (auto ref: specs[i].inputs) { - executor->addInput(_executors[ref.executor]->outputs()[ref.output]); } - executor->inputs_done(); - uint32_t out_cnt = specs[i].output_types.size(); - for (uint32_t out_idx = 0; out_idx < out_cnt; ++out_idx) { - executor->bindOutput(my_mdl.allocFeature(specs[i].output_types[out_idx])); - } - executor->outputs_done(); - _executors.push_back(executor); - } - add_unboxing_executors(my_mdl); - _match_data = my_mdl.createMatchData(); - for (auto executor: _executors) { + executor->bind_inputs(inputs); + executor->bind_outputs(outputs); executor->bind_match_data(*_match_data); + _executors.push_back(executor); } + add_unboxing_executors(features, feature_offset, total_features); compile(); } diff --git a/searchlib/src/vespa/searchlib/fef/rank_program.h b/searchlib/src/vespa/searchlib/fef/rank_program.h index d5e2fbb88cc..46308559e2a 100644 --- a/searchlib/src/vespa/searchlib/fef/rank_program.h +++ b/searchlib/src/vespa/searchlib/fef/rank_program.h @@ -11,6 +11,7 @@ #include <vespa/vespalib/stllike/string.h> #include <vector> #include <memory.h> +#include <vespa/vespalib/util/array.h> namespace search { namespace fef { @@ -30,18 +31,19 @@ private: using MappedValues = std::map<const NumberOrObject *, const NumberOrObject *>; BlueprintResolver::SP _resolver; - FeatureExecutor::SharedInputs _shared_inputs; std::vector<FeatureExecutor *> _program; MatchData::UP _match_data; vespalib::Stash _stash; std::vector<FeatureExecutor *> _executors; MappedValues _unboxed_seeds; + size_t count_features() const; + /** * Add unboxing executors for seeds that are object features to * make sure all output values are numbers. **/ - void add_unboxing_executors(MatchDataLayout &my_mdl); + void add_unboxing_executors(vespalib::ArrayRef<NumberOrObject> features, size_t feature_offset, size_t total_features); /** * Prepare the final program and evaluate all constant features. |