diff options
20 files changed, 79 insertions, 450 deletions
diff --git a/config-model/src/main/java/com/yahoo/searchdefinition/SearchBuilder.java b/config-model/src/main/java/com/yahoo/searchdefinition/SearchBuilder.java index d9039ebbe6e..ba0ede77b55 100644 --- a/config-model/src/main/java/com/yahoo/searchdefinition/SearchBuilder.java +++ b/config-model/src/main/java/com/yahoo/searchdefinition/SearchBuilder.java @@ -177,34 +177,6 @@ public class SearchBuilder { } /** - * Registers the given search object to the internal list of objects to be processed during {@link #build()}. A - * {@link Search} object is considered to be "processed" if it has not already been processed. This is the case for most - * programmatically constructed search objects used in unit tests. - * - * @param processed The object to import. - * @return The name of the imported object. - * @throws IllegalArgumentException Thrown if the given search object has already been processed. - */ - public String importProcessedSearch(Search processed) { - if (processed.getName() == null) { - throw new IllegalArgumentException("Search has no name."); - } - String rawName = processed.getName(); - if (!processed.isProcessed()) { - throw new IllegalArgumentException("A search definition with a search section called '" + rawName + - "' has not been processed."); - } - for (Search search : searchList) { - if (rawName.equals(search.getName())) { - throw new IllegalArgumentException("A search definition with a search section called '" + rawName + - "' has already been added."); - } - } - searchList.add(processed); - return rawName; - } - - /** * Only for testing. * * Processes and finalizes the imported search definitions so that they become available through the {@link @@ -380,6 +352,8 @@ public class SearchBuilder { return builder; } + // TODO: The build methods below just call the create methods above - remove + /** * Convenience factory method to import and build a {@link Search} object from a file. Only for testing. * diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/Logserver.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/Logserver.java index 4f873b112cf..28cf18802cf 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/Logserver.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/Logserver.java @@ -11,6 +11,7 @@ import com.yahoo.vespa.model.AbstractService; * @author gjoranv */ public class Logserver extends AbstractService { + private static final long serialVersionUID = 1L; private static final String logArchiveDir = "$ROOT/logs/vespa/logarchive"; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java index 7512e14643f..b4e537b2370 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java @@ -307,7 +307,7 @@ public class ApplicationController { // Delete zones not listed in DeploymentSpec, if allowed // We do this at deployment time to be able to return a validation failure message when necessary - application = deleteRemovedDeployments(application); + application = deleteRemovedDeployments(application, lock); // Clean up deployment jobs that are no longer referenced by deployment spec application = deleteUnreferencedDeploymentJobs(application); @@ -354,7 +354,7 @@ public class ApplicationController { return new ActivateResult(new RevisionId(applicationPackage.hash()), prepareResponse); } - private Application deleteRemovedDeployments(Application application) { + private Application deleteRemovedDeployments(Application application, Lock lock) { List<Deployment> deploymentsToRemove = application.deployments().values().stream() .filter(deployment -> deployment.zone().environment() == Environment.prod) .filter(deployment -> ! application.deploymentSpec().includes(deployment.zone().environment(), @@ -375,7 +375,7 @@ public class ApplicationController { Application applicationWithRemoval = application; for (Deployment deployment : deploymentsToRemove) - applicationWithRemoval = deactivate(applicationWithRemoval, deployment, false); + applicationWithRemoval = deactivate(applicationWithRemoval, deployment.zone(), lock); return applicationWithRemoval; } @@ -556,22 +556,32 @@ public class ApplicationController { private Application deactivate(Application application, Zone zone, Optional<Deployment> deployment, boolean requireThatDeploymentHasExpired) { try (Lock lock = lock(application.id())) { - if (deployment.isPresent() && requireThatDeploymentHasExpired && ! DeploymentExpirer.hasExpired( - controller.zoneRegistry(), deployment.get(), clock.instant())) { + application = controller.applications().require(application.id()); // re-get with lock + if (deployment.isPresent() && requireThatDeploymentHasExpired && + ! DeploymentExpirer.hasExpired(controller.zoneRegistry(), deployment.get(), clock.instant())) { return application; } - - try { - configserverClient.deactivate(new DeploymentId(application.id(), zone)); - } catch (NoInstanceException ignored) { - // ok; already gone - } - application = application.withoutDeploymentIn(zone); + application = deactivate(application, zone, lock); store(application, lock); return application; } } + /** + * Deactivates a locked application without storing it + * + * @return the application with the deployment in the given zone removed + */ + private Application deactivate(Application application, Zone zone, Lock lock) { + try { + configserverClient.deactivate(new DeploymentId(application.id(), zone)); + } + catch (NoInstanceException ignored) { + // ok; already gone + } + return application.withoutDeploymentIn(zone); + } + public DeploymentTrigger deploymentTrigger() { return deploymentTrigger; } private ApplicationId dashToUnderscore(ApplicationId id) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java index 3f5877addb7..c8479f448ba 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java @@ -123,6 +123,8 @@ public class DeploymentTrigger { applications = applications.notPullRequest(); for (Application application : applications.asList()) { try (Lock lock = applications().lock(application.id())) { + application = controller.applications().get(application.id()).orElse(null); // re-get with lock + if (application == null) continue; // application removed triggerReadyJobs(application, lock); } } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ClusterInfoMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ClusterInfoMaintainer.java index baf60b612b0..a6bf0112e5d 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ClusterInfoMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ClusterInfoMaintainer.java @@ -87,6 +87,9 @@ public class ClusterInfoMaintainer extends Maintainer { protected void maintain() { for (Application application : controller().applications().asList()) { try (Lock lock = controller().applications().lock(application.id())) { + application = controller.applications().get(application.id()).orElse(null); // re-get inside lock + if (application == null) continue; // application removed + for (Deployment deployment : application.deployments().values()) { DeploymentId deploymentId = new DeploymentId(application.id(), deployment.zone()); try { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ClusterUtilizationMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ClusterUtilizationMaintainer.java index 8806651c60d..f9a3de87cab 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ClusterUtilizationMaintainer.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ClusterUtilizationMaintainer.java @@ -44,9 +44,10 @@ public class ClusterUtilizationMaintainer extends Maintainer { @Override protected void maintain() { - for (Application application : controller().applications().asList()) { try (Lock lock = controller().applications().lock(application.id())) { + application = controller.applications().get(application.id()).orElse(null); // re-get inside lock + if (application == null) continue; // application removed for (Deployment deployment : application.deployments().values()) { Map<ClusterSpec.Id, ClusterUtilization> clusterUtilization = getUpdatedClusterUtilizations(application.id(), deployment.zone()); Application app = application.with(deployment.withClusterUtils(clusterUtilization)); diff --git a/eval/src/tests/eval/function_speed/function_speed_test.cpp b/eval/src/tests/eval/function_speed/function_speed_test.cpp index f1924331065..ac1471544ed 100644 --- a/eval/src/tests/eval/function_speed/function_speed_test.cpp +++ b/eval/src/tests/eval/function_speed/function_speed_test.cpp @@ -5,9 +5,11 @@ #include <vespa/vespalib/util/benchmark_timer.h> #include <vespa/eval/eval/interpreted_function.h> #include <vespa/vespalib/util/benchmark_timer.h> +#include <vespa/eval/tensor/default_tensor_engine.h> using namespace vespalib::eval; using vespalib::BenchmarkTimer; +using vespalib::tensor::DefaultTensorEngine; double budget = 0.25; @@ -38,12 +40,14 @@ double big_gcc_function(double p, double o, double q, double f, double w) { struct Fixture { Function function; InterpretedFunction interpreted; + InterpretedFunction interpreted_simple; CompiledFunction separate; CompiledFunction array; CompiledFunction lazy; Fixture(const vespalib::string &expr) : function(Function::parse(expr)), - interpreted(SimpleTensorEngine::ref(), function, NodeTypes()), + interpreted(DefaultTensorEngine::ref(), function, NodeTypes()), + interpreted_simple(SimpleTensorEngine::ref(), function, NodeTypes()), separate(function, PassParams::SEPARATE), array(function, PassParams::ARRAY), lazy(function, PassParams::LAZY) {} @@ -74,12 +78,16 @@ TEST("measure small function eval/jit/gcc speed") { double interpret_time = fixture.interpreted.estimate_cost_us(test_params, budget); fprintf(stderr, "interpret: %g us\n", interpret_time); + double interpret_simple_time = fixture.interpreted_simple.estimate_cost_us(test_params, budget); + fprintf(stderr, "interpret (simple): %g us\n", interpret_simple_time); double jit_time = estimate_cost_us(test_params, fixture.separate.get_function<5>()); fprintf(stderr, "jit compiled: %g us\n", jit_time); double gcc_time = estimate_cost_us(test_params, fun); fprintf(stderr, "gcc compiled: %g us\n", gcc_time); + double simple_vs_default_speed = (1.0/interpret_simple_time)/(1.0/interpret_time); double jit_vs_interpret_speed = (1.0/jit_time)/(1.0/interpret_time); double gcc_vs_jit_speed = (1.0/gcc_time)/(1.0/jit_time); + fprintf(stderr, "simple vs default interpret speed: %g\n", simple_vs_default_speed); fprintf(stderr, "jit speed compared to interpret: %g\n", jit_vs_interpret_speed); fprintf(stderr, "gcc speed compared to jit: %g\n", gcc_vs_jit_speed); @@ -104,12 +112,16 @@ TEST("measure big function eval/jit/gcc speed") { double interpret_time = fixture.interpreted.estimate_cost_us(test_params, budget); fprintf(stderr, "interpret: %g us\n", interpret_time); + double interpret_simple_time = fixture.interpreted_simple.estimate_cost_us(test_params, budget); + fprintf(stderr, "interpret (simple): %g us\n", interpret_time); double jit_time = estimate_cost_us(test_params, fixture.separate.get_function<5>()); fprintf(stderr, "jit compiled: %g us\n", jit_time); double gcc_time = estimate_cost_us(test_params, fun); fprintf(stderr, "gcc compiled: %g us\n", gcc_time); + double simple_vs_default_speed = (1.0/interpret_simple_time)/(1.0/interpret_time); double jit_vs_interpret_speed = (1.0/jit_time)/(1.0/interpret_time); double gcc_vs_jit_speed = (1.0/gcc_time)/(1.0/jit_time); + fprintf(stderr, "simple vs default interpret speed: %g\n", simple_vs_default_speed); fprintf(stderr, "jit speed compared to interpret: %g\n", jit_vs_interpret_speed); fprintf(stderr, "gcc speed compared to jit: %g\n", gcc_vs_jit_speed); diff --git a/eval/src/tests/eval/simple_tensor/simple_tensor_test.cpp b/eval/src/tests/eval/simple_tensor/simple_tensor_test.cpp index f0dca274fb4..beec4ed928b 100644 --- a/eval/src/tests/eval/simple_tensor/simple_tensor_test.cpp +++ b/eval/src/tests/eval/simple_tensor/simple_tensor_test.cpp @@ -94,7 +94,7 @@ TEST("require that simple tensors can have their values negated") { auto result = tensor->map([](double a){ return -a; }); EXPECT_EQUAL(*expect, *result); Stash stash; - const Value &result2 = SimpleTensorEngine::ref().map(operation::Neg(), *tensor, stash); + const Value &result2 = SimpleTensorEngine::ref().map(TensorValue(*tensor), operation::Neg::f, stash); EXPECT_EQUAL(*expect, unwrap(result2)); } @@ -119,7 +119,7 @@ TEST("require that simple tensors can be multiplied with each other") { auto result = SimpleTensor::join(*lhs, *rhs, [](double a, double b){ return (a * b); }); EXPECT_EQUAL(*expect, *result); Stash stash; - const Value &result2 = SimpleTensorEngine::ref().apply(operation::Mul(), *lhs, *rhs, stash); + const Value &result2 = SimpleTensorEngine::ref().join(TensorValue(*lhs), TensorValue(*rhs), operation::Mul::f, stash); EXPECT_EQUAL(*expect, unwrap(result2)); } @@ -150,10 +150,10 @@ TEST("require that simple tensors support dimension reduction") { EXPECT_EQUAL(*expect_sum_y, *result_sum_y); EXPECT_EQUAL(*expect_sum_x, *result_sum_x); EXPECT_EQUAL(*expect_sum_all, *result_sum_all); - const Value &result_sum_y_2 = SimpleTensorEngine::ref().reduce(*tensor, operation::Add(), {"y"}, stash); - const Value &result_sum_x_2 = SimpleTensorEngine::ref().reduce(*tensor, operation::Add(), {"x"}, stash); - const Value &result_sum_all_2 = SimpleTensorEngine::ref().reduce(*tensor, operation::Add(), {"x", "y"}, stash); - const Value &result_sum_all_3 = SimpleTensorEngine::ref().reduce(*tensor, operation::Add(), {}, stash); + const Value &result_sum_y_2 = SimpleTensorEngine::ref().reduce(TensorValue(*tensor), Aggr::SUM, {"y"}, stash); + const Value &result_sum_x_2 = SimpleTensorEngine::ref().reduce(TensorValue(*tensor), Aggr::SUM, {"x"}, stash); + const Value &result_sum_all_2 = SimpleTensorEngine::ref().reduce(TensorValue(*tensor), Aggr::SUM, {"x", "y"}, stash); + const Value &result_sum_all_3 = SimpleTensorEngine::ref().reduce(TensorValue(*tensor), Aggr::SUM, {}, stash); EXPECT_EQUAL(*expect_sum_y, unwrap(result_sum_y_2)); EXPECT_EQUAL(*expect_sum_x, unwrap(result_sum_x_2)); EXPECT_TRUE(result_sum_all_2.is_double()); diff --git a/eval/src/vespa/eval/eval/interpreted_function.cpp b/eval/src/vespa/eval/eval/interpreted_function.cpp index 10fc60dd27d..e6e11dbd7c3 100644 --- a/eval/src/vespa/eval/eval/interpreted_function.cpp +++ b/eval/src/vespa/eval/eval/interpreted_function.cpp @@ -48,12 +48,12 @@ void op_load_let(State &state, uint64_t param) { template <typename OP1> void op_unary(State &state, uint64_t) { - state.replace(1, OP1().perform(state.peek(0), state.stash)); + state.replace(1, state.engine.map(state.peek(0), OP1::f, state.stash)); } template <typename OP2> void op_binary(State &state, uint64_t) { - state.replace(2, OP2().perform(state.peek(1), state.peek(0), state.stash)); + state.replace(2, state.engine.join(state.peek(1), state.peek(0), OP2::f, state.stash)); } //----------------------------------------------------------------------------- @@ -102,20 +102,12 @@ void op_not_member(State &state, uint64_t) { //----------------------------------------------------------------------------- void op_tensor_sum(State &state, uint64_t) { - const eval::Tensor *tensor = state.peek(0).as_tensor(); - if (tensor != nullptr) { - state.replace(1, tensor->engine().reduce(*tensor, operation::Add(), {}, state.stash)); - } + state.replace(1, state.engine.reduce(state.peek(0), Aggr::SUM, {}, state.stash)); } void op_tensor_sum_dimension(State &state, uint64_t param) { - const eval::Tensor *tensor = state.peek(0).as_tensor(); - if (tensor != nullptr) { - const vespalib::string &dimension = unwrap_param<vespalib::string>(param); - state.replace(1, tensor->engine().reduce(*tensor, operation::Add(), {dimension}, state.stash)); - } else { - state.replace(1, state.stash.create<ErrorValue>()); - } + const vespalib::string &dimension = unwrap_param<vespalib::string>(param); + state.replace(1, state.engine.reduce(state.peek(0), Aggr::SUM, {dimension}, state.stash)); } //----------------------------------------------------------------------------- diff --git a/eval/src/vespa/eval/eval/operation.cpp b/eval/src/vespa/eval/eval/operation.cpp index 16514cafaf4..e417f434536 100644 --- a/eval/src/vespa/eval/eval/operation.cpp +++ b/eval/src/vespa/eval/eval/operation.cpp @@ -9,62 +9,6 @@ namespace vespalib { namespace eval { -const Value & -UnaryOperation::perform(const Value &lhs, Stash &stash) const { - if (lhs.is_error()) { - return stash.create<ErrorValue>(); - } else if (lhs.is_double()) { - return stash.create<DoubleValue>(eval(lhs.as_double())); - } else { - return lhs.apply(*this, stash); - } -} - -const Value & -BinaryOperation::perform(const Value &lhs, const Value &rhs, Stash &stash) const { - if (lhs.is_error() || rhs.is_error()) { - return stash.create<ErrorValue>(); - } else if (lhs.is_double() && rhs.is_double()) { - return stash.create<DoubleValue>(eval(lhs.as_double(), rhs.as_double())); - } else if (lhs.is_double()) { - BindLeft unary_op(*this, lhs.as_double()); - return rhs.apply(unary_op, stash); - } else if (rhs.is_double()) { - BindRight unary_op(*this, rhs.as_double()); - return lhs.apply(unary_op, stash); - } else { - return lhs.apply(*this, rhs, stash); - } -} - -__thread const UnaryOperation *UnaryOperationProxy::_ctx = nullptr; -double UnaryOperationProxy::eval_proxy(double a) { return _ctx->eval(a); } -UnaryOperationProxy::UnaryOperationProxy(const UnaryOperation &op) - : _my_ctx(&op), - _old_ctx(_ctx) -{ - _ctx = _my_ctx; -} -UnaryOperationProxy::~UnaryOperationProxy() -{ - assert(_ctx == _my_ctx); - _ctx = _old_ctx; -} - -__thread const BinaryOperation *BinaryOperationProxy::_ctx = nullptr; -double BinaryOperationProxy::eval_proxy(double a, double b) { return _ctx->eval(a, b); } -BinaryOperationProxy::BinaryOperationProxy(const BinaryOperation &op) - : _my_ctx(&op), - _old_ctx(_ctx) -{ - _ctx = _my_ctx; -} -BinaryOperationProxy::~BinaryOperationProxy() -{ - assert(_ctx == _my_ctx); - _ctx = _old_ctx; -} - template <typename T> void Op1<T>::accept(OperationVisitor &visitor) const { visitor.visit(static_cast<const T&>(*this)); } @@ -176,6 +120,7 @@ template struct Op2<operation::Max>; template struct Op1<operation::IsNan>; template struct Op1<operation::Relu>; template struct Op1<operation::Sigmoid>; +template struct Op1<CustomUnaryOperation>; } // namespace vespalib::eval } // namespace vespalib diff --git a/eval/src/vespa/eval/eval/operation.h b/eval/src/vespa/eval/eval/operation.h index bd81f17aae9..6395412731c 100644 --- a/eval/src/vespa/eval/eval/operation.h +++ b/eval/src/vespa/eval/eval/operation.h @@ -39,7 +39,6 @@ const T *as(const Operation &op) { return dynamic_cast<const T *>(&op); } * value. **/ struct UnaryOperation : Operation { - const Value &perform(const Value &a, Stash &stash) const; virtual double eval(double a) const = 0; virtual op1_fun_t get_f() const = 0; }; @@ -48,7 +47,6 @@ struct UnaryOperation : Operation { * An Operation performing a calculation based on two input values. **/ struct BinaryOperation : Operation { - const Value &perform(const Value &a, const Value &b, Stash &stash) const; virtual double eval(double a, double b) const = 0; virtual std::unique_ptr<BinaryOperation> clone() const = 0; virtual op2_fun_t get_f() const = 0; @@ -56,42 +54,6 @@ struct BinaryOperation : Operation { //----------------------------------------------------------------------------- -/** - * Utility class used to adapt stateless function pointers to stateful - * functors by using thread-local bindings. - **/ -class UnaryOperationProxy { -private: - static __thread const UnaryOperation *_ctx; - static double eval_proxy(double a); - const UnaryOperation *_my_ctx; - const UnaryOperation *_old_ctx; -public: - using fun_t = double (*)(double); - UnaryOperationProxy(const UnaryOperation &op); - operator fun_t() const { return eval_proxy; } - ~UnaryOperationProxy(); -}; - -/** - * Utility class used to adapt stateless function pointers to stateful - * functors by using thread-local bindings. - **/ -class BinaryOperationProxy { -private: - static __thread const BinaryOperation *_ctx; - static double eval_proxy(double a, double b); - const BinaryOperation *_my_ctx; - const BinaryOperation *_old_ctx; -public: - using fun_t = double (*)(double, double); - BinaryOperationProxy(const BinaryOperation &op); - operator fun_t() const { return eval_proxy; } - ~BinaryOperationProxy(); -}; - -//----------------------------------------------------------------------------- - template <typename T> struct Op1 : UnaryOperation { virtual void accept(OperationVisitor &visitor) const override; @@ -119,36 +81,6 @@ struct CustomUnaryOperation : Op1<CustomUnaryOperation> { //----------------------------------------------------------------------------- -/** - * This class binds the first parameter of a binary operation to a - * numeric value, acting as a custom unary operation. - **/ -class BindLeft : public CustomUnaryOperation -{ -private: - const BinaryOperation &_op; - double _a; -public: - BindLeft(const BinaryOperation &op, double a) : _op(op), _a(a) {} - double eval(double b) const override { return _op.eval(_a, b); } -}; - -/** - * This class binds the second parameter of a binary operation to a - * numeric value, acting as a custom unary operation. - **/ -class BindRight : public CustomUnaryOperation -{ -private: - const BinaryOperation &_op; - double _b; -public: - BindRight(const BinaryOperation &op, double b) : _op(op), _b(b) {} - double eval(double a) const override { return _op.eval(a, _b); } -}; - -//----------------------------------------------------------------------------- - namespace operation { struct Neg : Op1<Neg> { static double f(double a); }; struct Not : Op1<Not> { static double f(double a); }; diff --git a/eval/src/vespa/eval/eval/simple_tensor_engine.cpp b/eval/src/vespa/eval/eval/simple_tensor_engine.cpp index 18c0f8b471a..d69715cab22 100644 --- a/eval/src/vespa/eval/eval/simple_tensor_engine.cpp +++ b/eval/src/vespa/eval/eval/simple_tensor_engine.cpp @@ -10,15 +10,6 @@ namespace eval { namespace { -struct Op2Aggr : Aggregator { - double res = 0.0; - const BinaryOperation &op; - Op2Aggr(const BinaryOperation &op_in) : op(op_in) {} - void first(double value) override { res = value; } - void next(double value) override { res = op.eval(res, value); } - double result() const override { return res; } -}; - const SimpleTensor &to_simple(const Tensor &tensor) { assert(&tensor.engine() == &SimpleTensorEngine::ref()); return static_cast<const SimpleTensor&>(tensor); @@ -117,27 +108,6 @@ SimpleTensorEngine::create(const TensorSpec &spec) const //----------------------------------------------------------------------------- -const Value & -SimpleTensorEngine::reduce(const eval::Tensor &tensor, const BinaryOperation &op, const std::vector<vespalib::string> &dimensions, Stash &stash) const -{ - Op2Aggr aggr(op); - return to_value(to_simple(tensor).reduce(aggr, dimensions), stash); -} - -const Value & -SimpleTensorEngine::map(const UnaryOperation &op, const eval::Tensor &a, Stash &stash) const -{ - return to_value(to_simple(a).map(UnaryOperationProxy(op)), stash); -} - -const Value & -SimpleTensorEngine::apply(const BinaryOperation &op, const eval::Tensor &a, const eval::Tensor &b, Stash &stash) const -{ - return to_value(SimpleTensor::join(to_simple(a), to_simple(b), BinaryOperationProxy(op)), stash); -} - -//----------------------------------------------------------------------------- - void SimpleTensorEngine::encode(const Value &value, nbostream &output, Stash &stash) const { @@ -155,12 +125,18 @@ SimpleTensorEngine::decode(nbostream &input, Stash &stash) const const Value & SimpleTensorEngine::map(const Value &a, map_fun_t function, Stash &stash) const { + if (a.is_double()) { + return stash.create<DoubleValue>(function(a.as_double())); + } return to_value(to_simple(a, stash).map(function), stash); } const Value & SimpleTensorEngine::join(const Value &a, const Value &b, join_fun_t function, Stash &stash) const { + if (a.is_double() && b.is_double()) { + return stash.create<DoubleValue>(function(a.as_double(), b.as_double())); + } return to_value(SimpleTensor::join(to_simple(a, stash), to_simple(b, stash), function), stash); } diff --git a/eval/src/vespa/eval/eval/simple_tensor_engine.h b/eval/src/vespa/eval/eval/simple_tensor_engine.h index aebb11036ff..bc6d0166bd1 100644 --- a/eval/src/vespa/eval/eval/simple_tensor_engine.h +++ b/eval/src/vespa/eval/eval/simple_tensor_engine.h @@ -25,9 +25,6 @@ public: TensorSpec to_spec(const Tensor &tensor) const override; std::unique_ptr<Tensor> create(const TensorSpec &spec) const override; - const Value &reduce(const Tensor &tensor, const BinaryOperation &op, const std::vector<vespalib::string> &dimensions, Stash &stash) const override; - const Value &map(const UnaryOperation &op, const Tensor &a, Stash &stash) const override; - const Value &apply(const BinaryOperation &op, const Tensor &a, const Tensor &b, Stash &stash) const override; void encode(const Value &value, nbostream &output, Stash &stash) const override; const Value &decode(nbostream &input, Stash &stash) const override; diff --git a/eval/src/vespa/eval/eval/tensor_engine.h b/eval/src/vespa/eval/eval/tensor_engine.h index ef269d934d5..d33c1ba0ed2 100644 --- a/eval/src/vespa/eval/eval/tensor_engine.h +++ b/eval/src/vespa/eval/eval/tensor_engine.h @@ -20,8 +20,6 @@ namespace eval { class Value; class Tensor; class TensorSpec; -struct UnaryOperation; -struct BinaryOperation; /** * Top-level API for a tensor implementation. All Tensor operations @@ -40,8 +38,6 @@ struct TensorEngine using Value = eval::Value; using map_fun_t = double (*)(double); using join_fun_t = double (*)(double, double); - using BinaryOperation = eval::BinaryOperation; - using UnaryOperation = eval::UnaryOperation; using Aggr = eval::Aggr; virtual ValueType type_of(const Tensor &tensor) const = 0; @@ -52,9 +48,6 @@ struct TensorEngine virtual TensorFunction::UP compile(tensor_function::Node_UP expr) const { return std::move(expr); } virtual std::unique_ptr<Tensor> create(const TensorSpec &spec) const = 0; - virtual const Value &reduce(const Tensor &tensor, const BinaryOperation &op, const std::vector<vespalib::string> &dimensions, Stash &stash) const = 0; - virtual const Value &map(const UnaryOperation &op, const Tensor &a, Stash &stash) const = 0; - virtual const Value &apply(const BinaryOperation &op, const Tensor &a, const Tensor &b, Stash &stash) const = 0; // havardpe: new API, WIP virtual void encode(const Value &value, nbostream &output, Stash &stash) const = 0; diff --git a/eval/src/vespa/eval/eval/test/tensor_conformance.cpp b/eval/src/vespa/eval/eval/test/tensor_conformance.cpp index ebfd3c11634..fc6a5006e82 100644 --- a/eval/src/vespa/eval/eval/test/tensor_conformance.cpp +++ b/eval/src/vespa/eval/eval/test/tensor_conformance.cpp @@ -173,39 +173,6 @@ const Value &make_value(const TensorEngine &engine, const TensorSpec &spec, Stas return stash.create<TensorValue>(engine.create(spec)); } -// evaluate tensor reduce operation using tensor engine immediate api -struct ImmediateReduceOld : Eval { - const BinaryOperation &op; - std::vector<vespalib::string> dimensions; - ImmediateReduceOld(const BinaryOperation &op_in) : op(op_in), dimensions() {} - ImmediateReduceOld(const BinaryOperation &op_in, const vespalib::string &dimension) - : op(op_in), dimensions({dimension}) {} - Result eval(const TensorEngine &engine, const TensorSpec &a) const override { - Stash stash; - return Result(engine.reduce(*engine.create(a), op, dimensions, stash)); - } -}; - -// evaluate tensor map operation using tensor engine immediate api -struct ImmediateMapOld : Eval { - const UnaryOperation &op; - ImmediateMapOld(const UnaryOperation &op_in) : op(op_in) {} - Result eval(const TensorEngine &engine, const TensorSpec &a) const override { - Stash stash; - return Result(engine.map(op, *engine.create(a), stash)); - } -}; - -// evaluate tensor apply operation using tensor engine immediate api -struct ImmediateApplyOld : Eval { - const BinaryOperation &op; - ImmediateApplyOld(const BinaryOperation &op_in) : op(op_in) {} - Result eval(const TensorEngine &engine, const TensorSpec &a, const TensorSpec &b) const override { - Stash stash; - return Result(engine.apply(op, *engine.create(a), *engine.create(b), stash)); - } -}; - //----------------------------------------------------------------------------- // evaluate tensor reduce operation using tensor engine immediate api @@ -234,7 +201,7 @@ struct ImmediateMap : Eval { } }; -// evaluate tensor map operation using tensor engine immediate api +// evaluate tensor join operation using tensor engine immediate api struct ImmediateJoin : Eval { using fun_t = double (*)(double, double); fun_t function; @@ -491,44 +458,6 @@ struct TestContext { TEST_DO(verify_result(eval.eval(engine, a), expect)); } - void test_reduce_op(Aggr aggr, const BinaryOperation &op, const Sequence &seq) { - std::vector<Layout> layouts = { - {x(3)}, - {x(3),y(5)}, - {x(3),y(5),z(7)}, - {x({"a","b","c"})}, - {x({"a","b","c"}),y({"foo","bar"})}, - {x({"a","b","c"}),y({"foo","bar"}),z({"i","j","k","l"})}, - {x(3),y({"foo", "bar"}),z(7)}, - {x({"a","b","c"}),y(5),z({"i","j","k","l"})} - }; - for (const Layout &layout: layouts) { - TensorSpec input = spec(layout, seq); - for (const Domain &domain: layout) { - Eval::Result expect = ImmediateReduceOld(op, domain.dimension).eval(ref_engine, input); - TEST_STATE(make_string("shape: %s, reduce dimension: %s", - infer_type(layout).c_str(), domain.dimension.c_str()).c_str()); - vespalib::string expr = make_string("reduce(a,%s,%s)", - AggrNames::name_of(aggr)->c_str(), domain.dimension.c_str()); - TEST_DO(verify_reduce_result(Expr_T(expr), input, expect)); - TEST_DO(verify_reduce_result(ImmediateReduceOld(op, domain.dimension), input, expect)); - TEST_DO(verify_reduce_result(ImmediateReduce(aggr, domain.dimension), input, expect)); - TEST_DO(verify_reduce_result(RetainedReduce(aggr, domain.dimension), input, expect)); - } - { - Eval::Result expect = ImmediateReduceOld(op).eval(ref_engine, input); - TEST_STATE(make_string("shape: %s, reduce all dimensions", - infer_type(layout).c_str()).c_str()); - vespalib::string expr = make_string("reduce(a,%s)", - AggrNames::name_of(aggr)->c_str()); - TEST_DO(verify_reduce_result(Expr_T(expr), input, expect)); - TEST_DO(verify_reduce_result(ImmediateReduceOld(op), input, expect)); - TEST_DO(verify_reduce_result(ImmediateReduce(aggr), input, expect)); - TEST_DO(verify_reduce_result(RetainedReduce(aggr), input, expect)); - } - } - } - void test_reduce_op(Aggr aggr, const Sequence &seq) { std::vector<Layout> layouts = { {x(3)}, @@ -567,10 +496,10 @@ struct TestContext { void test_tensor_reduce() { TEST_DO(test_reduce_op(Aggr::AVG, N())); TEST_DO(test_reduce_op(Aggr::COUNT, N())); - TEST_DO(test_reduce_op(Aggr::PROD, operation::Mul(), Sigmoid(N()))); - TEST_DO(test_reduce_op(Aggr::SUM, operation::Add(), N())); - TEST_DO(test_reduce_op(Aggr::MAX, operation::Max(), N())); - TEST_DO(test_reduce_op(Aggr::MIN, operation::Min(), N())); + TEST_DO(test_reduce_op(Aggr::PROD, Sigmoid(N()))); + TEST_DO(test_reduce_op(Aggr::SUM, N())); + TEST_DO(test_reduce_op(Aggr::MAX, N())); + TEST_DO(test_reduce_op(Aggr::MIN, N())); } //------------------------------------------------------------------------- @@ -593,7 +522,6 @@ struct TestContext { } void test_map_op(const vespalib::string &expr, const UnaryOperation &op, const Sequence &seq) { - TEST_DO(test_map_op(ImmediateMapOld(op), op, seq)); TEST_DO(test_map_op(ImmediateMap(op.get_f()), op, seq)); TEST_DO(test_map_op(RetainedMap(op.get_f()), op, seq)); TEST_DO(test_map_op(Expr_T(expr), op, seq)); @@ -861,7 +789,7 @@ struct TestContext { TEST_STATE(make_string("lhs shape: %s, rhs shape: %s", lhs_input.type().c_str(), rhs_input.type().c_str()).c_str()); - Eval::Result expect = ImmediateApplyOld(op).eval(ref_engine, lhs_input, rhs_input); + Eval::Result expect = ImmediateJoin(op.get_f()).eval(ref_engine, lhs_input, rhs_input); TEST_DO(verify_result(safe(eval).eval(engine, lhs_input, rhs_input), expect)); } TEST_DO(test_fixed_sparse_cases_apply_op(eval, op)); @@ -869,7 +797,6 @@ struct TestContext { } void test_apply_op(const vespalib::string &expr, const BinaryOperation &op, const Sequence &seq) { - TEST_DO(test_apply_op(ImmediateApplyOld(op), op, seq)); TEST_DO(test_apply_op(ImmediateJoin(op.get_f()), op, seq)); TEST_DO(test_apply_op(RetainedJoin(op.get_f()), op, seq)); TEST_DO(test_apply_op(Expr_TT(expr), op, seq)); diff --git a/eval/src/vespa/eval/eval/value.cpp b/eval/src/vespa/eval/eval/value.cpp index d5111187157..e11d5f01985 100644 --- a/eval/src/vespa/eval/eval/value.cpp +++ b/eval/src/vespa/eval/eval/value.cpp @@ -7,18 +7,6 @@ namespace vespalib { namespace eval { -const Value & -Value::apply(const UnaryOperation &, Stash &stash) const -{ - return stash.create<ErrorValue>(); -} - -const Value & -Value::apply(const BinaryOperation &, const Value &, Stash &stash) const -{ - return stash.create<ErrorValue>(); -} - ErrorValue ErrorValue::instance; bool @@ -27,22 +15,6 @@ TensorValue::equal(const Value &rhs) const return (rhs.is_tensor() && _tensor->engine().equal(*_tensor, *rhs.as_tensor())); } -const Value & -TensorValue::apply(const UnaryOperation &op, Stash &stash) const -{ - return _tensor->engine().map(op, *_tensor, stash); -} - -const Value & -TensorValue::apply(const BinaryOperation &op, const Value &rhs, Stash &stash) const -{ - const Tensor *other = rhs.as_tensor(); - if ((other == nullptr) || (&other->engine() != &_tensor->engine())) { - return stash.create<ErrorValue>(); - } - return _tensor->engine().apply(op, *_tensor, *other, stash); -} - ValueType TensorValue::type() const { diff --git a/eval/src/vespa/eval/eval/value.h b/eval/src/vespa/eval/eval/value.h index 45b4f59ecd5..42dddeef188 100644 --- a/eval/src/vespa/eval/eval/value.h +++ b/eval/src/vespa/eval/eval/value.h @@ -15,12 +15,8 @@ class Tensor; constexpr double error_value = 31212.0; -struct UnaryOperation; -struct BinaryOperation; - /** - * An abstract Value. Calculation using abstract values should be done - * using the perform function on the appropriate Operation. + * An abstract Value. **/ struct Value { typedef std::unique_ptr<Value> UP; @@ -32,8 +28,6 @@ struct Value { virtual bool as_bool() const { return false; } virtual const Tensor *as_tensor() const { return nullptr; } virtual bool equal(const Value &rhs) const = 0; - virtual const Value &apply(const UnaryOperation &op, Stash &stash) const; - virtual const Value &apply(const BinaryOperation &op, const Value &rhs, Stash &stash) const; virtual ValueType type() const = 0; virtual ~Value() {} }; @@ -72,8 +66,6 @@ public: bool is_tensor() const override { return true; } const Tensor *as_tensor() const override { return _tensor; } bool equal(const Value &rhs) const override; - const Value &apply(const UnaryOperation &op, Stash &stash) const override; - const Value &apply(const BinaryOperation &op, const Value &rhs, Stash &stash) const override; ValueType type() const override; }; diff --git a/eval/src/vespa/eval/tensor/default_tensor_engine.cpp b/eval/src/vespa/eval/tensor/default_tensor_engine.cpp index 771a457509c..6a536497bdd 100644 --- a/eval/src/vespa/eval/tensor/default_tensor_engine.cpp +++ b/eval/src/vespa/eval/tensor/default_tensor_engine.cpp @@ -130,12 +130,6 @@ DefaultTensorEngine::compile(eval::tensor_function::Node_UP expr) const return DenseTensorFunctionCompiler::compile(std::move(expr)); } -struct IsAddOperation : public eval::DefaultOperationVisitor { - bool result = false; - void visitDefault(const eval::Operation &) override {} - void visit(const eval::operation::Add &) override { result = true; } -}; - std::unique_ptr<eval::Tensor> DefaultTensorEngine::create(const TensorSpec &spec) const { @@ -183,41 +177,6 @@ DefaultTensorEngine::create(const TensorSpec &spec) const } } -const eval::Value & -DefaultTensorEngine::reduce(const Tensor &tensor, const BinaryOperation &op, const std::vector<vespalib::string> &dimensions, Stash &stash) const -{ - assert(&tensor.engine() == this); - const tensor::Tensor &my_tensor = static_cast<const tensor::Tensor &>(tensor); - if (!tensor::Tensor::supported({my_tensor.getType()})) { - return to_default(simple_engine().reduce(to_simple(my_tensor, stash), op, dimensions, stash), stash); - } - IsAddOperation check; - op.accept(check); - tensor::Tensor::UP result; - if (check.result) { - if (dimensions.empty()) { // sum - return stash.create<eval::DoubleValue>(my_tensor.sum()); - } else { // dimension sum - for (const auto &dimension: dimensions) { - if (result) { - result = result->sum(dimension); - } else { - result = my_tensor.sum(dimension); - } - } - } - } else { - result = my_tensor.reduce(op, dimensions); - } - return to_value(std::move(result), stash); -} - -struct CellFunctionOpAdapter : tensor::CellFunction { - const eval::UnaryOperation &op; - CellFunctionOpAdapter(const eval::UnaryOperation &op_in) : op(op_in) {} - virtual double apply(double value) const override { return op.eval(value); } -}; - struct CellFunctionFunAdapter : tensor::CellFunction { map_fun_t fun; CellFunctionFunAdapter(map_fun_t fun_in) : fun(fun_in) {} @@ -238,69 +197,6 @@ struct CellFunctionBindRightAdapter : tensor::CellFunction { virtual double apply(double a) const override { return fun(a, b); } }; -const eval::Value & -DefaultTensorEngine::map(const UnaryOperation &op, const Tensor &a, Stash &stash) const -{ - assert(&a.engine() == this); - const tensor::Tensor &my_a = static_cast<const tensor::Tensor &>(a); - if (!tensor::Tensor::supported({my_a.getType()})) { - return to_default(simple_engine().map(op, to_simple(my_a, stash), stash), stash); - } - CellFunctionOpAdapter cell_function(op); - return to_value(my_a.apply(cell_function), stash); -} - -struct TensorOperationOverride : eval::DefaultOperationVisitor { - const tensor::Tensor &lhs; - const tensor::Tensor &rhs; - tensor::Tensor::UP result; - TensorOperationOverride(const tensor::Tensor &lhs_in, - const tensor::Tensor &rhs_in) - : lhs(lhs_in), rhs(rhs_in), result() {} - virtual void visitDefault(const eval::Operation &op) override { - // empty result indicates error - const eval::BinaryOperation *binaryOp = - dynamic_cast<const eval::BinaryOperation *>(&op); - if (binaryOp) { - result = lhs.apply(*binaryOp, rhs); - } - } - virtual void visit(const eval::operation::Add &) override { - result = lhs.add(rhs); - } - virtual void visit(const eval::operation::Sub &) override { - result = lhs.subtract(rhs); - } - virtual void visit(const eval::operation::Mul &) override { - if (lhs.getType() == rhs.getType()) { - result = lhs.match(rhs); - } else { - result = lhs.multiply(rhs); - } - } - virtual void visit(const eval::operation::Min &) override { - result = lhs.min(rhs); - } - virtual void visit(const eval::operation::Max &) override { - result = lhs.max(rhs); - } -}; - -const eval::Value & -DefaultTensorEngine::apply(const BinaryOperation &op, const Tensor &a, const Tensor &b, Stash &stash) const -{ - assert(&a.engine() == this); - assert(&b.engine() == this); - const tensor::Tensor &my_a = static_cast<const tensor::Tensor &>(a); - const tensor::Tensor &my_b = static_cast<const tensor::Tensor &>(b); - if (!tensor::Tensor::supported({my_a.getType(), my_b.getType()})) { - return to_default(simple_engine().apply(op, to_simple(my_a, stash), to_simple(my_b, stash), stash), stash); - } - TensorOperationOverride tensor_override(my_a, my_b); - op.accept(tensor_override); - return to_value(std::move(tensor_override.result), stash); -} - //----------------------------------------------------------------------------- void @@ -371,8 +267,12 @@ DefaultTensorEngine::join(const Value &a, const Value &b, join_fun_t function, S if (!tensor::Tensor::supported({my_a.getType(), my_b.getType()})) { return fallback_join(a, b, function, stash); } - if ((function == eval::operation::Mul::f) && (my_a.getType() == my_b.getType())) { - return to_value(my_a.match(my_b), stash); + if (function == eval::operation::Mul::f) { + if (my_a.getType() == my_b.getType()) { + return to_value(my_a.match(my_b), stash); + } else { + return to_value(my_a.multiply(my_b), stash); + } } else { return to_value(my_a.join(function, my_b), stash); } diff --git a/eval/src/vespa/eval/tensor/default_tensor_engine.h b/eval/src/vespa/eval/tensor/default_tensor_engine.h index b4bd717525b..abdce6edb62 100644 --- a/eval/src/vespa/eval/tensor/default_tensor_engine.h +++ b/eval/src/vespa/eval/tensor/default_tensor_engine.h @@ -27,9 +27,6 @@ public: virtual eval::TensorFunction::UP compile(eval::tensor_function::Node_UP expr) const override; std::unique_ptr<Tensor> create(const TensorSpec &spec) const override; - const Value &reduce(const Tensor &tensor, const BinaryOperation &op, const std::vector<vespalib::string> &dimensions, Stash &stash) const override; - const Value &map(const UnaryOperation &op, const Tensor &a, Stash &stash) const override; - const Value &apply(const BinaryOperation &op, const Tensor &a, const Tensor &b, Stash &stash) const override; void encode(const Value &value, nbostream &output, Stash &stash) const override; const Value &decode(nbostream &input, Stash &stash) const override; diff --git a/searchlib/src/vespa/searchlib/docstore/logdatastore.cpp b/searchlib/src/vespa/searchlib/docstore/logdatastore.cpp index 4e3178fcdb1..1850db7b02b 100644 --- a/searchlib/src/vespa/searchlib/docstore/logdatastore.cpp +++ b/searchlib/src/vespa/searchlib/docstore/logdatastore.cpp @@ -356,6 +356,7 @@ double LogDataStore::getMaxBucketSpread() const { double maxSpread(1.0); + LockGuard guard(_updateLock); for (const FileChunk::UP & fc : _fileChunks) { if (fc) { if (_bucketizer && fc->frozen()) { @@ -1000,6 +1001,7 @@ LogDataStore::computeNumberOfSignificantBucketIdBits(const IBucketizer & bucketi void LogDataStore::verify(bool reportOnly) const { + LockGuard guard(_updateLock); for (const FileChunk::UP & fc : _fileChunks) { if (fc) { fc->verify(reportOnly); @@ -1107,6 +1109,7 @@ double LogDataStore::getVisitCost() const { uint32_t totalChunks = 0; + LockGuard guard(_updateLock); for (auto &fc : _fileChunks) { totalChunks += fc->getNumChunks(); } |