From 5638fb88e06e835a03cc0d9c70725a37dacd2974 Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Wed, 30 Sep 2020 13:23:34 +0000 Subject: now needs less casting to DenseTensor --- .../vespa/eval/tensor/dense/dense_generic_join.hpp | 12 ++--- .../tensor/dense/dense_replace_type_function.cpp | 7 +-- .../vespa/eval/tensor/dense/dense_tensor_view.cpp | 59 +++++++--------------- .../vespa/eval/tensor/dense/dense_tensor_view.h | 4 +- eval/src/vespa/eval/tensor/dense/onnx_wrapper.cpp | 6 +-- 5 files changed, 28 insertions(+), 60 deletions(-) diff --git a/eval/src/vespa/eval/tensor/dense/dense_generic_join.hpp b/eval/src/vespa/eval/tensor/dense/dense_generic_join.hpp index cdc89b30fff..033ea3631be 100644 --- a/eval/src/vespa/eval/tensor/dense/dense_generic_join.hpp +++ b/eval/src/vespa/eval/tensor/dense/dense_generic_join.hpp @@ -53,14 +53,10 @@ template std::unique_ptr generic_join(const DenseTensorView &lhs, const Tensor &rhs, Function &&func) { - const DenseTensorView *view = dynamic_cast(&rhs); - if (view) { - DenseDimensionCombiner combiner(lhs.fast_type(), view->fast_type()); - TypedCells lhsCells = lhs.cellsRef(); - TypedCells rhsCells = view->cellsRef(); - return dispatch_2(lhsCells, rhsCells, combiner, std::move(func)); - } - return Tensor::UP(); + DenseDimensionCombiner combiner(lhs.fast_type(), rhs.type()); + TypedCells lhsCells = lhs.cells(); + TypedCells rhsCells = rhs.cells(); + return dispatch_2(lhsCells, rhsCells, combiner, std::move(func)); } } diff --git a/eval/src/vespa/eval/tensor/dense/dense_replace_type_function.cpp b/eval/src/vespa/eval/tensor/dense/dense_replace_type_function.cpp index e2dc2241555..6e4756dbe3c 100644 --- a/eval/src/vespa/eval/tensor/dense/dense_replace_type_function.cpp +++ b/eval/src/vespa/eval/tensor/dense/dense_replace_type_function.cpp @@ -15,14 +15,9 @@ using namespace eval::tensor_function; namespace { -TypedCells getCellsRef(const eval::Value &value) { - const DenseTensorView &denseTensor = static_cast(value); - return denseTensor.cellsRef(); -} - void my_replace_type_op(eval::InterpretedFunction::State &state, uint64_t param) { const ValueType *type = (const ValueType *)(param); - TypedCells cells = getCellsRef(state.peek(0)); + TypedCells cells = state.peek(0).cells(); state.pop_push(state.stash.create(*type, cells)); } diff --git a/eval/src/vespa/eval/tensor/dense/dense_tensor_view.cpp b/eval/src/vespa/eval/tensor/dense/dense_tensor_view.cpp index b845ccf93a5..9303488bbc5 100644 --- a/eval/src/vespa/eval/tensor/dense/dense_tensor_view.cpp +++ b/eval/src/vespa/eval/tensor/dense/dense_tensor_view.cpp @@ -39,48 +39,34 @@ dimensionsAsString(const eval::ValueType &type) return oss.str(); } -size_t -calcCellsSize(const eval::ValueType &type) -{ - size_t cellsSize = 1; - for (const auto &dim : type.dimensions()) { - cellsSize *= dim.size; - } - return cellsSize; -} - - void -checkCellsSize(const DenseTensorView &arg) +checkCellsSize(const eval::ValueType &type, TypedCells cells) { - auto cellsSize = calcCellsSize(arg.fast_type()); - if (arg.cellsRef().size != cellsSize) { + auto cellsSize = type.dense_subspace_size(); + if (cells.size != cellsSize) { throw IllegalStateException(make_string("wrong cell size, " "expected=%zu, " "actual=%zu", cellsSize, - arg.cellsRef().size)); + cells.size)); } } void -checkDimensions(const DenseTensorView &lhs, const DenseTensorView &rhs, +checkDimensions(const eval::ValueType &lhs, const eval::ValueType &rhs, vespalib::stringref operation) { - if (lhs.fast_type().dimensions() != rhs.fast_type().dimensions()) { + if (lhs.dimensions() != rhs.dimensions()) { throw IllegalStateException(make_string("mismatching dimensions for " "dense tensor %s, " "lhs dimensions = '%s', " "rhs dimensions = '%s'", operation.data(), - dimensionsAsString(lhs.fast_type()).c_str(), - dimensionsAsString(rhs.fast_type()).c_str())); + dimensionsAsString(lhs).c_str(), + dimensionsAsString(rhs).c_str())); } - checkCellsSize(lhs); - checkCellsSize(rhs); } - /* * Join the cells of two tensors. * @@ -122,28 +108,20 @@ struct CallJoin } }; -template -Tensor::UP -joinDenseTensors(const DenseTensorView &lhs, const DenseTensorView &rhs, - Function &&func) -{ - TypedCells lhsCells = lhs.cellsRef(); - TypedCells rhsCells = rhs.cellsRef(); - return dispatch_2(lhsCells, rhsCells, lhs.fast_type().dimensions(), std::move(func)); -} - template Tensor::UP joinDenseTensors(const DenseTensorView &lhs, const Tensor &rhs, vespalib::stringref operation, Function &&func) { - auto view = dynamic_cast(&rhs); - if (view) { - checkDimensions(lhs, *view, operation); - return joinDenseTensors(lhs, *view, func); - } - return Tensor::UP(); + const auto & lhs_type = lhs.fast_type(); + const auto & rhs_type = rhs.type(); + TypedCells lhs_cells = lhs.cells(); + TypedCells rhs_cells = rhs.cells(); + checkDimensions(lhs_type, rhs_type, operation); + checkCellsSize(lhs_type, lhs_cells); + checkCellsSize(rhs_type, rhs_cells); + return dispatch_2(lhs_cells, rhs_cells, lhs_type.dimensions(), std::move(func)); } bool sameCells(TypedCells lhs, TypedCells rhs) @@ -215,9 +193,8 @@ DenseTensorView::apply(const CellFunction &func) const bool DenseTensorView::equals(const Tensor &arg) const { - auto view = dynamic_cast(&arg); - if (view) { - return *this == *view; + if (fast_type() == arg.type()) { + return sameCells(cells(), arg.cells()); } return false; } diff --git a/eval/src/vespa/eval/tensor/dense/dense_tensor_view.h b/eval/src/vespa/eval/tensor/dense/dense_tensor_view.h index 2cea3c855a6..e192b73add1 100644 --- a/eval/src/vespa/eval/tensor/dense/dense_tensor_view.h +++ b/eval/src/vespa/eval/tensor/dense/dense_tensor_view.h @@ -52,10 +52,10 @@ public: } template static ConstArrayRef typify_cells(const eval::Value &self) { - return static_cast(self).cellsRef().typify(); + return self.cells().typify(); } template static ConstArrayRef unsafe_typify_cells(const eval::Value &self) { - return static_cast(self).cellsRef().unsafe_typify(); + return self.cells().unsafe_typify(); } protected: explicit DenseTensorView(const eval::ValueType &type_in) diff --git a/eval/src/vespa/eval/tensor/dense/onnx_wrapper.cpp b/eval/src/vespa/eval/tensor/dense/onnx_wrapper.cpp index b581ce787e3..5db533a4655 100644 --- a/eval/src/vespa/eval/tensor/dense/onnx_wrapper.cpp +++ b/eval/src/vespa/eval/tensor/dense/onnx_wrapper.cpp @@ -346,7 +346,7 @@ template void Onnx::EvalContext::adapt_param(EvalContext &self, size_t idx, const eval::Value ¶m) { - const auto &cells_ref = static_cast(param).cellsRef(); + const auto &cells_ref = param.cells(); auto cells = unconstify(cells_ref.typify()); const auto &sizes = self._wire_info.onnx_inputs[idx].dimensions; self._param_values[idx] = Ort::Value::CreateTensor(self._cpu_memory, cells.begin(), cells.size(), sizes.data(), sizes.size()); @@ -356,7 +356,7 @@ template void Onnx::EvalContext::convert_param(EvalContext &self, size_t idx, const eval::Value ¶m) { - auto cells = static_cast(param).cellsRef().typify(); + auto cells = param.cells().typify(); size_t n = cells.size(); const SRC *src = cells.begin(); DST *dst = self._param_values[idx].GetTensorMutableData(); @@ -369,7 +369,7 @@ template void Onnx::EvalContext::convert_result(EvalContext &self, size_t idx) { - const auto &cells_ref = static_cast(*self._results[idx]).cellsRef(); + const auto &cells_ref = (*self._results[idx]).cells(); auto cells = unconstify(cells_ref.typify()); size_t n = cells.size(); DST *dst = cells.begin(); -- cgit v1.2.3 From e3dc8d27a19e75323ac16322dc0e43a58c49dbae Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Fri, 2 Oct 2020 13:52:56 +0000 Subject: less casting to DenseTensorView --- .../dense_replace_type_function_test.cpp | 2 +- .../direct_dense_tensor_builder_test.cpp | 5 ++--- eval/src/tests/tensor/onnx_wrapper/onnx_wrapper_test.cpp | 6 +++--- eval/src/vespa/eval/tensor/default_tensor_engine.cpp | 3 +-- 4 files changed, 7 insertions(+), 9 deletions(-) diff --git a/eval/src/tests/tensor/dense_replace_type_function/dense_replace_type_function_test.cpp b/eval/src/tests/tensor/dense_replace_type_function/dense_replace_type_function_test.cpp index 732fc9c3e69..9ebcb0ec77c 100644 --- a/eval/src/tests/tensor/dense_replace_type_function/dense_replace_type_function_test.cpp +++ b/eval/src/tests/tensor/dense_replace_type_function/dense_replace_type_function_test.cpp @@ -16,7 +16,7 @@ using namespace vespalib; const TensorEngine &engine = DefaultTensorEngine::ref(); TypedCells getCellsRef(const eval::Value &value) { - return static_cast(value).cellsRef(); + return value.cells(); } struct ChildMock : Leaf { diff --git a/eval/src/tests/tensor/direct_dense_tensor_builder/direct_dense_tensor_builder_test.cpp b/eval/src/tests/tensor/direct_dense_tensor_builder/direct_dense_tensor_builder_test.cpp index 75e9c7868ce..4c96145862c 100644 --- a/eval/src/tests/tensor/direct_dense_tensor_builder/direct_dense_tensor_builder_test.cpp +++ b/eval/src/tests/tensor/direct_dense_tensor_builder/direct_dense_tensor_builder_test.cpp @@ -29,9 +29,8 @@ void assertTensor(const vespalib::string &type_spec, const std::vector &expCells, const Tensor &tensor) { - const DenseTensorView &realTensor = dynamic_cast(tensor); - EXPECT_EQUAL(ValueType::from_spec(type_spec), realTensor.type()); - EXPECT_EQUAL(expCells, dispatch_1(realTensor.cellsRef())); + EXPECT_EQUAL(ValueType::from_spec(type_spec), tensor.type()); + EXPECT_EQUAL(expCells, dispatch_1(tensor.cells())); } void assertTensorSpec(const TensorSpec &expSpec, const Tensor &tensor) { diff --git a/eval/src/tests/tensor/onnx_wrapper/onnx_wrapper_test.cpp b/eval/src/tests/tensor/onnx_wrapper/onnx_wrapper_test.cpp index d1d8bc796ba..fce7ccc6411 100644 --- a/eval/src/tests/tensor/onnx_wrapper/onnx_wrapper_test.cpp +++ b/eval/src/tests/tensor/onnx_wrapper/onnx_wrapper_test.cpp @@ -162,7 +162,7 @@ TEST(OnnxTest, simple_onnx_model_can_be_evaluated) ctx.bind_param(1, attribute); ctx.bind_param(2, bias); ctx.eval(); - auto cells = static_cast(output).cellsRef(); + auto cells = output.cells(); EXPECT_EQ(cells.type, ValueType::CellType::FLOAT); EXPECT_EQ(cells.size, 1); EXPECT_EQ(GetCell::from(cells, 0), 79.0); @@ -208,7 +208,7 @@ TEST(OnnxTest, dynamic_onnx_model_can_be_evaluated) ctx.bind_param(1, attribute); ctx.bind_param(2, bias); ctx.eval(); - auto cells = static_cast(output).cellsRef(); + auto cells = output.cells(); EXPECT_EQ(cells.type, ValueType::CellType::FLOAT); EXPECT_EQ(cells.size, 1); EXPECT_EQ(GetCell::from(cells, 0), 79.0); @@ -254,7 +254,7 @@ TEST(OnnxTest, int_types_onnx_model_can_be_evaluated) ctx.bind_param(1, attribute); ctx.bind_param(2, bias); ctx.eval(); - auto cells = static_cast(output).cellsRef(); + auto cells = output.cells(); EXPECT_EQ(cells.type, ValueType::CellType::DOUBLE); EXPECT_EQ(cells.size, 1); EXPECT_EQ(GetCell::from(cells, 0), 79.0); diff --git a/eval/src/vespa/eval/tensor/default_tensor_engine.cpp b/eval/src/vespa/eval/tensor/default_tensor_engine.cpp index 7d4bff21380..11c1ce74eca 100644 --- a/eval/src/vespa/eval/tensor/default_tensor_engine.cpp +++ b/eval/src/vespa/eval/tensor/default_tensor_engine.cpp @@ -445,8 +445,7 @@ struct CallAppendVector { template void append_vector(OCT *&pos, const Value &value) { if (auto tensor = value.as_tensor()) { - const DenseTensorView *view = static_cast(tensor); - dispatch_1 >(view->cellsRef(), pos); + dispatch_1 >(tensor->cells(), pos); } else { *pos++ = value.as_double(); } -- cgit v1.2.3 From 344029ef873291cc03e30d0a0ad2e9f80ba48c6d Mon Sep 17 00:00:00 2001 From: Ola Aunrønning Date: Tue, 6 Oct 2020 14:33:27 +0200 Subject: Enable policy for listing billing information in public system --- .../main/java/com/yahoo/vespa/hosted/controller/api/role/Policy.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Policy.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Policy.java index 548ad0af484..8c21e6facec 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Policy.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/role/Policy.java @@ -166,7 +166,7 @@ enum Policy { /** Read the generated bills */ billingInformationRead(Privilege.grant(Action.read) .on(PathGroup.billingList) - .in(SystemName.PublicCd)), + .in(SystemName.PublicCd, SystemName.Public)), /** Invoice management */ hostedAccountant(Privilege.grant(Action.all()) -- cgit v1.2.3 From d093fdab1ae901e03a7aa77747af996dcc4d44f4 Mon Sep 17 00:00:00 2001 From: Bjørn Christian Seime Date: Tue, 6 Oct 2020 15:17:43 +0200 Subject: Don't use request headers for remote address/port in hosted Vespa Control which headers are used for remote address/port in access log through config model. --- .../vespa/model/admin/LogserverContainer.java | 4 +- .../ClusterControllerContainer.java | 2 +- .../admin/metricsproxy/MetricsProxyContainer.java | 2 +- .../model/builder/xml/dom/DomAdminV4Builder.java | 4 +- .../model/container/ApplicationContainer.java | 2 +- .../com/yahoo/vespa/model/container/Container.java | 10 ++--- .../model/container/http/JettyHttpServer.java | 21 +++++++-- .../container/http/xml/JettyHttpServerBuilder.java | 2 +- .../model/container/xml/ContainerModelBuilder.java | 2 +- jdisc_http_service/abi-spec.json | 45 ++++++++++++++++++- .../http/server/jetty/AccessLogRequestLog.java | 50 +++++++++++----------- .../jdisc/http/server/jetty/JettyHttpServer.java | 2 +- .../jdisc.http.jdisc.http.server.def | 8 +++- .../http/server/jetty/AccessLogRequestLogTest.java | 22 +++++++--- 14 files changed, 124 insertions(+), 52 deletions(-) diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/LogserverContainer.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/LogserverContainer.java index e94fa9bf040..b3ad8db0df1 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/LogserverContainer.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/LogserverContainer.java @@ -12,8 +12,8 @@ import com.yahoo.vespa.model.container.component.AccessLogComponent; */ public class LogserverContainer extends Container { - public LogserverContainer(AbstractConfigProducer parent) { - super(parent, "" + 0, 0); + public LogserverContainer(AbstractConfigProducer parent, boolean isHostedVespa) { + super(parent, "" + 0, 0, isHostedVespa); addComponent(new AccessLogComponent(AccessLogComponent.AccessLogType.jsonAccessLog, ((LogserverContainerCluster) parent).getName(), true)); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainer.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainer.java index 5b3e4e1479e..8bd4506aedc 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainer.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainer.java @@ -34,7 +34,7 @@ public class ClusterControllerContainer extends Container implements private final Set bundles = new TreeSet<>(); public ClusterControllerContainer(AbstractConfigProducer parent, int index, boolean runStandaloneZooKeeper, boolean isHosted) { - super(parent, "" + index, index); + super(parent, "" + index, index, isHosted); addHandler("clustercontroller-status", "com.yahoo.vespa.clustercontroller.apps.clustercontroller.StatusHandler", "/clustercontroller-status/*"); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainer.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainer.java index fccacc3210d..2e89d885ec9 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainer.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainer.java @@ -40,7 +40,7 @@ public class MetricsProxyContainer extends Container implements final boolean isHostedVespa; public MetricsProxyContainer(AbstractConfigProducer parent, String hostname, int index, boolean isHostedVespa) { - super(parent, hostname, index); + super(parent, hostname, index, isHostedVespa); this.isHostedVespa = isHostedVespa; setProp("clustertype", "admin"); setProp("index", String.valueOf(index)); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV4Builder.java b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV4Builder.java index a47c9fdb15b..12d5e8d32ed 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV4Builder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/builder/xml/dom/DomAdminV4Builder.java @@ -6,7 +6,6 @@ import com.yahoo.config.model.ConfigModelContext; import com.yahoo.config.model.api.ConfigServerSpec; import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.provision.ClusterSpec; -import java.util.logging.Level; import com.yahoo.vespa.model.HostResource; import com.yahoo.vespa.model.HostSystem; import com.yahoo.vespa.model.admin.Admin; @@ -22,6 +21,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.Optional; +import java.util.logging.Level; import java.util.stream.Collectors; /** @@ -105,7 +105,7 @@ public class DomAdminV4Builder extends DomAdminBuilderBase { ContainerModel logserverClusterModel = new ContainerModel(context.withParent(admin).withId(logServerCluster.getSubId())); logserverClusterModel.setCluster(logServerCluster); - LogserverContainer container = new LogserverContainer(logServerCluster); + LogserverContainer container = new LogserverContainer(logServerCluster, deployState.isHosted()); container.setHostResource(hostResource); container.initService(deployState.getDeployLogger()); logServerCluster.addContainer(container); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainer.java b/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainer.java index 232552ea4ce..cb8abb919ac 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainer.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainer.java @@ -26,7 +26,7 @@ public final class ApplicationContainer extends Container implements QrStartConf } public ApplicationContainer(AbstractConfigProducer parent, String name, boolean retired, int index, boolean isHostedVespa) { - super(parent, name, retired, index); + super(parent, name, retired, index, isHostedVespa); this.isHostedVespa = isHostedVespa; addComponent(getFS4ResourcePool()); // TODO Remove when FS4 based search protocol is gone diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/Container.java b/config-model/src/main/java/com/yahoo/vespa/model/container/Container.java index c6de198c06a..536928bbc9d 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/Container.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/Container.java @@ -73,19 +73,19 @@ public abstract class Container extends AbstractService implements private final ComponentGroup> handlers = new ComponentGroup<>(this, "handler"); private final ComponentGroup> components = new ComponentGroup<>(this, "components"); - private final JettyHttpServer defaultHttpServer = new JettyHttpServer(new ComponentId("DefaultHttpServer")); + private final JettyHttpServer defaultHttpServer; - protected Container(AbstractConfigProducer parent, String name, int index) { - this(parent, name, false, index); + protected Container(AbstractConfigProducer parent, String name, int index, boolean isHostedVespa) { + this(parent, name, false, index, isHostedVespa); } - protected Container(AbstractConfigProducer parent, String name, boolean retired, int index) { + protected Container(AbstractConfigProducer parent, String name, boolean retired, int index, boolean isHostedVespa) { super(parent, name); this.name = name; this.parent = parent; this.retired = retired; this.index = index; - + this.defaultHttpServer = new JettyHttpServer(new ComponentId("DefaultHttpServer"), isHostedVespa); if (getHttp() == null) { addChild(defaultHttpServer); } diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/http/JettyHttpServer.java b/config-model/src/main/java/com/yahoo/vespa/model/container/http/JettyHttpServer.java index 1350e105406..98fde2e7859 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/http/JettyHttpServer.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/http/JettyHttpServer.java @@ -15,19 +15,21 @@ import java.util.List; import static com.yahoo.component.ComponentSpecification.fromString; /** - * @author Einar M R Rosenvinge - * @since 5.16.0 + * @author Einar M R Rosenvinge + * @author bjorncs */ public class JettyHttpServer extends SimpleComponent implements ServerConfig.Producer { - private List connectorFactories = new ArrayList<>(); + private final boolean isHostedVespa; + private final List connectorFactories = new ArrayList<>(); - public JettyHttpServer(ComponentId id) { + public JettyHttpServer(ComponentId id, boolean isHostedVespa) { super(new ComponentModel( new BundleInstantiationSpecification(id, fromString("com.yahoo.jdisc.http.server.jetty.JettyHttpServer"), fromString("jdisc_http_service")) )); + this.isHostedVespa = isHostedVespa; final FilterBindingsProviderComponent filterBindingsProviderComponent = new FilterBindingsProviderComponent(id); addChild(filterBindingsProviderComponent); inject(filterBindingsProviderComponent); @@ -56,6 +58,17 @@ public class JettyHttpServer extends SimpleComponent implements ServerConfig.Pro .monitoringHandlerPaths(List.of("/state/v1", "/status.html")) .searchHandlerPaths(List.of("/search")) ); + if (isHostedVespa) { + // Proxy-protocol v1/v2 is used in hosted Vespa for remote address/port + builder.accessLog(new ServerConfig.AccessLog.Builder() + .remoteAddressHeaders(List.of()) + .remotePortHeaders(List.of())); + } else { + // TODO Vespa 8: Remove legacy Yahoo headers + builder.accessLog(new ServerConfig.AccessLog.Builder() + .remoteAddressHeaders(List.of("x-forwarded-for", "y-ra", "yahooremoteip", "client-ip")) + .remotePortHeaders(List.of("X-Forwarded-Port", "y-rp"))); + } } static ComponentModel providerComponentModel(final ComponentId parentId, String className) { diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/http/xml/JettyHttpServerBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/http/xml/JettyHttpServerBuilder.java index 3f38b2b16fa..cc9cd61df36 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/http/xml/JettyHttpServerBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/http/xml/JettyHttpServerBuilder.java @@ -17,7 +17,7 @@ public class JettyHttpServerBuilder extends VespaDomBuilder.DomConfigProducerBui @Override protected JettyHttpServer doBuild(DeployState deployState, AbstractConfigProducer ancestor, Element http) { - JettyHttpServer jettyHttpServer = new JettyHttpServer(new ComponentId("jdisc-jetty")); + JettyHttpServer jettyHttpServer = new JettyHttpServer(new ComponentId("jdisc-jetty"), deployState.isHosted()); for (Element serverSpec: XML.getChildren(http, "server")) { ConnectorFactory connectorFactory = new JettyConnectorBuilder().build(deployState, ancestor, serverSpec); jettyHttpServer.addConnector(connectorFactory); diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java index 638c02caf55..dee03fb58d3 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java @@ -353,7 +353,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder { cluster.setHttp(new Http(new FilterChains(cluster))); } if(cluster.getHttp().getHttpServer().isEmpty()) { - JettyHttpServer defaultHttpServer = new JettyHttpServer(new ComponentId("DefaultHttpServer")); + JettyHttpServer defaultHttpServer = new JettyHttpServer(new ComponentId("DefaultHttpServer"), cluster.isHostedVespa()); cluster.getHttp().setHttpServer(defaultHttpServer); defaultHttpServer.addConnector(new ConnectorFactory("SearchServer", Defaults.getDefaults().vespaWebServicePort())); } diff --git a/jdisc_http_service/abi-spec.json b/jdisc_http_service/abi-spec.json index f6bfe769997..3f68009cd42 100644 --- a/jdisc_http_service/abi-spec.json +++ b/jdisc_http_service/abi-spec.json @@ -682,6 +682,44 @@ ], "fields": [] }, + "com.yahoo.jdisc.http.ServerConfig$AccessLog$Builder": { + "superClass": "java.lang.Object", + "interfaces": [ + "com.yahoo.config.ConfigBuilder" + ], + "attributes": [ + "public" + ], + "methods": [ + "public void ()", + "public void (com.yahoo.jdisc.http.ServerConfig$AccessLog)", + "public com.yahoo.jdisc.http.ServerConfig$AccessLog$Builder remoteAddressHeaders(java.lang.String)", + "public com.yahoo.jdisc.http.ServerConfig$AccessLog$Builder remoteAddressHeaders(java.util.Collection)", + "public com.yahoo.jdisc.http.ServerConfig$AccessLog$Builder remotePortHeaders(java.lang.String)", + "public com.yahoo.jdisc.http.ServerConfig$AccessLog$Builder remotePortHeaders(java.util.Collection)", + "public com.yahoo.jdisc.http.ServerConfig$AccessLog build()" + ], + "fields": [ + "public java.util.List remoteAddressHeaders", + "public java.util.List remotePortHeaders" + ] + }, + "com.yahoo.jdisc.http.ServerConfig$AccessLog": { + "superClass": "com.yahoo.config.InnerNode", + "interfaces": [], + "attributes": [ + "public", + "final" + ], + "methods": [ + "public void (com.yahoo.jdisc.http.ServerConfig$AccessLog$Builder)", + "public java.util.List remoteAddressHeaders()", + "public java.lang.String remoteAddressHeaders(int)", + "public java.util.List remotePortHeaders()", + "public java.lang.String remotePortHeaders(int)" + ], + "fields": [] + }, "com.yahoo.jdisc.http.ServerConfig$Builder": { "superClass": "java.lang.Object", "interfaces": [ @@ -704,6 +742,7 @@ "public com.yahoo.jdisc.http.ServerConfig$Builder stopTimeout(double)", "public com.yahoo.jdisc.http.ServerConfig$Builder jmx(com.yahoo.jdisc.http.ServerConfig$Jmx$Builder)", "public com.yahoo.jdisc.http.ServerConfig$Builder metric(com.yahoo.jdisc.http.ServerConfig$Metric$Builder)", + "public com.yahoo.jdisc.http.ServerConfig$Builder accessLog(com.yahoo.jdisc.http.ServerConfig$AccessLog$Builder)", "public final boolean dispatchGetConfig(com.yahoo.config.ConfigInstance$Producer)", "public final java.lang.String getDefMd5()", "public final java.lang.String getDefName()", @@ -713,7 +752,8 @@ "fields": [ "public java.util.List filter", "public com.yahoo.jdisc.http.ServerConfig$Jmx$Builder jmx", - "public com.yahoo.jdisc.http.ServerConfig$Metric$Builder metric" + "public com.yahoo.jdisc.http.ServerConfig$Metric$Builder metric", + "public com.yahoo.jdisc.http.ServerConfig$AccessLog$Builder accessLog" ] }, "com.yahoo.jdisc.http.ServerConfig$Filter$Builder": { @@ -854,7 +894,8 @@ "public int maxWorkerThreads()", "public double stopTimeout()", "public com.yahoo.jdisc.http.ServerConfig$Jmx jmx()", - "public com.yahoo.jdisc.http.ServerConfig$Metric metric()" + "public com.yahoo.jdisc.http.ServerConfig$Metric metric()", + "public com.yahoo.jdisc.http.ServerConfig$AccessLog accessLog()" ], "fields": [ "public static final java.lang.String CONFIG_DEF_MD5", diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLog.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLog.java index 7085f07585a..e8fd92f8e19 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLog.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLog.java @@ -4,6 +4,7 @@ package com.yahoo.jdisc.http.server.jetty; import com.google.common.base.Objects; import com.yahoo.container.logging.AccessLog; import com.yahoo.container.logging.AccessLogEntry; +import com.yahoo.jdisc.http.ServerConfig; import com.yahoo.jdisc.http.servlet.ServletRequest; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.RequestLog; @@ -15,6 +16,7 @@ import java.security.Principal; import java.security.cert.X509Certificate; import java.util.List; import java.util.Optional; +import java.util.OptionalInt; import java.util.logging.Level; import java.util.logging.Logger; @@ -27,25 +29,21 @@ import static com.yahoo.jdisc.http.core.HttpServletRequestUtils.getConnectorLoca * @author Oyvind Bakksjo * @author bjorncs */ -public class AccessLogRequestLog extends AbstractLifeCycle implements RequestLog { +class AccessLogRequestLog extends AbstractLifeCycle implements RequestLog { private static final Logger logger = Logger.getLogger(AccessLogRequestLog.class.getName()); - // TODO These hardcoded headers should be provided by config instead - private static final String HEADER_NAME_X_FORWARDED_FOR = "x-forwarded-for"; - private static final String HEADER_NAME_X_FORWARDED_PORT = "X-Forwarded-Port"; - private static final String HEADER_NAME_Y_RA = "y-ra"; - private static final String HEADER_NAME_Y_RP = "y-rp"; - private static final String HEADER_NAME_YAHOOREMOTEIP = "yahooremoteip"; - private static final String HEADER_NAME_CLIENT_IP = "client-ip"; - // HTTP headers that are logged as extra key-value-pairs in access log entries private static final List LOGGED_REQUEST_HEADERS = List.of("Vespa-Client-Version"); private final AccessLog accessLog; + private final List remoteAddressHeaders; + private final List remotePortHeaders; - public AccessLogRequestLog(AccessLog accessLog) { + AccessLogRequestLog(AccessLog accessLog, ServerConfig.AccessLog config) { this.accessLog = accessLog; + this.remoteAddressHeaders = config.remoteAddressHeaders(); + this.remotePortHeaders = config.remotePortHeaders(); } @Override @@ -121,26 +119,30 @@ public class AccessLogRequestLog extends AbstractLifeCycle implements RequestLog } } - private static String getRemoteAddress(HttpServletRequest request) { - return Optional.ofNullable(request.getHeader(HEADER_NAME_X_FORWARDED_FOR)) - .or(() -> Optional.ofNullable(request.getHeader(HEADER_NAME_Y_RA))) - .or(() -> Optional.ofNullable(request.getHeader(HEADER_NAME_YAHOOREMOTEIP))) - .or(() -> Optional.ofNullable(request.getHeader(HEADER_NAME_CLIENT_IP))) - .orElseGet(request::getRemoteAddr); + private String getRemoteAddress(HttpServletRequest request) { + for (String header : remoteAddressHeaders) { + String value = request.getHeader(header); + if (value != null) return value; + } + return request.getRemoteAddr(); } - private static int getRemotePort(HttpServletRequest request) { - return Optional.ofNullable(request.getHeader(HEADER_NAME_X_FORWARDED_PORT)) - .or(() -> Optional.ofNullable(request.getHeader(HEADER_NAME_Y_RP))) - .flatMap(AccessLogRequestLog::parsePort) - .orElseGet(request::getRemotePort); + private int getRemotePort(HttpServletRequest request) { + for (String header : remotePortHeaders) { + String value = request.getHeader(header); + if (value != null) { + OptionalInt maybePort = parsePort(value); + if (maybePort.isPresent()) return maybePort.getAsInt(); + } + } + return request.getRemotePort(); } - private static Optional parsePort(String port) { + private static OptionalInt parsePort(String port) { try { - return Optional.of(Integer.valueOf(port)); + return OptionalInt.of(Integer.parseInt(port)); } catch (IllegalArgumentException e) { - return Optional.empty(); + return OptionalInt.empty(); } } diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java index b050a9a6d1c..d881fae8e26 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java @@ -147,7 +147,7 @@ public class JettyHttpServer extends AbstractServerProvider { server = new Server(); server.setStopTimeout((long)(serverConfig.stopTimeout() * 1000.0)); - server.setRequestLog(new AccessLogRequestLog(accessLog)); + server.setRequestLog(new AccessLogRequestLog(accessLog, serverConfig.accessLog())); setupJmx(server, serverConfig); ((QueuedThreadPool)server.getThreadPool()).setMaxThreads(serverConfig.maxWorkerThreads()); diff --git a/jdisc_http_service/src/main/resources/configdefinitions/jdisc.http.jdisc.http.server.def b/jdisc_http_service/src/main/resources/configdefinitions/jdisc.http.jdisc.http.server.def index 0cb5b89b20c..3118a7dea64 100644 --- a/jdisc_http_service/src/main/resources/configdefinitions/jdisc.http.jdisc.http.server.def +++ b/jdisc_http_service/src/main/resources/configdefinitions/jdisc.http.jdisc.http.server.def @@ -43,4 +43,10 @@ jmx.listenPort int default = 1099 metric.monitoringHandlerPaths[] string # Paths that should be reported with search dimensions where applicable -metric.searchHandlerPaths[] string \ No newline at end of file +metric.searchHandlerPaths[] string + +# HTTP request headers that contain remote address +accessLog.remoteAddressHeaders[] string + +# HTTP request headers that contain remote port +accessLog.remotePortHeaders[] string diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLogTest.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLogTest.java index 69535be034c..a4fd7c9bc5f 100644 --- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLogTest.java +++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/AccessLogRequestLogTest.java @@ -3,6 +3,7 @@ package com.yahoo.jdisc.http.server.jetty; import com.yahoo.container.logging.AccessLog; import com.yahoo.container.logging.AccessLogEntry; +import com.yahoo.jdisc.http.ServerConfig; import org.eclipse.jetty.http.MetaData; import org.eclipse.jetty.server.HttpChannel; import org.eclipse.jetty.server.HttpConnection; @@ -11,6 +12,7 @@ import org.eclipse.jetty.server.Response; import org.eclipse.jetty.server.ServerConnector; import org.junit.Test; +import java.util.List; import java.util.Optional; import static org.hamcrest.CoreMatchers.is; @@ -33,7 +35,7 @@ public class AccessLogRequestLogTest { when(jettyRequest.getRequestURI()).thenReturn("/search/"); when(jettyRequest.getQueryString()).thenReturn("query=year:>2010"); - new AccessLogRequestLog(mock(AccessLog.class)).log(jettyRequest, createResponseMock()); + doAccessLoggingOfRequest(jettyRequest); assertThat(accessLogEntry.getRawPath(), is(not(nullValue()))); assertTrue(accessLogEntry.getRawQuery().isPresent()); @@ -48,7 +50,7 @@ public class AccessLogRequestLogTest { final String query = "query=year%252010+%3B&customParameter=something"; when(jettyRequest.getQueryString()).thenReturn(query); - new AccessLogRequestLog(mock(AccessLog.class)).log(jettyRequest, createResponseMock()); + doAccessLoggingOfRequest(jettyRequest); assertThat(accessLogEntry.getRawPath(), is(path)); assertThat(accessLogEntry.getRawQuery().get(), is(query)); @@ -64,7 +66,7 @@ public class AccessLogRequestLogTest { String rawQuery = "q=%%2"; when(jettyRequest.getQueryString()).thenReturn(rawQuery); - new AccessLogRequestLog(mock(AccessLog.class)).log(jettyRequest, createResponseMock()); + doAccessLoggingOfRequest(jettyRequest); assertThat(accessLogEntry.getRawPath(), is(rawPath)); Optional actualRawQuery = accessLogEntry.getRawQuery(); assertThat(actualRawQuery.isPresent(), is(true)); @@ -80,7 +82,7 @@ public class AccessLogRequestLogTest { when(jettyRequest.getHeader("x-forwarded-for")).thenReturn("1.2.3.4"); when(jettyRequest.getHeader("y-ra")).thenReturn("2.3.4.5"); - new AccessLogRequestLog(mock(AccessLog.class)).log(jettyRequest, createResponseMock()); + doAccessLoggingOfRequest(jettyRequest); assertThat(accessLogEntry.getRemoteAddress(), is("1.2.3.4")); } @@ -93,7 +95,7 @@ public class AccessLogRequestLogTest { when(jettyRequest.getHeader("X-Forwarded-Port")).thenReturn("80"); when(jettyRequest.getHeader("y-rp")).thenReturn("8080"); - new AccessLogRequestLog(mock(AccessLog.class)).log(jettyRequest, createResponseMock()); + doAccessLoggingOfRequest(jettyRequest); assertThat(accessLogEntry.getRemotePort(), is(80)); } @@ -105,11 +107,19 @@ public class AccessLogRequestLogTest { when(jettyRequest.getHeader("X-Forwarded-Port")).thenReturn("8o8o"); when(jettyRequest.getRemotePort()).thenReturn(80); - new AccessLogRequestLog(mock(AccessLog.class)).log(jettyRequest, createResponseMock()); + doAccessLoggingOfRequest(jettyRequest); assertThat(accessLogEntry.getRemotePort(), is(0)); assertThat(accessLogEntry.getPeerPort(), is(80)); } + private void doAccessLoggingOfRequest(Request jettyRequest) { + ServerConfig.AccessLog config = new ServerConfig.AccessLog( + new ServerConfig.AccessLog.Builder() + .remoteAddressHeaders(List.of("x-forwarded-for", "y-ra")) + .remotePortHeaders(List.of("X-Forwarded-Port", "y-rp"))); + new AccessLogRequestLog(mock(AccessLog.class), config).log(jettyRequest, createResponseMock()); + } + private static Request createRequestMock(AccessLogEntry entry) { ServerConnector serverConnector = mock(ServerConnector.class); when(serverConnector.getLocalPort()).thenReturn(1234); -- cgit v1.2.3 From da57ae5990f0021e31fe708e9cdd25914f1f1e45 Mon Sep 17 00:00:00 2001 From: Martin Polden Date: Tue, 6 Oct 2020 15:31:12 +0200 Subject: Remove config lock from node-repository --- .../provision/applications/Applications.java | 5 +-- .../provision/maintenance/LoadBalancerExpirer.java | 11 +++--- .../persistence/CuratorDatabaseClient.java | 24 ------------- .../provisioning/LoadBalancerProvisioner.java | 40 ++++++++-------------- 4 files changed, 19 insertions(+), 61 deletions(-) diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/Applications.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/Applications.java index 56e52d9f658..17f8d73c88f 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/Applications.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/Applications.java @@ -25,10 +25,7 @@ public class Applications { // read and write all to make sure they are stored in the latest version of the serialized format for (ApplicationId id : ids()) { try (Mutex lock = db.lock(id)) { - // TODO(mpolden): Remove inner lock - try (Mutex innerLock = db.configLock(id)) { - get(id).ifPresent(application -> put(application, lock)); - } + get(id).ifPresent(application -> put(application, lock)); } } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirer.java index 55d17f3e080..90cf3ba8f54 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/LoadBalancerExpirer.java @@ -124,13 +124,10 @@ public class LoadBalancerExpirer extends NodeRepositoryMaintainer { private void withLoadBalancersIn(LoadBalancer.State state, Consumer operation) { for (var id : db.readLoadBalancerIds()) { try (var lock = db.lock(id.application())) { - // TODO(mpolden): Remove inner lock - try (var innerLock = db.configLock(id.application())) { - var loadBalancer = db.readLoadBalancer(id); - if (loadBalancer.isEmpty()) continue; // Load balancer was removed during loop - if (loadBalancer.get().state() != state) continue; // Wrong state - operation.accept(loadBalancer.get()); - } + var loadBalancer = db.readLoadBalancer(id); + if (loadBalancer.isEmpty()) continue; // Load balancer was removed during loop + if (loadBalancer.get().state() != state) continue; // Wrong state + operation.accept(loadBalancer.get()); } } } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java index 2c00b4bab68..91c683d139e 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java @@ -61,7 +61,6 @@ public class CuratorDatabaseClient { private static final Path root = Path.fromString("/provision/v1"); private static final Path lockPath = root.append("locks"); - private static final Path configLockPath = Path.fromString("/config/v2/locks/"); private static final Path loadBalancersPath = root.append("loadBalancers"); private static final Path applicationsPath = root.append("applications"); private static final Path inactiveJobsPath = root.append("inactiveJobs"); @@ -328,15 +327,6 @@ public class CuratorDatabaseClient { return lockPath; } - /** Creates and returns the config lock path for this application */ - // TODO(mpolden): Remove - private Path configLockPath(ApplicationId application) { - // This must match the lock path used by com.yahoo.vespa.config.server.application.TenantApplications - Path lockPath = configLockPath.append(application.tenant().value()).append(application.serializedForm()); - db.create(lockPath); - return lockPath; - } - private String toDir(Node.State state) { switch (state) { case active: return "allocated"; // legacy name @@ -372,20 +362,6 @@ public class CuratorDatabaseClient { } } - // TODO(mpolden): Remove - private Lock configLock(ApplicationId application, Duration timeout) { - try { - return db.lock(configLockPath(application), timeout); - } catch (UncheckedTimeoutException e) { - throw new ApplicationLockException(e); - } - } - - // TODO(mpolden): Remove - public Lock configLock(ApplicationId application) { - return configLock(application, defaultLockTimeout); - } - // Applications ----------------------------------------------------------- public List readApplicationIds() { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java index 09a33093654..cc0b5e411ca 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/LoadBalancerProvisioner.java @@ -60,11 +60,8 @@ public class LoadBalancerProvisioner { // Read and write all load balancers to make sure they are stored in the latest version of the serialization format for (var id : db.readLoadBalancerIds()) { try (var lock = db.lock(id.application())) { - // TODO(mpolden): Remove inner lock - try (var innerLock = db.configLock(id.application())) { - var loadBalancer = db.readLoadBalancer(id); - loadBalancer.ifPresent(db::writeLoadBalancer); - } + var loadBalancer = db.readLoadBalancer(id); + loadBalancer.ifPresent(db::writeLoadBalancer); } } } @@ -83,12 +80,9 @@ public class LoadBalancerProvisioner { if (!canForwardTo(requestedNodes.type(), cluster)) return; // Nothing to provision for this node and cluster type if (application.instance().isTester()) return; // Do not provision for tester instances try (var lock = db.lock(application)) { - // TODO(mpolden): Remove inner lock - try (var innerLock = db.configLock(application)) { - ClusterSpec.Id clusterId = effectiveId(cluster); - List nodes = nodesOf(clusterId, application); - provision(application, clusterId, nodes, false, lock); - } + ClusterSpec.Id clusterId = effectiveId(cluster); + List nodes = nodesOf(clusterId, application); + provision(application, clusterId, nodes, false, lock); } } @@ -105,18 +99,15 @@ public class LoadBalancerProvisioner { public void activate(ApplicationId application, Set clusters, @SuppressWarnings("unused") Mutex applicationLock, NestedTransaction transaction) { try (var lock = db.lock(application)) { - // TODO(mpolden): Remove inner lock - try (var innerLock = db.configLock(application)) { - for (var cluster : loadBalancedClustersOf(application).entrySet()) { - // Provision again to ensure that load balancer instance is re-configured with correct nodes - provision(application, cluster.getKey(), cluster.getValue(), true, lock); - } - // Deactivate any surplus load balancers, i.e. load balancers for clusters that have been removed - var surplusLoadBalancers = surplusLoadBalancersOf(application, clusters.stream() - .map(LoadBalancerProvisioner::effectiveId) - .collect(Collectors.toSet())); - deactivate(surplusLoadBalancers, transaction); + for (var cluster : loadBalancedClustersOf(application).entrySet()) { + // Provision again to ensure that load balancer instance is re-configured with correct nodes + provision(application, cluster.getKey(), cluster.getValue(), true, lock); } + // Deactivate any surplus load balancers, i.e. load balancers for clusters that have been removed + var surplusLoadBalancers = surplusLoadBalancersOf(application, clusters.stream() + .map(LoadBalancerProvisioner::effectiveId) + .collect(Collectors.toSet())); + deactivate(surplusLoadBalancers, transaction); } } @@ -126,10 +117,7 @@ public class LoadBalancerProvisioner { */ public void deactivate(ApplicationId application, NestedTransaction transaction) { try (var lock = nodeRepository.lock(application)) { - // TODO(mpolden): Remove inner lock - try (var innerLock = db.configLock(application)) { - deactivate(nodeRepository.loadBalancers(application).asList(), transaction); - } + deactivate(nodeRepository.loadBalancers(application).asList(), transaction); } } -- cgit v1.2.3 From 8f0d3c5539fd8a8b8c9c962c81ad3aeb5c70b2a6 Mon Sep 17 00:00:00 2001 From: Martin Polden Date: Wed, 7 Oct 2020 13:15:13 +0200 Subject: Remove unused weighted-dns-per-region flag --- flags/src/main/java/com/yahoo/vespa/flags/Flags.java | 7 ------- 1 file changed, 7 deletions(-) diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index 29a924c3033..dbe14ad86d1 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -332,13 +332,6 @@ public class Flags { APPLICATION_ID ); - public static final UnboundBooleanFlag WEIGHTED_DNS_PER_REGION = defineFeatureFlag( - "weighted-dns-per-region", true, - "Whether to create weighted DNS records per region in global endpoints", - "Takes effect on next deployment through controller", - APPLICATION_ID - ); - public static final UnboundBooleanFlag ONLY_PUBLIC_ACCESS = defineFeatureFlag( "enable-public-only", false, "Only access public hosts from container", -- cgit v1.2.3 From 246e73d7aeec45c54111201e7c074810c96122da Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Wed, 7 Oct 2020 13:16:16 +0200 Subject: Autoscale to at most half the host vcpu --- .../autoscale/AllocatableClusterResources.java | 17 ++++++++++++----- .../vespa/hosted/provision/autoscale/Autoscaler.java | 6 ------ .../hosted/provision/autoscale/AutoscalingTest.java | 16 ++++++++-------- 3 files changed, 20 insertions(+), 19 deletions(-) diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocatableClusterResources.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocatableClusterResources.java index 267bfefa332..d1d15baa5dc 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocatableClusterResources.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocatableClusterResources.java @@ -138,21 +138,20 @@ public class AllocatableClusterResources { Limits applicationLimits, NodeRepository nodeRepository) { var systemLimits = new NodeResourceLimits(nodeRepository); - if ( !exclusive && nodeRepository.zone().getCloud().allowHostSharing()) { // Check if any flavor can fit these hosts + if ( !exclusive && nodeRepository.zone().getCloud().allowHostSharing()) { // We decide resources: Add overhead to what we'll request (advertised) to make sure real becomes (at least) cappedNodeResources NodeResources advertisedResources = nodeRepository.resourcesCalculator().realToRequest(wantedResources.nodeResources()); advertisedResources = systemLimits.enlargeToLegal(advertisedResources, clusterType); // Attempt to ask for something legal advertisedResources = applicationLimits.cap(advertisedResources); // Overrides other conditions, even if it will then fail NodeResources realResources = nodeRepository.resourcesCalculator().requestToReal(advertisedResources); // ... thus, what we really get may change if ( ! systemLimits.isWithinRealLimits(realResources, clusterType)) return Optional.empty(); - for (Flavor flavor : nodeRepository.flavors().getFlavors()) { - if (flavor.resources().satisfies(advertisedResources)) + if (matchesAny(nodeRepository.flavors().getFlavors(), advertisedResources)) return Optional.of(new AllocatableClusterResources(wantedResources.with(realResources), advertisedResources, wantedResources.nodeResources(), clusterType)); - } - return Optional.empty(); + else + return Optional.empty(); } else { // Return the cheapest flavor satisfying the requested resources, if any NodeResources cappedWantedResources = applicationLimits.cap(wantedResources.nodeResources()); @@ -185,6 +184,14 @@ public class AllocatableClusterResources { } } + /** Returns true if the given resources could be allocated on any of the given flavors */ + private static boolean matchesAny(List flavors, NodeResources advertisedResources) { + // Tenant nodes should not consume more than half the resources of the biggest hosts + // to make it easier to shift them between hosts. + return flavors.stream().anyMatch(flavor -> flavor.resources().withVcpu(flavor.resources().vcpu() / 2) + .satisfies(advertisedResources)); + } + private static boolean between(NodeResources min, NodeResources max, NodeResources r) { if ( ! min.isUnspecified() && ! min.justNonNumbers().compatibleWith(r.justNonNumbers())) return false; if ( ! max.isUnspecified() && ! max.justNonNumbers().compatibleWith(r.justNonNumbers())) return false; diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaler.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaler.java index d359b86ae85..a267d59f1dc 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaler.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaler.java @@ -1,7 +1,6 @@ // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.provision.autoscale; -import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ClusterResources; import com.yahoo.config.provision.ClusterSpec; import com.yahoo.vespa.hosted.provision.Node; @@ -9,13 +8,8 @@ import com.yahoo.vespa.hosted.provision.NodeRepository; import com.yahoo.vespa.hosted.provision.applications.Cluster; import java.time.Duration; -import java.time.Instant; -import java.util.ArrayList; -import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.Optional; -import java.util.stream.Collectors; /** * The autoscaler makes decisions about the flavor and node count that should be allocated to a cluster diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java index 5b4b0b5bcb2..d615d418c50 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java @@ -78,7 +78,7 @@ public class AutoscalingTest { NodeResources resources = new NodeResources(3, 100, 100, 1); ClusterResources min = new ClusterResources( 2, 1, new NodeResources(1, 1, 1, 1)); ClusterResources max = new ClusterResources(20, 1, new NodeResources(100, 1000, 1000, 1)); - AutoscalingTester tester = new AutoscalingTester(resources); + AutoscalingTester tester = new AutoscalingTester(resources.withVcpu(resources.vcpu() * 2)); ApplicationId application1 = tester.applicationId("application1"); ClusterSpec cluster1 = tester.clusterSpec(ClusterSpec.Type.container, "cluster1"); @@ -131,7 +131,7 @@ public class AutoscalingTest { @Test public void autoscaling_respects_upper_limit() { - NodeResources hostResources = new NodeResources(3, 100, 100, 1); + NodeResources hostResources = new NodeResources(6, 100, 100, 1); ClusterResources min = new ClusterResources( 2, 1, new NodeResources(1, 1, 1, 1)); ClusterResources max = new ClusterResources( 6, 1, new NodeResources(2.4, 78, 79, 1)); AutoscalingTester tester = new AutoscalingTester(hostResources); @@ -155,7 +155,7 @@ public class AutoscalingTest { NodeResources resources = new NodeResources(3, 100, 100, 1); ClusterResources min = new ClusterResources( 4, 1, new NodeResources(1.8, 7.4, 8.5, 1)); ClusterResources max = new ClusterResources( 6, 1, new NodeResources(2.4, 78, 79, 1)); - AutoscalingTester tester = new AutoscalingTester(resources); + AutoscalingTester tester = new AutoscalingTester(resources.withVcpu(resources.vcpu() * 2)); ApplicationId application1 = tester.applicationId("application1"); ClusterSpec cluster1 = tester.clusterSpec(ClusterSpec.Type.container, "cluster1"); @@ -209,7 +209,7 @@ public class AutoscalingTest { NodeResources resources = new NodeResources(3, 100, 100, 1); ClusterResources min = new ClusterResources( 2, 1, new NodeResources(1, 1, 1, 1)); ClusterResources max = min; - AutoscalingTester tester = new AutoscalingTester(resources); + AutoscalingTester tester = new AutoscalingTester(resources.withVcpu(resources.vcpu() * 2)); ApplicationId application1 = tester.applicationId("application1"); ClusterSpec cluster1 = tester.clusterSpec(ClusterSpec.Type.container, "cluster1"); @@ -227,7 +227,7 @@ public class AutoscalingTest { NodeResources resources = new NodeResources(3, 100, 100, 1); ClusterResources min = new ClusterResources( 2, 2, new NodeResources(1, 1, 1, 1)); ClusterResources max = new ClusterResources(20, 20, new NodeResources(100, 1000, 1000, 1)); - AutoscalingTester tester = new AutoscalingTester(resources); + AutoscalingTester tester = new AutoscalingTester(resources.withVcpu(resources.vcpu() * 2)); ApplicationId application1 = tester.applicationId("application1"); ClusterSpec cluster1 = tester.clusterSpec(ClusterSpec.Type.container, "cluster1"); @@ -245,7 +245,7 @@ public class AutoscalingTest { NodeResources resources = new NodeResources(3, 100, 100, 1); ClusterResources min = new ClusterResources( 3, 1, new NodeResources(1, 1, 1, 1)); ClusterResources max = new ClusterResources(21, 7, new NodeResources(100, 1000, 1000, 1)); - AutoscalingTester tester = new AutoscalingTester(resources); + AutoscalingTester tester = new AutoscalingTester(resources.withVcpu(resources.vcpu() * 2)); ApplicationId application1 = tester.applicationId("application1"); ClusterSpec cluster1 = tester.clusterSpec(ClusterSpec.Type.container, "cluster1"); @@ -278,7 +278,7 @@ public class AutoscalingTest { @Test public void autoscaling_avoids_illegal_configurations() { - NodeResources hostResources = new NodeResources(3, 100, 100, 1); + NodeResources hostResources = new NodeResources(6, 100, 100, 1); ClusterResources min = new ClusterResources( 2, 1, new NodeResources(1, 1, 1, 1)); ClusterResources max = new ClusterResources(20, 1, new NodeResources(100, 1000, 1000, 1)); AutoscalingTester tester = new AutoscalingTester(hostResources); @@ -287,7 +287,7 @@ public class AutoscalingTest { ClusterSpec cluster1 = tester.clusterSpec(ClusterSpec.Type.content, "cluster1"); // deploy - tester.deploy(application1, cluster1, 6, 1, hostResources); + tester.deploy(application1, cluster1, 6, 1, hostResources.withVcpu(hostResources.vcpu() / 2)); tester.addMeasurements(Resource.memory, 0.02f, 0.95f, 120, application1); tester.assertResources("Scaling down", 6, 1, 2.8, 4.0, 95.0, -- cgit v1.2.3 From 9bb9d8e14827ecc4dba2d43e2d9e76248c120e1d Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Wed, 7 Oct 2020 11:16:08 +0000 Subject: Add noexcept as indicated by -Wnoeexcept --- .../src/vespa/document/repo/documenttyperepo.cpp | 4 +- .../src/vespa/document/repo/documenttyperepo.h | 4 +- document/src/vespa/document/repo/fixedtyperepo.cpp | 2 +- document/src/vespa/document/repo/fixedtyperepo.h | 8 ++-- eval/src/tests/ann/hnsw-like.h | 2 +- eval/src/tests/ann/nns.h | 4 +- fsa/src/vespa/fsa/fsa.h | 26 +++++------ fsa/src/vespa/fsa/segmenter.h | 8 ++-- .../tests/empty_forwarder/empty_forwarder_test.cpp | 2 +- .../src/tests/rpc_forwarder/rpc_forwarder_test.cpp | 2 +- persistence/src/vespa/persistence/spi/bucket.h | 2 +- persistencetypes/src/persistence/spi/types.h | 8 ++-- .../searchcommon/attribute/iattributevector.h | 4 +- .../apps/vespa-feed-bm/bm_storage_link_context.h | 2 +- .../proton/documentdb/feedview/feedview_test.cpp | 2 +- .../lid_space_compaction_test.cpp | 5 +-- .../maintenancecontroller_test.cpp | 2 +- .../storeonlyfeedview/storeonlyfeedview_test.cpp | 4 +- .../documentmetastore/documentmetastore_test.cpp | 2 +- .../tests/proton/flushengine/flushengine_test.cpp | 4 +- .../prepare_restart_flush_strategy_test.cpp | 6 +-- .../shrink_lid_space_flush_target_test.cpp | 16 ++++--- .../src/tests/proton/matching/matching_test.cpp | 2 +- .../document_db_reference_resolver_test.cpp | 2 +- .../gid_to_lid_change_handler_test.cpp | 2 +- .../reprocessing_runner_test.cpp | 46 ++++--------------- .../src/tests/proton/server/feedstates_test.cpp | 2 +- .../proton/server/memoryflush/memoryflush_test.cpp | 29 +++++------- .../proton/attribute/attribute_populator.cpp | 2 +- .../vespa/searchcore/proton/common/doctypename.cpp | 4 +- .../vespa/searchcore/proton/common/doctypename.h | 6 +-- .../proton/documentmetastore/gid_compare.h | 7 ++- .../searchcore/proton/flushengine/iflushhandler.h | 2 +- .../searchcore/proton/initializer/task_runner.h | 2 +- .../searchcore/proton/matching/sessionmanager.h | 2 +- .../vespa/searchcore/proton/server/feedhandler.cpp | 2 +- .../vespa/searchcore/proton/server/feedstates.h | 4 +- .../proton/server/move_operation_limiter.cpp | 2 +- .../vespa/searchcore/proton/server/packetwrapper.h | 3 +- .../vespa/searchcore/proton/server/rpc_hooks.cpp | 2 +- .../proton/test/document_meta_store_observer.h | 51 ++++++++++------------ .../searchcore/proton/test/dummy_flush_handler.h | 19 +++----- .../searchcore/proton/test/dummy_flush_target.h | 30 +++++-------- .../proton/test/mock_gid_to_lid_change_handler.h | 4 +- .../src/vespa/searchcorespi/flush/iflushtarget.h | 10 ++--- .../vespa-ranking-expression-analyzer.cpp | 5 ++- .../posting_list_merger_test.cpp | 12 ++--- .../attribute_searchable_adapter_test.cpp | 9 ++-- .../field_index_remover_test.cpp | 4 +- searchlib/src/tests/query/querybuilder_test.cpp | 4 +- .../predicate/predicate_blueprint_test.cpp | 8 ++-- searchlib/src/tests/transactionlog/chunks_test.cpp | 2 +- .../tests/transactionlog/translogclient_test.cpp | 2 +- .../searchlib/attribute/bitvector_search_cache.h | 10 ++--- .../attribute/imported_search_context.cpp | 6 +-- .../src/vespa/searchlib/attribute/interlock.h | 12 ++--- .../src/vespa/searchlib/attribute/multivalue.h | 13 +++--- .../src/vespa/searchlib/common/gatecallback.h | 4 +- .../src/vespa/searchlib/common/lid_usage_stats.h | 4 +- searchlib/src/vespa/searchlib/common/rankedhit.h | 14 +++--- .../vespa/searchlib/common/scheduletaskcallback.h | 10 ++--- .../src/vespa/searchlib/common/tunefileinfo.h | 22 +++++----- .../searchlib/diskindex/zc4_posting_writer_base.h | 2 +- searchlib/src/vespa/searchlib/docstore/filechunk.h | 4 +- searchlib/src/vespa/searchlib/docstore/lid_info.h | 6 +-- .../src/vespa/searchlib/docstore/storebybucket.cpp | 2 +- .../src/vespa/searchlib/docstore/storebybucket.h | 13 +++--- .../src/vespa/searchlib/docstore/visitcache.h | 2 +- .../searchlib/docstore/writeablefilechunk.cpp | 2 +- searchlib/src/vespa/searchlib/engine/docsumreply.h | 6 +-- .../src/vespa/searchlib/engine/docsumrequest.h | 4 +- searchlib/src/vespa/searchlib/engine/searchreply.h | 8 ++-- .../src/vespa/searchlib/features/array_parser.h | 2 +- .../src/vespa/searchlib/features/bm25_feature.h | 2 +- .../vespa/searchlib/features/fieldmatch/computer.h | 2 +- searchlib/src/vespa/searchlib/fef/rank_program.cpp | 2 +- .../vespa/searchlib/fef/termmatchdatamerger.cpp | 2 +- .../src/vespa/searchlib/fef/termmatchdatamerger.h | 22 ++++------ .../src/vespa/searchlib/grouping/hyperloglog.h | 2 +- searchlib/src/vespa/searchlib/grouping/sketch.h | 14 +++--- .../src/vespa/searchlib/index/docidandfeatures.h | 18 ++++---- .../searchlib/memoryindex/field_index_remover.h | 8 ++-- .../vespa/searchlib/memoryindex/field_inverter.h | 7 ++- .../searchlib/parsequery/stackdumpiterator.cpp | 2 +- .../src/vespa/searchlib/query/streaming/hit.h | 2 +- .../searchlib/query/tree/predicate_query_term.h | 10 +---- .../vespa/searchlib/query/tree/queryreplicator.h | 2 +- .../src/vespa/searchlib/queryeval/global_filter.h | 6 +-- .../searchlib/queryeval/sourceblendersearch.h | 2 +- .../vespa/searchlib/queryeval/wand/wand_parts.h | 11 ++--- .../src/vespa/searchlib/tensor/hnsw_index.cpp | 2 +- .../src/vespa/searchlib/tensor/hnsw_index_utils.h | 4 +- .../searchlib/tensor/nearest_neighbor_index.h | 6 +-- .../searchlib/test/imported_attribute_fixture.h | 2 +- staging_vespalib/src/tests/metrics/mock_tick.h | 4 +- .../polymorphicarray/polymorphicarray_test.cpp | 6 +-- .../adaptive_sequenced_executor_test.cpp | 2 +- .../foregroundtaskexecutor_test.cpp | 2 +- .../sequencedtaskexecutor_test.cpp | 2 +- .../vespalib/metrics/dummy_metrics_manager.cpp | 8 ++-- .../vespa/vespalib/metrics/dummy_metrics_manager.h | 10 ++--- .../src/vespa/vespalib/objects/identifiable.hpp | 10 ++--- storage/src/tests/bucketdb/lockablemaptest.cpp | 2 +- .../src/tests/persistence/persistencetestutils.h | 2 +- .../vespa/storage/bucketdb/abstract_bucket_map.h | 14 +++--- storage/src/vespa/storage/bucketdb/bucketcopy.h | 4 +- .../src/vespa/storage/distributor/distributor.cpp | 2 +- .../src/vespa/storage/distributor/messagetracker.h | 2 +- .../storage/distributor/operation_sequencer.h | 2 +- .../operations/external/updateoperation.h | 2 +- .../operations/idealstate/mergemetadata.h | 14 ++---- .../persistence/filestorage/filestormanager.cpp | 2 +- .../storageserver/configurable_bucket_resolver.h | 2 +- .../storageserver/service_layer_error_listener.h | 2 +- .../vespa/storage/storageserver/statemanager.cpp | 2 +- .../src/vespa/storageapi/buckets/bucketinfo.cpp | 16 +++---- .../src/vespa/storageapi/buckets/bucketinfo.h | 14 +++--- storageapi/src/vespa/storageapi/defs.h | 4 +- storageapi/src/vespa/storageapi/message/bucket.h | 6 +-- storageapi/src/vespa/storageapi/message/visitor.h | 12 ++--- .../searchvisitor/matching_elements_filler.cpp | 2 +- .../src/vespa/searchvisitor/searchvisitor.h | 4 +- vbench/src/vbench/core/time_queue.h | 9 ++-- vbench/src/vbench/core/time_queue.hpp | 3 ++ vespalib/src/tests/delegatelist/delegatelist.cpp | 8 ++-- .../explore_modern_cpp/explore_modern_cpp_test.cpp | 2 +- .../net/async_resolver/async_resolver_test.cpp | 4 +- .../tests/priority_queue/priority_queue_test.cpp | 10 ++--- vespalib/src/tests/stllike/hash_test.cpp | 12 ++--- .../src/tests/stllike/uniq_by_sort_map_hash.cpp | 16 +++---- .../generation_handler_stress_test.cpp | 2 +- vespalib/src/vespa/vespalib/btree/btree_key_data.h | 8 ++-- vespalib/src/vespa/vespalib/data/databuffer.cpp | 2 +- vespalib/src/vespa/vespalib/data/databuffer.h | 10 ++--- vespalib/src/vespa/vespalib/data/slime/slime.h | 2 +- vespalib/src/vespa/vespalib/datastore/entryref.h | 26 +++++------ vespalib/src/vespa/vespalib/datastore/entryref.hpp | 2 +- vespalib/src/vespa/vespalib/stllike/string.h | 2 +- vespalib/src/vespa/vespalib/util/alloc.cpp | 4 +- vespalib/src/vespa/vespalib/util/alloc.h | 14 +++--- vespalib/src/vespa/vespalib/util/arrayqueue.hpp | 15 ++++--- .../src/vespa/vespalib/util/compressionconfig.h | 8 ++-- .../src/vespa/vespalib/util/count_down_latch.h | 2 +- vespalib/src/vespa/vespalib/util/gate.h | 2 +- vespalib/src/vespa/vespalib/util/memory.h | 18 ++++---- vespalib/src/vespa/vespalib/util/sync.h | 10 +++-- 146 files changed, 471 insertions(+), 548 deletions(-) diff --git a/document/src/vespa/document/repo/documenttyperepo.cpp b/document/src/vespa/document/repo/documenttyperepo.cpp index bcd4edb3ab8..677da16190d 100644 --- a/document/src/vespa/document/repo/documenttyperepo.cpp +++ b/document/src/vespa/document/repo/documenttyperepo.cpp @@ -547,13 +547,13 @@ DocumentTypeRepo::~DocumentTypeRepo() { } const DocumentType * -DocumentTypeRepo::getDocumentType(int32_t type_id) const { +DocumentTypeRepo::getDocumentType(int32_t type_id) const noexcept { const DataTypeRepo *repo = FindPtr(*_doc_types, type_id); return repo ? repo->doc_type : nullptr; } const DocumentType * -DocumentTypeRepo::getDocumentType(stringref name) const { +DocumentTypeRepo::getDocumentType(stringref name) const noexcept { DocumentTypeMap::const_iterator it = _doc_types->find(DocumentType::createId(name)); if (it != _doc_types->end() && it->second->doc_type->getName() == name) { diff --git a/document/src/vespa/document/repo/documenttyperepo.h b/document/src/vespa/document/repo/documenttyperepo.h index f0c59918a74..fd17bd5640a 100644 --- a/document/src/vespa/document/repo/documenttyperepo.h +++ b/document/src/vespa/document/repo/documenttyperepo.h @@ -34,8 +34,8 @@ public: explicit DocumentTypeRepo(const DocumenttypesConfig & config); ~DocumentTypeRepo(); - const DocumentType *getDocumentType(int32_t doc_type_id) const; - const DocumentType *getDocumentType(vespalib::stringref name) const; + const DocumentType *getDocumentType(int32_t doc_type_id) const noexcept; + const DocumentType *getDocumentType(vespalib::stringref name) const noexcept; const DataType *getDataType(const DocumentType &doc_type, int32_t id) const; const DataType *getDataType(const DocumentType &doc_type, vespalib::stringref name) const; const AnnotationType *getAnnotationType(const DocumentType &doc_type, int32_t id) const; diff --git a/document/src/vespa/document/repo/fixedtyperepo.cpp b/document/src/vespa/document/repo/fixedtyperepo.cpp index 81a26265830..20865a8a6ca 100644 --- a/document/src/vespa/document/repo/fixedtyperepo.cpp +++ b/document/src/vespa/document/repo/fixedtyperepo.cpp @@ -5,7 +5,7 @@ namespace document { -FixedTypeRepo::FixedTypeRepo(const DocumentTypeRepo &repo, const vespalib::string &type) +FixedTypeRepo::FixedTypeRepo(const DocumentTypeRepo &repo, const vespalib::string &type) noexcept : _repo(&repo), _doc_type(repo.getDocumentType(type)) { assert(_doc_type); diff --git a/document/src/vespa/document/repo/fixedtyperepo.h b/document/src/vespa/document/repo/fixedtyperepo.h index 67e7571e31d..29bef846e36 100644 --- a/document/src/vespa/document/repo/fixedtyperepo.h +++ b/document/src/vespa/document/repo/fixedtyperepo.h @@ -13,17 +13,17 @@ class FixedTypeRepo { const DocumentType *_doc_type; public: - explicit FixedTypeRepo(const DocumentTypeRepo &repo) + explicit FixedTypeRepo(const DocumentTypeRepo &repo) noexcept : _repo(&repo), _doc_type(repo.getDefaultDocType()) {} - FixedTypeRepo(const DocumentTypeRepo &repo, const DocumentType &doc_type) + FixedTypeRepo(const DocumentTypeRepo &repo, const DocumentType &doc_type) noexcept : _repo(&repo), _doc_type(&doc_type) {} - FixedTypeRepo(const DocumentTypeRepo &repo, const vespalib::string &type); + FixedTypeRepo(const DocumentTypeRepo &repo, const vespalib::string &type) noexcept; const DataType *getDataType(int32_t id) const { return _repo->getDataType(*_doc_type, id); } const DataType *getDataType(const vespalib::string &name) const { return _repo->getDataType(*_doc_type, name); } const AnnotationType *getAnnotationType(int32_t id) const { return _repo->getAnnotationType(*_doc_type, id); } const DocumentTypeRepo &getDocumentTypeRepo() const { return *_repo; } - const DocumentType &getDocumentType() const { return *_doc_type; } + const DocumentType &getDocumentType() const noexcept { return *_doc_type; } }; } // namespace document diff --git a/eval/src/tests/ann/hnsw-like.h b/eval/src/tests/ann/hnsw-like.h index 841957c1ccb..ac4a346993d 100644 --- a/eval/src/tests/ann/hnsw-like.h +++ b/eval/src/tests/ann/hnsw-like.h @@ -86,7 +86,7 @@ struct VisitedSetPool struct HnswHit { double dist; uint32_t docid; - HnswHit(uint32_t di, SqDist sq) : dist(sq.distance), docid(di) {} + HnswHit(uint32_t di, SqDist sq) noexcept : dist(sq.distance), docid(di) {} }; struct GreaterDist { diff --git a/eval/src/tests/ann/nns.h b/eval/src/tests/ann/nns.h index 7a20a132248..6351733fb59 100644 --- a/eval/src/tests/ann/nns.h +++ b/eval/src/tests/ann/nns.h @@ -8,13 +8,13 @@ struct SqDist { double distance; - explicit SqDist(double d) : distance(d) {} + explicit SqDist(double d) noexcept : distance(d) {} }; struct NnsHit { uint32_t docid; SqDist sq; - NnsHit(uint32_t di, SqDist sqD) + NnsHit(uint32_t di, SqDist sqD) noexcept : docid(di), sq(sqD) {} }; struct NnsHitComparatorLessDistance { diff --git a/fsa/src/vespa/fsa/fsa.h b/fsa/src/vespa/fsa/fsa.h index e4d3246d924..9c668f1f85f 100644 --- a/fsa/src/vespa/fsa/fsa.h +++ b/fsa/src/vespa/fsa/fsa.h @@ -304,7 +304,7 @@ public: * * @param f Reference to FSA. */ - State(const FSA& f) : _fsa(&f), _state(_fsa->start()) {} + State(const FSA& f) noexcept : _fsa(&f), _state(_fsa->start()) {} /** * @brief Constructor. @@ -314,7 +314,7 @@ public: * * @param f Pointer to FSA. */ - State(const FSA* f) : _fsa(f), _state(_fsa->start()) {} + State(const FSA* f) noexcept : _fsa(f), _state(_fsa->start()) {} /** * @brief Copy constructor. @@ -325,14 +325,14 @@ public: * * @param s Reference to state to be duplicated. */ - State(const State& s) : _fsa(s._fsa), _state(s._state) {} + State(const State& s) noexcept : _fsa(s._fsa), _state(s._state) {} /** * @brief Destructor. * * Destructor, does nothing special. */ - virtual ~State() {} + virtual ~State() = default; /** * @brief Check if the automaton has perfect hash built in. @@ -1043,7 +1043,7 @@ public: * * @param f Reference to FSA. */ - WordCounterState(const FSA& f) : State(f), _counter(0) {} + WordCounterState(const FSA& f) noexcept : State(f), _counter(0) {} /** * @brief Constructor. @@ -1053,7 +1053,7 @@ public: * * @param f Pointer to FSA. */ - WordCounterState(const FSA* f) : State(f), _counter(0) {} + WordCounterState(const FSA* f) noexcept : State(f), _counter(0) {} /** * @brief Copy constructor. @@ -1062,12 +1062,12 @@ public: * * @param s Reference to hashed state to copy. */ - WordCounterState(const WordCounterState& s) : State(s), _counter(s._counter) {} + WordCounterState(const WordCounterState& s) noexcept : State(s), _counter(s._counter) {} /** * @brief Destructor. */ - virtual ~WordCounterState() {} + virtual ~WordCounterState() = default; /** * @brief Set the state to the starting state of the automaton. @@ -1798,7 +1798,7 @@ public: * * @param f Reference to FSA. */ - HashedWordCounterState(const FSA& f) : State(f), _hash(0), _counter(0) {} + HashedWordCounterState(const FSA& f) noexcept : State(f), _hash(0), _counter(0) {} /** * @brief Constructor. @@ -1808,7 +1808,7 @@ public: * * @param f Pointer to FSA. */ - HashedWordCounterState(const FSA* f) : State(f), _hash(0), _counter(0) {} + HashedWordCounterState(const FSA* f) noexcept : State(f), _hash(0), _counter(0) {} /** * @brief Copy constructor. @@ -1817,12 +1817,12 @@ public: * * @param s Reference to hashed state to copy. */ - HashedWordCounterState(const HashedWordCounterState& s) : State(s), _hash(s._hash), _counter(s._counter) {} + HashedWordCounterState(const HashedWordCounterState& s) noexcept : State(s), _hash(s._hash), _counter(s._counter) {} /** * @brief Destructor. */ - virtual ~HashedWordCounterState() {} + virtual ~HashedWordCounterState() = default; /** * @brief Set the state to the starting state of the automaton. @@ -2120,7 +2120,7 @@ public: * * @return Index of the start state (0 if the %FSA is empty). */ - state_t start() const + state_t start() const noexcept { return _start; } diff --git a/fsa/src/vespa/fsa/segmenter.h b/fsa/src/vespa/fsa/segmenter.h index a2720c6de0c..f2826f9c967 100644 --- a/fsa/src/vespa/fsa/segmenter.h +++ b/fsa/src/vespa/fsa/segmenter.h @@ -147,7 +147,7 @@ public: * * Null segment at postion zero. */ - Segment() : _beg(0), _end(0), _conn(0) {} + Segment() noexcept : _beg(0), _end(0), _conn(0) {} /** * @brief Constructor. @@ -156,7 +156,7 @@ public: * @param e End of the segment (the position after the last term). * @param c Connexity of the segment. */ - Segment(unsigned int b, unsigned int e, unsigned int c) : + Segment(unsigned int b, unsigned int e, unsigned int c) noexcept : _beg(b), _end(e), _conn(c) {} /** @@ -164,12 +164,12 @@ public: * * @param s Segment object to copy. */ - Segment(const Segment &s) : _beg(s._beg), _end(s._end), _conn(s._conn) {} + Segment(const Segment &s) noexcept : _beg(s._beg), _end(s._end), _conn(s._conn) {} /** * @brief Destructor. */ - ~Segment() {} + ~Segment() = default; /** * @brief Set the segment parameters. diff --git a/logd/src/tests/empty_forwarder/empty_forwarder_test.cpp b/logd/src/tests/empty_forwarder/empty_forwarder_test.cpp index d1194f30c40..dbc76e694f2 100644 --- a/logd/src/tests/empty_forwarder/empty_forwarder_test.cpp +++ b/logd/src/tests/empty_forwarder/empty_forwarder_test.cpp @@ -11,7 +11,7 @@ using vespalib::metrics::DummyMetricsManager; struct MockMetricsManager : public DummyMetricsManager { int add_count; - MockMetricsManager() : DummyMetricsManager(), add_count(0) {} + MockMetricsManager() noexcept : DummyMetricsManager(), add_count(0) {} void add(Counter::Increment) override { ++add_count; } diff --git a/logd/src/tests/rpc_forwarder/rpc_forwarder_test.cpp b/logd/src/tests/rpc_forwarder/rpc_forwarder_test.cpp index 15a1dc36a87..d39a9ade0a8 100644 --- a/logd/src/tests/rpc_forwarder/rpc_forwarder_test.cpp +++ b/logd/src/tests/rpc_forwarder/rpc_forwarder_test.cpp @@ -85,7 +85,7 @@ make_log_line(const std::string& level, const std::string& payload) struct MockMetricsManager : public DummyMetricsManager { int add_count; - MockMetricsManager() : DummyMetricsManager(), add_count(0) {} + MockMetricsManager() noexcept : DummyMetricsManager(), add_count(0) {} void add(Counter::Increment) override { ++add_count; } diff --git a/persistence/src/vespa/persistence/spi/bucket.h b/persistence/src/vespa/persistence/spi/bucket.h index 175aba376a9..b7d04150108 100644 --- a/persistence/src/vespa/persistence/spi/bucket.h +++ b/persistence/src/vespa/persistence/spi/bucket.h @@ -36,7 +36,7 @@ public: /** Convert easily to a document bucket id to make class easy to use. */ operator document::BucketId() const { return _bucket.getBucketId(); } - bool operator==(const Bucket& o) const { + bool operator==(const Bucket& o) const noexcept { return (_bucket == o._bucket && _partition == o._partition); } diff --git a/persistencetypes/src/persistence/spi/types.h b/persistencetypes/src/persistence/spi/types.h index c11f3dacf06..59891e04710 100644 --- a/persistencetypes/src/persistence/spi/types.h +++ b/persistencetypes/src/persistence/spi/types.h @@ -29,10 +29,10 @@ namespace document { typedef type Type; \ name() noexcept : _value() {} \ explicit name(type v) noexcept : _value(v) {} \ - operator type() const { return _value; } \ - operator type&() { return _value; } \ - type getValue() const { return _value; } \ - name& operator=(type val) { _value = val; return *this; } \ + operator type() const noexcept { return _value; } \ + operator type&() noexcept { return _value; } \ + type getValue() const noexcept { return _value; } \ + name& operator=(type val) noexcept { _value = val; return *this; } \ friend vespalib::nbostream & \ operator<<(vespalib::nbostream &os, const name &wrapped); \ friend vespalib::nbostream & \ diff --git a/searchcommon/src/vespa/searchcommon/attribute/iattributevector.h b/searchcommon/src/vespa/searchcommon/attribute/iattributevector.h index 71319d782f1..c26906ac7c0 100644 --- a/searchcommon/src/vespa/searchcommon/attribute/iattributevector.h +++ b/searchcommon/src/vespa/searchcommon/attribute/iattributevector.h @@ -36,8 +36,8 @@ private: int32_t _weight; public: - WeightedType() : _value(T()), _weight(1) { } - WeightedType(T value_, int32_t weight_ = 1) : _value(value_), _weight(weight_) { } + WeightedType() noexcept : _value(T()), _weight(1) { } + WeightedType(T value_, int32_t weight_ = 1) noexcept : _value(value_), _weight(weight_) { } const T & getValue() const { return _value; } const T & value() const { return _value; } void setValue(const T & v) { _value = v; } diff --git a/searchcore/src/apps/vespa-feed-bm/bm_storage_link_context.h b/searchcore/src/apps/vespa-feed-bm/bm_storage_link_context.h index adb2a13ec10..f2df20f1f66 100644 --- a/searchcore/src/apps/vespa-feed-bm/bm_storage_link_context.h +++ b/searchcore/src/apps/vespa-feed-bm/bm_storage_link_context.h @@ -10,7 +10,7 @@ class BmStorageLink; struct BmStorageLinkContext { BmStorageLink* bm_link; - BmStorageLinkContext() + BmStorageLinkContext() noexcept : bm_link(nullptr) { } diff --git a/searchcore/src/tests/proton/documentdb/feedview/feedview_test.cpp b/searchcore/src/tests/proton/documentdb/feedview/feedview_test.cpp index b275064c0f2..e6751a0cb98 100644 --- a/searchcore/src/tests/proton/documentdb/feedview/feedview_test.cpp +++ b/searchcore/src/tests/proton/documentdb/feedview/feedview_test.cpp @@ -176,7 +176,7 @@ struct MyGidToLidChangeHandler : public MockGidToLidChangeHandler uint32_t _changes; std::map _gidToLid; public: - MyGidToLidChangeHandler() + MyGidToLidChangeHandler() noexcept : MockGidToLidChangeHandler(), _changeGid(), _changeLid(std::numeric_limits::max()), diff --git a/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_compaction_test.cpp b/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_compaction_test.cpp index 04444647b5d..8890a6cfdda 100644 --- a/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_compaction_test.cpp +++ b/searchcore/src/tests/proton/documentdb/lid_space_compaction/lid_space_compaction_test.cpp @@ -212,11 +212,10 @@ MyDocumentStore::~MyDocumentStore() = default; struct MyDocumentRetriever : public DocumentRetrieverBaseForTest { std::shared_ptr repo; const MyDocumentStore& store; - MyDocumentRetriever(std::shared_ptr repo_in, const MyDocumentStore& store_in) + MyDocumentRetriever(std::shared_ptr repo_in, const MyDocumentStore& store_in) noexcept : repo(std::move(repo_in)), store(store_in) - { - } + {} const document::DocumentTypeRepo& getDocumentTypeRepo() const override { return *repo; } void getBucketMetaData(const storage::spi::Bucket&, DocumentMetaData::Vector&) const override { abort(); } DocumentMetaData getDocumentMetaData(const DocumentId&) const override { abort(); } diff --git a/searchcore/src/tests/proton/documentdb/maintenancecontroller/maintenancecontroller_test.cpp b/searchcore/src/tests/proton/documentdb/maintenancecontroller/maintenancecontroller_test.cpp index 70b2100521f..63c7a873eaf 100644 --- a/searchcore/src/tests/proton/documentdb/maintenancecontroller/maintenancecontroller_test.cpp +++ b/searchcore/src/tests/proton/documentdb/maintenancecontroller/maintenancecontroller_test.cpp @@ -147,7 +147,7 @@ struct MyDocumentRetriever : public DocumentRetrieverBaseForTest { MyDocumentSubDB &_subDB; - explicit MyDocumentRetriever(MyDocumentSubDB &subDB) + explicit MyDocumentRetriever(MyDocumentSubDB &subDB) noexcept : _subDB(subDB) { } diff --git a/searchcore/src/tests/proton/documentdb/storeonlyfeedview/storeonlyfeedview_test.cpp b/searchcore/src/tests/proton/documentdb/storeonlyfeedview/storeonlyfeedview_test.cpp index 8d1276497bd..e6e71d51e47 100644 --- a/searchcore/src/tests/proton/documentdb/storeonlyfeedview/storeonlyfeedview_test.cpp +++ b/searchcore/src/tests/proton/documentdb/storeonlyfeedview/storeonlyfeedview_test.cpp @@ -171,7 +171,7 @@ struct MoveOperationFeedView : public MyMinimalFeedView { struct MoveOperationCallback : public IDestructorCallback { int &outstandingMoveOps; - MoveOperationCallback(int &outstandingMoveOps_) : outstandingMoveOps(outstandingMoveOps_) { + explicit MoveOperationCallback(int &outstandingMoveOps_) noexcept : outstandingMoveOps(outstandingMoveOps_) { ++outstandingMoveOps; } ~MoveOperationCallback() override { @@ -194,7 +194,7 @@ struct FixtureBase { documentmetastore::LidReuseDelayerConfig lidReuseDelayerConfig; typename FeedViewType::UP feedview; - FixtureBase(SubDbType subDbType = SubDbType::READY) + explicit FixtureBase(SubDbType subDbType = SubDbType::READY) : removeCount(0), putCount(0), heartbeatCount(0), diff --git a/searchcore/src/tests/proton/documentmetastore/documentmetastore_test.cpp b/searchcore/src/tests/proton/documentmetastore/documentmetastore_test.cpp index 9ca22bafe9a..60d96e37e15 100644 --- a/searchcore/src/tests/proton/documentmetastore/documentmetastore_test.cpp +++ b/searchcore/src/tests/proton/documentmetastore/documentmetastore_test.cpp @@ -2090,7 +2090,7 @@ public: size_t remove_batch_cnt; size_t remove_cnt; - MockOperationListener() + MockOperationListener() noexcept : remove_batch_cnt(0), remove_cnt(0) { diff --git a/searchcore/src/tests/proton/flushengine/flushengine_test.cpp b/searchcore/src/tests/proton/flushengine/flushengine_test.cpp index 08fa1b11229..38fca35ea87 100644 --- a/searchcore/src/tests/proton/flushengine/flushengine_test.cpp +++ b/searchcore/src/tests/proton/flushengine/flushengine_test.cpp @@ -361,14 +361,14 @@ public: public: typedef std::shared_ptr SP; - SimpleStrategy() {} + SimpleStrategy() noexcept : _targets() {} uint32_t indexOf(const IFlushTarget::SP &target) const { IFlushTarget *raw = target.get(); CachedFlushTarget *cached = dynamic_cast(raw); - if (cached != NULL) { + if (cached != nullptr) { raw = cached->getFlushTarget().get(); } WrappedFlushTarget *wrapped = dynamic_cast(raw); diff --git a/searchcore/src/tests/proton/flushengine/prepare_restart_flush_strategy/prepare_restart_flush_strategy_test.cpp b/searchcore/src/tests/proton/flushengine/prepare_restart_flush_strategy/prepare_restart_flush_strategy_test.cpp index 5f93f97f165..1b8be388c13 100644 --- a/searchcore/src/tests/proton/flushengine/prepare_restart_flush_strategy/prepare_restart_flush_strategy_test.cpp +++ b/searchcore/src/tests/proton/flushengine/prepare_restart_flush_strategy/prepare_restart_flush_strategy_test.cpp @@ -27,16 +27,16 @@ struct SimpleFlushTarget : public test::DummyFlushTarget const Type &type, SerialNum flushedSerial_, uint64_t approxDiskBytes_, - double replay_operation_cost_) + double replay_operation_cost_) noexcept : test::DummyFlushTarget(name, type, Component::OTHER), flushedSerial(flushedSerial_), approxDiskBytes(approxDiskBytes_), replay_operation_cost(replay_operation_cost_) {} - virtual SerialNum getFlushedSerialNum() const override { + SerialNum getFlushedSerialNum() const override { return flushedSerial; } - virtual uint64_t getApproxBytesToWriteToDisk() const override { + uint64_t getApproxBytesToWriteToDisk() const override { return approxDiskBytes; } double get_replay_operation_cost() const override { diff --git a/searchcore/src/tests/proton/flushengine/shrink_lid_space_flush_target/shrink_lid_space_flush_target_test.cpp b/searchcore/src/tests/proton/flushengine/shrink_lid_space_flush_target/shrink_lid_space_flush_target_test.cpp index 964cd47afa5..6c502bccce1 100644 --- a/searchcore/src/tests/proton/flushengine/shrink_lid_space_flush_target/shrink_lid_space_flush_target_test.cpp +++ b/searchcore/src/tests/proton/flushengine/shrink_lid_space_flush_target/shrink_lid_space_flush_target_test.cpp @@ -18,26 +18,26 @@ class MyLidSpace : public search::common::ICompactableLidSpace bool _canShrink; size_t _canFree; public: - MyLidSpace() + MyLidSpace() noexcept : _canShrink(true), _canFree(16) { } - virtual ~MyLidSpace() override {} + ~MyLidSpace() override = default; - virtual void compactLidSpace(uint32_t wantedDocLidLimit) override { + void compactLidSpace(uint32_t wantedDocLidLimit) override { (void) wantedDocLidLimit; } - virtual bool canShrinkLidSpace() const override { + bool canShrinkLidSpace() const override { return _canShrink; } - virtual size_t getEstimatedShrinkLidSpaceGain() const override { + size_t getEstimatedShrinkLidSpaceGain() const override { return _canShrink ? _canFree : 0; } - virtual void shrinkLidSpace() override { + void shrinkLidSpace() override { if (_canShrink) { _canFree = 0; _canShrink = false; @@ -62,9 +62,11 @@ struct Fixture { } - ~Fixture() { } + ~Fixture(); }; +Fixture::~Fixture() = default; + TEST_F("require that flush target returns estimated memory gain", Fixture) { auto memoryGain = f._ft->getApproxMemoryGain(); diff --git a/searchcore/src/tests/proton/matching/matching_test.cpp b/searchcore/src/tests/proton/matching/matching_test.cpp index 22dc19167f6..30f83273bd7 100644 --- a/searchcore/src/tests/proton/matching/matching_test.cpp +++ b/searchcore/src/tests/proton/matching/matching_test.cpp @@ -284,7 +284,7 @@ struct MyWorld { struct MySearchHandler : ISearchHandler { Matcher::SP _matcher; - MySearchHandler(Matcher::SP matcher) : _matcher(matcher) {} + MySearchHandler(Matcher::SP matcher) noexcept : _matcher(std::move(matcher)) {} DocsumReply::UP getDocsums(const DocsumRequest &) override { return DocsumReply::UP(); diff --git a/searchcore/src/tests/proton/reference/document_db_reference_resolver/document_db_reference_resolver_test.cpp b/searchcore/src/tests/proton/reference/document_db_reference_resolver/document_db_reference_resolver_test.cpp index 48b2a269f6b..bacdd85dcc5 100644 --- a/searchcore/src/tests/proton/reference/document_db_reference_resolver/document_db_reference_resolver_test.cpp +++ b/searchcore/src/tests/proton/reference/document_db_reference_resolver/document_db_reference_resolver_test.cpp @@ -61,7 +61,7 @@ struct MyDocumentDBReference : public MockDocumentDBReference { std::shared_ptr _gidToLidChangeHandler; MyDocumentDBReference(MyGidToLidMapperFactory::SP factory_, - std::shared_ptr gidToLidChangeHandler) + std::shared_ptr gidToLidChangeHandler) noexcept : factory(std::move(factory_)), _gidToLidChangeHandler(std::move(gidToLidChangeHandler)) { diff --git a/searchcore/src/tests/proton/reference/gid_to_lid_change_handler/gid_to_lid_change_handler_test.cpp b/searchcore/src/tests/proton/reference/gid_to_lid_change_handler/gid_to_lid_change_handler_test.cpp index 920472c6a01..a10d48ee7fe 100644 --- a/searchcore/src/tests/proton/reference/gid_to_lid_change_handler/gid_to_lid_change_handler_test.cpp +++ b/searchcore/src/tests/proton/reference/gid_to_lid_change_handler/gid_to_lid_change_handler_test.cpp @@ -39,7 +39,7 @@ class ListenerStats { uint32_t _destroyedListeners; public: - ListenerStats() + ListenerStats() noexcept : _lock(), _putChanges(0u), _removeChanges(0u), diff --git a/searchcore/src/tests/proton/reprocessing/reprocessing_runner/reprocessing_runner_test.cpp b/searchcore/src/tests/proton/reprocessing/reprocessing_runner/reprocessing_runner_test.cpp index 0053a03105f..242ad91271e 100644 --- a/searchcore/src/tests/proton/reprocessing/reprocessing_runner/reprocessing_runner_test.cpp +++ b/searchcore/src/tests/proton/reprocessing/reprocessing_runner/reprocessing_runner_test.cpp @@ -32,7 +32,7 @@ struct MyTask : public IReprocessingTask double initProgress, double middleProgress, double finalProgress, - double weight) + double weight) noexcept : _runner(runner), _initProgress(initProgress), _middleProgress(middleProgress), @@ -42,9 +42,7 @@ struct MyTask : public IReprocessingTask { } - virtual void - run() override - { + void run() override { ASSERT_EQUAL(_initProgress, _runner.getProgress()); _myProgress = 0.5; ASSERT_EQUAL(_middleProgress, _runner.getProgress()); @@ -52,9 +50,7 @@ struct MyTask : public IReprocessingTask ASSERT_EQUAL(_finalProgress, _runner.getProgress()); } - virtual Progress - getProgress() const override - { + Progress getProgress() const override { return Progress(_myProgress, _weight); } @@ -65,11 +61,7 @@ struct MyTask : public IReprocessingTask double finalProgress, double weight) { - return std::make_shared(runner, - initProgress, - middleProgress, - finalProgress, - weight); + return std::make_shared(runner, initProgress, middleProgress, finalProgress, weight); } }; @@ -77,16 +69,8 @@ TEST_F("require that progress is calculated when tasks are executed", Fixture) { TaskList tasks; EXPECT_EQUAL(0.0, f._runner.getProgress()); - tasks.push_back(MyTask::create(f._runner, - 0.0, - 0.1, - 0.2, - 1.0)); - tasks.push_back(MyTask::create(f._runner, - 0.2, - 0.6, - 1.0, - 4.0)); + tasks.push_back(MyTask::create(f._runner, 0.0, 0.1, 0.2, 1.0)); + tasks.push_back(MyTask::create(f._runner, 0.2, 0.6, 1.0, 4.0)); f._runner.addTasks(tasks); tasks.clear(); EXPECT_EQUAL(0.0, f._runner.getProgress()); @@ -99,11 +83,7 @@ TEST_F("require that runner can be reset", Fixture) { TaskList tasks; EXPECT_EQUAL(0.0, f._runner.getProgress()); - tasks.push_back(MyTask::create(f._runner, - 0.0, - 0.5, - 1.0, - 1.0)); + tasks.push_back(MyTask::create(f._runner, 0.0, 0.5, 1.0, 1.0)); f._runner.addTasks(tasks); tasks.clear(); EXPECT_EQUAL(0.0, f._runner.getProgress()); @@ -111,21 +91,13 @@ TEST_F("require that runner can be reset", Fixture) EXPECT_EQUAL(1.0, f._runner.getProgress()); f._runner.reset(); EXPECT_EQUAL(0.0, f._runner.getProgress()); - tasks.push_back(MyTask::create(f._runner, - 0.0, - 0.5, - 1.0, - 1.0)); + tasks.push_back(MyTask::create(f._runner, 0.0, 0.5, 1.0, 1.0)); f._runner.addTasks(tasks); tasks.clear(); EXPECT_EQUAL(0.0, f._runner.getProgress()); f._runner.reset(); EXPECT_EQUAL(0.0, f._runner.getProgress()); - tasks.push_back(MyTask::create(f._runner, - 0.0, - 0.5, - 1.0, - 4.0)); + tasks.push_back(MyTask::create(f._runner, 0.0, 0.5, 1.0, 4.0)); f._runner.addTasks(tasks); tasks.clear(); EXPECT_EQUAL(0.0, f._runner.getProgress()); diff --git a/searchcore/src/tests/proton/server/feedstates_test.cpp b/searchcore/src/tests/proton/server/feedstates_test.cpp index 42d88328d14..59e504de6ce 100644 --- a/searchcore/src/tests/proton/server/feedstates_test.cpp +++ b/searchcore/src/tests/proton/server/feedstates_test.cpp @@ -124,7 +124,7 @@ TEST_F("require that replay progress is tracked", Fixture) { RemoveOperationContext opCtx(10); TlsReplayProgress progress("test", 5, 15); - PacketWrapper::SP wrap(new PacketWrapper(*opCtx.packet, &progress)); + auto wrap = std::make_shared(*opCtx.packet, &progress); ForegroundThreadExecutor executor; f.state.receive(wrap, executor); diff --git a/searchcore/src/tests/proton/server/memoryflush/memoryflush_test.cpp b/searchcore/src/tests/proton/server/memoryflush/memoryflush_test.cpp index 7f650847183..305f5d8c9ba 100644 --- a/searchcore/src/tests/proton/server/memoryflush/memoryflush_test.cpp +++ b/searchcore/src/tests/proton/server/memoryflush/memoryflush_test.cpp @@ -22,19 +22,13 @@ typedef IFlushTarget::DiskGain DiskGain; class MyFlushHandler : public IFlushHandler { public: - MyFlushHandler(const vespalib::string &name) : IFlushHandler(name) {} - // Implements IFlushHandler - virtual std::vector getFlushTargets() override { + MyFlushHandler(const vespalib::string &name) noexcept : IFlushHandler(name) {} + std::vector getFlushTargets() override { return std::vector(); } - virtual SerialNum getCurrentSerialNumber() const override { return 0; } - virtual void flushDone(SerialNum oldestSerial) override { (void) oldestSerial; } - - virtual void - syncTls(search::SerialNum syncTo) override - { - (void) syncTo; - } + SerialNum getCurrentSerialNumber() const override { return 0; } + void flushDone(SerialNum oldestSerial) override { (void) oldestSerial; } + void syncTls(search::SerialNum syncTo) override {(void) syncTo;} }; class MyFlushTarget : public test::DummyFlushTarget { @@ -47,7 +41,7 @@ private: public: MyFlushTarget(const vespalib::string &name, MemoryGain memoryGain, DiskGain diskGain, SerialNum flushedSerial, - system_time lastFlushTime, bool urgentFlush) : + system_time lastFlushTime, bool urgentFlush) noexcept : test::DummyFlushTarget(name), _memoryGain(memoryGain), _diskGain(diskGain), @@ -56,12 +50,11 @@ public: _urgentFlush(urgentFlush) { } - // Implements IFlushTarget - virtual MemoryGain getApproxMemoryGain() const override { return _memoryGain; } - virtual DiskGain getApproxDiskGain() const override { return _diskGain; } - virtual SerialNum getFlushedSerialNum() const override { return _flushedSerial; } - virtual system_time getLastFlushTime() const override { return _lastFlushTime; } - virtual bool needUrgentFlush() const override { return _urgentFlush; } + MemoryGain getApproxMemoryGain() const override { return _memoryGain; } + DiskGain getApproxDiskGain() const override { return _diskGain; } + SerialNum getFlushedSerialNum() const override { return _flushedSerial; } + system_time getLastFlushTime() const override { return _lastFlushTime; } + bool needUrgentFlush() const override { return _urgentFlush; } }; struct StringList : public std::vector { diff --git a/searchcore/src/vespa/searchcore/proton/attribute/attribute_populator.cpp b/searchcore/src/vespa/searchcore/proton/attribute/attribute_populator.cpp index 5208195a968..33a5776cb8a 100644 --- a/searchcore/src/vespa/searchcore/proton/attribute/attribute_populator.cpp +++ b/searchcore/src/vespa/searchcore/proton/attribute/attribute_populator.cpp @@ -18,7 +18,7 @@ class PopulateDoneContext : public IDestructorCallback { std::shared_ptr _doc; public: - PopulateDoneContext(std::shared_ptr doc) + PopulateDoneContext(std::shared_ptr doc) noexcept : _doc(std::move(doc)) { } diff --git a/searchcore/src/vespa/searchcore/proton/common/doctypename.cpp b/searchcore/src/vespa/searchcore/proton/common/doctypename.cpp index 21b27ef6ffd..4d76018d66b 100644 --- a/searchcore/src/vespa/searchcore/proton/common/doctypename.cpp +++ b/searchcore/src/vespa/searchcore/proton/common/doctypename.cpp @@ -7,12 +7,12 @@ namespace proton { -DocTypeName::DocTypeName(const search::engine::Request &request) +DocTypeName::DocTypeName(const search::engine::Request &request) noexcept : _name(request.propertiesMap.matchProperties().lookup("documentdb", "searchdoctype").get("")) {} -DocTypeName::DocTypeName(const document::DocumentType &docType) +DocTypeName::DocTypeName(const document::DocumentType &docType) noexcept : _name(docType.getName()) {} diff --git a/searchcore/src/vespa/searchcore/proton/common/doctypename.h b/searchcore/src/vespa/searchcore/proton/common/doctypename.h index 661d54ab8d4..f0fa3c72ee9 100644 --- a/searchcore/src/vespa/searchcore/proton/common/doctypename.h +++ b/searchcore/src/vespa/searchcore/proton/common/doctypename.h @@ -13,9 +13,9 @@ class DocTypeName vespalib::string _name; public: - explicit DocTypeName(const vespalib::string &name) : _name(name) { } - explicit DocTypeName(const search::engine::Request &request); - explicit DocTypeName(const document::DocumentType &docType); + explicit DocTypeName(const vespalib::string &name) noexcept : _name(name) { } + explicit DocTypeName(const search::engine::Request &request) noexcept; + explicit DocTypeName(const document::DocumentType &docType) noexcept; const vespalib::string & getName() const { return _name; } diff --git a/searchcore/src/vespa/searchcore/proton/documentmetastore/gid_compare.h b/searchcore/src/vespa/searchcore/proton/documentmetastore/gid_compare.h index dd32abcab6e..c16dd8a7292 100644 --- a/searchcore/src/vespa/searchcore/proton/documentmetastore/gid_compare.h +++ b/searchcore/src/vespa/searchcore/proton/documentmetastore/gid_compare.h @@ -14,7 +14,7 @@ class IGidCompare public: typedef std::shared_ptr SP; - virtual ~IGidCompare() {} + virtual ~IGidCompare() = default; virtual bool operator()(const document::GlobalId &lhs, const document::GlobalId &rhs) const = 0; @@ -30,14 +30,13 @@ private: document::GlobalId::BucketOrderCmp _comp; public: - DefaultGidCompare() + DefaultGidCompare() noexcept : IGidCompare(), _comp() { } - virtual bool operator()(const document::GlobalId &lhs, - const document::GlobalId &rhs) const override { + bool operator()(const document::GlobalId &lhs, const document::GlobalId &rhs) const override { return _comp(lhs, rhs); } }; diff --git a/searchcore/src/vespa/searchcore/proton/flushengine/iflushhandler.h b/searchcore/src/vespa/searchcore/proton/flushengine/iflushhandler.h index 914de9df30c..ca23f27ed49 100644 --- a/searchcore/src/vespa/searchcore/proton/flushengine/iflushhandler.h +++ b/searchcore/src/vespa/searchcore/proton/flushengine/iflushhandler.h @@ -28,7 +28,7 @@ public: * * @param name The unique name of this handler. */ - IFlushHandler(const vespalib::string &name) + IFlushHandler(const vespalib::string &name) noexcept : _name(name) { } diff --git a/searchcore/src/vespa/searchcore/proton/initializer/task_runner.h b/searchcore/src/vespa/searchcore/proton/initializer/task_runner.h index f28c46334bc..bd0d82b0b2c 100644 --- a/searchcore/src/vespa/searchcore/proton/initializer/task_runner.h +++ b/searchcore/src/vespa/searchcore/proton/initializer/task_runner.h @@ -28,7 +28,7 @@ class TaskRunner { using SP = std::shared_ptr; Context(InitializerTask::SP rootTask, vespalib::Executor &contextExecutor, - vespalib::Executor::Task::UP doneTask) + vespalib::Executor::Task::UP doneTask) noexcept : _rootTask(rootTask), _contextExecutor(contextExecutor), _doneTask(std::move(doneTask)) diff --git a/searchcore/src/vespa/searchcore/proton/matching/sessionmanager.h b/searchcore/src/vespa/searchcore/proton/matching/sessionmanager.h index 96c83270735..0cff5583bbd 100644 --- a/searchcore/src/vespa/searchcore/proton/matching/sessionmanager.h +++ b/searchcore/src/vespa/searchcore/proton/matching/sessionmanager.h @@ -37,7 +37,7 @@ public: vespalib::steady_time doom; SearchSessionInfo(const vespalib::string &id_in, vespalib::steady_time created_in, - vespalib::steady_time doom_in) + vespalib::steady_time doom_in) noexcept : id(id_in), created(created_in), doom(doom_in) {} }; diff --git a/searchcore/src/vespa/searchcore/proton/server/feedhandler.cpp b/searchcore/src/vespa/searchcore/proton/server/feedhandler.cpp index 174ba090842..a52be734c13 100644 --- a/searchcore/src/vespa/searchcore/proton/server/feedhandler.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/feedhandler.cpp @@ -111,7 +111,7 @@ TlsMgrWriter::sync(SerialNum syncTo) class OnCommitDone : public search::IDestructorCallback { public: - OnCommitDone(Executor & executor, std::unique_ptr task) + OnCommitDone(Executor & executor, std::unique_ptr task) noexcept : _executor(executor), _task(std::move(task)) {} diff --git a/searchcore/src/vespa/searchcore/proton/server/feedstates.h b/searchcore/src/vespa/searchcore/proton/server/feedstates.h index 0c2e9109cce..3d24f4068a2 100644 --- a/searchcore/src/vespa/searchcore/proton/server/feedstates.h +++ b/searchcore/src/vespa/searchcore/proton/server/feedstates.h @@ -24,7 +24,7 @@ class InitState : public FeedState { vespalib::string _doc_type_name; public: - InitState(const vespalib::string &name) + InitState(const vespalib::string &name) noexcept : FeedState(INIT), _doc_type_name(name) { @@ -73,7 +73,7 @@ class NormalState : public FeedState { FeedHandler &_handler; public: - NormalState(FeedHandler &handler) + NormalState(FeedHandler &handler) noexcept : FeedState(NORMAL), _handler(handler) { } diff --git a/searchcore/src/vespa/searchcore/proton/server/move_operation_limiter.cpp b/searchcore/src/vespa/searchcore/proton/server/move_operation_limiter.cpp index e535b05393c..c84abb373f8 100644 --- a/searchcore/src/vespa/searchcore/proton/server/move_operation_limiter.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/move_operation_limiter.cpp @@ -10,7 +10,7 @@ using BlockedReason = IBlockableMaintenanceJob::BlockedReason; struct MoveOperationLimiter::Callback : public search::IDestructorCallback { MoveOperationLimiter::SP _limiter; - Callback(MoveOperationLimiter::SP limiter) : _limiter(std::move(limiter)) {} + Callback(MoveOperationLimiter::SP limiter) noexcept : _limiter(std::move(limiter)) {} virtual ~Callback() { _limiter->endOperation(); } }; diff --git a/searchcore/src/vespa/searchcore/proton/server/packetwrapper.h b/searchcore/src/vespa/searchcore/proton/server/packetwrapper.h index c36652ec847..6f132604662 100644 --- a/searchcore/src/vespa/searchcore/proton/server/packetwrapper.h +++ b/searchcore/src/vespa/searchcore/proton/server/packetwrapper.h @@ -20,8 +20,7 @@ struct PacketWrapper { search::transactionlog::client::RPC::Result result; vespalib::Gate gate; - PacketWrapper(const search::transactionlog::Packet &p, - TlsReplayProgress *progress_) + PacketWrapper(const search::transactionlog::Packet &p, TlsReplayProgress *progress_) noexcept : packet(p), progress(progress_), result(search::transactionlog::client::RPC::ERROR), diff --git a/searchcore/src/vespa/searchcore/proton/server/rpc_hooks.cpp b/searchcore/src/vespa/searchcore/proton/server/rpc_hooks.cpp index c724e1065e9..62ffdb3e5a8 100644 --- a/searchcore/src/vespa/searchcore/proton/server/rpc_hooks.cpp +++ b/searchcore/src/vespa/searchcore/proton/server/rpc_hooks.cpp @@ -19,7 +19,7 @@ namespace { struct Pair { string key; string value; - Pair(const string &k, const string &v) + Pair(const string &k, const string &v) noexcept : key(k), value(v) { diff --git a/searchcore/src/vespa/searchcore/proton/test/document_meta_store_observer.h b/searchcore/src/vespa/searchcore/proton/test/document_meta_store_observer.h index 66cc5ed0fea..a6ff6d86d0d 100644 --- a/searchcore/src/vespa/searchcore/proton/test/document_meta_store_observer.h +++ b/searchcore/src/vespa/searchcore/proton/test/document_meta_store_observer.h @@ -15,38 +15,36 @@ struct DocumentMetaStoreObserver : public IDocumentMetaStore DocId _compactLidSpaceLidLimit; uint32_t _holdUnblockShrinkLidSpaceCnt; - DocumentMetaStoreObserver(IDocumentMetaStore &store) + DocumentMetaStoreObserver(IDocumentMetaStore &store) noexcept : _store(store), _removeCompleteCnt(0), _removeCompleteLid(0), _compactLidSpaceLidLimit(0), _holdUnblockShrinkLidSpaceCnt(0) - { - } + {} /** * Implements search::IDocumentMetaStore **/ - virtual bool getGid(DocId lid, GlobalId &gid) const override { + bool getGid(DocId lid, GlobalId &gid) const override { return _store.getGid(lid, gid); } - virtual bool getGidEvenIfMoved(DocId lid, GlobalId &gid) const override { + bool getGidEvenIfMoved(DocId lid, GlobalId &gid) const override { return _store.getGidEvenIfMoved(lid, gid); } - virtual bool getLid(const GlobalId &gid, DocId &lid) const override { + bool getLid(const GlobalId &gid, DocId &lid) const override { return _store.getLid(gid, lid); } - virtual search::DocumentMetaData getMetaData(const GlobalId &gid) const override { + search::DocumentMetaData getMetaData(const GlobalId &gid) const override { return _store.getMetaData(gid); } - virtual void getMetaData(const BucketId &bucketId, - search::DocumentMetaData::Vector &result) const override { + void getMetaData(const BucketId &bucketId, search::DocumentMetaData::Vector &result) const override { _store.getMetaData(bucketId, result); } - virtual search::LidUsageStats getLidUsageStats() const override { + search::LidUsageStats getLidUsageStats() const override { return _store.getLidUsageStats(); } - virtual search::queryeval::Blueprint::UP createWhiteListBlueprint() const override { + search::queryeval::Blueprint::UP createWhiteListBlueprint() const override { return _store.createWhiteListBlueprint(); } uint64_t getCurrentGeneration() const override { @@ -57,40 +55,39 @@ struct DocumentMetaStoreObserver : public IDocumentMetaStore /** * Implements documentmetastore::IStore. */ - virtual Result inspectExisting(const GlobalId &gid, uint64_t prepare_serial_num) override { + Result inspectExisting(const GlobalId &gid, uint64_t prepare_serial_num) override { return _store.inspectExisting(gid, prepare_serial_num); } - virtual Result inspect(const GlobalId &gid, uint64_t prepare_serial_num) override { + Result inspect(const GlobalId &gid, uint64_t prepare_serial_num) override { return _store.inspect(gid, prepare_serial_num); } - virtual Result put(const GlobalId &gid, - const BucketId &bucketId, - const Timestamp ×tamp, - uint32_t docSize, - DocId lid, - uint64_t prepare_serial_num) override { + Result put(const GlobalId &gid, + const BucketId &bucketId, + const Timestamp ×tamp, + uint32_t docSize, + DocId lid, + uint64_t prepare_serial_num) override + { return _store.put(gid, bucketId, timestamp, docSize, lid, prepare_serial_num); } - virtual bool updateMetaData(DocId lid, - const BucketId &bucketId, - const Timestamp ×tamp) override { + bool updateMetaData(DocId lid, const BucketId &bucketId, const Timestamp ×tamp) override { return _store.updateMetaData(lid, bucketId, timestamp); } - virtual bool remove(DocId lid, uint64_t prepare_serial_num) override { + bool remove(DocId lid, uint64_t prepare_serial_num) override { return _store.remove(lid, prepare_serial_num); } - virtual void removeComplete(DocId lid) override { + void removeComplete(DocId lid) override { ++_removeCompleteCnt; _removeCompleteLid = lid; _store.removeComplete(lid); } - virtual void move(DocId fromLid, DocId toLid, uint64_t prepare_serial_num) override { + void move(DocId fromLid, DocId toLid, uint64_t prepare_serial_num) override { _store.move(fromLid, toLid, prepare_serial_num); } - virtual bool validLid(DocId lid) const override { + bool validLid(DocId lid) const override { return _store.validLid(lid); } - virtual void removeBatch(const std::vector &lidsToRemove, + void removeBatch(const std::vector &lidsToRemove, const DocId docIdLimit) override { _store.removeBatch(lidsToRemove, docIdLimit); } diff --git a/searchcore/src/vespa/searchcore/proton/test/dummy_flush_handler.h b/searchcore/src/vespa/searchcore/proton/test/dummy_flush_handler.h index 53fa8a9c3fb..af2f4c63443 100644 --- a/searchcore/src/vespa/searchcore/proton/test/dummy_flush_handler.h +++ b/searchcore/src/vespa/searchcore/proton/test/dummy_flush_handler.h @@ -3,37 +3,32 @@ #include -namespace proton { - -namespace test { +namespace proton::test { /** * Default implementation used for testing. */ struct DummyFlushHandler : public IFlushHandler { - DummyFlushHandler(const vespalib::string &name) + DummyFlushHandler(const vespalib::string &name) noexcept : IFlushHandler(name) {} - // Implements IFlushHandler - virtual std::vector getFlushTargets() override { + std::vector getFlushTargets() override { return std::vector(); } - virtual SerialNum getCurrentSerialNumber() const override { + SerialNum getCurrentSerialNumber() const override { return 0; } - virtual void flushDone(SerialNum oldestSerial) override { + void flushDone(SerialNum oldestSerial) override { (void) oldestSerial; } - virtual void syncTls(SerialNum syncTo) override { + void syncTls(SerialNum syncTo) override { (void) syncTo; } }; -} // namespace test - -} // namespace proton +} diff --git a/searchcore/src/vespa/searchcore/proton/test/dummy_flush_target.h b/searchcore/src/vespa/searchcore/proton/test/dummy_flush_target.h index 8e291126430..dd3cd49df2f 100644 --- a/searchcore/src/vespa/searchcore/proton/test/dummy_flush_target.h +++ b/searchcore/src/vespa/searchcore/proton/test/dummy_flush_target.h @@ -3,39 +3,33 @@ #include -namespace proton { - -namespace test { +namespace proton::test { struct DummyFlushTarget : public searchcorespi::IFlushTarget { - DummyFlushTarget(const vespalib::string &name) + DummyFlushTarget(const vespalib::string &name) noexcept : searchcorespi::IFlushTarget(name) {} DummyFlushTarget(const vespalib::string &name, const Type &type, - const Component &component) + const Component &component) noexcept : searchcorespi::IFlushTarget(name, type, component) {} - // Implements searchcorespi::IFlushTarget - virtual MemoryGain getApproxMemoryGain() const override { return MemoryGain(0, 0); } - virtual DiskGain getApproxDiskGain() const override { return DiskGain(0, 0); } - virtual SerialNum getFlushedSerialNum() const override { return 0; } - virtual Time getLastFlushTime() const override { return Time(); } - virtual bool needUrgentFlush() const override { return false; } - virtual searchcorespi::FlushTask::UP initFlush(SerialNum) override { + MemoryGain getApproxMemoryGain() const override { return MemoryGain(0, 0); } + DiskGain getApproxDiskGain() const override { return DiskGain(0, 0); } + SerialNum getFlushedSerialNum() const override { return 0; } + Time getLastFlushTime() const override { return Time(); } + bool needUrgentFlush() const override { return false; } + searchcorespi::FlushTask::UP initFlush(SerialNum) override { return searchcorespi::FlushTask::UP(); } - virtual searchcorespi::FlushStats getLastFlushStats() const override { + searchcorespi::FlushStats getLastFlushStats() const override { return searchcorespi::FlushStats(); } - virtual uint64_t getApproxBytesToWriteToDisk() const override { + uint64_t getApproxBytesToWriteToDisk() const override { return 0; } }; -} // namespace test - -} // namespace proton - +} diff --git a/searchcore/src/vespa/searchcore/proton/test/mock_gid_to_lid_change_handler.h b/searchcore/src/vespa/searchcore/proton/test/mock_gid_to_lid_change_handler.h index e808f721e83..f9531486e9b 100644 --- a/searchcore/src/vespa/searchcore/proton/test/mock_gid_to_lid_change_handler.h +++ b/searchcore/src/vespa/searchcore/proton/test/mock_gid_to_lid_change_handler.h @@ -23,7 +23,7 @@ private: std::vector> _listeners; public: - MockGidToLidChangeHandler() + MockGidToLidChangeHandler() noexcept : IGidToLidChangeHandler(), _adds(), _removes(), @@ -31,7 +31,7 @@ public: { } - ~MockGidToLidChangeHandler() override { } + ~MockGidToLidChangeHandler() override = default; void addListener(std::unique_ptr listener) override { _adds.emplace_back(listener->getDocTypeName(), listener->getName()); diff --git a/searchcorespi/src/vespa/searchcorespi/flush/iflushtarget.h b/searchcorespi/src/vespa/searchcorespi/flush/iflushtarget.h index 10ed19a7244..3a67333c9d5 100644 --- a/searchcorespi/src/vespa/searchcorespi/flush/iflushtarget.h +++ b/searchcorespi/src/vespa/searchcorespi/flush/iflushtarget.h @@ -45,8 +45,8 @@ public: template class Gain { public: - Gain() : _before(0), _after(0) { } - Gain(T before, T after) : _before(before), _after(after) { } + Gain() noexcept : _before(0), _after(0) { } + Gain(T before, T after) noexcept : _before(before), _after(after) { } T getBefore() const { return _before; } T getAfter() const { return _after; } T gain() const { return _before - _after; } @@ -74,7 +74,7 @@ public: * * @param name The handler-wide unique name of this target. */ - IFlushTarget(const vespalib::string &name) + IFlushTarget(const vespalib::string &name) noexcept : _name(name), _type(Type::OTHER), _component(Component::OTHER) @@ -89,7 +89,7 @@ public: */ IFlushTarget(const vespalib::string &name, const Type &type, - const Component &component) + const Component &component) noexcept : _name(name), _type(type), _component(component) @@ -98,7 +98,7 @@ public: /** * Virtual destructor required for inheritance. */ - virtual ~IFlushTarget() { } + virtual ~IFlushTarget() = default; /** * Returns the handler-wide unique name of this target. diff --git a/searchlib/src/apps/vespa-ranking-expression-analyzer/vespa-ranking-expression-analyzer.cpp b/searchlib/src/apps/vespa-ranking-expression-analyzer/vespa-ranking-expression-analyzer.cpp index 65b27dd411a..ae4b55948dc 100644 --- a/searchlib/src/apps/vespa-ranking-expression-analyzer/vespa-ranking-expression-analyzer.cpp +++ b/searchlib/src/apps/vespa-ranking-expression-analyzer/vespa-ranking-expression-analyzer.cpp @@ -62,8 +62,9 @@ struct InputInfo { std::vector cmp_with; double usage_probability; double expected_usage; - InputInfo(vespalib::stringref name_in, double usage_probability_in, double expected_usage_in) - : name(name_in), cmp_with(), usage_probability(usage_probability_in), expected_usage(expected_usage_in) {} + InputInfo(vespalib::stringref name_in, double usage_probability_in, double expected_usage_in) noexcept + : name(name_in), cmp_with(), usage_probability(usage_probability_in), expected_usage(expected_usage_in) + {} double select_value() const { return cmp_with.empty() ? 0.5 : cmp_with[(cmp_with.size()-1)/2]; } diff --git a/searchlib/src/tests/attribute/posting_list_merger/posting_list_merger_test.cpp b/searchlib/src/tests/attribute/posting_list_merger/posting_list_merger_test.cpp index 99b22e02e2e..5976b02c5cf 100644 --- a/searchlib/src/tests/attribute/posting_list_merger/posting_list_merger_test.cpp +++ b/searchlib/src/tests/attribute/posting_list_merger/posting_list_merger_test.cpp @@ -11,17 +11,17 @@ using search::attribute::PostingListMerger; struct Posting { uint32_t lid; int32_t weight; - Posting(uint32_t lid_, int32_t weight_) + Posting(uint32_t lid_, int32_t weight_) noexcept : lid(lid_), weight(weight_) { } - bool operator==(const Posting &rhs) const { + bool operator==(const Posting &rhs) const noexcept { return ((lid == rhs.lid) && (weight == rhs.weight)); } - bool operator<(const Posting &rhs) const { return lid < rhs.lid; } + bool operator<(const Posting &rhs) const noexcept { return lid < rhs.lid; } }; std::ostream &operator<<(std::ostream &os, const Posting &posting) @@ -35,11 +35,11 @@ class WeightedPostingList { std::vector _entries; public: - WeightedPostingList(std::vector entries) + WeightedPostingList(std::vector entries) noexcept : _entries(std::move(entries)) { } - ~WeightedPostingList() { } + ~WeightedPostingList() = default; template void foreach(Func func) const { @@ -67,7 +67,7 @@ struct WeightedFixture { } - ~WeightedFixture() { } + ~WeightedFixture() = default; void reserveArray(uint32_t postingsCount, size_t postingsSize) { _merger.reserveArray(postingsCount, postingsSize); } diff --git a/searchlib/src/tests/attribute/searchable/attribute_searchable_adapter_test.cpp b/searchlib/src/tests/attribute/searchable/attribute_searchable_adapter_test.cpp index 87aea2e3e8c..7328fe2c7ff 100644 --- a/searchlib/src/tests/attribute/searchable/attribute_searchable_adapter_test.cpp +++ b/searchlib/src/tests/attribute/searchable/attribute_searchable_adapter_test.cpp @@ -140,8 +140,9 @@ struct Result { uint32_t docid; double raw_score; int32_t match_weight; - Hit(uint32_t id, double raw, int32_t match_weight_in) - : docid(id), raw_score(raw), match_weight(match_weight_in) {} + Hit(uint32_t id, double raw, int32_t match_weight_in) noexcept + : docid(id), raw_score(raw), match_weight(match_weight_in) + {} }; size_t est_hits; bool est_empty; @@ -590,7 +591,7 @@ TEST("require that attribute weighted set term works") { TEST("require that predicate query in non-predicate field yields empty.") { MyAttributeManager attribute_manager = makeAttributeManager("foo"); - PredicateQueryTerm::UP term(new PredicateQueryTerm); + auto term = std::make_unique(); SimplePredicateQuery node(std::move(term), field, 0, Weight(1)); Result result = do_search(attribute_manager, node, true); EXPECT_TRUE(result.est_empty); @@ -605,7 +606,7 @@ TEST("require that predicate query in predicate field yields results.") { const_cast(attr->getIntervalRangeVector())[2] = 1u; MyAttributeManager attribute_manager(attr); - PredicateQueryTerm::UP term(new PredicateQueryTerm); + auto term = std::make_unique(); SimplePredicateQuery node(std::move(term), field, 0, Weight(1)); Result result = do_search(attribute_manager, node, true); EXPECT_FALSE(result.est_empty); diff --git a/searchlib/src/tests/memoryindex/field_index_remover/field_index_remover_test.cpp b/searchlib/src/tests/memoryindex/field_index_remover/field_index_remover_test.cpp index af7ea9be481..584a8121b16 100644 --- a/searchlib/src/tests/memoryindex/field_index_remover/field_index_remover_test.cpp +++ b/searchlib/src/tests/memoryindex/field_index_remover/field_index_remover_test.cpp @@ -16,10 +16,10 @@ using namespace search::memoryindex; struct WordFieldPair { vespalib::string _word; uint32_t _fieldId; - WordFieldPair(vespalib::stringref word, uint32_t fieldId) + WordFieldPair(vespalib::stringref word, uint32_t fieldId) noexcept : _word(word), _fieldId(fieldId) {} - bool operator<(const WordFieldPair &rhs) const { + bool operator<(const WordFieldPair &rhs) const noexcept { if (_word != rhs._word) { return _word < rhs._word; } diff --git a/searchlib/src/tests/query/querybuilder_test.cpp b/searchlib/src/tests/query/querybuilder_test.cpp index 269600d26d4..5a5a5eafb2c 100644 --- a/searchlib/src/tests/query/querybuilder_test.cpp +++ b/searchlib/src/tests/query/querybuilder_test.cpp @@ -38,7 +38,7 @@ const uint32_t x_aspect = 0; const Location location(position, max_distance, x_aspect); PredicateQueryTerm::UP getPredicateQueryTerm() { - PredicateQueryTerm::UP pqt(new PredicateQueryTerm); + auto pqt = std::make_unique(); pqt->addFeature("key", "value"); pqt->addRangeFeature("key2", 42, 0xfff); return pqt; @@ -242,7 +242,7 @@ void checkQueryTreeTypes(Node *node) { EXPECT_TRUE(checkTerm(string_term, str[5], view[5], id[5], weight[5])); auto* predicateQuery = as_node(and_node->getChildren()[5]); - PredicateQueryTerm::UP pqt(new PredicateQueryTerm); + auto pqt = std::make_unique(); EXPECT_TRUE(checkTerm(predicateQuery, getPredicateQueryTerm(), view[3], id[3], weight[3])); auto* dotProduct = as_node(and_node->getChildren()[6]); diff --git a/searchlib/src/tests/queryeval/predicate/predicate_blueprint_test.cpp b/searchlib/src/tests/queryeval/predicate/predicate_blueprint_test.cpp index 28b0d103040..3dd2ec26dea 100644 --- a/searchlib/src/tests/queryeval/predicate/predicate_blueprint_test.cpp +++ b/searchlib/src/tests/queryeval/predicate/predicate_blueprint_test.cpp @@ -40,9 +40,9 @@ struct Fixture { Fixture() : field(42, 0), - attribute(new PredicateAttribute("f", attribute::Config(attribute::BasicType::PREDICATE))), - query(PredicateQueryTerm::UP(new PredicateQueryTerm), - "view", 0, Weight(1)) { + attribute(std::make_shared("f", attribute::Config(attribute::BasicType::PREDICATE))), + query(std::make_unique(),"view", 0, Weight(1)) + { query.getTerm()->addFeature("key", "value"); query.getTerm()->addRangeFeature("range_key", 42); } @@ -219,7 +219,7 @@ TEST_F("require that blueprint can set up search with subqueries", Fixture) { std::vector{{0x0002ffff}}; f.indexDocument(doc_id, annotations); - SimplePredicateQuery query(PredicateQueryTerm::UP(new PredicateQueryTerm), + SimplePredicateQuery query(std::make_unique(), "view", 0, Weight(1)); query.getTerm()->addFeature("key", "value", 1); query.getTerm()->addFeature("key2", "value", 2); diff --git a/searchlib/src/tests/transactionlog/chunks_test.cpp b/searchlib/src/tests/transactionlog/chunks_test.cpp index 4a74dd7f5bc..a3cf9f8bd92 100644 --- a/searchlib/src/tests/transactionlog/chunks_test.cpp +++ b/searchlib/src/tests/transactionlog/chunks_test.cpp @@ -79,7 +79,7 @@ TEST("test empty commitchunk") { struct Counter : public search::IDestructorCallback { std::atomic & _counter; - Counter(std::atomic & counter) : _counter(counter) { _counter++; } + Counter(std::atomic & counter) noexcept : _counter(counter) { _counter++; } ~Counter() override { _counter--; } }; diff --git a/searchlib/src/tests/transactionlog/translogclient_test.cpp b/searchlib/src/tests/transactionlog/translogclient_test.cpp index fffb70467a3..e097eebd42c 100644 --- a/searchlib/src/tests/transactionlog/translogclient_test.cpp +++ b/searchlib/src/tests/transactionlog/translogclient_test.cpp @@ -311,7 +311,7 @@ using Counter = std::atomic; class CountDone : public IDestructorCallback { public: - explicit CountDone(Counter & inFlight) : _inFlight(inFlight) { ++_inFlight; } + explicit CountDone(Counter & inFlight) noexcept : _inFlight(inFlight) { ++_inFlight; } ~CountDone() override { --_inFlight; } private: Counter & _inFlight; diff --git a/searchlib/src/vespa/searchlib/attribute/bitvector_search_cache.h b/searchlib/src/vespa/searchlib/attribute/bitvector_search_cache.h index a889120f8df..1cd07cf4e23 100644 --- a/searchlib/src/vespa/searchlib/attribute/bitvector_search_cache.h +++ b/searchlib/src/vespa/searchlib/attribute/bitvector_search_cache.h @@ -8,11 +8,8 @@ #include #include -namespace search { - -class BitVector; - -namespace attribute { +namespace search { class BitVector; } +namespace search::attribute { /** * Class that caches posting lists (as bit vectors) for a set of search terms. @@ -31,7 +28,7 @@ public: ReadGuardUP dmsReadGuard; BitVectorSP bitVector; uint32_t docIdLimit; - Entry(ReadGuardUP dmsReadGuard_, BitVectorSP bitVector_, uint32_t docIdLimit_) + Entry(ReadGuardUP dmsReadGuard_, BitVectorSP bitVector_, uint32_t docIdLimit_) noexcept : dmsReadGuard(std::move(dmsReadGuard_)), bitVector(std::move(bitVector_)), docIdLimit(docIdLimit_) {} }; @@ -52,4 +49,3 @@ public: }; } -} diff --git a/searchlib/src/vespa/searchlib/attribute/imported_search_context.cpp b/searchlib/src/vespa/searchlib/attribute/imported_search_context.cpp index bc0d965bcc1..9c40de26db6 100644 --- a/searchlib/src/vespa/searchlib/attribute/imported_search_context.cpp +++ b/searchlib/src/vespa/searchlib/attribute/imported_search_context.cpp @@ -93,7 +93,7 @@ struct WeightedRef { EntryRef revMapIdx; int32_t weight; - WeightedRef(EntryRef revMapIdx_, int32_t weight_) + WeightedRef(EntryRef revMapIdx_, int32_t weight_) noexcept : revMapIdx(revMapIdx_), weight(weight_) { @@ -118,11 +118,11 @@ class ReverseMappingBitVector const ReverseMapping &_reverseMapping; EntryRef _revMapIdx; public: - ReverseMappingBitVector(const ReverseMapping &reverseMapping, EntryRef revMapIdx) + ReverseMappingBitVector(const ReverseMapping &reverseMapping, EntryRef revMapIdx) noexcept : _reverseMapping(reverseMapping), _revMapIdx(revMapIdx) {} - ~ReverseMappingBitVector() { } + ~ReverseMappingBitVector() = default; template void foreach_key(Func func) const { diff --git a/searchlib/src/vespa/searchlib/attribute/interlock.h b/searchlib/src/vespa/searchlib/attribute/interlock.h index cf9298686c3..30efeaf32fd 100644 --- a/searchlib/src/vespa/searchlib/attribute/interlock.h +++ b/searchlib/src/vespa/searchlib/attribute/interlock.h @@ -4,11 +4,7 @@ #include -namespace search -{ - -namespace attribute -{ +namespace search::attribute { class InterlockGuard; @@ -35,7 +31,7 @@ class Interlock { std::mutex _mutex; friend class InterlockGuard; public: - Interlock() + Interlock() noexcept : _mutex() { } @@ -60,6 +56,4 @@ public: }; -} - -} +} \ No newline at end of file diff --git a/searchlib/src/vespa/searchlib/attribute/multivalue.h b/searchlib/src/vespa/searchlib/attribute/multivalue.h index c59f975e00a..54c10bdc7cf 100644 --- a/searchlib/src/vespa/searchlib/attribute/multivalue.h +++ b/searchlib/src/vespa/searchlib/attribute/multivalue.h @@ -10,12 +10,9 @@ template class Value { public: typedef T ValueType; - Value() - : _v() - { - } - Value(T v) : _v(v) { } - Value(T v, int32_t w) : _v(v) { (void) w; } + Value() noexcept : _v() {} + Value(T v) noexcept : _v(v) { } + Value(T v, int32_t w) noexcept : _v(v) { (void) w; } T value() const { return _v; } operator T () const { return _v; } operator T & () { return _v; } @@ -36,8 +33,8 @@ template class WeightedValue { public: typedef T ValueType; - WeightedValue() : _v(), _w(1) { } - WeightedValue(T v, int32_t w) : _v(v), _w(w) { } + WeightedValue() noexcept : _v(), _w(1) { } + WeightedValue(T v, int32_t w) noexcept : _v(v), _w(w) { } T value() const { return _v; } operator T () const { return _v; } operator T & () { return _v; } diff --git a/searchlib/src/vespa/searchlib/common/gatecallback.h b/searchlib/src/vespa/searchlib/common/gatecallback.h index 1e85d796089..b6e7690f820 100644 --- a/searchlib/src/vespa/searchlib/common/gatecallback.h +++ b/searchlib/src/vespa/searchlib/common/gatecallback.h @@ -9,7 +9,7 @@ namespace search { class GateCallback : public IDestructorCallback { public: - GateCallback(vespalib::Gate & gate) : _gate(gate) {} + GateCallback(vespalib::Gate & gate) noexcept : _gate(gate) {} ~GateCallback() override; private: vespalib::Gate & _gate; @@ -17,7 +17,7 @@ private: class IgnoreCallback : public IDestructorCallback { public: - IgnoreCallback() { } + IgnoreCallback() noexcept { } ~IgnoreCallback() override = default; }; diff --git a/searchlib/src/vespa/searchlib/common/lid_usage_stats.h b/searchlib/src/vespa/searchlib/common/lid_usage_stats.h index 1dd3881892f..97e16ffe768 100644 --- a/searchlib/src/vespa/searchlib/common/lid_usage_stats.h +++ b/searchlib/src/vespa/searchlib/common/lid_usage_stats.h @@ -20,7 +20,7 @@ private: uint32_t _highestUsedLid; public: - LidUsageStats() + LidUsageStats() noexcept : _lidLimit(0), _usedLids(0), _lowestFreeLid(0), @@ -30,7 +30,7 @@ public: LidUsageStats(uint32_t lidLimit, uint32_t usedLids, uint32_t lowestFreeLid, - uint32_t highestUsedLid) + uint32_t highestUsedLid) noexcept : _lidLimit(lidLimit), _usedLids(usedLids), _lowestFreeLid(lowestFreeLid), diff --git a/searchlib/src/vespa/searchlib/common/rankedhit.h b/searchlib/src/vespa/searchlib/common/rankedhit.h index 635f6e350a5..8a0efb4ce3c 100644 --- a/searchlib/src/vespa/searchlib/common/rankedhit.h +++ b/searchlib/src/vespa/searchlib/common/rankedhit.h @@ -9,8 +9,8 @@ namespace search { struct RankedHit { - RankedHit() : _docId(0), _rankValue(zero_rank_value) { } - RankedHit(unsigned int docId, HitRank rank = zero_rank_value) : _docId(docId), _rankValue(rank) { } + RankedHit() noexcept : _docId(0), _rankValue(zero_rank_value) { } + RankedHit(unsigned int docId, HitRank rank = zero_rank_value) noexcept : _docId(docId), _rankValue(rank) { } unsigned int getDocId() const { return _docId & 0x7fffffff; } bool hasMore() const { return _docId & 0x80000000; } HitRank getRank() const { return _rankValue; } @@ -21,13 +21,13 @@ struct RankedHit { class RankedHitIterator { public: - RankedHitIterator(const RankedHit * h, size_t sz) : _h(h), _sz(sz), _pos(0) { } - bool hasNext() const { return _pos < _sz; } - uint32_t next() { return _h[_pos++].getDocId(); } + RankedHitIterator(const RankedHit * h, size_t sz) noexcept : _h(h), _sz(sz), _pos(0) { } + bool hasNext() const noexcept { return _pos < _sz; } + uint32_t next() noexcept { return _h[_pos++].getDocId(); } private: const RankedHit *_h; - const size_t _sz; - size_t _pos; + const size_t _sz; + size_t _pos; }; } // namespace search diff --git a/searchlib/src/vespa/searchlib/common/scheduletaskcallback.h b/searchlib/src/vespa/searchlib/common/scheduletaskcallback.h index 27bbe751532..00f0d4b29f5 100644 --- a/searchlib/src/vespa/searchlib/common/scheduletaskcallback.h +++ b/searchlib/src/vespa/searchlib/common/scheduletaskcallback.h @@ -4,8 +4,7 @@ #include "idestructorcallback.h" #include -namespace search -{ +namespace search { /** * Class that schedules a task when instance is destroyed. Typically a @@ -20,12 +19,11 @@ class ScheduleTaskCallback : public IDestructorCallback vespalib::Executor::Task::UP _task; public: ScheduleTaskCallback(vespalib::Executor &executor, - vespalib::Executor::Task::UP task) + vespalib::Executor::Task::UP task) noexcept : _executor(executor), _task(std::move(task)) - { - } - virtual ~ScheduleTaskCallback() { + {} + ~ScheduleTaskCallback() override { _executor.execute(std::move(_task)); } }; diff --git a/searchlib/src/vespa/searchlib/common/tunefileinfo.h b/searchlib/src/vespa/searchlib/common/tunefileinfo.h index bcd6765845b..e27290d35de 100644 --- a/searchlib/src/vespa/searchlib/common/tunefileinfo.h +++ b/searchlib/src/vespa/searchlib/common/tunefileinfo.h @@ -19,7 +19,7 @@ private: TuneControl _tuneControl; public: - TuneFileSeqRead() : _tuneControl(NORMAL) { } + TuneFileSeqRead() noexcept : _tuneControl(NORMAL) { } void setWantDirectIO() { _tuneControl = DIRECTIO; } bool getWantDirectIO() const { return _tuneControl == DIRECTIO; } @@ -62,7 +62,7 @@ private: TuneControl _tuneControl; public: - TuneFileSeqWrite() : _tuneControl(NORMAL) { } + TuneFileSeqWrite() noexcept : _tuneControl(NORMAL) { } void setWantDirectIO() { _tuneControl = DIRECTIO; } bool getWantDirectIO() const { return _tuneControl == DIRECTIO; } bool getWantSyncWrites() const { return _tuneControl == OSYNC; } @@ -99,7 +99,7 @@ private: int _mmapFlags; int _advise; public: - TuneFileRandRead() + TuneFileRandRead() noexcept : _tuneControl(NORMAL), _mmapFlags(0), _advise(0) @@ -139,9 +139,9 @@ public: TuneFileSeqRead _read; TuneFileSeqWrite _write; - TuneFileIndexing() : _read(), _write() {} + TuneFileIndexing() noexcept : _read(), _write() {} - TuneFileIndexing(const TuneFileSeqRead &r, const TuneFileSeqWrite &w) : _read(r), _write(w) { } + TuneFileIndexing(const TuneFileSeqRead &r, const TuneFileSeqWrite &w) noexcept : _read(r), _write(w) { } bool operator==(const TuneFileIndexing &rhs) const { return _read == rhs._read && _write == rhs._write; @@ -161,8 +161,8 @@ class TuneFileSearch public: TuneFileRandRead _read; - TuneFileSearch() : _read() { } - TuneFileSearch(const TuneFileRandRead &r) : _read(r) { } + TuneFileSearch() noexcept : _read() { } + TuneFileSearch(const TuneFileRandRead &r) noexcept : _read(r) { } bool operator==(const TuneFileSearch &rhs) const { return _read == rhs._read; } bool operator!=(const TuneFileSearch &rhs) const { return _read != rhs._read; } }; @@ -178,7 +178,7 @@ public: TuneFileIndexing _indexing; TuneFileSearch _search; - TuneFileIndexManager() : _indexing(), _search() { } + TuneFileIndexManager() noexcept : _indexing(), _search() { } bool operator==(const TuneFileIndexManager &rhs) const { return _indexing == rhs._indexing && _search == rhs._search; @@ -198,7 +198,7 @@ class TuneFileAttributes public: TuneFileSeqWrite _write; - TuneFileAttributes() : _write() { } + TuneFileAttributes() noexcept : _write() { } bool operator==(const TuneFileAttributes &rhs) const { return _write == rhs._write; @@ -220,7 +220,7 @@ public: TuneFileSeqWrite _write; TuneFileRandRead _randRead; - TuneFileSummary() : _seqRead(), _write(), _randRead() { } + TuneFileSummary() noexcept : _seqRead(), _write(), _randRead() { } bool operator==(const TuneFileSummary &rhs) const { return _seqRead == rhs._seqRead && @@ -248,7 +248,7 @@ public: TuneFileAttributes _attr; TuneFileSummary _summary; - TuneFileDocumentDB() : _index(), _attr(), _summary() { } + TuneFileDocumentDB() noexcept : _index(), _attr(), _summary() { } bool operator==(const TuneFileDocumentDB &rhs) const { return _index == rhs._index && diff --git a/searchlib/src/vespa/searchlib/diskindex/zc4_posting_writer_base.h b/searchlib/src/vespa/searchlib/diskindex/zc4_posting_writer_base.h index e9b1efa5c7d..4e71a8356c0 100644 --- a/searchlib/src/vespa/searchlib/diskindex/zc4_posting_writer_base.h +++ b/searchlib/src/vespa/searchlib/diskindex/zc4_posting_writer_base.h @@ -24,7 +24,7 @@ public: uint32_t _field_length; uint32_t _num_occs; uint32_t _features_size; - DocIdAndFeatureSize(uint32_t doc_id, uint32_t field_length, uint32_t num_occs, uint32_t features_size) + DocIdAndFeatureSize(uint32_t doc_id, uint32_t field_length, uint32_t num_occs, uint32_t features_size) noexcept : _doc_id(doc_id), _field_length(field_length), _num_occs(num_occs), diff --git a/searchlib/src/vespa/searchlib/docstore/filechunk.h b/searchlib/src/vespa/searchlib/docstore/filechunk.h index b68db801d60..6839423d6db 100644 --- a/searchlib/src/vespa/searchlib/docstore/filechunk.h +++ b/searchlib/src/vespa/searchlib/docstore/filechunk.h @@ -76,7 +76,7 @@ public: using LockGuard = vespalib::LockGuard; class NameId { public: - explicit NameId(size_t id) : _id(id) { } + explicit NameId(size_t id) noexcept : _id(id) { } uint64_t getId() const { return _id; } vespalib::string createName(const vespalib::string &baseName) const; bool operator == (const NameId & rhs) const { return _id == rhs._id; } @@ -90,7 +90,7 @@ public: }; class FileId { public: - explicit FileId(uint32_t id) : _id(id) { } + explicit FileId(uint32_t id) noexcept : _id(id) { } uint32_t getId() const { return _id; } bool operator != (const FileId & rhs) const { return _id != rhs._id; } bool operator == (const FileId & rhs) const { return _id == rhs._id; } diff --git a/searchlib/src/vespa/searchlib/docstore/lid_info.h b/searchlib/src/vespa/searchlib/docstore/lid_info.h index 10ddd868c41..8444a9e3575 100644 --- a/searchlib/src/vespa/searchlib/docstore/lid_info.h +++ b/searchlib/src/vespa/searchlib/docstore/lid_info.h @@ -11,8 +11,8 @@ namespace search { class LidInfo { public: - LidInfo() : _value() { } - LidInfo(uint64_t rep) { _value.r = rep; } + LidInfo() noexcept : _value() { } + LidInfo(uint64_t rep) noexcept { _value.r = rep; } LidInfo(uint32_t fileId, uint32_t chunkId, uint32_t size); uint32_t getFileId() const { return _value.v.fileId; } uint32_t getChunkId() const { return _value.v.chunkId; } @@ -57,7 +57,7 @@ private: class LidInfoWithLid : public LidInfo { public: - LidInfoWithLid(LidInfo lidInfo, uint32_t lid) : LidInfo(lidInfo), _lid(lid) { } + LidInfoWithLid(LidInfo lidInfo, uint32_t lid) noexcept : LidInfo(lidInfo), _lid(lid) { } uint32_t getLid() const { return _lid; } private: uint32_t _lid; diff --git a/searchlib/src/vespa/searchlib/docstore/storebybucket.cpp b/searchlib/src/vespa/searchlib/docstore/storebybucket.cpp index 5e595d0bb14..d7711b61d78 100644 --- a/searchlib/src/vespa/searchlib/docstore/storebybucket.cpp +++ b/searchlib/src/vespa/searchlib/docstore/storebybucket.cpp @@ -12,7 +12,7 @@ using document::BucketId; using vespalib::makeTask; using vespalib::makeClosure; -StoreByBucket::StoreByBucket(MemoryDataStore & backingMemory, Executor & executor, const CompressionConfig & compression) +StoreByBucket::StoreByBucket(MemoryDataStore & backingMemory, Executor & executor, const CompressionConfig & compression) noexcept : _chunkSerial(0), _current(), _where(), diff --git a/searchlib/src/vespa/searchlib/docstore/storebybucket.h b/searchlib/src/vespa/searchlib/docstore/storebybucket.h index 8be0610b588..1365dcb4416 100644 --- a/searchlib/src/vespa/searchlib/docstore/storebybucket.h +++ b/searchlib/src/vespa/searchlib/docstore/storebybucket.h @@ -24,9 +24,12 @@ class StoreByBucket using ConstBufferRef = vespalib::ConstBufferRef; using CompressionConfig = vespalib::compression::CompressionConfig; public: - StoreByBucket(vespalib::MemoryDataStore & backingMemory, const CompressionConfig & compression); - StoreByBucket(MemoryDataStore & backingMemory, Executor & executor, const CompressionConfig & compression); - StoreByBucket(StoreByBucket &&) = default; + StoreByBucket(MemoryDataStore & backingMemory, Executor & executor, const CompressionConfig & compression) noexcept; + //TODO Putting the below move constructor into cpp file fails for some unknown reason. Needs to be resolved. + StoreByBucket(StoreByBucket &&) noexcept = default; + StoreByBucket(const StoreByBucket &) = delete; + StoreByBucket & operator=(StoreByBucket &&) noexcept = delete; + StoreByBucket & operator = (const StoreByBucket &) = delete; ~StoreByBucket(); class IWrite { public: @@ -52,10 +55,10 @@ private: void closeChunk(Chunk::UP chunk); struct Index { using BucketId=document::BucketId; - Index(BucketId bucketId, uint32_t id, uint32_t chunkId, uint32_t entry) : + Index(BucketId bucketId, uint32_t id, uint32_t chunkId, uint32_t entry) noexcept : _bucketId(bucketId), _id(id), _chunkId(chunkId), _lid(entry) { } - bool operator < (const Index & b) const { + bool operator < (const Index & b) const noexcept { return BucketId::bucketIdToKey(_bucketId.getRawId()) < BucketId::bucketIdToKey(b._bucketId.getRawId()); } BucketId _bucketId; diff --git a/searchlib/src/vespa/searchlib/docstore/visitcache.h b/searchlib/src/vespa/searchlib/docstore/visitcache.h index 8a06794ee35..25a7be5ee6c 100644 --- a/searchlib/src/vespa/searchlib/docstore/visitcache.h +++ b/searchlib/src/vespa/searchlib/docstore/visitcache.h @@ -40,7 +40,7 @@ class BlobSet { public: class LidPosition { public: - LidPosition(uint32_t lid, uint32_t offset, uint32_t size) : _lid(lid), _offset(offset), _size(size) { } + LidPosition(uint32_t lid, uint32_t offset, uint32_t size) noexcept : _lid(lid), _offset(offset), _size(size) { } uint32_t lid() const { return _lid; } uint32_t offset() const { return _offset; } uint32_t size() const { return _size; } diff --git a/searchlib/src/vespa/searchlib/docstore/writeablefilechunk.cpp b/searchlib/src/vespa/searchlib/docstore/writeablefilechunk.cpp index 3517595d00a..beece528795 100644 --- a/searchlib/src/vespa/searchlib/docstore/writeablefilechunk.cpp +++ b/searchlib/src/vespa/searchlib/docstore/writeablefilechunk.cpp @@ -204,7 +204,7 @@ seek_past(LidInfoWithLidV::const_iterator begin, LidInfoWithLidV::const_iterator } struct LidAndBuffer { - LidAndBuffer(uint32_t lid, uint32_t sz, vespalib::alloc::Alloc buf) : _lid(lid), _size(sz), _buf(std::move(buf)) {} + LidAndBuffer(uint32_t lid, uint32_t sz, vespalib::alloc::Alloc buf) noexcept : _lid(lid), _size(sz), _buf(std::move(buf)) {} uint32_t _lid; uint32_t _size; vespalib::alloc::Alloc _buf; diff --git a/searchlib/src/vespa/searchlib/engine/docsumreply.h b/searchlib/src/vespa/searchlib/engine/docsumreply.h index 37431030253..0150b1eda3d 100644 --- a/searchlib/src/vespa/searchlib/engine/docsumreply.h +++ b/searchlib/src/vespa/searchlib/engine/docsumreply.h @@ -22,9 +22,9 @@ struct DocsumReply document::GlobalId gid; Blob data; - Docsum() : docid(0), gid(), data(0) {} - Docsum(document::GlobalId gid_) : docid(0), gid(gid_), data(0) { } - Docsum(document::GlobalId gid_, const char *buf, uint32_t len) : docid(0), gid(gid_), data(len) { + Docsum() noexcept : docid(0), gid(), data(0) {} + Docsum(document::GlobalId gid_) noexcept : docid(0), gid(gid_), data(0) { } + Docsum(document::GlobalId gid_, const char *buf, uint32_t len) noexcept : docid(0), gid(gid_), data(len) { memcpy(data.str(), buf, len); } Docsum & setData(const char *buf, uint32_t len) { diff --git a/searchlib/src/vespa/searchlib/engine/docsumrequest.h b/searchlib/src/vespa/searchlib/engine/docsumrequest.h index 8aa8d036b73..8fe5aa6f465 100644 --- a/searchlib/src/vespa/searchlib/engine/docsumrequest.h +++ b/searchlib/src/vespa/searchlib/engine/docsumrequest.h @@ -23,8 +23,8 @@ public: class Hit { public: - Hit() : gid(), docid(0), path(0) {} - Hit(const document::GlobalId & gid_) : gid(gid_), docid(0), path(0) {} + Hit() noexcept : gid(), docid(0), path(0) {} + Hit(const document::GlobalId & gid_) noexcept : gid(gid_), docid(0), path(0) {} document::GlobalId gid; mutable uint32_t docid; // converted in backend diff --git a/searchlib/src/vespa/searchlib/engine/searchreply.h b/searchlib/src/vespa/searchlib/engine/searchreply.h index d9468216a90..531f94d2b6f 100644 --- a/searchlib/src/vespa/searchlib/engine/searchreply.h +++ b/searchlib/src/vespa/searchlib/engine/searchreply.h @@ -18,7 +18,7 @@ public: class Hit { public: - Hit() : gid(), metric(0), path(0), _distributionKey(0) {} + Hit() noexcept : gid(), metric(0), path(0), _distributionKey(0) {} void setDistributionKey(uint32_t key) { _distributionKey = key; } uint32_t getDistributionKey() const { return _distributionKey; } document::GlobalId gid; @@ -30,9 +30,9 @@ public: class Coverage { public: - Coverage() : Coverage(0) { } - Coverage(uint64_t active) : Coverage(active, active) { } - Coverage(uint64_t active, uint64_t covered) + Coverage() noexcept : Coverage(0) { } + Coverage(uint64_t active) noexcept : Coverage(active, active) { } + Coverage(uint64_t active, uint64_t covered) noexcept : _covered(covered), _active(active), _soonActive(active), _degradeReason(0), _nodesQueried(1), _nodesReplied(1) { } diff --git a/searchlib/src/vespa/searchlib/features/array_parser.h b/searchlib/src/vespa/searchlib/features/array_parser.h index 8bf6e9ca365..7de766efddc 100644 --- a/searchlib/src/vespa/searchlib/features/array_parser.h +++ b/searchlib/src/vespa/searchlib/features/array_parser.h @@ -29,7 +29,7 @@ public: class ValueAndIndex { public: typedef T ValueType; - ValueAndIndex(T value, uint32_t index) : _value(value), _index(index) { } + ValueAndIndex(T value, uint32_t index) noexcept : _value(value), _index(index) { } T getValue() const { return _value; } uint32_t getIndex() const { return _index; } bool operator < (const ValueAndIndex & b) const { return _index < b._index; } diff --git a/searchlib/src/vespa/searchlib/features/bm25_feature.h b/searchlib/src/vespa/searchlib/features/bm25_feature.h index 0afd14e7ac8..72dcb7e2ef7 100644 --- a/searchlib/src/vespa/searchlib/features/bm25_feature.h +++ b/searchlib/src/vespa/searchlib/features/bm25_feature.h @@ -14,7 +14,7 @@ private: fef::TermFieldHandle handle; const fef::TermFieldMatchData* tfmd; double idf_mul_k1_plus_one; - QueryTerm(fef::TermFieldHandle handle_, double inverse_doc_freq, double k1_param) + QueryTerm(fef::TermFieldHandle handle_, double inverse_doc_freq, double k1_param) noexcept : handle(handle_), tfmd(nullptr), idf_mul_k1_plus_one(inverse_doc_freq * (k1_param + 1)) diff --git a/searchlib/src/vespa/searchlib/features/fieldmatch/computer.h b/searchlib/src/vespa/searchlib/features/fieldmatch/computer.h index e4dbde1248a..a4699b17457 100644 --- a/searchlib/src/vespa/searchlib/features/fieldmatch/computer.h +++ b/searchlib/src/vespa/searchlib/features/fieldmatch/computer.h @@ -289,7 +289,7 @@ private: struct SegmentData { SegmentData() : segment(), valid(false) {} - SegmentData(SegmentStart::SP ss, bool v = false) : segment(std::move(ss)), valid(v) {} + SegmentData(SegmentStart::SP ss, bool v = false) noexcept : segment(std::move(ss)), valid(v) {} SegmentStart::SP segment; bool valid; }; diff --git a/searchlib/src/vespa/searchlib/fef/rank_program.cpp b/searchlib/src/vespa/searchlib/fef/rank_program.cpp index 0bc85a63ceb..dd1e774607f 100644 --- a/searchlib/src/vespa/searchlib/fef/rank_program.cpp +++ b/searchlib/src/vespa/searchlib/fef/rank_program.cpp @@ -23,7 +23,7 @@ struct Override BlueprintResolver::FeatureRef ref; feature_t value; - Override(const BlueprintResolver::FeatureRef &r, feature_t v) + Override(const BlueprintResolver::FeatureRef &r, feature_t v) noexcept : ref(r), value(v) {} bool operator<(const Override &rhs) const { diff --git a/searchlib/src/vespa/searchlib/fef/termmatchdatamerger.cpp b/searchlib/src/vespa/searchlib/fef/termmatchdatamerger.cpp index 973e11fc0d2..11a5dd80b4c 100644 --- a/searchlib/src/vespa/searchlib/fef/termmatchdatamerger.cpp +++ b/searchlib/src/vespa/searchlib/fef/termmatchdatamerger.cpp @@ -24,7 +24,7 @@ TermMatchDataMerger::TermMatchDataMerger(const Inputs &allinputs, } } -TermMatchDataMerger::~TermMatchDataMerger() {} +TermMatchDataMerger::~TermMatchDataMerger() = default; void TermMatchDataMerger::merge(uint32_t docid) diff --git a/searchlib/src/vespa/searchlib/fef/termmatchdatamerger.h b/searchlib/src/vespa/searchlib/fef/termmatchdatamerger.h index e0addc374b2..fb129b792b8 100644 --- a/searchlib/src/vespa/searchlib/fef/termmatchdatamerger.h +++ b/searchlib/src/vespa/searchlib/fef/termmatchdatamerger.h @@ -6,8 +6,7 @@ #include "termfieldmatchdata.h" #include -namespace search { -namespace fef { +namespace search::fef { class TermMatchDataMerger { @@ -16,8 +15,8 @@ public: const TermFieldMatchData *matchData; double exactness; - Input() : matchData(NULL), exactness(0.0) {} - Input(const TermFieldMatchData *arg_matchData, double arg_exactness) + Input() : matchData(nullptr), exactness(0.0) {} + Input(const TermFieldMatchData *arg_matchData, double arg_exactness) noexcept : matchData(arg_matchData), exactness(arg_exactness) {} }; @@ -27,21 +26,16 @@ private: const TermFieldMatchDataArray _output; std::vector _scratch; - TermMatchDataMerger(const TermMatchDataMerger &); - TermMatchDataMerger &operator=(const TermMatchDataMerger &); - - void merge(uint32_t docid, - const Inputs &in, - TermFieldMatchData &out); + void merge(uint32_t docid, const Inputs &in, TermFieldMatchData &out); public: + TermMatchDataMerger(const TermMatchDataMerger &) = delete; + TermMatchDataMerger &operator=(const TermMatchDataMerger &) = delete; - TermMatchDataMerger(const Inputs &allinputs, - const TermFieldMatchDataArray &outputs); + TermMatchDataMerger(const Inputs &allinputs, const TermFieldMatchDataArray &outputs); ~TermMatchDataMerger(); void merge(uint32_t docid); }; -} // namespace fef -} // namespace search +} diff --git a/searchlib/src/vespa/searchlib/grouping/hyperloglog.h b/searchlib/src/vespa/searchlib/grouping/hyperloglog.h index 931b832c76d..2206f4ccbe1 100644 --- a/searchlib/src/vespa/searchlib/grouping/hyperloglog.h +++ b/searchlib/src/vespa/searchlib/grouping/hyperloglog.h @@ -28,7 +28,7 @@ template class ExchangerSketch : public SparseSketch { typename Sketch::UP &_sketch_ptr; - virtual int aggregate(HashT hash) override { + int aggregate(HashT hash) override { if (this->getSize() > SPARSE_SKETCH_LIMIT) { NormalSketch *normal_sketch = new NormalSketch; diff --git a/searchlib/src/vespa/searchlib/grouping/sketch.h b/searchlib/src/vespa/searchlib/grouping/sketch.h index 317c1bfef9d..c105b480a3d 100644 --- a/searchlib/src/vespa/searchlib/grouping/sketch.h +++ b/searchlib/src/vespa/searchlib/grouping/sketch.h @@ -70,21 +70,21 @@ struct SparseSketch : Sketch { enum { classId = IDENTIFIABLE_CLASSID_NS(search, SparseSketch) }; struct IdentityHash { - size_t operator()(HashT hash) const { return hash; } + size_t operator()(HashT hash) const noexcept { return hash; } }; std::unordered_set hash_set; size_t getSize() const { return hash_set.size(); } - virtual int aggregate(HashT hash) override { + int aggregate(HashT hash) override { return hash_set.insert(hash).second ? 1 : 0; } - virtual uint32_t getClassId() const override { return classId; } - virtual void serialize(vespalib::Serializer &os) const override; - virtual void deserialize(vespalib::Deserializer &is) override; + uint32_t getClassId() const override { return classId; } + void serialize(vespalib::Serializer &os) const override; + void deserialize(vespalib::Deserializer &is) override; - virtual bool operator==(const SketchType &other) const override { + bool operator==(const SketchType &other) const override { const SparseSketch *other_sparse = dynamic_cast *>(&other); if (!other_sparse) { @@ -101,7 +101,7 @@ struct SparseSketch : Sketch { return true; } - virtual void print(std::ostream &out) const override { + void print(std::ostream &out) const override { out << " (" << hash_set.size() << " elements)"; for (auto hash : hash_set) { out << " 0x" << std::hex; diff --git a/searchlib/src/vespa/searchlib/index/docidandfeatures.h b/searchlib/src/vespa/searchlib/index/docidandfeatures.h index 5372d5ef3aa..6ee80721038 100644 --- a/searchlib/src/vespa/searchlib/index/docidandfeatures.h +++ b/searchlib/src/vespa/searchlib/index/docidandfeatures.h @@ -26,14 +26,14 @@ private: uint32_t _elementLen; public: - WordDocElementFeatures() + WordDocElementFeatures() noexcept : _elementId(0u), _numOccs(0u), _weight(1), _elementLen(SEARCHLIB_FEF_UNKNOWN_FIELD_LENGTH) {} - WordDocElementFeatures(uint32_t elementId) + WordDocElementFeatures(uint32_t elementId) noexcept : _elementId(elementId), _numOccs(0u), _weight(1), @@ -42,7 +42,7 @@ public: WordDocElementFeatures(uint32_t elementId, uint32_t weight, - uint32_t elementLen) + uint32_t elementLen) noexcept : _elementId(elementId), _numOccs(0u), _weight(weight), @@ -71,11 +71,11 @@ private: uint32_t _wordPos; public: - WordDocElementWordPosFeatures() + WordDocElementWordPosFeatures() noexcept : _wordPos(0u) {} - WordDocElementWordPosFeatures(uint32_t wordPos) + WordDocElementWordPosFeatures(uint32_t wordPos) noexcept : _wordPos(wordPos) {} @@ -101,17 +101,17 @@ protected: std::vector _word_positions; // Raw data (file format specific, packed) - RawData _blob; // Feature data for (word, docid) pair + RawData _blob; // Feature data for (word, docid) pair uint32_t _bit_offset; // Offset of feature start ([0..63]) uint32_t _bit_length; // Length of features - bool _has_raw_data; + bool _has_raw_data; public: DocIdAndFeatures(); DocIdAndFeatures(const DocIdAndFeatures &); DocIdAndFeatures & operator = (const DocIdAndFeatures &); - DocIdAndFeatures(DocIdAndFeatures &&) = default; - DocIdAndFeatures & operator = (DocIdAndFeatures &&) = default; + DocIdAndFeatures(DocIdAndFeatures &&) noexcept = default; + DocIdAndFeatures & operator = (DocIdAndFeatures &&) noexcept = default; ~DocIdAndFeatures(); void clear_features() { diff --git a/searchlib/src/vespa/searchlib/memoryindex/field_index_remover.h b/searchlib/src/vespa/searchlib/memoryindex/field_index_remover.h index 36d2286cfb1..717f21528bb 100644 --- a/searchlib/src/vespa/searchlib/memoryindex/field_index_remover.h +++ b/searchlib/src/vespa/searchlib/memoryindex/field_index_remover.h @@ -21,11 +21,11 @@ private: struct WordFieldDocTuple { vespalib::datastore::EntryRef _wordRef; uint32_t _docId; - WordFieldDocTuple() : + WordFieldDocTuple() noexcept : _wordRef(0), _docId(0) { } - WordFieldDocTuple(vespalib::datastore::EntryRef wordRef, uint32_t docId) : + WordFieldDocTuple(vespalib::datastore::EntryRef wordRef, uint32_t docId) noexcept : _wordRef(wordRef), _docId(docId) { } @@ -44,12 +44,12 @@ private: CompactWordsStore _store; CompactWordsStore::Builder::UP _builder; - std::vector _wordFieldDocTuples; + std::vector _wordFieldDocTuples; const WordStore &_wordStore; public: FieldIndexRemover(const WordStore &wordStore); - ~FieldIndexRemover(); + ~FieldIndexRemover() override; void remove(uint32_t docId, IFieldIndexRemoveListener &inverter); CompactWordsStore &getStore() { return _store; } const CompactWordsStore &getStore() const { return _store; } diff --git a/searchlib/src/vespa/searchlib/memoryindex/field_inverter.h b/searchlib/src/vespa/searchlib/memoryindex/field_inverter.h index 78a1cf6c171..3a18d0c2f8c 100644 --- a/searchlib/src/vespa/searchlib/memoryindex/field_inverter.h +++ b/searchlib/src/vespa/searchlib/memoryindex/field_inverter.h @@ -39,7 +39,7 @@ public: static constexpr uint32_t _elemRemoved = std::numeric_limits::max(); - PosInfo() + PosInfo() noexcept : _wordNum(0), _docId(0), _elemId(0), @@ -51,7 +51,7 @@ public: PosInfo(uint32_t wordRef, uint32_t docId, uint32_t elemId, - uint32_t wordPos, uint32_t elemRef) + uint32_t wordPos, uint32_t elemRef) noexcept : _wordNum(wordRef), _docId(docId), _elemId(elemId), @@ -60,8 +60,7 @@ public: { } - PosInfo(uint32_t wordRef, - uint32_t docId) + PosInfo(uint32_t wordRef, uint32_t docId) noexcept : _wordNum(wordRef), _docId(docId), _elemId(_elemRemoved), diff --git a/searchlib/src/vespa/searchlib/parsequery/stackdumpiterator.cpp b/searchlib/src/vespa/searchlib/parsequery/stackdumpiterator.cpp index 1820fb0e969..6039a86580c 100644 --- a/searchlib/src/vespa/searchlib/parsequery/stackdumpiterator.cpp +++ b/searchlib/src/vespa/searchlib/parsequery/stackdumpiterator.cpp @@ -225,7 +225,7 @@ SimpleQueryStackDumpIterator::next() case ParseItem::ITEM_PREDICATE_QUERY: try { _curr_index_name = read_stringref(p); - _predicate_query_term.reset(new PredicateQueryTerm); + _predicate_query_term = std::make_unique(); size_t count = readCompressedPositiveInt(p); for (size_t i = 0; i < count; ++i) { diff --git a/searchlib/src/vespa/searchlib/query/streaming/hit.h b/searchlib/src/vespa/searchlib/query/streaming/hit.h index 64b71a70df9..fabaa3ad50c 100644 --- a/searchlib/src/vespa/searchlib/query/streaming/hit.h +++ b/searchlib/src/vespa/searchlib/query/streaming/hit.h @@ -9,7 +9,7 @@ namespace search::streaming { class Hit { public: - Hit(uint32_t pos_, uint32_t context_, uint32_t elemId_, int32_t weight_) + Hit(uint32_t pos_, uint32_t context_, uint32_t elemId_, int32_t weight_) noexcept : _position(pos_ | (context_<<24)), _elemId(elemId_), _weight(weight_) diff --git a/searchlib/src/vespa/searchlib/query/tree/predicate_query_term.h b/searchlib/src/vespa/searchlib/query/tree/predicate_query_term.h index 0a92546e414..8602eb1ac57 100644 --- a/searchlib/src/vespa/searchlib/query/tree/predicate_query_term.h +++ b/searchlib/src/vespa/searchlib/query/tree/predicate_query_term.h @@ -22,7 +22,7 @@ class PredicateQueryTerm { public: Entry(const vespalib::string &key, const ValueType &value, - uint64_t sub_query_bitmap = ALL_SUB_QUERIES) + uint64_t sub_query_bitmap = ALL_SUB_QUERIES) noexcept : _key(key), _value(value), _sub_query_bitmap(sub_query_bitmap) {} vespalib::string getKey() const { return _key; } @@ -41,13 +41,7 @@ class PredicateQueryTerm { public: typedef std::unique_ptr UP; - PredicateQueryTerm() : _features(), _range_features() {} - - PredicateQueryTerm(const std::vector> &features, - const std::vector> &range_features) - : _features(features), - _range_features(range_features) { - } + PredicateQueryTerm() noexcept : _features(), _range_features() {} void addFeature(const vespalib::string &key, const vespalib::string &value, uint64_t sub_query_bitmask = ALL_SUB_QUERIES) { diff --git a/searchlib/src/vespa/searchlib/query/tree/queryreplicator.h b/searchlib/src/vespa/searchlib/query/tree/queryreplicator.h index 9289df7cbe9..600249c3e1e 100644 --- a/searchlib/src/vespa/searchlib/query/tree/queryreplicator.h +++ b/searchlib/src/vespa/searchlib/query/tree/queryreplicator.h @@ -153,7 +153,7 @@ private: void visit(PredicateQuery &node) override { replicate(node, _builder.addPredicateQuery( - PredicateQueryTerm::UP(new PredicateQueryTerm(*node.getTerm())), + std::make_unique(*node.getTerm()), node.getView(), node.getId(), node.getWeight())); } diff --git a/searchlib/src/vespa/searchlib/queryeval/global_filter.h b/searchlib/src/vespa/searchlib/queryeval/global_filter.h index b30162affa7..3e57ded4898 100644 --- a/searchlib/src/vespa/searchlib/queryeval/global_filter.h +++ b/searchlib/src/vespa/searchlib/queryeval/global_filter.h @@ -21,15 +21,15 @@ private: struct ctor_tag {}; std::unique_ptr bit_vector; +public: GlobalFilter(const GlobalFilter &) = delete; GlobalFilter(GlobalFilter &&) = delete; -public: - GlobalFilter(ctor_tag, std::unique_ptr bit_vector_in) + GlobalFilter(ctor_tag, std::unique_ptr bit_vector_in) noexcept : bit_vector(std::move(bit_vector_in)) {} - GlobalFilter(ctor_tag) : bit_vector() {} + GlobalFilter(ctor_tag) noexcept : bit_vector() {} ~GlobalFilter() {} diff --git a/searchlib/src/vespa/searchlib/queryeval/sourceblendersearch.h b/searchlib/src/vespa/searchlib/queryeval/sourceblendersearch.h index f209e0f7fd8..b9812036637 100644 --- a/searchlib/src/vespa/searchlib/queryeval/sourceblendersearch.h +++ b/searchlib/src/vespa/searchlib/queryeval/sourceblendersearch.h @@ -29,7 +29,7 @@ public: SearchIterator *search; uint32_t sourceId; Child() : search(nullptr), sourceId(0) { } - Child(SearchIterator *s, uint32_t id) : search(s), sourceId(id) {} + Child(SearchIterator *s, uint32_t id) noexcept : search(s), sourceId(id) {} }; typedef std::vector Children; diff --git a/searchlib/src/vespa/searchlib/queryeval/wand/wand_parts.h b/searchlib/src/vespa/searchlib/queryeval/wand/wand_parts.h index 071d6d99470..937e7a089df 100644 --- a/searchlib/src/vespa/searchlib/queryeval/wand/wand_parts.h +++ b/searchlib/src/vespa/searchlib/queryeval/wand/wand_parts.h @@ -41,11 +41,12 @@ struct Term { uint32_t estHits; fef::TermFieldMatchData *matchData; score_t maxScore = 0.0; // <- only used by rise wand test - Term(SearchIterator *s, int32_t w, uint32_t e, fef::TermFieldMatchData *tfmd) - : search(s), weight(w), estHits(e), matchData(tfmd) {} - Term() : Term(nullptr, 0, 0, nullptr){} - Term(SearchIterator *s, int32_t w, uint32_t e) : Term(s, w, e, nullptr) {} - Term(SearchIterator::UP s, int32_t w, uint32_t e) : Term(s.release(), w, e, nullptr) {} + Term(SearchIterator *s, int32_t w, uint32_t e, fef::TermFieldMatchData *tfmd) noexcept + : search(s), weight(w), estHits(e), matchData(tfmd) + {} + Term() noexcept : Term(nullptr, 0, 0, nullptr){} + Term(SearchIterator *s, int32_t w, uint32_t e) noexcept : Term(s, w, e, nullptr) {} + Term(SearchIterator::UP s, int32_t w, uint32_t e) noexcept : Term(s.release(), w, e, nullptr) {} }; //----------------------------------------------------------------------------- diff --git a/searchlib/src/vespa/searchlib/tensor/hnsw_index.cpp b/searchlib/src/vespa/searchlib/tensor/hnsw_index.cpp index 9def5a7b0a8..6488b525b7c 100644 --- a/searchlib/src/vespa/searchlib/tensor/hnsw_index.cpp +++ b/searchlib/src/vespa/searchlib/tensor/hnsw_index.cpp @@ -40,7 +40,7 @@ struct PairDist { uint32_t id_first; uint32_t id_second; double distance; - PairDist(uint32_t i1, uint32_t i2, double d) + PairDist(uint32_t i1, uint32_t i2, double d) noexcept : id_first(i1), id_second(i2), distance(d) {} }; diff --git a/searchlib/src/vespa/searchlib/tensor/hnsw_index_utils.h b/searchlib/src/vespa/searchlib/tensor/hnsw_index_utils.h index 99266505780..d206b055071 100644 --- a/searchlib/src/vespa/searchlib/tensor/hnsw_index_utils.h +++ b/searchlib/src/vespa/searchlib/tensor/hnsw_index_utils.h @@ -16,9 +16,9 @@ struct HnswCandidate { uint32_t docid; HnswGraph::NodeRef node_ref; double distance; - HnswCandidate(uint32_t docid_in, double distance_in) + HnswCandidate(uint32_t docid_in, double distance_in) noexcept : docid(docid_in), node_ref(), distance(distance_in) {} - HnswCandidate(uint32_t docid_in, HnswGraph::NodeRef node_ref_in, double distance_in) + HnswCandidate(uint32_t docid_in, HnswGraph::NodeRef node_ref_in, double distance_in) noexcept : docid(docid_in), node_ref(node_ref_in), distance(distance_in) {} }; diff --git a/searchlib/src/vespa/searchlib/tensor/nearest_neighbor_index.h b/searchlib/src/vespa/searchlib/tensor/nearest_neighbor_index.h index 74f14cea21b..c14da0d058f 100644 --- a/searchlib/src/vespa/searchlib/tensor/nearest_neighbor_index.h +++ b/searchlib/src/vespa/searchlib/tensor/nearest_neighbor_index.h @@ -29,12 +29,12 @@ public: struct Neighbor { uint32_t docid; double distance; - Neighbor(uint32_t id, double dist) + Neighbor(uint32_t id, double dist) noexcept : docid(id), distance(dist) {} - Neighbor() : docid(0), distance(0.0) {} + Neighbor() noexcept : docid(0), distance(0.0) {} }; - virtual ~NearestNeighborIndex() {} + virtual ~NearestNeighborIndex() = default; virtual void add_document(uint32_t docid) = 0; /** diff --git a/searchlib/src/vespa/searchlib/test/imported_attribute_fixture.h b/searchlib/src/vespa/searchlib/test/imported_attribute_fixture.h index e2fcbf444a2..0a04e1e79fd 100644 --- a/searchlib/src/vespa/searchlib/test/imported_attribute_fixture.h +++ b/searchlib/src/vespa/searchlib/test/imported_attribute_fixture.h @@ -23,7 +23,7 @@ namespace search { struct MockDocumentMetaStoreContext : public IDocumentMetaStoreContext { mutable size_t get_read_guard_cnt; - MockDocumentMetaStoreContext() : get_read_guard_cnt(0) {} + MockDocumentMetaStoreContext() noexcept : get_read_guard_cnt(0) {} IReadGuard::UP getReadGuard() const override; }; diff --git a/staging_vespalib/src/tests/metrics/mock_tick.h b/staging_vespalib/src/tests/metrics/mock_tick.h index 3f244ea6c9f..e8f621508ae 100644 --- a/staging_vespalib/src/tests/metrics/mock_tick.h +++ b/staging_vespalib/src/tests/metrics/mock_tick.h @@ -55,7 +55,7 @@ private: } public: - MockTick(TimeStamp first_value) + explicit MockTick(TimeStamp first_value) : _first_value(first_value), _lock(), _cond(), _alive(true), _prev(), _next() {} TimeStamp first() override { return _first_value; } TimeStamp next(TimeStamp prev) override { @@ -81,7 +81,7 @@ class TickProxy : public Tick { private: std::shared_ptr _tick; public: - TickProxy(std::shared_ptr tick) : _tick(std::move(tick)) {} + explicit TickProxy(std::shared_ptr tick) noexcept : _tick(std::move(tick)) {} TimeStamp first() override { return _tick->first(); } TimeStamp next(TimeStamp prev) override { return _tick->next(prev); } bool alive() const override { return _tick->alive(); } diff --git a/staging_vespalib/src/tests/polymorphicarray/polymorphicarray_test.cpp b/staging_vespalib/src/tests/polymorphicarray/polymorphicarray_test.cpp index e3ff7533d58..49bc52d6239 100644 --- a/staging_vespalib/src/tests/polymorphicarray/polymorphicarray_test.cpp +++ b/staging_vespalib/src/tests/polymorphicarray/polymorphicarray_test.cpp @@ -7,7 +7,7 @@ using namespace vespalib; class A { public: - virtual ~A() { } + virtual ~A() = default; virtual void assign(const A & rhs) { (void) rhs; assert(false); } // Required by the primitive array. virtual A * clone() const { assert(false); return nullptr; } // Required for the complex array. @@ -19,7 +19,7 @@ public: class Primitive : public A { public: - Primitive(size_t v=11) : _v(v) { } + Primitive(size_t v=11) noexcept : _v(v) { } size_t value() const { return _v; } bool operator == (const A & rhs) const override { return dynamic_cast(rhs).value() == value(); @@ -38,7 +38,7 @@ private: class Complex : public A { public: - Complex(size_t v=11) : _v(v) { } + Complex(size_t v=11) noexcept : _v(v) { } size_t value() const { return _v; } bool operator == (const A & rhs) const override { return dynamic_cast(rhs).value() == value(); diff --git a/staging_vespalib/src/tests/sequencedtaskexecutor/adaptive_sequenced_executor_test.cpp b/staging_vespalib/src/tests/sequencedtaskexecutor/adaptive_sequenced_executor_test.cpp index 2ca49105610..e75cdd09c8c 100644 --- a/staging_vespalib/src/tests/sequencedtaskexecutor/adaptive_sequenced_executor_test.cpp +++ b/staging_vespalib/src/tests/sequencedtaskexecutor/adaptive_sequenced_executor_test.cpp @@ -31,7 +31,7 @@ public: int _fail; int _val; - TestObj() + TestObj() noexcept : _m(), _cv(), _done(0), diff --git a/staging_vespalib/src/tests/sequencedtaskexecutor/foregroundtaskexecutor_test.cpp b/staging_vespalib/src/tests/sequencedtaskexecutor/foregroundtaskexecutor_test.cpp index a2671bb81a7..03ec64b771e 100644 --- a/staging_vespalib/src/tests/sequencedtaskexecutor/foregroundtaskexecutor_test.cpp +++ b/staging_vespalib/src/tests/sequencedtaskexecutor/foregroundtaskexecutor_test.cpp @@ -33,7 +33,7 @@ public: int _fail; int _val; - TestObj() + TestObj() noexcept : _m(), _cv(), _done(0), diff --git a/staging_vespalib/src/tests/sequencedtaskexecutor/sequencedtaskexecutor_test.cpp b/staging_vespalib/src/tests/sequencedtaskexecutor/sequencedtaskexecutor_test.cpp index df94e70f9d6..29b25cd0471 100644 --- a/staging_vespalib/src/tests/sequencedtaskexecutor/sequencedtaskexecutor_test.cpp +++ b/staging_vespalib/src/tests/sequencedtaskexecutor/sequencedtaskexecutor_test.cpp @@ -33,7 +33,7 @@ public: int _fail; int _val; - TestObj() + TestObj() noexcept : _m(), _cv(), _done(0), diff --git a/staging_vespalib/src/vespa/vespalib/metrics/dummy_metrics_manager.cpp b/staging_vespalib/src/vespa/vespalib/metrics/dummy_metrics_manager.cpp index cd7a3abb1eb..7909ec7486e 100644 --- a/staging_vespalib/src/vespa/vespalib/metrics/dummy_metrics_manager.cpp +++ b/staging_vespalib/src/vespa/vespalib/metrics/dummy_metrics_manager.cpp @@ -1,10 +1,9 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "dummy_metrics_manager.h" -namespace vespalib { -namespace metrics { +namespace vespalib::metrics { -DummyMetricsManager::~DummyMetricsManager() {} +DummyMetricsManager::~DummyMetricsManager() = default; Snapshot DummyMetricsManager::snapshot() @@ -20,5 +19,4 @@ DummyMetricsManager::totalSnapshot() return snap; } -} // namespace vespalib::metrics -} // namespace vespalib +} diff --git a/staging_vespalib/src/vespa/vespalib/metrics/dummy_metrics_manager.h b/staging_vespalib/src/vespa/vespalib/metrics/dummy_metrics_manager.h index 6aeb1137732..1536888014a 100644 --- a/staging_vespalib/src/vespa/vespalib/metrics/dummy_metrics_manager.h +++ b/staging_vespalib/src/vespa/vespalib/metrics/dummy_metrics_manager.h @@ -10,8 +10,7 @@ #include "metrics_manager.h" #include "clock.h" -namespace vespalib { -namespace metrics { +namespace vespalib::metrics { /** * Dummy manager that discards everything, use @@ -21,9 +20,9 @@ namespace metrics { class DummyMetricsManager : public MetricsManager { protected: - DummyMetricsManager() {} + DummyMetricsManager() noexcept {} public: - ~DummyMetricsManager(); + ~DummyMetricsManager() override; static std::shared_ptr create() { return std::shared_ptr(new DummyMetricsManager()); @@ -58,5 +57,4 @@ public: void sample(Gauge::Measurement) override {} }; -} // namespace vespalib::metrics -} // namespace vespalib +} diff --git a/staging_vespalib/src/vespa/vespalib/objects/identifiable.hpp b/staging_vespalib/src/vespa/vespalib/objects/identifiable.hpp index 2c34ba306ab..b1c4a128feb 100644 --- a/staging_vespalib/src/vespa/vespalib/objects/identifiable.hpp +++ b/staging_vespalib/src/vespa/vespalib/objects/identifiable.hpp @@ -46,16 +46,16 @@ class IdentifiablePtr : public CloneablePtr { public: IdentifiablePtr(const T &t) : CloneablePtr(t.clone()) {} - IdentifiablePtr(IdentifiablePtr &&) = default; - IdentifiablePtr & operator = (IdentifiablePtr &&) = default; + IdentifiablePtr(IdentifiablePtr &&) noexcept = default; + IdentifiablePtr & operator = (IdentifiablePtr &&) noexcept = default; IdentifiablePtr(const IdentifiablePtr &) = default; IdentifiablePtr & operator = (const IdentifiablePtr &) = default; - IdentifiablePtr(T * p=NULL) : CloneablePtr(p) { } - IdentifiablePtr(std::unique_ptr &&rhs) + IdentifiablePtr(T * p=NULL) noexcept : CloneablePtr(p) { } + IdentifiablePtr(std::unique_ptr &&rhs) noexcept : CloneablePtr(std::move(rhs)) { } - IdentifiablePtr &operator=(std::unique_ptr &&rhs) + IdentifiablePtr &operator=(std::unique_ptr &&rhs) noexcept { CloneablePtr::operator=(std::move(rhs)); return *this; diff --git a/storage/src/tests/bucketdb/lockablemaptest.cpp b/storage/src/tests/bucketdb/lockablemaptest.cpp index 709e158e531..527c0a927b4 100644 --- a/storage/src/tests/bucketdb/lockablemaptest.cpp +++ b/storage/src/tests/bucketdb/lockablemaptest.cpp @@ -148,7 +148,7 @@ namespace { template struct NonConstProcessor { - typename Map::Decision operator()(int key, A& a) { + typename Map::Decision operator() (int key, A& a) const noexcept { (void) key; ++a._val2; return Map::UPDATE; diff --git a/storage/src/tests/persistence/persistencetestutils.h b/storage/src/tests/persistence/persistencetestutils.h index 3d25a205017..d4cbd896d11 100644 --- a/storage/src/tests/persistence/persistencetestutils.h +++ b/storage/src/tests/persistence/persistencetestutils.h @@ -42,7 +42,7 @@ public: class NoBucketLock : public FileStorHandler::BucketLockInterface { public: - NoBucketLock(document::Bucket bucket) : _bucket(bucket) { } + NoBucketLock(document::Bucket bucket) noexcept : _bucket(bucket) { } const document::Bucket &getBucket() const override { return _bucket; } diff --git a/storage/src/vespa/storage/bucketdb/abstract_bucket_map.h b/storage/src/vespa/storage/bucketdb/abstract_bucket_map.h index 9b738b26994..be83970e2b7 100644 --- a/storage/src/vespa/storage/bucketdb/abstract_bucket_map.h +++ b/storage/src/vespa/storage/bucketdb/abstract_bucket_map.h @@ -36,7 +36,7 @@ public: key_type _key; bool _locked; - LockKeeper(AbstractBucketMap& map, key_type key) + LockKeeper(AbstractBucketMap& map, key_type key) noexcept : _map(map), _key(key), _locked(true) {} void unlock() { _map.unlock(_key); _locked = false; } public: @@ -44,7 +44,7 @@ public: }; struct WrappedEntry { - WrappedEntry() + WrappedEntry() noexcept : _exists(false), _preExisted(false), _lockKeeper(), @@ -103,16 +103,16 @@ public: key_type _key; const char* _owner; - LockId() : _key(0), _owner("none - empty token") {} - LockId(key_type key, const char* owner) + LockId() noexcept : _key(0), _owner("none - empty token") {} + LockId(key_type key, const char* owner) noexcept : _key(key), _owner(owner) { assert(_owner); } - size_t hash() const { return _key; } - size_t operator%(size_t val) const { return _key % val; } - bool operator==(const LockId& id) const { return (_key == id._key); } + size_t hash() const noexcept { return _key; } + size_t operator%(size_t val) const noexcept { return _key % val; } + bool operator==(const LockId& id) const noexcept { return (_key == id._key); } operator key_type() const { return _key; } }; diff --git a/storage/src/vespa/storage/bucketdb/bucketcopy.h b/storage/src/vespa/storage/bucketdb/bucketcopy.h index 09f86ef1324..94a1e63e53e 100644 --- a/storage/src/vespa/storage/bucketdb/bucketcopy.h +++ b/storage/src/vespa/storage/bucketdb/bucketcopy.h @@ -15,12 +15,12 @@ private: public: static const int TRUSTED = 1; - BucketCopy() + BucketCopy() noexcept : _timestamp(0), _flags(0), _node(0xffff) {} BucketCopy(uint64_t timestamp, uint16_t nodeIdx, - const api::BucketInfo& info) + const api::BucketInfo& info) noexcept : _timestamp(timestamp), _info(info), _flags(0), diff --git a/storage/src/vespa/storage/distributor/distributor.cpp b/storage/src/vespa/storage/distributor/distributor.cpp index cfd8d7f1753..e577d219f1b 100644 --- a/storage/src/vespa/storage/distributor/distributor.cpp +++ b/storage/src/vespa/storage/distributor/distributor.cpp @@ -28,7 +28,7 @@ class Distributor::Status { bool _done; public: - Status(const DelegatedStatusRequest& request) + Status(const DelegatedStatusRequest& request) noexcept : _request(request), _monitor(), _done(false) diff --git a/storage/src/vespa/storage/distributor/messagetracker.h b/storage/src/vespa/storage/distributor/messagetracker.h index 75ae287d98f..11d8c36082e 100644 --- a/storage/src/vespa/storage/distributor/messagetracker.h +++ b/storage/src/vespa/storage/distributor/messagetracker.h @@ -17,7 +17,7 @@ class MessageTracker { public: class ToSend { public: - ToSend(std::shared_ptr msg, uint16_t target) : + ToSend(std::shared_ptr msg, uint16_t target) noexcept : _msg(std::move(msg)), _target(target) {}; std::shared_ptr _msg; diff --git a/storage/src/vespa/storage/distributor/operation_sequencer.h b/storage/src/vespa/storage/distributor/operation_sequencer.h index cf687e721a8..7a523b61e53 100644 --- a/storage/src/vespa/storage/distributor/operation_sequencer.h +++ b/storage/src/vespa/storage/distributor/operation_sequencer.h @@ -24,7 +24,7 @@ class SequencingHandle { OperationSequencer* _sequencer; document::GlobalId _gid; public: - SequencingHandle() : _sequencer(nullptr) {} + SequencingHandle() noexcept : _sequencer(nullptr) {} SequencingHandle(OperationSequencer& sequencer, const document::GlobalId& gid) : _sequencer(&sequencer), _gid(gid) diff --git a/storage/src/vespa/storage/distributor/operations/external/updateoperation.h b/storage/src/vespa/storage/distributor/operations/external/updateoperation.h index c66026f02f5..9d9bce9160b 100644 --- a/storage/src/vespa/storage/distributor/operations/external/updateoperation.h +++ b/storage/src/vespa/storage/distributor/operations/external/updateoperation.h @@ -55,7 +55,7 @@ private: class PreviousDocumentVersion { public: - PreviousDocumentVersion(document::BucketId b, const api::BucketInfo& info, uint64_t o, uint16_t node) : + PreviousDocumentVersion(document::BucketId b, const api::BucketInfo& info, uint64_t o, uint16_t node) noexcept : bucketId(b), bucketInfo(info), oldTs(o), nodeId(node) {} document::BucketId bucketId; diff --git a/storage/src/vespa/storage/distributor/operations/idealstate/mergemetadata.h b/storage/src/vespa/storage/distributor/operations/idealstate/mergemetadata.h index fb84441b654..273603ca6ad 100644 --- a/storage/src/vespa/storage/distributor/operations/idealstate/mergemetadata.h +++ b/storage/src/vespa/storage/distributor/operations/idealstate/mergemetadata.h @@ -3,26 +3,22 @@ #pragma once #include -#include -namespace storage { -namespace distributor { +namespace storage::distributor { struct MergeMetaData { uint16_t _nodeIndex; bool _sourceOnly; const BucketCopy* _copy; - MergeMetaData() : _nodeIndex(0), _sourceOnly(false), _copy(0) {} - MergeMetaData(uint16_t nodeIndex, const BucketCopy& copy) + MergeMetaData() noexcept : _nodeIndex(0), _sourceOnly(false), _copy(nullptr) {} + MergeMetaData(uint16_t nodeIndex, const BucketCopy& copy) noexcept : _nodeIndex(nodeIndex), _sourceOnly(false), _copy(©) {} bool trusted() const { - assert(_copy != 0); return _copy->trusted(); } uint32_t checksum() const { - assert(_copy != 0); return _copy->getChecksum(); } bool source_only() const noexcept { return _sourceOnly; } @@ -30,6 +26,4 @@ struct MergeMetaData { vespalib::asciistream& operator<<(vespalib::asciistream& out, const MergeMetaData& e); -} // distributor -} // storage - +} diff --git a/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp b/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp index 85604299e85..0341e34d39d 100644 --- a/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp +++ b/storage/src/vespa/storage/persistence/filestorage/filestormanager.cpp @@ -871,7 +871,7 @@ FileStorManager::reportHtmlStatus(std::ostream& out, const framework::HttpUrlPat namespace { struct Deactivator { - StorBucketDatabase::Decision operator()(document::BucketId::Type, StorBucketDatabase::Entry& data) + StorBucketDatabase::Decision operator() (document::BucketId::Type, StorBucketDatabase::Entry& data) noexcept { data.info.setActive(false); return StorBucketDatabase::Decision::UPDATE; diff --git a/storage/src/vespa/storage/storageserver/configurable_bucket_resolver.h b/storage/src/vespa/storage/storageserver/configurable_bucket_resolver.h index 0800cc9612e..34e30aaea48 100644 --- a/storage/src/vespa/storage/storageserver/configurable_bucket_resolver.h +++ b/storage/src/vespa/storage/storageserver/configurable_bucket_resolver.h @@ -21,7 +21,7 @@ public: using BucketSpaceMapping = std::unordered_map>; const BucketSpaceMapping _type_to_space; public: - explicit ConfigurableBucketResolver(BucketSpaceMapping type_to_space) + explicit ConfigurableBucketResolver(BucketSpaceMapping type_to_space) noexcept : _type_to_space(std::move(type_to_space)) {} diff --git a/storage/src/vespa/storage/storageserver/service_layer_error_listener.h b/storage/src/vespa/storage/storageserver/service_layer_error_listener.h index 6995459e333..5dd01329ff4 100644 --- a/storage/src/vespa/storage/storageserver/service_layer_error_listener.h +++ b/storage/src/vespa/storage/storageserver/service_layer_error_listener.h @@ -23,7 +23,7 @@ class ServiceLayerErrorListener : public ProviderErrorListener { std::atomic _shutdown_initiated; public: ServiceLayerErrorListener(StorageComponent& component, - MergeThrottler& merge_throttler) + MergeThrottler& merge_throttler) noexcept : _component(component), _merge_throttler(merge_throttler), _shutdown_initiated(false) diff --git a/storage/src/vespa/storage/storageserver/statemanager.cpp b/storage/src/vespa/storage/storageserver/statemanager.cpp index da221e312e2..f9b5e284b93 100644 --- a/storage/src/vespa/storage/storageserver/statemanager.cpp +++ b/storage/src/vespa/storage/storageserver/statemanager.cpp @@ -224,7 +224,7 @@ StateManager::removeStateListener(StateListener& listener) struct StateManager::ExternalStateLock : public NodeStateUpdater::Lock { StateManager& _manager; - explicit ExternalStateLock(StateManager& manager) : _manager(manager) {} + explicit ExternalStateLock(StateManager& manager) noexcept : _manager(manager) {} ~ExternalStateLock() override { { vespalib::MonitorGuard lock(_manager._stateLock); diff --git a/storageapi/src/vespa/storageapi/buckets/bucketinfo.cpp b/storageapi/src/vespa/storageapi/buckets/bucketinfo.cpp index c3980784106..7466d5c603e 100644 --- a/storageapi/src/vespa/storageapi/buckets/bucketinfo.cpp +++ b/storageapi/src/vespa/storageapi/buckets/bucketinfo.cpp @@ -5,7 +5,7 @@ namespace storage::api { -BucketInfo::BucketInfo() +BucketInfo::BucketInfo() noexcept : _lastModified(0), _checksum(0), _docCount(0), @@ -17,7 +17,7 @@ BucketInfo::BucketInfo() {} BucketInfo::BucketInfo(uint32_t checksum, uint32_t docCount, - uint32_t totDocSize) + uint32_t totDocSize) noexcept : _lastModified(0), _checksum(checksum), _docCount(docCount), @@ -30,7 +30,7 @@ BucketInfo::BucketInfo(uint32_t checksum, uint32_t docCount, BucketInfo::BucketInfo(uint32_t checksum, uint32_t docCount, uint32_t totDocSize, uint32_t metaCount, - uint32_t usedFileSize) + uint32_t usedFileSize) noexcept : _lastModified(0), _checksum(checksum), _docCount(docCount), @@ -44,7 +44,7 @@ BucketInfo::BucketInfo(uint32_t checksum, uint32_t docCount, BucketInfo::BucketInfo(uint32_t checksum, uint32_t docCount, uint32_t totDocSize, uint32_t metaCount, uint32_t usedFileSize, - bool ready, bool active) + bool ready, bool active) noexcept : _lastModified(0), _checksum(checksum), _docCount(docCount), @@ -58,7 +58,7 @@ BucketInfo::BucketInfo(uint32_t checksum, uint32_t docCount, BucketInfo::BucketInfo(uint32_t checksum, uint32_t docCount, uint32_t totDocSize, uint32_t metaCount, uint32_t usedFileSize, - bool ready, bool active, Timestamp lastModified) + bool ready, bool active, Timestamp lastModified) noexcept : _lastModified(lastModified), _checksum(checksum), _docCount(docCount), @@ -69,9 +69,9 @@ BucketInfo::BucketInfo(uint32_t checksum, uint32_t docCount, _active(active) {} -BucketInfo::BucketInfo(const BucketInfo &) = default; -BucketInfo & BucketInfo::operator = (const BucketInfo &) = default; -BucketInfo::~BucketInfo() {} +BucketInfo::BucketInfo(const BucketInfo &) noexcept = default; +BucketInfo & BucketInfo::operator = (const BucketInfo &) noexcept = default; +BucketInfo::~BucketInfo() = default; bool BucketInfo::operator==(const BucketInfo& info) const diff --git a/storageapi/src/vespa/storageapi/buckets/bucketinfo.h b/storageapi/src/vespa/storageapi/buckets/bucketinfo.h index a80595527e2..f15d99588ca 100644 --- a/storageapi/src/vespa/storageapi/buckets/bucketinfo.h +++ b/storageapi/src/vespa/storageapi/buckets/bucketinfo.h @@ -31,18 +31,18 @@ class BucketInfo : public vespalib::AsciiPrintable bool _active; public: - BucketInfo(); - BucketInfo(uint32_t checksum, uint32_t docCount, uint32_t totDocSize); + BucketInfo() noexcept; + BucketInfo(uint32_t checksum, uint32_t docCount, uint32_t totDocSize) noexcept; BucketInfo(uint32_t checksum, uint32_t docCount, uint32_t totDocSize, - uint32_t metaCount, uint32_t usedFileSize); + uint32_t metaCount, uint32_t usedFileSize) noexcept; BucketInfo(uint32_t checksum, uint32_t docCount, uint32_t totDocSize, uint32_t metaCount, uint32_t usedFileSize, - bool ready, bool active); + bool ready, bool active) noexcept; BucketInfo(uint32_t checksum, uint32_t docCount, uint32_t totDocSize, uint32_t metaCount, uint32_t usedFileSize, - bool ready, bool active, Timestamp lastModified); - BucketInfo(const BucketInfo &); - BucketInfo & operator = (const BucketInfo &); + bool ready, bool active, Timestamp lastModified) noexcept; + BucketInfo(const BucketInfo &) noexcept; + BucketInfo & operator = (const BucketInfo &) noexcept; ~BucketInfo(); Timestamp getLastModified() const { return _lastModified; } diff --git a/storageapi/src/vespa/storageapi/defs.h b/storageapi/src/vespa/storageapi/defs.h index e5445f22870..531a1b13120 100644 --- a/storageapi/src/vespa/storageapi/defs.h +++ b/storageapi/src/vespa/storageapi/defs.h @@ -8,8 +8,7 @@ #include -namespace storage { -namespace api { +namespace storage:: api { typedef uint64_t Timestamp; typedef uint32_t VisitorId; @@ -17,4 +16,3 @@ typedef uint32_t VisitorId; const Timestamp MAX_TIMESTAMP = (Timestamp)-1ll; } -} diff --git a/storageapi/src/vespa/storageapi/message/bucket.h b/storageapi/src/vespa/storageapi/message/bucket.h index 45af2296e8a..2b09967d95b 100644 --- a/storageapi/src/vespa/storageapi/message/bucket.h +++ b/storageapi/src/vespa/storageapi/message/bucket.h @@ -104,10 +104,10 @@ public: uint16_t index; bool sourceOnly; - Node(uint16_t index_, bool sourceOnly_ = false) + Node(uint16_t index_, bool sourceOnly_ = false) noexcept : index(index_), sourceOnly(sourceOnly_) {} - bool operator==(const Node& n) const + bool operator==(const Node& n) const noexcept { return (index == n.index && sourceOnly == n.sourceOnly); } }; @@ -123,7 +123,7 @@ public: Timestamp maxTimestamp, uint32_t clusterStateVersion = 0, const std::vector& chain = std::vector()); - ~MergeBucketCommand(); + ~MergeBucketCommand() override; const std::vector& getNodes() const { return _nodes; } Timestamp getMaxTimestamp() const { return _maxTimestamp; } diff --git a/storageapi/src/vespa/storageapi/message/visitor.h b/storageapi/src/vespa/storageapi/message/visitor.h index 7189cc67195..67c41a0cc4d 100644 --- a/storageapi/src/vespa/storageapi/message/visitor.h +++ b/storageapi/src/vespa/storageapi/message/visitor.h @@ -186,12 +186,12 @@ public: document::BucketId bucketId; Timestamp timestamp; - BucketTimestampPair() : bucketId(), timestamp(0) {} - BucketTimestampPair(const document::BucketId& bucket, - const Timestamp& ts) - : bucketId(bucket), timestamp(ts) {} + BucketTimestampPair() noexcept : bucketId(), timestamp(0) {} + BucketTimestampPair(const document::BucketId& bucket, const Timestamp& ts) noexcept + : bucketId(bucket), timestamp(ts) + {} - bool operator==(const BucketTimestampPair& other) const { + bool operator==(const BucketTimestampPair& other) const noexcept { return (bucketId == other.bucketId && timestamp && other.timestamp); } }; @@ -203,7 +203,7 @@ private: public: VisitorInfoCommand(); - ~VisitorInfoCommand(); + ~VisitorInfoCommand() override; void setErrorCode(const ReturnCode& code) { _error = code; } void setCompleted() { _completed = true; } diff --git a/streamingvisitors/src/vespa/searchvisitor/matching_elements_filler.cpp b/streamingvisitors/src/vespa/searchvisitor/matching_elements_filler.cpp index d51bd57e942..13b396f5284 100644 --- a/streamingvisitors/src/vespa/searchvisitor/matching_elements_filler.cpp +++ b/streamingvisitors/src/vespa/searchvisitor/matching_elements_filler.cpp @@ -30,7 +30,7 @@ struct SubFieldTerm vespalib::string _field_name; const QueryTerm* _term; public: - SubFieldTerm(vespalib::string field_name, const QueryTerm* term) + SubFieldTerm(vespalib::string field_name, const QueryTerm* term) noexcept : _field_name(std::move(field_name)), _term(term) { diff --git a/streamingvisitors/src/vespa/searchvisitor/searchvisitor.h b/streamingvisitors/src/vespa/searchvisitor/searchvisitor.h index 9f1887c3b53..eb3879863a5 100644 --- a/streamingvisitors/src/vespa/searchvisitor/searchvisitor.h +++ b/streamingvisitors/src/vespa/searchvisitor/searchvisitor.h @@ -57,7 +57,7 @@ private: * @param fid the field id of the attribute field. * @param attr a guard to the attribute vector. **/ - AttrInfo(vsm::FieldIdT fid, search::AttributeGuard::UP attr) : + AttrInfo(vsm::FieldIdT fid, search::AttributeGuard::UP attr) noexcept : _field(fid), _ascending(true), _converter(nullptr), @@ -71,7 +71,7 @@ private: * @param ascending whether this attribute should be sorted ascending or not. * @param converter is a converter to apply to the attribute before sorting. **/ - AttrInfo(vsm::FieldIdT fid, search::AttributeGuard::UP attr, bool ascending, const search::common::BlobConverter * converter) : + AttrInfo(vsm::FieldIdT fid, search::AttributeGuard::UP attr, bool ascending, const search::common::BlobConverter * converter) noexcept : _field(fid), _ascending(ascending), _converter(converter), diff --git a/vbench/src/vbench/core/time_queue.h b/vbench/src/vbench/core/time_queue.h index 2af6f3edbcd..5237a6bc0fb 100644 --- a/vbench/src/vbench/core/time_queue.h +++ b/vbench/src/vbench/core/time_queue.h @@ -21,14 +21,14 @@ private: struct Entry { std::unique_ptr object; double time; - Entry(std::unique_ptr obj, double t) : object(std::move(obj)), time(t) {} - Entry(Entry &&rhs) : object(std::move(rhs.object)), time(rhs.time) {} - Entry &operator=(Entry &&rhs) { + Entry(std::unique_ptr obj, double t) noexcept : object(std::move(obj)), time(t) {} + Entry(Entry &&rhs) noexcept : object(std::move(rhs.object)), time(rhs.time) {} + Entry &operator=(Entry &&rhs) noexcept { object = std::move(rhs.object); time = rhs.time; return *this; } - bool operator<(const Entry &rhs) const { + bool operator<(const Entry &rhs) const noexcept { return (time < rhs.time); } }; @@ -42,6 +42,7 @@ private: public: TimeQueue(double window, double tick); + ~TimeQueue(); void close() override; void discard(); void insert(std::unique_ptr obj, double time); diff --git a/vbench/src/vbench/core/time_queue.hpp b/vbench/src/vbench/core/time_queue.hpp index 4a70e258935..0a1a74d72db 100644 --- a/vbench/src/vbench/core/time_queue.hpp +++ b/vbench/src/vbench/core/time_queue.hpp @@ -13,6 +13,9 @@ TimeQueue::TimeQueue(double window, double tick) { } +template +TimeQueue::~TimeQueue() = default; + template void TimeQueue::close() diff --git a/vespalib/src/tests/delegatelist/delegatelist.cpp b/vespalib/src/tests/delegatelist/delegatelist.cpp index 070864dd85a..4dc7e5c97d7 100644 --- a/vespalib/src/tests/delegatelist/delegatelist.cpp +++ b/vespalib/src/tests/delegatelist/delegatelist.cpp @@ -77,15 +77,15 @@ struct Command int cmd; int cnt; Handler *handler; - Command(DL *dl_, int cmd_, int cnt_, Handler *handler_) + Command(DL *dl_, int cmd_, int cnt_, Handler *handler_) noexcept : dl(dl_), cmd(cmd_), cnt(cnt_), handler(handler_) {} - Command(const Command &rhs) + Command(const Command &rhs) noexcept : dl(rhs.dl), cmd(rhs.cmd), cnt(rhs.cnt), handler(rhs.handler) {} - Command &operator=(const Command &rhs) { + Command &operator=(const Command &rhs) noexcept { memcpy(this, &rhs, sizeof(Command)); return *this; } - bool operator==(const Command &rhs) { + bool operator==(const Command &rhs) noexcept { return memcmp(this, &rhs, sizeof(Command)) == 0; } }; diff --git a/vespalib/src/tests/explore_modern_cpp/explore_modern_cpp_test.cpp b/vespalib/src/tests/explore_modern_cpp/explore_modern_cpp_test.cpp index 1ba368b39a3..29a82138d29 100644 --- a/vespalib/src/tests/explore_modern_cpp/explore_modern_cpp_test.cpp +++ b/vespalib/src/tests/explore_modern_cpp/explore_modern_cpp_test.cpp @@ -5,7 +5,7 @@ TEST("verify how std::function copies lambda closures") { size_t count = 0; size_t value = 0; - auto closure = [count,&value]()mutable{ ++count; value += count; }; + auto closure = [count,&value]() mutable noexcept { ++count; value += count; }; closure(); EXPECT_EQUAL(0u, count); EXPECT_EQUAL(1u, value); // +1 diff --git a/vespalib/src/tests/net/async_resolver/async_resolver_test.cpp b/vespalib/src/tests/net/async_resolver/async_resolver_test.cpp index 434cb8d6a69..eca43ea5cec 100644 --- a/vespalib/src/tests/net/async_resolver/async_resolver_test.cpp +++ b/vespalib/src/tests/net/async_resolver/async_resolver_test.cpp @@ -11,7 +11,7 @@ using namespace vespalib; struct ResultSetter : public AsyncResolver::ResultHandler { SocketAddress &addr; std::atomic done; - ResultSetter(SocketAddress &addr_out) : addr(addr_out), done(false) {} + ResultSetter(SocketAddress &addr_out) noexcept : addr(addr_out), done(false) {} void handle_result(SocketAddress result) override { addr = result; done = true; @@ -31,7 +31,7 @@ struct MyClock : public AsyncResolver::Clock { struct BlockingHostResolver : public AsyncResolver::HostResolver { CountDownLatch callers; Gate barrier; - BlockingHostResolver(size_t num_callers) + BlockingHostResolver(size_t num_callers) noexcept : callers(num_callers), barrier() {} vespalib::string ip_address(const vespalib::string &) override { callers.countDown(); diff --git a/vespalib/src/tests/priority_queue/priority_queue_test.cpp b/vespalib/src/tests/priority_queue/priority_queue_test.cpp index 3457c2bde6b..af8dd853c6c 100644 --- a/vespalib/src/tests/priority_queue/priority_queue_test.cpp +++ b/vespalib/src/tests/priority_queue/priority_queue_test.cpp @@ -154,11 +154,11 @@ TEST("require that priority queue works with move-only objects") { struct MyItem { int value; int *ref; - MyItem(int v, int &r) : value(v), ref(&r) {} - MyItem(const MyItem &) = delete; - MyItem &operator=(const MyItem &) = delete; - MyItem(MyItem &&rhs) : value(rhs.value), ref(rhs.ref) { rhs.ref = 0; } - MyItem &operator=(MyItem &&rhs) { + MyItem(int v, int &r) noexcept : value(v), ref(&r) {} + MyItem(const MyItem &) noexcept = delete; + MyItem &operator=(const MyItem &) noexcept = delete; + MyItem(MyItem &&rhs) noexcept : value(rhs.value), ref(rhs.ref) { rhs.ref = 0; } + MyItem &operator=(MyItem &&rhs) noexcept { value = rhs.value; ref = rhs.ref; rhs.ref = 0; diff --git a/vespalib/src/tests/stllike/hash_test.cpp b/vespalib/src/tests/stllike/hash_test.cpp index e86a9ad020a..d6f5c2ba65d 100644 --- a/vespalib/src/tests/stllike/hash_test.cpp +++ b/vespalib/src/tests/stllike/hash_test.cpp @@ -15,14 +15,14 @@ namespace { struct Foo { int i; - Foo() : i(0) {} - Foo(int i_) : i(i_) {} + Foo() noexcept : i(0) {} + Foo(int i_) noexcept : i(i_) {} - bool operator==(const Foo& f) const + bool operator==(const Foo& f) const noexcept { return (i == f.i); } struct hash { - size_t operator() (const Foo& f) const { + size_t operator() (const Foo& f) const noexcept { return (f.i % 16); } }; @@ -332,10 +332,10 @@ TEST("test hash map with simple key and value type") class S { public: - explicit S(uint64_t l=0) : _a(l&0xfffffffful), _b(l>>32) { } + explicit S(uint64_t l=0) noexcept : _a(l&0xfffffffful), _b(l>>32) { } uint32_t hash() const { return _a; } uint32_t a() const { return _a; } - friend bool operator == (const S & a, const S & b) { return a._a == b._a && a._b == b._b; } + friend bool operator == (const S & a, const S & b) noexcept { return a._a == b._a && a._b == b._b; } private: uint32_t _a, _b; }; diff --git a/vespalib/src/tests/stllike/uniq_by_sort_map_hash.cpp b/vespalib/src/tests/stllike/uniq_by_sort_map_hash.cpp index 72a0c93c719..faee7680426 100644 --- a/vespalib/src/tests/stllike/uniq_by_sort_map_hash.cpp +++ b/vespalib/src/tests/stllike/uniq_by_sort_map_hash.cpp @@ -71,13 +71,13 @@ class Gid { public: struct hash { - size_t operator () (const Gid & g) const { return g.getGid()[0]; } + size_t operator () (const Gid & g) const noexcept { return g.getGid()[0]; } }; - Gid(unsigned int v=0) : _gid() { _gid[0] = _gid[1] = _gid[2] = v; } + Gid(unsigned int v=0) noexcept : _gid() { _gid[0] = _gid[1] = _gid[2] = v; } const unsigned int * getGid() const { return _gid; } - int cmp(const Gid & b) const { return memcmp(_gid, b._gid, sizeof(_gid)); } - bool operator < (const Gid & b) const { return cmp(b) < 0; } - bool operator == (const Gid & b) const { return cmp(b) == 0; } + int cmp(const Gid & b) const noexcept { return memcmp(_gid, b._gid, sizeof(_gid)); } + bool operator < (const Gid & b) const noexcept { return cmp(b) < 0; } + bool operator == (const Gid & b) const noexcept { return cmp(b) == 0; } private: unsigned int _gid[3]; }; @@ -85,15 +85,15 @@ private: class Slot { public: - Slot(unsigned int v=0) : _gid(v) { } + Slot(unsigned int v=0) noexcept : _gid(v) { } const Gid & getGid() const { return _gid; } - int cmp(const Slot & b) const { return _gid.cmp(b.getGid()); } + int cmp(const Slot & b) const noexcept { return _gid.cmp(b.getGid()); } private: Gid _gid; }; struct IndirectCmp : public std::binary_function { - bool operator()(const Slot* s1, const Slot* s2) { + bool operator() (const Slot* s1, const Slot* s2) noexcept { return s1->cmp(*s2) < 0; } }; diff --git a/vespalib/src/tests/util/generationhandler_stress/generation_handler_stress_test.cpp b/vespalib/src/tests/util/generationhandler_stress/generation_handler_stress_test.cpp index a61f9fb5bca..2c1463e506a 100644 --- a/vespalib/src/tests/util/generationhandler_stress/generation_handler_stress_test.cpp +++ b/vespalib/src/tests/util/generationhandler_stress/generation_handler_stress_test.cpp @@ -15,7 +15,7 @@ struct WorkContext { uint64_t _generation; - WorkContext() + WorkContext() noexcept : _generation(0) { } diff --git a/vespalib/src/vespa/vespalib/btree/btree_key_data.h b/vespalib/src/vespa/vespalib/btree/btree_key_data.h index dc44faf00a9..5d4929694d5 100644 --- a/vespalib/src/vespa/vespalib/btree/btree_key_data.h +++ b/vespalib/src/vespa/vespalib/btree/btree_key_data.h @@ -28,12 +28,12 @@ public: KeyT _key; DataT _data; - BTreeKeyData() + BTreeKeyData() noexcept : _key(), _data() {} - BTreeKeyData(const KeyT &key, const DataT &data) + BTreeKeyData(const KeyT &key, const DataT &data) noexcept : _key(key), _data(data) {} @@ -60,9 +60,9 @@ public: KeyT _key; - BTreeKeyData() : _key() {} + BTreeKeyData() noexcept : _key() {} - BTreeKeyData(const KeyT &key, const BTreeNoLeafData &) + BTreeKeyData(const KeyT &key, const BTreeNoLeafData &) noexcept : _key(key) { } diff --git a/vespalib/src/vespa/vespalib/data/databuffer.cpp b/vespalib/src/vespa/vespalib/data/databuffer.cpp index 04c4b1e225b..6b98226e50e 100644 --- a/vespalib/src/vespa/vespalib/data/databuffer.cpp +++ b/vespalib/src/vespa/vespalib/data/databuffer.cpp @@ -11,7 +11,7 @@ size_t padbefore(size_t alignment, const char *buf) { } } -DataBuffer::DataBuffer(size_t len, size_t alignment, const Alloc & initial) +DataBuffer::DataBuffer(size_t len, size_t alignment, const Alloc & initial) noexcept : _alignment(alignment), _externalBuf(nullptr), _bufstart(nullptr), diff --git a/vespalib/src/vespa/vespalib/data/databuffer.h b/vespalib/src/vespa/vespalib/data/databuffer.h index 7c4cd63a7b1..93da2b92379 100644 --- a/vespalib/src/vespa/vespalib/data/databuffer.h +++ b/vespalib/src/vespa/vespalib/data/databuffer.h @@ -44,8 +44,8 @@ public: typedef std::unique_ptr UP; DataBuffer(const DataBuffer &) = delete; DataBuffer &operator=(const DataBuffer &) = delete; - DataBuffer(DataBuffer &&) = default; - DataBuffer &operator=(DataBuffer &&) = default; + DataBuffer(DataBuffer &&) noexcept = default; + DataBuffer &operator=(DataBuffer &&) noexcept = default; /** * Construct a databuffer. @@ -53,7 +53,7 @@ public: * @param len the initial size of the buffer. * @param alignment required memory alignment for data start **/ - DataBuffer(size_t len = 1024, size_t alignment = 1, const Alloc & initial = Alloc::alloc(0)); + DataBuffer(size_t len = 1024, size_t alignment = 1, const Alloc & initial = Alloc::alloc(0)) noexcept; /** * Construct a databuffer using externally allocated memory. Note @@ -63,7 +63,7 @@ public: * @param buf pointer to preallocated memory * @param len length of preallocated memory **/ - DataBuffer(void *buf, size_t len) : + DataBuffer(void *buf, size_t len) noexcept : _alignment(1), _externalBuf(static_cast(buf)), _bufstart(_externalBuf), @@ -73,7 +73,7 @@ public: _buffer(Alloc::alloc(0)) { } - DataBuffer(const void *buf, size_t len) : + DataBuffer(const void *buf, size_t len) noexcept : _alignment(1), _externalBuf(static_cast(const_cast(buf))), _bufstart(_externalBuf), diff --git a/vespalib/src/vespa/vespalib/data/slime/slime.h b/vespalib/src/vespa/vespalib/data/slime/slime.h index 6523cd1dac0..3ee608799a6 100644 --- a/vespalib/src/vespa/vespalib/data/slime/slime.h +++ b/vespalib/src/vespa/vespalib/data/slime/slime.h @@ -86,7 +86,7 @@ public: ~Slime(); - Slime(Slime &&rhs) : + Slime(Slime &&rhs) noexcept : _names(std::move(rhs._names)), _stash(std::move(rhs._stash)), _root(std::move(rhs._root)) diff --git a/vespalib/src/vespa/vespalib/datastore/entryref.h b/vespalib/src/vespa/vespalib/datastore/entryref.h index 51a4e0699fc..4a5123ee1b3 100644 --- a/vespalib/src/vespa/vespalib/datastore/entryref.h +++ b/vespalib/src/vespa/vespalib/datastore/entryref.h @@ -13,13 +13,13 @@ class EntryRef { protected: uint32_t _ref; public: - EntryRef() : _ref(0u) { } - explicit EntryRef(uint32_t ref_) : _ref(ref_) { } - uint32_t ref() const { return _ref; } - bool valid() const { return _ref != 0u; } - bool operator==(const EntryRef &rhs) const { return _ref == rhs._ref; } - bool operator!=(const EntryRef &rhs) const { return _ref != rhs._ref; } - bool operator <(const EntryRef &rhs) const { return _ref < rhs._ref; } + EntryRef() noexcept : _ref(0u) { } + explicit EntryRef(uint32_t ref_) noexcept : _ref(ref_) { } + uint32_t ref() const noexcept { return _ref; } + bool valid() const noexcept { return _ref != 0u; } + bool operator==(const EntryRef &rhs) const noexcept { return _ref == rhs._ref; } + bool operator!=(const EntryRef &rhs) const noexcept { return _ref != rhs._ref; } + bool operator <(const EntryRef &rhs) const noexcept { return _ref < rhs._ref; } }; /** @@ -29,9 +29,9 @@ public: template class EntryRefT : public EntryRef { public: - EntryRefT() : EntryRef() {} - EntryRefT(size_t offset_, uint32_t bufferId_); - EntryRefT(const EntryRef & ref_) : EntryRef(ref_.ref()) {} + EntryRefT() noexcept : EntryRef() {} + EntryRefT(size_t offset_, uint32_t bufferId_) noexcept; + EntryRefT(const EntryRef & ref_) noexcept : EntryRef(ref_.ref()) {} size_t offset() const { return _ref & (offsetSize() - 1); } uint32_t bufferId() const { return _ref >> OffsetBits; } static size_t offsetSize() { return 1ul << OffsetBits; } @@ -55,10 +55,10 @@ private: typedef EntryRefT ParentType; static const uint32_t PadConstant = ((1 << OffsetAlign) - 1); public: - AlignedEntryRefT() : ParentType() {} - AlignedEntryRefT(size_t offset_, uint32_t bufferId_) : + AlignedEntryRefT() noexcept : ParentType() {} + AlignedEntryRefT(size_t offset_, uint32_t bufferId_) noexcept : ParentType(align(offset_) >> OffsetAlign, bufferId_) {} - AlignedEntryRefT(const EntryRef & ref_) : ParentType(ref_) {} + AlignedEntryRefT(const EntryRef & ref_) noexcept : ParentType(ref_) {} size_t offset() const { return ParentType::offset() << OffsetAlign; } static size_t offsetSize() { return ParentType::offsetSize() << OffsetAlign; } static size_t align(size_t val) { return val + pad(val); } diff --git a/vespalib/src/vespa/vespalib/datastore/entryref.hpp b/vespalib/src/vespa/vespalib/datastore/entryref.hpp index 56ebe62dfab..34b8d5355e3 100644 --- a/vespalib/src/vespa/vespalib/datastore/entryref.hpp +++ b/vespalib/src/vespa/vespalib/datastore/entryref.hpp @@ -8,7 +8,7 @@ namespace vespalib::datastore { template -EntryRefT::EntryRefT(size_t offset_, uint32_t bufferId_) : +EntryRefT::EntryRefT(size_t offset_, uint32_t bufferId_) noexcept : EntryRef((bufferId_ << OffsetBits) + offset_) { ASSERT_ONCE_OR_LOG(offset_ < offsetSize(), "EntryRefT.offset_overflow", 10000); diff --git a/vespalib/src/vespa/vespalib/stllike/string.h b/vespalib/src/vespa/vespalib/stllike/string.h index 7d4ed06c411..f1207c1d9b8 100644 --- a/vespalib/src/vespa/vespalib/stllike/string.h +++ b/vespalib/src/vespa/vespalib/stllike/string.h @@ -180,7 +180,7 @@ public: small_string(const char * s) noexcept : _buf(_stack), _sz(s ? strlen(s) : 0) { init(s); } small_string(const void * s, size_type sz) noexcept : _buf(_stack), _sz(sz) { init(s); } small_string(stringref s) noexcept : _buf(_stack), _sz(s.size()) { init(s.data()); } - small_string(const std::string & s) : _buf(_stack), _sz(s.size()) { init(s.data()); } + small_string(const std::string & s) noexcept : _buf(_stack), _sz(s.size()) { init(s.data()); } small_string(small_string && rhs) noexcept : _sz(rhs.size()), _bufferSize(rhs._bufferSize) { diff --git a/vespalib/src/vespa/vespalib/util/alloc.cpp b/vespalib/src/vespa/vespalib/util/alloc.cpp index 29285932f63..1303e7ffdee 100644 --- a/vespalib/src/vespa/vespalib/util/alloc.cpp +++ b/vespalib/src/vespa/vespalib/util/alloc.cpp @@ -510,13 +510,13 @@ Alloc::allocMMap(size_t sz) } Alloc -Alloc::alloc() +Alloc::alloc() noexcept { return Alloc(&AutoAllocator::getDefault()); } Alloc -Alloc::alloc(size_t sz, size_t mmapLimit, size_t alignment) +Alloc::alloc(size_t sz, size_t mmapLimit, size_t alignment) noexcept { return Alloc(&AutoAllocator::getAllocator(mmapLimit, alignment), sz); } diff --git a/vespalib/src/vespa/vespalib/util/alloc.h b/vespalib/src/vespa/vespalib/util/alloc.h index 449cdde5fc7..cf7135736e5 100644 --- a/vespalib/src/vespa/vespalib/util/alloc.h +++ b/vespalib/src/vespa/vespalib/util/alloc.h @@ -81,18 +81,18 @@ public: } return *this; } - Alloc() : _alloc(nullptr, 0), _allocator(nullptr) { } + Alloc() noexcept : _alloc(nullptr, 0), _allocator(nullptr) { } ~Alloc() { if (_alloc.first != nullptr) { _allocator->free(_alloc); _alloc.first = nullptr; } } - void swap(Alloc & rhs) { + void swap(Alloc & rhs) noexcept { std::swap(_alloc, rhs._alloc); std::swap(_allocator, rhs._allocator); } - Alloc create(size_t sz) const { + Alloc create(size_t sz) const noexcept { return (sz == 0) ? Alloc(_allocator) : Alloc(_allocator, sz); } @@ -103,11 +103,11 @@ public: * Optional alignment is assumed to be <= system page size, since mmap * is always used when size is above limit. */ - static Alloc alloc(size_t sz, size_t mmapLimit = MemoryAllocator::HUGEPAGE_SIZE, size_t alignment=0); - static Alloc alloc(); + static Alloc alloc(size_t sz, size_t mmapLimit = MemoryAllocator::HUGEPAGE_SIZE, size_t alignment=0) noexcept; + static Alloc alloc() noexcept; private: - Alloc(const MemoryAllocator * allocator, size_t sz) : _alloc(allocator->alloc(sz)), _allocator(allocator) { } - Alloc(const MemoryAllocator * allocator) : _alloc(nullptr, 0), _allocator(allocator) { } + Alloc(const MemoryAllocator * allocator, size_t sz) noexcept : _alloc(allocator->alloc(sz)), _allocator(allocator) { } + Alloc(const MemoryAllocator * allocator) noexcept : _alloc(nullptr, 0), _allocator(allocator) { } void clear() { _alloc.first = nullptr; _alloc.second = 0; diff --git a/vespalib/src/vespa/vespalib/util/arrayqueue.hpp b/vespalib/src/vespa/vespalib/util/arrayqueue.hpp index 17a8c02dbf5..9af446e7a0f 100644 --- a/vespalib/src/vespa/vespalib/util/arrayqueue.hpp +++ b/vespalib/src/vespa/vespalib/util/arrayqueue.hpp @@ -101,15 +101,16 @@ public: /** * Create an empty queue with an initial capacity of 0. **/ - ArrayQueue() : _data(0), _capacity(0), _used(0), _skew(0) {} + ArrayQueue() noexcept : _data(0), _capacity(0), _used(0), _skew(0) {} /** * Create an empty queue with the given initial capacity. * * @param cap initial capacity **/ - explicit ArrayQueue(uint32_t cap) : _data((T*)malloc(sizeof(T) * cap)), - _capacity(cap), _used(0), _skew(0) {} + explicit ArrayQueue(uint32_t cap) noexcept + : _data((T*)malloc(sizeof(T) * cap)), _capacity(cap), _used(0), _skew(0) + {} /** * Create a queue that is a copy of another queue. Now with funky @@ -119,8 +120,8 @@ public: * @param q the queue that should be copied **/ ArrayQueue(typename std::conditional::value, void_tag, const ArrayQueue &>::type q) = delete; - ArrayQueue(typename std::conditional::value, const ArrayQueue &, void_tag>::type q) : _data((T*)malloc(sizeof(T) * q._capacity)), - _capacity(q._capacity), _used(0), _skew(0) + ArrayQueue(typename std::conditional::value, const ArrayQueue &, void_tag>::type q) + : _data((T*)malloc(sizeof(T) * q._capacity)), _capacity(q._capacity), _used(0), _skew(0) { try { q.copyInto(*this); @@ -136,7 +137,7 @@ public: * * @param q the queue that should be moved **/ - ArrayQueue(ArrayQueue &&q) : _data(0), _capacity(0), _used(0), _skew(0) + ArrayQueue(ArrayQueue &&q) noexcept : _data(0), _capacity(0), _used(0), _skew(0) { swap(q); } @@ -358,7 +359,7 @@ public: * * @param q the queue we want to swap state with **/ - void swap(ArrayQueue &q) { + void swap(ArrayQueue &q) noexcept { std::swap(_data, q._data); std::swap(_capacity, q._capacity); std::swap(_used, q._used); diff --git a/vespalib/src/vespa/vespalib/util/compressionconfig.h b/vespalib/src/vespa/vespalib/util/compressionconfig.h index c0010e8e05c..88563c181a1 100644 --- a/vespalib/src/vespa/vespalib/util/compressionconfig.h +++ b/vespalib/src/vespa/vespalib/util/compressionconfig.h @@ -20,15 +20,15 @@ struct CompressionConfig { ZSTD = 7 }; - CompressionConfig() + CompressionConfig() noexcept : type(NONE), compressionLevel(0), threshold(90), minSize(0) {} - CompressionConfig(Type t) + CompressionConfig(Type t) noexcept : type(t), compressionLevel(9), threshold(90), minSize(0) {} - CompressionConfig(Type t, uint8_t level, uint8_t minRes) + CompressionConfig(Type t, uint8_t level, uint8_t minRes) noexcept : type(t), compressionLevel(level), threshold(minRes), minSize(0) {} - CompressionConfig(Type t, uint8_t lvl, uint8_t minRes, size_t minSz) + CompressionConfig(Type t, uint8_t lvl, uint8_t minRes, size_t minSz) noexcept : type(t), compressionLevel(lvl), threshold(minRes), minSize(minSz) {} bool operator==(const CompressionConfig& o) const { diff --git a/vespalib/src/vespa/vespalib/util/count_down_latch.h b/vespalib/src/vespa/vespalib/util/count_down_latch.h index 66ef1e44cee..420be8f7ab2 100644 --- a/vespalib/src/vespa/vespalib/util/count_down_latch.h +++ b/vespalib/src/vespa/vespalib/util/count_down_latch.h @@ -37,7 +37,7 @@ public: * * @param cnt initial count **/ - CountDownLatch(uint32_t cnt) : _lock(), _cond(), _count(cnt) {} + CountDownLatch(uint32_t cnt) noexcept : _lock(), _cond(), _count(cnt) {} /** * Count down this latch. When the count reaches 0, all threads diff --git a/vespalib/src/vespa/vespalib/util/gate.h b/vespalib/src/vespa/vespalib/util/gate.h index 7d913a7a039..5505a3676df 100644 --- a/vespalib/src/vespa/vespalib/util/gate.h +++ b/vespalib/src/vespa/vespalib/util/gate.h @@ -16,7 +16,7 @@ public: /** * Sets the initial count to 1. **/ - Gate() : CountDownLatch(1) {} + Gate() noexcept : CountDownLatch(1) {} }; } // namespace vespalib diff --git a/vespalib/src/vespa/vespalib/util/memory.h b/vespalib/src/vespa/vespalib/util/memory.h index f1c7c7820f1..2e0d631725a 100644 --- a/vespalib/src/vespa/vespalib/util/memory.h +++ b/vespalib/src/vespa/vespalib/util/memory.h @@ -200,7 +200,7 @@ public: * with default sz=0 you get an empty container * @param sz the number of bytes to allocate **/ - MallocPtr(const size_t sz=0) : _sz(sz), _p(_sz ? malloc(sz) : nullptr) { + MallocPtr(const size_t sz=0) noexcept : _sz(sz), _p(_sz ? malloc(sz) : nullptr) { if (_p == nullptr) { _sz = 0; } @@ -208,7 +208,7 @@ public: /** @brief destructor doing free() if needed */ ~MallocPtr() { cleanup(); } - MallocPtr(MallocPtr && rhs) : + MallocPtr(MallocPtr && rhs) noexcept : _sz(rhs.size()), _p(rhs._p) { rhs._sz = 0; @@ -221,7 +221,7 @@ public: * Does deep copy of contents (using memcpy). * @param rhs container to copy **/ - MallocPtr(const MallocPtr & rhs) + MallocPtr(const MallocPtr & rhs) noexcept : _sz(rhs.size()), _p(_sz ? malloc(_sz) : nullptr) { if (_p == nullptr) { @@ -238,14 +238,14 @@ public: * works like destruct + copy construct. * @param rhs container to copy **/ - MallocPtr & operator = (const MallocPtr & rhs) { + MallocPtr & operator = (const MallocPtr & rhs) noexcept { if (this != &rhs) { MallocPtr tmp(rhs); swap(tmp); } return *this; } - MallocPtr & operator = (MallocPtr && rhs) { + MallocPtr & operator = (MallocPtr && rhs) noexcept { if (this != &rhs) { cleanup(); _sz = rhs._sz; @@ -261,7 +261,7 @@ public: * * does not copy anything, just swaps pointers. **/ - void swap(MallocPtr & rhs) { + void swap(MallocPtr & rhs) noexcept { std::swap(_sz, rhs._sz); std::swap(_p, rhs._p); } @@ -307,7 +307,7 @@ public: } } private: - void cleanup() { + void cleanup() noexcept { if (_p) { free(_p); _p = nullptr; @@ -350,7 +350,7 @@ public: } /** @brief move constructor, takes over ownership */ - CloneablePtr(std::unique_ptr &&rhs) + CloneablePtr(std::unique_ptr &&rhs) noexcept : _p(rhs.release()) { } @@ -373,7 +373,7 @@ public: } /** @brief swap contents */ - void swap(CloneablePtr & rhs) { std::swap(_p, rhs._p); } + void swap(CloneablePtr & rhs) noexcept { std::swap(_p, rhs._p); } /** @brief value access */ const T * get() const { return _p; } diff --git a/vespalib/src/vespa/vespalib/util/sync.h b/vespalib/src/vespa/vespalib/util/sync.h index 8458bc19629..043bb477c89 100644 --- a/vespalib/src/vespa/vespalib/util/sync.h +++ b/vespalib/src/vespa/vespalib/util/sync.h @@ -32,9 +32,10 @@ public: * * Creates a Lock that has mutex instrumentation disabled. **/ - Lock() : _mutex() {} + Lock() noexcept : _mutex() {} + //TODO Remove The below methods are very bad and have a dodgy history. Lock(const Lock &) : Lock() { } - Lock(Lock &&) : Lock() { } + Lock(Lock &&) noexcept : Lock() { } Lock &operator=(const Lock &) { return *this; } Lock &operator=(Lock &&) { return *this; } }; @@ -66,9 +67,10 @@ public: * * Creates a Monitor that has mutex instrumentation disabled. **/ - Monitor() : Lock(), _cond() {} + Monitor() noexcept : Lock(), _cond() {} + //TODO Remove The below methods are very bad and have a dodgy history. Monitor(const Monitor &) : Monitor() { } - Monitor(Monitor &&) : Monitor() { } + Monitor(Monitor &&) noexcept : Monitor() { } Monitor &operator=(const Monitor &) { return *this; } Monitor &operator=(Monitor &&) { return *this; } }; -- cgit v1.2.3 From 3f75460d657edeb6ee6b66035b57b418569a34c3 Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Tue, 6 Oct 2020 08:17:02 +0000 Subject: simplify TensorSpec::add() * the old multiple-add semantics weren't used for anything, so make it more predictable and constrained. --- .../tests/instruction/generic_concat/generic_concat_test.cpp | 4 ++-- eval/src/vespa/eval/eval/tensor_spec.cpp | 11 ++++++++--- eval/src/vespa/eval/eval/tensor_spec.h | 9 +-------- 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/eval/src/tests/instruction/generic_concat/generic_concat_test.cpp b/eval/src/tests/instruction/generic_concat/generic_concat_test.cpp index a2510fdd2fa..2e3901c5ee8 100644 --- a/eval/src/tests/instruction/generic_concat/generic_concat_test.cpp +++ b/eval/src/tests/instruction/generic_concat/generic_concat_test.cpp @@ -101,8 +101,8 @@ TensorSpec reference_concat(const TensorSpec &a, const TensorSpec &b, const std: TensorSpec::Address addr_a; TensorSpec::Address addr_b; if (concat_addresses(cell_a.first, cell_b.first, concat_dim, b_offset, addr_a, addr_b)) { - result.set(addr_a, cell_a.second); - result.set(addr_b, cell_b.second); + result.add(addr_a, cell_a.second); + result.add(addr_b, cell_b.second); } } } diff --git a/eval/src/vespa/eval/eval/tensor_spec.cpp b/eval/src/vespa/eval/eval/tensor_spec.cpp index d3aafc7632a..4273cb347c6 100644 --- a/eval/src/vespa/eval/eval/tensor_spec.cpp +++ b/eval/src/vespa/eval/eval/tensor_spec.cpp @@ -46,9 +46,14 @@ TensorSpec & TensorSpec::operator = (const TensorSpec &) = default; TensorSpec::~TensorSpec() { } TensorSpec & -TensorSpec::set(Address address, double value) { - auto res = _cells.emplace(std::move(address), value); - if (!res.second) { assert(res.first->second.value == value); } +TensorSpec::add(Address address, double value) { + auto [iter, inserted] = _cells.emplace(std::move(address), value); + if (! inserted) { + // to simplify reference concat implementation, allow + // adding the same address several times to a Spec, but + // only with the same value every time: + assert(iter->second.value == value); + } return *this; } diff --git a/eval/src/vespa/eval/eval/tensor_spec.h b/eval/src/vespa/eval/eval/tensor_spec.h index 8a4343b3faa..8f02e56f860 100644 --- a/eval/src/vespa/eval/eval/tensor_spec.h +++ b/eval/src/vespa/eval/eval/tensor_spec.h @@ -68,14 +68,7 @@ public: TensorSpec(const TensorSpec &); TensorSpec & operator = (const TensorSpec &); ~TensorSpec(); - TensorSpec &set(Address address, double value); - TensorSpec &add(Address address, double value) { - auto res = _cells.emplace(std::move(address), value); - if (!res.second) { - res.first->second.value += value; - } - return *this; - } + TensorSpec &add(Address address, double value); const vespalib::string &type() const { return _type; } const Cells &cells() const { return _cells; } vespalib::string to_string() const; -- cgit v1.2.3 From 7d8516aecc9caf10435cd63cd59973d29e5c07cc Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Tue, 6 Oct 2020 12:45:12 +0000 Subject: add TensorPartialUpdate with add/modify/remove --- eval/CMakeLists.txt | 3 + eval/src/tests/tensor/partial_add/CMakeLists.txt | 9 + .../tests/tensor/partial_add/partial_add_test.cpp | 98 ++++++ .../src/tests/tensor/partial_modify/CMakeLists.txt | 9 + .../tensor/partial_modify/partial_modify_test.cpp | 122 +++++++ .../src/tests/tensor/partial_remove/CMakeLists.txt | 9 + .../tensor/partial_remove/partial_remove_test.cpp | 106 ++++++ eval/src/vespa/eval/tensor/CMakeLists.txt | 1 + eval/src/vespa/eval/tensor/partial_update.cpp | 388 +++++++++++++++++++++ eval/src/vespa/eval/tensor/partial_update.h | 35 ++ .../vespa/eval/tensor/wrapped_simple_tensor.cpp | 1 - 11 files changed, 780 insertions(+), 1 deletion(-) create mode 100644 eval/src/tests/tensor/partial_add/CMakeLists.txt create mode 100644 eval/src/tests/tensor/partial_add/partial_add_test.cpp create mode 100644 eval/src/tests/tensor/partial_modify/CMakeLists.txt create mode 100644 eval/src/tests/tensor/partial_modify/partial_modify_test.cpp create mode 100644 eval/src/tests/tensor/partial_remove/CMakeLists.txt create mode 100644 eval/src/tests/tensor/partial_remove/partial_remove_test.cpp create mode 100644 eval/src/vespa/eval/tensor/partial_update.cpp create mode 100644 eval/src/vespa/eval/tensor/partial_update.h diff --git a/eval/CMakeLists.txt b/eval/CMakeLists.txt index 18c3676c366..ab793534c14 100644 --- a/eval/CMakeLists.txt +++ b/eval/CMakeLists.txt @@ -65,6 +65,9 @@ vespa_define_module( src/tests/tensor/instruction_benchmark src/tests/tensor/onnx_wrapper src/tests/tensor/packed_mappings + src/tests/tensor/partial_add + src/tests/tensor/partial_modify + src/tests/tensor/partial_remove src/tests/tensor/tensor_add_operation src/tests/tensor/tensor_address src/tests/tensor/tensor_conformance diff --git a/eval/src/tests/tensor/partial_add/CMakeLists.txt b/eval/src/tests/tensor/partial_add/CMakeLists.txt new file mode 100644 index 00000000000..f0d07a8e9cf --- /dev/null +++ b/eval/src/tests/tensor/partial_add/CMakeLists.txt @@ -0,0 +1,9 @@ +# Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +vespa_add_executable(eval_partial_add_test_app TEST + SOURCES + partial_add_test.cpp + DEPENDS + vespaeval + GTest::GTest +) +vespa_add_test(NAME eval_partial_add_test_app COMMAND eval_partial_add_test_app) diff --git a/eval/src/tests/tensor/partial_add/partial_add_test.cpp b/eval/src/tests/tensor/partial_add/partial_add_test.cpp new file mode 100644 index 00000000000..f31df131345 --- /dev/null +++ b/eval/src/tests/tensor/partial_add/partial_add_test.cpp @@ -0,0 +1,98 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +using namespace vespalib; +using namespace vespalib::eval; +using namespace vespalib::eval::test; + +using vespalib::make_string_short::fmt; + +std::vector add_layouts = { + {x({"a"})}, {x({"b"})}, + {x({"a","b"})}, {x({"a","c"})}, + float_cells({x({"a","b"})}), {x({"a","c"})}, + {x({"a","b"})}, float_cells({x({"a","c"})}), + float_cells({x({"a","b"})}), float_cells({x({"a","c"})}), + {x({"a","b","c"}),y({"d","e"})}, {x({"b","f"}),y({"d","g"})}, + {x(3),y({"a","b"})}, {x(3),y({"b","c"})} +}; + +TensorSpec reference_add(const TensorSpec &a, const TensorSpec &b) { + TensorSpec result(a.type()); + for (const auto &cell: b.cells()) { + result.add(cell.first, cell.second); + } + auto end_iter = b.cells().end(); + for (const auto &cell: a.cells()) { + auto iter = b.cells().find(cell.first); + if (iter == end_iter) { + result.add(cell.first, cell.second); + } + } + return result; +} + +TensorSpec perform_partial_add(const TensorSpec &a, const TensorSpec &b) { + const auto &factory = SimpleValueBuilderFactory::get(); + auto lhs = value_from_spec(a, factory); + auto rhs = value_from_spec(b, factory); + auto up = tensor::TensorPartialUpdate::add(*lhs, *rhs, factory); + if (up) { + return spec_from_value(*up); + } else { + return TensorSpec(a.type()); + } +} + +TensorSpec perform_old_add(const TensorSpec &a, const TensorSpec &b) { + const auto &engine = tensor::DefaultTensorEngine::ref(); + auto lhs = engine.from_spec(a); + auto rhs = engine.from_spec(b); + auto lhs_tensor = dynamic_cast(lhs.get()); + EXPECT_TRUE(lhs_tensor); + auto rhs_tensor = dynamic_cast(rhs.get()); + EXPECT_TRUE(rhs_tensor); + auto up = lhs_tensor->add(*rhs_tensor); + EXPECT_TRUE(up); + return engine.to_spec(*up); +} + + +TEST(PartialAddTest, partial_add_works_for_simple_values) { + ASSERT_TRUE((add_layouts.size() % 2) == 0); + for (size_t i = 0; i < add_layouts.size(); i += 2) { + TensorSpec lhs = spec(add_layouts[i], N()); + TensorSpec rhs = spec(add_layouts[i + 1], Div16(N())); + SCOPED_TRACE(fmt("\n===\nLHS: %s\nRHS: %s\n===\n", lhs.to_string().c_str(), rhs.to_string().c_str())); + auto expect = reference_add(lhs, rhs); + auto actual = perform_partial_add(lhs, rhs); + EXPECT_EQ(actual, expect); + } +} + +TEST(PartialAddTest, partial_add_works_like_old_add) { + ASSERT_TRUE((add_layouts.size() % 2) == 0); + for (size_t i = 0; i < add_layouts.size(); i += 2) { + TensorSpec lhs = spec(add_layouts[i], N()); + TensorSpec rhs = spec(add_layouts[i + 1], Div16(N())); + SCOPED_TRACE(fmt("\n===\nLHS: %s\nRHS: %s\n===\n", lhs.to_string().c_str(), rhs.to_string().c_str())); + auto expect = perform_old_add(lhs, rhs); + auto actual = perform_partial_add(lhs, rhs); + EXPECT_EQ(actual, expect); + printf("%s add %s -> %s\n", lhs.to_string().c_str(), rhs.to_string().c_str(), actual.to_string().c_str()); + + } +} + +GTEST_MAIN_RUN_ALL_TESTS() diff --git a/eval/src/tests/tensor/partial_modify/CMakeLists.txt b/eval/src/tests/tensor/partial_modify/CMakeLists.txt new file mode 100644 index 00000000000..42a08acaae6 --- /dev/null +++ b/eval/src/tests/tensor/partial_modify/CMakeLists.txt @@ -0,0 +1,9 @@ +# Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +vespa_add_executable(eval_partial_modify_test_app TEST + SOURCES + partial_modify_test.cpp + DEPENDS + vespaeval + GTest::GTest +) +vespa_add_test(NAME eval_partial_modify_test_app COMMAND eval_partial_modify_test_app) diff --git a/eval/src/tests/tensor/partial_modify/partial_modify_test.cpp b/eval/src/tests/tensor/partial_modify/partial_modify_test.cpp new file mode 100644 index 00000000000..a35c7597194 --- /dev/null +++ b/eval/src/tests/tensor/partial_modify/partial_modify_test.cpp @@ -0,0 +1,122 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +using namespace vespalib; +using namespace vespalib::eval; +using namespace vespalib::eval::test; + +using vespalib::make_string_short::fmt; + +std::vector modify_layouts = { + {x({"a"})}, {x({"a"})}, + {x({"a",""})}, {x({"b","c","d","e"})}, + {x(5)}, {x({"1","2","foo","17"})}, + {x({"a","b","c"}),y({"d","e"})}, {x({"b"}),y({"d"})}, + {x({"a","b","c"})}, {x({"b","c","d"})}, + {x(3),y(2)}, {x({"0","1"}),y({"0","1"})}, + {x({"a","","b"})}, {x({""})} +}; + +TensorSpec::Address sparsify(const TensorSpec::Address &input) { + TensorSpec::Address output; + for (const auto & kv : input) { + if (kv.second.is_indexed()) { + auto val = fmt("%zu", kv.second.index); + output.emplace(kv.first, val); + } else { + output.emplace(kv.first, kv.second); + } + } + return output; +} + +TensorSpec reference_modify(const TensorSpec &a, const TensorSpec &b, join_fun_t fun) { + TensorSpec result(a.type()); + auto end_iter = b.cells().end(); + for (const auto &cell: a.cells()) { + double v = cell.second; + auto sparse_addr = sparsify(cell.first); + auto iter = b.cells().find(sparse_addr); + if (iter == end_iter) { + result.add(cell.first, v); + } else { + result.add(cell.first, fun(v, iter->second)); + } + } + return result; +} + +TensorSpec perform_partial_modify(const TensorSpec &a, const TensorSpec &b, join_fun_t fun) { + const auto &factory = SimpleValueBuilderFactory::get(); + auto lhs = value_from_spec(a, factory); + auto rhs = value_from_spec(b, factory); + auto up = tensor::TensorPartialUpdate::modify(*lhs, fun, *rhs, factory); + if (up) { + return spec_from_value(*up); + } else { + return TensorSpec(a.type()); + } +} + +TensorSpec perform_old_modify(const TensorSpec &a, const TensorSpec &b, join_fun_t fun) { + const auto &engine = tensor::DefaultTensorEngine::ref(); + auto lhs = engine.from_spec(a); + auto rhs = engine.from_spec(b); + auto lhs_tensor = dynamic_cast(lhs.get()); + EXPECT_TRUE(lhs_tensor); + auto rhs_sparse = dynamic_cast(rhs.get()); + EXPECT_TRUE(rhs_sparse); + tensor::CellValues cell_values(*rhs_sparse); + auto up = lhs_tensor->modify(fun, cell_values); + EXPECT_TRUE(up); + return engine.to_spec(*up); +} + + +TEST(PartialModifyTest, partial_modify_works_for_simple_values) { + ASSERT_TRUE((modify_layouts.size() % 2) == 0); + for (size_t i = 0; i < modify_layouts.size(); i += 2) { + TensorSpec lhs = spec(modify_layouts[i], N()); + TensorSpec rhs = spec(modify_layouts[i + 1], Div16(N())); + SCOPED_TRACE(fmt("\n===\nLHS: %s\nRHS: %s\n===\n", lhs.to_string().c_str(), rhs.to_string().c_str())); + for (auto fun: {operation::Add::f, operation::Mul::f, operation::Sub::f}) { + auto expect = reference_modify(lhs, rhs, fun); + auto actual = perform_partial_modify(lhs, rhs, fun); + EXPECT_EQ(actual, expect); + } + auto fun = [](double, double keep) { return keep; }; + auto expect = reference_modify(lhs, rhs, fun); + auto actual = perform_partial_modify(lhs, rhs, fun); + EXPECT_EQ(actual, expect); + } +} + +TEST(PartialModifyTest, partial_modify_works_like_old_modify) { + ASSERT_TRUE((modify_layouts.size() % 2) == 0); + for (size_t i = 0; i < modify_layouts.size(); i += 2) { + TensorSpec lhs = spec(modify_layouts[i], N()); + TensorSpec rhs = spec(modify_layouts[i + 1], Div16(N())); + SCOPED_TRACE(fmt("\n===\nLHS: %s\nRHS: %s\n===\n", lhs.to_string().c_str(), rhs.to_string().c_str())); + for (auto fun: {operation::Add::f, operation::Mul::f, operation::Sub::f}) { + auto expect = perform_old_modify(lhs, rhs, fun); + auto actual = perform_partial_modify(lhs, rhs, fun); + EXPECT_EQ(actual, expect); + if (fun == operation::Max::f) { + printf("%s modify(sub) %s -> %s\n", lhs.to_string().c_str(), rhs.to_string().c_str(), actual.to_string().c_str()); + } + } + } +} + +GTEST_MAIN_RUN_ALL_TESTS() diff --git a/eval/src/tests/tensor/partial_remove/CMakeLists.txt b/eval/src/tests/tensor/partial_remove/CMakeLists.txt new file mode 100644 index 00000000000..1680324f574 --- /dev/null +++ b/eval/src/tests/tensor/partial_remove/CMakeLists.txt @@ -0,0 +1,9 @@ +# Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +vespa_add_executable(eval_partial_remove_test_app TEST + SOURCES + partial_remove_test.cpp + DEPENDS + vespaeval + GTest::GTest +) +vespa_add_test(NAME eval_partial_remove_test_app COMMAND eval_partial_remove_test_app) diff --git a/eval/src/tests/tensor/partial_remove/partial_remove_test.cpp b/eval/src/tests/tensor/partial_remove/partial_remove_test.cpp new file mode 100644 index 00000000000..7fb46790415 --- /dev/null +++ b/eval/src/tests/tensor/partial_remove/partial_remove_test.cpp @@ -0,0 +1,106 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +using namespace vespalib; +using namespace vespalib::eval; +using namespace vespalib::eval::test; + +using vespalib::make_string_short::fmt; + +std::vector remove_layouts = { + {x({"a"})}, {x({"b"})}, + {x({"a","b"})}, {x({"a","c"})}, + float_cells({x({"a","b"})}), {x({"a","c"})}, + {x({"a","b"})}, float_cells({x({"a","c"})}), + float_cells({x({"a","b"})}), float_cells({x({"a","c"})}), + {x({"a","b","c"}),y({"d","e"})}, {x({"b","f"}),y({"d","g"})}, + {x(3),y({"a","b"})}, {y({"b","c"})} +}; + +TensorSpec::Address only_sparse(const TensorSpec::Address &input) { + TensorSpec::Address output; + for (const auto & kv : input) { + if (kv.second.is_mapped()) { + output.emplace(kv.first, kv.second); + } + } + return output; +} + +TensorSpec reference_remove(const TensorSpec &a, const TensorSpec &b) { + TensorSpec result(a.type()); + auto end_iter = b.cells().end(); + for (const auto &cell: a.cells()) { + auto iter = b.cells().find(only_sparse(cell.first)); + if (iter == end_iter) { + result.add(cell.first, cell.second); + } + } + return result; +} + +TensorSpec perform_partial_remove(const TensorSpec &a, const TensorSpec &b) { + const auto &factory = SimpleValueBuilderFactory::get(); + auto lhs = value_from_spec(a, factory); + auto rhs = value_from_spec(b, factory); + auto up = tensor::TensorPartialUpdate::remove(*lhs, *rhs, factory); + if (up) { + return spec_from_value(*up); + } else { + return TensorSpec(a.type()); + } +} + +TensorSpec perform_old_remove(const TensorSpec &a, const TensorSpec &b) { + const auto &engine = tensor::DefaultTensorEngine::ref(); + auto lhs = engine.from_spec(a); + auto rhs = engine.from_spec(b); + auto lhs_tensor = dynamic_cast(lhs.get()); + EXPECT_TRUE(lhs_tensor); + auto rhs_sparse = dynamic_cast(rhs.get()); + EXPECT_TRUE(rhs_sparse); + tensor::CellValues cell_values(*rhs_sparse); + auto up = lhs_tensor->remove(cell_values); + EXPECT_TRUE(up); + return engine.to_spec(*up); +} + + +TEST(PartialAddTest, partial_remove_works_for_simple_values) { + ASSERT_TRUE((remove_layouts.size() % 2) == 0); + for (size_t i = 0; i < remove_layouts.size(); i += 2) { + TensorSpec lhs = spec(remove_layouts[i], N()); + TensorSpec rhs = spec(remove_layouts[i + 1], Div16(N())); + SCOPED_TRACE(fmt("\n===\nLHS: %s\nRHS: %s\n===\n", lhs.to_string().c_str(), rhs.to_string().c_str())); + auto expect = reference_remove(lhs, rhs); + auto actual = perform_partial_remove(lhs, rhs); + EXPECT_EQ(actual, expect); + } +} + +TEST(PartialAddTest, partial_remove_works_like_old_remove) { + ASSERT_TRUE((remove_layouts.size() % 2) == 0); + for (size_t i = 0; i < remove_layouts.size(); i += 2) { + TensorSpec lhs = spec(remove_layouts[i], N()); + TensorSpec rhs = spec(remove_layouts[i + 1], Div16(N())); + SCOPED_TRACE(fmt("\n===\nLHS: %s\nRHS: %s\n===\n", lhs.to_string().c_str(), rhs.to_string().c_str())); + auto expect = perform_old_remove(lhs, rhs); + auto actual = perform_partial_remove(lhs, rhs); + EXPECT_EQ(actual, expect); + // printf("%s remove %s -> %s\n", lhs.to_string().c_str(), rhs.to_string().c_str(), actual.to_string().c_str()); + + } +} + +GTEST_MAIN_RUN_ALL_TESTS() diff --git a/eval/src/vespa/eval/tensor/CMakeLists.txt b/eval/src/vespa/eval/tensor/CMakeLists.txt index 79f6f7e2a4f..b75b34098f5 100644 --- a/eval/src/vespa/eval/tensor/CMakeLists.txt +++ b/eval/src/vespa/eval/tensor/CMakeLists.txt @@ -3,6 +3,7 @@ vespa_add_library(eval_tensor OBJECT SOURCES default_tensor_engine.cpp default_value_builder_factory.cpp + partial_update.cpp tensor.cpp tensor_address.cpp wrapped_simple_tensor.cpp diff --git a/eval/src/vespa/eval/tensor/partial_update.cpp b/eval/src/vespa/eval/tensor/partial_update.cpp new file mode 100644 index 00000000000..0ba8b37e77f --- /dev/null +++ b/eval/src/vespa/eval/tensor/partial_update.cpp @@ -0,0 +1,388 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "partial_update.h" +#include +#include +#include +#include +#include + +#include +LOG_SETUP(".eval.tensor.partial_update"); + +using namespace vespalib::eval; + +namespace vespalib::tensor { + +namespace { + +using join_fun_t = double (*)(double, double); + +static constexpr size_t npos() { return -1; } + +enum class DimCase { + SKIP_MAPPED, SKIP_INDEXED, + MISSING_MAPPED, MISSING_INDEXED, + MAPPED_MATCH, INDEXED_MATCH, + CONV_TO_INDEXED, CONV_TO_MAPPED +}; + +struct DenseCoords { + std::vector dim_sizes; + std::vector dim_names; + size_t total_size = 1; + size_t offset; + size_t dim; + void clear() { offset = 0; dim = 0; } + void with(size_t coord) { + size_t cur = dim_sizes[dim]; + if (coord < cur) { + if (offset != npos()) { + offset *= cur; + offset += coord; + } + } else { + // "bad label{%s} in modifier tensor, was %zu, must be < %zu", dim_names[dim], coord, cur + offset = npos(); + } + ++dim; + } + void with(vespalib::stringref label) { + uint32_t result = 0; + for (char c : label) { + if (c < '0' || c > '9') { // bad char + // "bad label{%s} in modifier tensor, was '%s'", dim_names[dim], label.data() + offset = npos(); + ++dim; + return; + } + result = result * 10 + (c - '0'); + } + with(result); + } + void add_dim(const char *name, size_t sz) { + dim_sizes.push_back(sz); + dim_names.push_back(name); + total_size *= sz; + } + size_t get() const { + assert(dim == dim_sizes.size()); + return offset; + } + ~DenseCoords(); +}; +DenseCoords::~DenseCoords() = default; + +struct Addresses { + std::vector addr; + std::vector next_result_refs; + std::vector lookup_refs; + std::vector lookup_view_dims; + Addresses(size_t sz) + : addr(sz), next_result_refs(sz), lookup_refs(sz), lookup_view_dims(sz) + { + for (size_t i = 0; i < sz; ++i) { + next_result_refs[i] = &addr[i]; + lookup_refs[i] = &addr[i]; + lookup_view_dims[i] = i; + } + } + ~Addresses(); +}; +Addresses::~Addresses() = default; + +struct AddressHandler { + std::vector how; + DenseCoords target_coords; + Addresses for_output; + Addresses from_modifier; + bool valid; + + AddressHandler(const ValueType &input_type, + const ValueType &modifier_type) + : how(), target_coords(), + for_output(input_type.count_mapped_dimensions()), + from_modifier(modifier_type.count_mapped_dimensions()), + valid(true) + { + if (! modifier_type.is_sparse()) { + LOG(error, "Unexpected non-sparse modifier tensor, type is %s", + modifier_type.to_spec().c_str()); + valid = false; + return; + } + // analyse dimensions + auto visitor = overload { + [&](visit_ranges_either, const auto &) { valid = false; }, + [&](visit_ranges_both, const auto &a, const auto &) { + how.push_back(a.is_mapped() ? DimCase::MAPPED_MATCH : DimCase::CONV_TO_INDEXED); + } + }; + const auto & input_dims = input_type.dimensions(); + const auto & modifier_dims = modifier_type.dimensions(); + visit_ranges(visitor, + input_dims.begin(), input_dims.end(), + modifier_dims.begin(), modifier_dims.end(), + [](const auto &a, const auto &b){ return (a.name < b.name); }); + if ((! valid) || + (input_dims.size() != modifier_dims.size()) || + (input_dims.size() != how.size())) + { + LOG(error, "Value type %s does not match modifier type %s (should have same dimensions)", + input_type.to_spec().c_str(), + modifier_type.to_spec().c_str()); + valid = false; + return; + } + for (const auto & dim : input_type.dimensions()) { + if (dim.is_indexed()) { + target_coords.add_dim(dim.name.c_str(), dim.size); + } + } + } + + void handle_address() + { + target_coords.clear(); + auto out = for_output.addr.begin(); + for (size_t i = 0; i < how.size(); ++i) { + if (how[i] == DimCase::CONV_TO_INDEXED) { + target_coords.with(from_modifier.addr[i]); + } else { + *out++ = from_modifier.addr[i]; + } + } + assert(out == for_output.addr.end()); + assert(target_coords.dim == target_coords.dim_sizes.size()); + } + + ~AddressHandler(); +}; +AddressHandler::~AddressHandler() = default; + +template +Value::UP +copy_tensor(const Value &input, const ValueType &input_type, Addresses &helper, const ValueBuilderFactory &factory) +{ + const size_t num_mapped_in_input = input_type.count_mapped_dimensions(); + const size_t dsss = input_type.dense_subspace_size(); + const size_t expected_subspaces = input.index().size(); + auto builder = factory.create_value_builder(input_type, num_mapped_in_input, dsss, expected_subspaces); + auto view = input.index().create_view({}); + view->lookup({}); + auto input_cells = input.cells().typify(); + size_t input_subspace; + while (view->next_result(helper.next_result_refs, input_subspace)) { + size_t input_offset = input_subspace * dsss; + auto src = input_cells.begin() + input_offset; + auto dst = builder->add_subspace(helper.addr).begin(); + for (size_t i = 0; i < dsss; ++i) { + dst[i] = src[i]; + } + } + return builder->build(std::move(builder)); +} + +template +Value::UP +my_modify_value(const Value &input, join_fun_t function, const Value &modifier, const ValueBuilderFactory &factory) +{ + const ValueType &input_type = input.type(); + const size_t dsss = input_type.dense_subspace_size(); + const ValueType &modifier_type = modifier.type(); + AddressHandler handler(input_type, modifier_type); + if (! handler.valid) { + return Value::UP(); + } + // copy input to output + auto out = copy_tensor(input, input_type, handler.for_output, factory); + // need to overwrite some cells + auto output_cells = unconstify(out->cells().template typify()); + const auto modifier_cells = modifier.cells().typify(); + auto modifier_view = modifier.index().create_view({}); + auto lookup_view = out->index().create_view(handler.for_output.lookup_view_dims); + modifier_view->lookup({}); + size_t modifier_subspace_index; + while (modifier_view->next_result(handler.from_modifier.next_result_refs, modifier_subspace_index)) { + handler.handle_address(); + size_t dense_idx = handler.target_coords.get(); + if (dense_idx == npos()) { + continue; + } + lookup_view->lookup(handler.for_output.lookup_refs); + size_t output_subspace_index; + if (lookup_view->next_result({}, output_subspace_index)) { + size_t subspace_offset = dsss * output_subspace_index; + auto dst = output_cells.begin() + subspace_offset; + ICT lhs = dst[dense_idx]; + MCT rhs = modifier_cells[modifier_subspace_index]; + dst[dense_idx] = function(lhs, rhs); + } + } + return out; +} +struct PerformModify { + template + static Value::UP invoke(const Value &input, + join_fun_t function, + const Value &modifier, + const ValueBuilderFactory &factory) + { + return my_modify_value(input, function, modifier, factory); + } +}; + +//----------------------------------------------------------------------------- + + +template +Value::UP +my_add_cells(const Value &input, const Value &modifier, const ValueBuilderFactory &factory) +{ + const ValueType &input_type = input.type(); + const ValueType &modifier_type = modifier.type(); + if (input_type.dimensions() != modifier_type.dimensions()) { + LOG(error, "when adding cells to a tensor, dimensions must be equal"); + return Value::UP(); + } + const auto input_cells = input.cells().typify(); + const auto modifier_cells = modifier.cells().typify(); + const size_t num_mapped_in_input = input_type.count_mapped_dimensions(); + const size_t dsss = input_type.dense_subspace_size(); + const size_t expected_subspaces = input.index().size() + modifier.index().size(); + auto builder = factory.create_value_builder(input_type, num_mapped_in_input, dsss, expected_subspaces); + Addresses addrs(num_mapped_in_input); + std::set overwritten_subspaces; + auto modifier_view = modifier.index().create_view({}); + auto lookup_view = input.index().create_view(addrs.lookup_view_dims); + modifier_view->lookup({}); + size_t modifier_subspace_index; + while (modifier_view->next_result(addrs.next_result_refs, modifier_subspace_index)) { + size_t modifier_offset = dsss * modifier_subspace_index; + auto src = modifier_cells.begin() + modifier_offset; + auto dst = builder->add_subspace(addrs.addr).begin(); + for (size_t i = 0; i < dsss; ++i) { + dst[i] = src[i]; + } + lookup_view->lookup(addrs.lookup_refs); + size_t input_subspace_index; + if (lookup_view->next_result({}, input_subspace_index)) { + overwritten_subspaces.insert(input_subspace_index); + } + } + auto input_view = input.index().create_view({}); + input_view->lookup({}); + size_t input_subspace_index; + while (input_view->next_result(addrs.next_result_refs, input_subspace_index)) { + if (overwritten_subspaces.count(input_subspace_index) == 0) { + size_t input_offset = dsss * input_subspace_index; + auto src = input_cells.begin() + input_offset; + auto dst = builder->add_subspace(addrs.addr).begin(); + for (size_t i = 0; i < dsss; ++i) { + dst[i] = src[i]; + } + } + } + return builder->build(std::move(builder)); +} + +struct PerformAdd { + template + static Value::UP invoke(const Value &input, + const Value &modifier, + const ValueBuilderFactory &factory) + { + return my_add_cells(input, modifier, factory); + } +}; + +//----------------------------------------------------------------------------- + +template +Value::UP +my_remove_cells(const Value &input, const Value &modifier, const ValueBuilderFactory &factory) +{ + const ValueType &input_type = input.type(); + const ValueType &modifier_type = modifier.type(); + if (input_type.mapped_dimensions() != modifier_type.mapped_dimensions()) { + LOG(error, "when removing cells from a tensor, mapped dimensions must be equal"); + return Value::UP(); + } + if (input_type.mapped_dimensions().size() == 0) { + LOG(error, "cannot remove cells from a dense tensor"); + return Value::UP(); + } + const auto input_cells = input.cells().typify(); + const size_t num_mapped_in_input = input_type.count_mapped_dimensions(); + const size_t dsss = input_type.dense_subspace_size(); + Addresses addrs(num_mapped_in_input); + std::set removed_subspaces; + auto modifier_view = modifier.index().create_view({}); + auto lookup_view = input.index().create_view(addrs.lookup_view_dims); + modifier_view->lookup({}); + size_t modifier_subspace_index; + while (modifier_view->next_result(addrs.next_result_refs, modifier_subspace_index)) { + lookup_view->lookup(addrs.lookup_refs); + size_t input_subspace_index; + if (lookup_view->next_result({}, input_subspace_index)) { + removed_subspaces.insert(input_subspace_index); + } + } + const size_t expected_subspaces = input.index().size() - removed_subspaces.size(); + auto builder = factory.create_value_builder(input_type, num_mapped_in_input, dsss, expected_subspaces); + auto input_view = input.index().create_view({}); + input_view->lookup({}); + size_t input_subspace_index; + while (input_view->next_result(addrs.next_result_refs, input_subspace_index)) { + if (removed_subspaces.count(input_subspace_index) == 0) { + size_t input_offset = dsss * input_subspace_index; + auto src = input_cells.begin() + input_offset; + auto dst = builder->add_subspace(addrs.addr).begin(); + for (size_t i = 0; i < dsss; ++i) { + dst[i] = src[i]; + } + } + } + return builder->build(std::move(builder)); +} + +struct PerformRemove { + template + static Value::UP invoke(const Value &input, + const Value &modifier, + const ValueBuilderFactory &factory) + { + return my_remove_cells(input, modifier, factory); + } +}; + +} // namespace + +//----------------------------------------------------------------------------- + +Value::UP +TensorPartialUpdate::modify(const Value &input, join_fun_t function, + const Value &modifier, const ValueBuilderFactory &factory) +{ + return typify_invoke<2, TypifyCellType, PerformModify>( + input.cells().type, modifier.cells().type, + input, function, modifier, factory); +} + +Value::UP +TensorPartialUpdate::add(const Value &input, const Value &add_cells, const ValueBuilderFactory &factory) +{ + return typify_invoke<2, TypifyCellType, PerformAdd>( + input.cells().type, add_cells.cells().type, + input, add_cells, factory); +} + +Value::UP +TensorPartialUpdate::remove(const Value &input, const Value &remove_spec, const ValueBuilderFactory &factory) +{ + return typify_invoke<1, TypifyCellType, PerformRemove>( + input.cells().type, + input, remove_spec, factory); +} + +} // namespace diff --git a/eval/src/vespa/eval/tensor/partial_update.h b/eval/src/vespa/eval/tensor/partial_update.h new file mode 100644 index 00000000000..17448b8223e --- /dev/null +++ b/eval/src/vespa/eval/tensor/partial_update.h @@ -0,0 +1,35 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include + +namespace vespalib::tensor { + +struct TensorPartialUpdate { + using join_fun_t = double (*)(double, double); + using Value = vespalib::eval::Value; + using ValueBuilderFactory = vespalib::eval::ValueBuilderFactory; + + // make a copy of the input, but apply function(oldvalue, modifier.cellvalue) + // to cells which also exist in the "modifier". + // modifier.type() must be sparse with exactly the same dimension names + // as the input type. + // returns null pointer if this constraint is violated. + static Value::UP modify(const Value &input, join_fun_t function, + const Value &modifier, const ValueBuilderFactory &factory); + + // make a copy of the input, but add or overwrite cells from add_cells. + // requires same type for input and add_cells. + // returns null pointer if this constraint is violated. + static Value::UP add(const Value &input, const Value &add_cells, const ValueBuilderFactory &factory); + + // make a copy of the input, but remove cells present in remove_spec. + // cell values in remove_spec are ignored. + // requires same set of mapped imensions input and remove_spec. + // not valid for dense tensors, since removing cells for those are impossible. + // returns null pointer if these constraints are violated. + static Value::UP remove(const Value &input, const Value &remove_spec, const ValueBuilderFactory &factory); +}; + +} // namespace diff --git a/eval/src/vespa/eval/tensor/wrapped_simple_tensor.cpp b/eval/src/vespa/eval/tensor/wrapped_simple_tensor.cpp index 241b8026b59..178935fce00 100644 --- a/eval/src/vespa/eval/tensor/wrapped_simple_tensor.cpp +++ b/eval/src/vespa/eval/tensor/wrapped_simple_tensor.cpp @@ -145,7 +145,6 @@ WrappedSimpleTensor::add(const Tensor &arg) const if (!rhs || type() != rhs->type()) { return Tensor::UP(); } - TensorSpec oldTensor = toSpec(); TensorSpec argTensor = rhs->toSpec(); TensorSpec result(type().to_spec()); -- cgit v1.2.3 From bd344fae9aaad5b78ab0cf6c9e122d258e544338 Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Wed, 7 Oct 2020 12:17:21 +0000 Subject: add PartialModify unit test for bad input --- .../tensor/partial_modify/partial_modify_test.cpp | 30 ++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/eval/src/tests/tensor/partial_modify/partial_modify_test.cpp b/eval/src/tests/tensor/partial_modify/partial_modify_test.cpp index a35c7597194..77e7094ce0e 100644 --- a/eval/src/tests/tensor/partial_modify/partial_modify_test.cpp +++ b/eval/src/tests/tensor/partial_modify/partial_modify_test.cpp @@ -57,11 +57,15 @@ TensorSpec reference_modify(const TensorSpec &a, const TensorSpec &b, join_fun_t return result; } -TensorSpec perform_partial_modify(const TensorSpec &a, const TensorSpec &b, join_fun_t fun) { +Value::UP try_partial_modify(const TensorSpec &a, const TensorSpec &b, join_fun_t fun) { const auto &factory = SimpleValueBuilderFactory::get(); auto lhs = value_from_spec(a, factory); auto rhs = value_from_spec(b, factory); - auto up = tensor::TensorPartialUpdate::modify(*lhs, fun, *rhs, factory); + return tensor::TensorPartialUpdate::modify(*lhs, fun, *rhs, factory); +} + +TensorSpec perform_partial_modify(const TensorSpec &a, const TensorSpec &b, join_fun_t fun) { + auto up = try_partial_modify(a, b, fun); if (up) { return spec_from_value(*up); } else { @@ -119,4 +123,26 @@ TEST(PartialModifyTest, partial_modify_works_like_old_modify) { } } +std::vector bad_layouts = { + {x(3)}, {x(3)}, + {x(3),y({"a"})}, {x(3),y({"a"})}, + {x({"a"})}, {x({"a"}),y({"b"})}, + {x({"a"}),y({"b"})}, {x({"a"})}, + {x({"a"})}, {x({"a"}),y(1)} +}; + +TEST(PartialModifyTest, partial_modify_returns_nullptr_on_invalid_inputs) { + ASSERT_TRUE((bad_layouts.size() % 2) == 0); + for (size_t i = 0; i < bad_layouts.size(); i += 2) { + TensorSpec lhs = spec(bad_layouts[i], N()); + TensorSpec rhs = spec(bad_layouts[i + 1], Div16(N())); + SCOPED_TRACE(fmt("\n===\nLHS: %s\nRHS: %s\n===\n", lhs.to_string().c_str(), rhs.to_string().c_str())); + for (auto fun: {operation::Add::f}) { + auto actual = try_partial_modify(lhs, rhs, fun); + auto expect = Value::UP(); + EXPECT_EQ(actual, expect); + } + } +} + GTEST_MAIN_RUN_ALL_TESTS() -- cgit v1.2.3 From bf22d0d633e3c10c3983082f77a0ce94886cd142 Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Wed, 7 Oct 2020 12:23:32 +0000 Subject: add PartialAdd unit test for bad input --- .../tests/tensor/partial_add/partial_add_test.cpp | 31 +++++++++++++++++++--- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/eval/src/tests/tensor/partial_add/partial_add_test.cpp b/eval/src/tests/tensor/partial_add/partial_add_test.cpp index f31df131345..36c9220e0bf 100644 --- a/eval/src/tests/tensor/partial_add/partial_add_test.cpp +++ b/eval/src/tests/tensor/partial_add/partial_add_test.cpp @@ -43,11 +43,16 @@ TensorSpec reference_add(const TensorSpec &a, const TensorSpec &b) { return result; } -TensorSpec perform_partial_add(const TensorSpec &a, const TensorSpec &b) { +Value::UP try_partial_add(const TensorSpec &a, const TensorSpec &b) { const auto &factory = SimpleValueBuilderFactory::get(); auto lhs = value_from_spec(a, factory); auto rhs = value_from_spec(b, factory); - auto up = tensor::TensorPartialUpdate::add(*lhs, *rhs, factory); + return tensor::TensorPartialUpdate::add(*lhs, *rhs, factory); +} + +TensorSpec perform_partial_add(const TensorSpec &a, const TensorSpec &b) { + auto up = try_partial_add(a, b); + EXPECT_TRUE(up); if (up) { return spec_from_value(*up); } else { @@ -90,8 +95,28 @@ TEST(PartialAddTest, partial_add_works_like_old_add) { auto expect = perform_old_add(lhs, rhs); auto actual = perform_partial_add(lhs, rhs); EXPECT_EQ(actual, expect); - printf("%s add %s -> %s\n", lhs.to_string().c_str(), rhs.to_string().c_str(), actual.to_string().c_str()); + } +} + +std::vector bad_layouts = { + {x(3)}, {x(3),y(1)}, + {x(3),y(1)}, {x(3)}, + {x(3),y(3)}, {x(3),y({"a"})}, + {x(3),y({"a"})}, {x(3),y(3)}, + {x({"a"})}, {x({"a"}),y({"b"})}, + {x({"a"}),y({"b"})}, {x({"a"})}, + {x({"a"})}, {x({"a"}),y(1)} +}; +TEST(PartialAddTest, partial_add_returns_nullptr_on_invalid_inputs) { + ASSERT_TRUE((bad_layouts.size() % 2) == 0); + for (size_t i = 0; i < bad_layouts.size(); i += 2) { + TensorSpec lhs = spec(bad_layouts[i], N()); + TensorSpec rhs = spec(bad_layouts[i + 1], Div16(N())); + SCOPED_TRACE(fmt("\n===\nLHS: %s\nRHS: %s\n===\n", lhs.to_string().c_str(), rhs.to_string().c_str())); + auto actual = try_partial_add(lhs, rhs); + auto expect = Value::UP(); + EXPECT_EQ(actual, expect); } } -- cgit v1.2.3 From 04f832a3306c838b3febe21f85d7364e14306695 Mon Sep 17 00:00:00 2001 From: Arne Juul Date: Wed, 7 Oct 2020 12:27:30 +0000 Subject: add PartialRemove unit test for bad input --- .../tensor/partial_remove/partial_remove_test.cpp | 28 ++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/eval/src/tests/tensor/partial_remove/partial_remove_test.cpp b/eval/src/tests/tensor/partial_remove/partial_remove_test.cpp index 7fb46790415..afefe925bec 100644 --- a/eval/src/tests/tensor/partial_remove/partial_remove_test.cpp +++ b/eval/src/tests/tensor/partial_remove/partial_remove_test.cpp @@ -21,6 +21,7 @@ using vespalib::make_string_short::fmt; std::vector remove_layouts = { {x({"a"})}, {x({"b"})}, {x({"a","b"})}, {x({"a","c"})}, + {x({"a","b"})}, {x({"a","b"})}, float_cells({x({"a","b"})}), {x({"a","c"})}, {x({"a","b"})}, float_cells({x({"a","c"})}), float_cells({x({"a","b"})}), float_cells({x({"a","c"})}), @@ -50,11 +51,15 @@ TensorSpec reference_remove(const TensorSpec &a, const TensorSpec &b) { return result; } -TensorSpec perform_partial_remove(const TensorSpec &a, const TensorSpec &b) { +Value::UP try_partial_remove(const TensorSpec &a, const TensorSpec &b) { const auto &factory = SimpleValueBuilderFactory::get(); auto lhs = value_from_spec(a, factory); auto rhs = value_from_spec(b, factory); - auto up = tensor::TensorPartialUpdate::remove(*lhs, *rhs, factory); + return tensor::TensorPartialUpdate::remove(*lhs, *rhs, factory); +} + +TensorSpec perform_partial_remove(const TensorSpec &a, const TensorSpec &b) { + auto up = try_partial_remove(a, b); if (up) { return spec_from_value(*up); } else { @@ -103,4 +108,23 @@ TEST(PartialAddTest, partial_remove_works_like_old_remove) { } } +std::vector bad_layouts = { + {x(3)}, {x(3)}, + {x(3),y({"a"})}, {x(3)}, + {x({"a"})}, {y({"a"})}, + {x({"a"})}, {x({"a"}),y({"b"})} +}; + +TEST(PartialRemoveTest, partial_remove_returns_nullptr_on_invalid_inputs) { + ASSERT_TRUE((bad_layouts.size() % 2) == 0); + for (size_t i = 0; i < bad_layouts.size(); i += 2) { + TensorSpec lhs = spec(bad_layouts[i], N()); + TensorSpec rhs = spec(bad_layouts[i + 1], Div16(N())); + SCOPED_TRACE(fmt("\n===\nLHS: %s\nRHS: %s\n===\n", lhs.to_string().c_str(), rhs.to_string().c_str())); + auto actual = try_partial_remove(lhs, rhs); + auto expect = Value::UP(); + EXPECT_EQ(actual, expect); + } +} + GTEST_MAIN_RUN_ALL_TESTS() -- cgit v1.2.3 From 0422bd14dd0ffb4d1a46ab5d4254e77c4fb0f7b0 Mon Sep 17 00:00:00 2001 From: Håvard Pettersen Date: Mon, 5 Oct 2020 13:32:35 +0000 Subject: fast value to enable inlined sparse operations use full overlap sparse join as initial test of full inlining. also improve simple sparse map performance by pre-calculating string hashes and using hash values for equality checks. --- .../simple_sparse_map/simple_sparse_map_test.cpp | 12 +- .../instruction/generic_join/generic_join_test.cpp | 15 +- .../instruction_benchmark.cpp | 11 +- eval/src/vespa/eval/eval/CMakeLists.txt | 1 + eval/src/vespa/eval/eval/fast_value.cpp | 38 ++++ eval/src/vespa/eval/eval/fast_value.h | 25 +++ eval/src/vespa/eval/eval/fast_value.hpp | 81 ++++++++ eval/src/vespa/eval/eval/simple_sparse_map.h | 227 +++++++++------------ eval/src/vespa/eval/eval/simple_value.cpp | 81 ++++---- eval/src/vespa/eval/eval/simple_value.h | 22 +- eval/src/vespa/eval/instruction/generic_join.cpp | 90 +++++--- eval/src/vespa/eval/instruction/generic_join.h | 22 ++ 12 files changed, 409 insertions(+), 216 deletions(-) create mode 100644 eval/src/vespa/eval/eval/fast_value.cpp create mode 100644 eval/src/vespa/eval/eval/fast_value.h create mode 100644 eval/src/vespa/eval/eval/fast_value.hpp diff --git a/eval/src/tests/eval/simple_sparse_map/simple_sparse_map_test.cpp b/eval/src/tests/eval/simple_sparse_map/simple_sparse_map_test.cpp index 8062881ced9..368b35f65ac 100644 --- a/eval/src/tests/eval/simple_sparse_map/simple_sparse_map_test.cpp +++ b/eval/src/tests/eval/simple_sparse_map/simple_sparse_map_test.cpp @@ -24,7 +24,7 @@ public: } } ~StringList(); - const std::vector direct_str() const { return _str_list; } + ConstArrayRef direct_str() const { return _str_list; } ConstArrayRef direct_ref() const { return _ref_list; } ConstArrayRef indirect_ref() const { return _ref_ptr_list; } }; @@ -55,10 +55,12 @@ TEST(SimpleSparseMapTest, simple_sparse_map_basic_usage_works) { EXPECT_EQ(map.lookup(a4.direct_ref()), map.npos()); EXPECT_EQ(map.lookup(a4.indirect_ref()), map.npos()); EXPECT_EQ(SimpleSparseMap::npos(), map.npos()); - SL expect_labels({"a","a","a", - "a","a","b", - "a","b","a"}); - EXPECT_EQ(map.labels(), expect_labels.direct_str()); + EXPECT_EQ(map.labels().size(), 9); + auto dump = [&](auto addr_tag, auto subspace, auto hash) { + auto addr = map.make_addr(addr_tag); + fprintf(stderr, " [%s,%s,%s]: %u (%zu)\n", addr[0].label.c_str(), addr[1].label.c_str(), addr[2].label.c_str(), subspace, hash); + }; + map.each_map_entry(dump); } TEST(SimpleSparseMapTest, simple_sparse_map_works_with_no_labels) { diff --git a/eval/src/tests/instruction/generic_join/generic_join_test.cpp b/eval/src/tests/instruction/generic_join/generic_join_test.cpp index 4821bf092da..25715f07bea 100644 --- a/eval/src/tests/instruction/generic_join/generic_join_test.cpp +++ b/eval/src/tests/instruction/generic_join/generic_join_test.cpp @@ -1,6 +1,7 @@ // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include +#include #include #include #include @@ -79,6 +80,16 @@ TensorSpec perform_generic_join(const TensorSpec &a, const TensorSpec &b, join_f return spec_from_value(single.eval(std::vector({*lhs,*rhs}))); } +TensorSpec perform_generic_join_fast(const TensorSpec &a, const TensorSpec &b, join_fun_t function) { + Stash stash; + const auto &factory = FastValueBuilderFactory::get(); + auto lhs = value_from_spec(a, factory); + auto rhs = value_from_spec(b, factory); + auto my_op = GenericJoin::make_instruction(lhs->type(), rhs->type(), function, factory, stash); + InterpretedFunction::EvalSingle single(my_op); + return spec_from_value(single.eval(std::vector({*lhs,*rhs}))); +} + TEST(GenericJoinTest, dense_join_plan_can_be_created) { auto lhs = ValueType::from_spec("tensor(a{},b[6],c[5],e[3],f[2],g{})"); auto rhs = ValueType::from_spec("tensor(a{},b[6],c[5],d[4],h{})"); @@ -121,7 +132,7 @@ TEST(GenericJoinTest, dense_join_plan_can_be_executed) { EXPECT_EQ(c, expect); } -TEST(GenericJoinTest, generic_join_works_for_simple_values) { +TEST(GenericJoinTest, generic_join_works_for_simple_and_fast_values) { ASSERT_TRUE((join_layouts.size() % 2) == 0); for (size_t i = 0; i < join_layouts.size(); i += 2) { TensorSpec lhs = spec(join_layouts[i], Div16(N())); @@ -130,7 +141,9 @@ TEST(GenericJoinTest, generic_join_works_for_simple_values) { SCOPED_TRACE(fmt("\n===\nLHS: %s\nRHS: %s\n===\n", lhs.to_string().c_str(), rhs.to_string().c_str())); auto expect = reference_join(lhs, rhs, fun); auto actual = perform_generic_join(lhs, rhs, fun); + auto fast = perform_generic_join_fast(lhs, rhs, fun); EXPECT_EQ(actual, expect); + EXPECT_EQ(fast, expect); } } } diff --git a/eval/src/tests/tensor/instruction_benchmark/instruction_benchmark.cpp b/eval/src/tests/tensor/instruction_benchmark/instruction_benchmark.cpp index 2c5b262ee0d..56e0edf2174 100644 --- a/eval/src/tests/tensor/instruction_benchmark/instruction_benchmark.cpp +++ b/eval/src/tests/tensor/instruction_benchmark/instruction_benchmark.cpp @@ -21,6 +21,7 @@ // verifying that all implementations produce the same result. #include +#include #include #include #include @@ -127,11 +128,12 @@ struct EngineImpl : Impl { //----------------------------------------------------------------------------- -EngineImpl simple_tensor_engine_impl(4, " SimpleTensorEngine", " SimpleT", SimpleTensorEngine::ref()); +EngineImpl simple_tensor_engine_impl(5, " SimpleTensorEngine", " SimpleT", SimpleTensorEngine::ref()); EngineImpl default_tensor_engine_impl(1, "DefaultTensorEngine", "OLD PROD", DefaultTensorEngine::ref()); -ValueImpl simple_value_impl(3, " SimpleValue", " SimpleV", SimpleValueBuilderFactory::get()); -ValueImpl packed_mixed_tensor_impl(2, " PackedMixedTensor", " Packed", PackedMixedTensorBuilderFactory::get()); -ValueImpl default_tensor_value_impl(0, " DefaultValue", "NEW PROD", DefaultValueBuilderFactory::get()); +ValueImpl simple_value_impl(2, " SimpleValue", " SimpleV", SimpleValueBuilderFactory::get()); +ValueImpl fast_value_impl(0, " FastValue", "NEW PROD", FastValueBuilderFactory::get()); +ValueImpl packed_mixed_tensor_impl(4, " PackedMixedTensor", " Packed", PackedMixedTensorBuilderFactory::get()); +ValueImpl default_tensor_value_impl(3, " DefaultValue", "DefaultV", DefaultValueBuilderFactory::get()); vespalib::string short_header("--------"); constexpr double budget = 5.0; @@ -142,6 +144,7 @@ constexpr double good_limit = 1.10; // GOOD: new prod has performance higher tha std::vector> impl_list = {simple_tensor_engine_impl, default_tensor_engine_impl, simple_value_impl, + fast_value_impl, packed_mixed_tensor_impl, default_tensor_value_impl}; diff --git a/eval/src/vespa/eval/eval/CMakeLists.txt b/eval/src/vespa/eval/eval/CMakeLists.txt index e2bcee94498..ddf2ede06e0 100644 --- a/eval/src/vespa/eval/eval/CMakeLists.txt +++ b/eval/src/vespa/eval/eval/CMakeLists.txt @@ -8,6 +8,7 @@ vespa_add_library(eval_eval OBJECT delete_node.cpp double_value_builder.cpp fast_forest.cpp + fast_value.cpp function.cpp gbdt.cpp interpreted_function.cpp diff --git a/eval/src/vespa/eval/eval/fast_value.cpp b/eval/src/vespa/eval/eval/fast_value.cpp new file mode 100644 index 00000000000..cad8bb57312 --- /dev/null +++ b/eval/src/vespa/eval/eval/fast_value.cpp @@ -0,0 +1,38 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "fast_value.h" +#include +#include "fast_value.hpp" + +namespace vespalib::eval { + +//----------------------------------------------------------------------------- + +namespace { + +struct CreateFastValueBuilderBase { + template static std::unique_ptr invoke(const ValueType &type, + size_t num_mapped_dims, size_t subspace_size, size_t expected_subspaces) + { + assert(check_cell_type(type.cell_type())); + return std::make_unique>(type, num_mapped_dims, subspace_size, expected_subspaces); + } +}; + +} // namespace + +//----------------------------------------------------------------------------- + +FastValueBuilderFactory::FastValueBuilderFactory() = default; +FastValueBuilderFactory FastValueBuilderFactory::_factory; + +std::unique_ptr +FastValueBuilderFactory::create_value_builder_base(const ValueType &type, size_t num_mapped_dims, size_t subspace_size, + size_t expected_subspaces) const +{ + return typify_invoke<1,TypifyCellType,CreateFastValueBuilderBase>(type.cell_type(), type, num_mapped_dims, subspace_size, expected_subspaces); +} + +//----------------------------------------------------------------------------- + +} diff --git a/eval/src/vespa/eval/eval/fast_value.h b/eval/src/vespa/eval/eval/fast_value.h new file mode 100644 index 00000000000..39dc8859204 --- /dev/null +++ b/eval/src/vespa/eval/eval/fast_value.h @@ -0,0 +1,25 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include "value.h" + +namespace vespalib::eval { + +/** + * A fast value is a value that uses a FastValueIndex to store its + * sparse mappings. A FastValueIndex uses the same implementation as a + * SimpleValueIndex but adds extra inlined functions that can be + * called directly from various instruction implementations. + **/ +class FastValueBuilderFactory : public ValueBuilderFactory { +private: + FastValueBuilderFactory(); + static FastValueBuilderFactory _factory; + std::unique_ptr create_value_builder_base(const ValueType &type, + size_t num_mapped_dims, size_t subspace_size, size_t expected_subspaces) const override; +public: + static const FastValueBuilderFactory &get() { return _factory; } +}; + +} diff --git a/eval/src/vespa/eval/eval/fast_value.hpp b/eval/src/vespa/eval/eval/fast_value.hpp new file mode 100644 index 00000000000..e21ac043e4d --- /dev/null +++ b/eval/src/vespa/eval/eval/fast_value.hpp @@ -0,0 +1,81 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "simple_value.h" +#include "inline_operation.h" +#include +#include + +namespace vespalib::eval { + +//----------------------------------------------------------------------------- + +// This is the class instructions will look for when optimizing sparse +// operations by calling inline functions directly. + +struct FastValueIndex final : SimpleValueIndex { + FastValueIndex(size_t num_mapped_dims_in, size_t expected_subspaces_in) + : SimpleValueIndex(num_mapped_dims_in, expected_subspaces_in) {} + + template + static const Value &sparse_full_overlap_join(const ValueType &res_type, const Fun &fun, + const FastValueIndex &lhs, const FastValueIndex &rhs, + ConstArrayRef lhs_cells, ConstArrayRef rhs_cells, Stash &stash); +}; + +//----------------------------------------------------------------------------- + +template +struct FastValue final : Value, ValueBuilder { + + ValueType my_type; + size_t my_subspace_size; + FastValueIndex my_index; + std::vector my_cells; + + FastValue(const ValueType &type_in, size_t num_mapped_dims_in, size_t subspace_size_in, size_t expected_subspaces_in) + : my_type(type_in), my_subspace_size(subspace_size_in), my_index(num_mapped_dims_in, expected_subspaces_in), my_cells() + { + my_cells.reserve(subspace_size_in * expected_subspaces_in); + } + ~FastValue() override; + const ValueType &type() const override { return my_type; } + const Value::Index &index() const override { return my_index; } + TypedCells cells() const override { return TypedCells(ConstArrayRef(my_cells)); } + ArrayRef add_subspace(ConstArrayRef addr) override { + size_t old_size = my_cells.size(); + my_index.map.add_mapping(addr); + my_cells.resize(old_size + my_subspace_size); + return ArrayRef(&my_cells[old_size], my_subspace_size); + } + std::unique_ptr build(std::unique_ptr> self) override { + ValueBuilder* me = this; + assert(me == self.get()); + self.release(); + return std::unique_ptr(this); + } +}; +template FastValue::~FastValue() = default; + +//----------------------------------------------------------------------------- + +template +const Value & +FastValueIndex::sparse_full_overlap_join(const ValueType &res_type, const Fun &fun, + const FastValueIndex &lhs, const FastValueIndex &rhs, + ConstArrayRef lhs_cells, ConstArrayRef rhs_cells, Stash &stash) +{ + auto &result = stash.create>(res_type, lhs.map.num_dims(), 1, lhs.map.size()); + lhs.map.each_map_entry([&](auto addr_tag, auto lhs_subspace, auto hash) + { + auto rhs_subspace = rhs.map.lookup(hash); + if (rhs_subspace != SimpleSparseMap::npos()) { + result.my_index.map.add_mapping(lhs.map.make_addr(addr_tag), hash); + result.my_cells.push_back(fun(lhs_cells[lhs_subspace], rhs_cells[rhs_subspace])); + } + }); + return result; +} + +//----------------------------------------------------------------------------- + +} diff --git a/eval/src/vespa/eval/eval/simple_sparse_map.h b/eval/src/vespa/eval/eval/simple_sparse_map.h index 61ff9f326e2..1316c2613c6 100644 --- a/eval/src/vespa/eval/eval/simple_sparse_map.h +++ b/eval/src/vespa/eval/eval/simple_sparse_map.h @@ -6,24 +6,23 @@ #include #include #include -#include +#include +#include namespace vespalib::eval { /** - * A simple wrapper around vespalib::hash_map, using it to map a list - * of labels (a sparse address) to an integer value (dense subspace - * index). Labels are stored in a separate vector and the map keys - * reference a slice of this vector. This is to avoid fragmentation - * caused by hash keys being vectors of values. In addition, labels + * A wrapper around vespalib::hash_map, using it to map a list of + * labels (a sparse address) to an integer value (dense subspace + * index). Labels are stored in a separate vector to avoid + * fragmentation caused by hash keys being vectors of values. Labels * can be specified in different ways during lookup and insert in - * order to reduce the need for data restructuring when using the - * map. To keep things simple, map iterators are kept away from the - * api. This will have a minor overhead during lookup since the end - * iterator needs to be translated to npos. All added mappings are - * checked for uniqueness with an assert. There is no real need for - * map entry iteration since you can just iterate the labels vector - * directly. + * order to reduce the need for data restructuring when used to + * integrate with the Value api. All labels are stored with a 64-bit + * hash. This hash is used as label equality (assuming no + * collisions). A order-sensitive 64bit hash constructed from + * individual label hashes is used for address equality (also assuming + * no collisions). The hash algorithm currently used is XXH3. * * 'add_mapping' will will bind the given address to an integer value * equal to the current (pre-insert) size of the map. The given @@ -35,167 +34,123 @@ namespace vespalib::eval { class SimpleSparseMap { public: - using DirectStr = ConstArrayRef; - using DirectRef = ConstArrayRef; - using IndirectRef = ConstArrayRef; + using hash_t = XXH64_hash_t; + + static hash_t hash_label(const vespalib::string &str) { + return XXH3_64bits(str.data(), str.size()); + } + static hash_t hash_label(vespalib::stringref str) { + return XXH3_64bits(str.data(), str.size()); + } + static hash_t hash_label(const vespalib::stringref *str) { + return XXH3_64bits(str->data(), str->size()); + } + + struct HashedLabel { + vespalib::string label; + hash_t hash; + HashedLabel() : label(), hash(0) {} + HashedLabel(const HashedLabel &rhs) = default; + HashedLabel &operator=(const HashedLabel &rhs) = default; + HashedLabel(HashedLabel &&rhs) = default; + HashedLabel &operator=(HashedLabel &&rhs) = default; + HashedLabel(const vespalib::string &str) : label(str), hash(hash_label(str)) {} + HashedLabel(vespalib::stringref str) : label(str), hash(hash_label(str)) {} + HashedLabel(const vespalib::stringref *str) : label(*str), hash(hash_label(*str)) {} + }; + + static hash_t hash_label(const HashedLabel &label) { + return label.hash; + } struct Key { uint32_t start; - uint32_t end; - Key() : start(0), end(0) {} - Key(uint32_t start_in, uint32_t end_in) - : start(start_in), end(end_in) {} + hash_t hash; + Key() : start(0), hash(0) {} + Key(uint32_t start_in, hash_t hash_in) + : start(start_in), hash(hash_in) {} }; struct Hash { - const std::vector *labels; - const vespalib::string &get_label(size_t i) const { return (*labels)[i]; } - Hash() : labels(nullptr) {} - Hash(const Hash &rhs) = default; - Hash &operator=(const Hash &rhs) = default; - Hash(const std::vector &labels_in) : labels(&labels_in) {} - size_t operator()(const Key &key) const { - size_t h = 0; - for (size_t i = key.start; i < key.end; ++i) { - const vespalib::string &str = get_label(i); - h = h * 31 + hashValue(str.data(), str.size()); - } - return h; - } - size_t operator()(const DirectStr &addr) const { - size_t h = 0; - for (const auto &str: addr) { - h = h * 31 + hashValue(str.data(), str.size()); - } - return h; - } - size_t operator()(const DirectRef &addr) const { - size_t h = 0; - for (const auto &str: addr) { - h = h * 31 + hashValue(str.data(), str.size()); - } - return h; - } - size_t operator()(const IndirectRef &addr) const { - size_t h = 0; - for (const auto *str: addr) { - h = h * 31 + hashValue(str->data(), str->size()); - } - return h; - } + hash_t operator()(const Key &key) const { return key.hash; } + hash_t operator()(hash_t hash) const { return hash; } }; struct Equal { - const std::vector *labels; - const vespalib::string &get_label(size_t i) const { return (*labels)[i]; } - Equal() : labels(nullptr) {} - Equal(const Equal &rhs) = default; - Equal &operator=(const Equal &rhs) = default; - Equal(const std::vector &labels_in) : labels(&labels_in) {} - bool operator()(const Key &a, const Key &b) const { - size_t len = (a.end - a.start); - if ((b.end - b.start) != len) { - return false; - } - for (size_t i = 0; i < len; ++i) { - if (get_label(a.start + i) != get_label(b.start + i)) { - return false; - } - } - return true; - } - bool operator()(const Key &a, const DirectStr &addr) const { - if (addr.size() != (a.end - a.start)) { - return false; - } - for (size_t i = 0; i < addr.size(); ++i) { - if (get_label(a.start + i) != addr[i]) { - return false; - } - } - return true; - } - bool operator()(const Key &a, const DirectRef &addr) const { - if (addr.size() != (a.end - a.start)) { - return false; - } - for (size_t i = 0; i < addr.size(); ++i) { - if (get_label(a.start + i) != addr[i]) { - return false; - } - } - return true; - } - bool operator()(const Key &a, const IndirectRef &addr) const { - if (addr.size() != (a.end - a.start)) { - return false; - } - for (size_t i = 0; i < addr.size(); ++i) { - if (get_label(a.start + i) != *addr[i]) { - return false; - } - } - return true; - } + bool operator()(const Key &a, hash_t b) const { return (a.hash == b); } + bool operator()(const Key &a, const Key &b) const { return (a.hash == b.hash); } }; using MapType = vespalib::hash_map; private: - std::vector _labels; + size_t _num_dims; + std::vector _labels; MapType _map; public: - SimpleSparseMap(size_t num_mapped_dims, size_t expected_subspaces) - : _labels(), _map(expected_subspaces * 2, Hash(_labels), Equal(_labels)) + SimpleSparseMap(size_t num_dims_in, size_t expected_subspaces) + : _num_dims(num_dims_in), _labels(), _map(expected_subspaces * 2) { - _labels.reserve(num_mapped_dims * expected_subspaces); + _labels.reserve(_num_dims * expected_subspaces); } ~SimpleSparseMap(); size_t size() const { return _map.size(); } + size_t num_dims() const { return _num_dims; } static constexpr size_t npos() { return -1; } - const std::vector &labels() const { return _labels; } - void add_mapping(DirectStr addr) { - uint32_t value = _map.size(); - uint32_t start = _labels.size(); + const std::vector &labels() const { return _labels; } + + ConstArrayRef make_addr(uint32_t start) const { + return ConstArrayRef(&_labels[start], _num_dims); + } + + template + hash_t hash_addr(ConstArrayRef addr) const { + hash_t h = 0; for (const auto &label: addr) { - _labels.emplace_back(label); + h = 31 * h + hash_label(label); } - uint32_t end = _labels.size(); - auto [ignore, was_inserted] = _map.insert(std::make_pair(Key(start, end), value)); - assert(was_inserted); + return h; } - void add_mapping(DirectRef addr) { + + template + void add_mapping(ConstArrayRef addr, hash_t hash) { uint32_t value = _map.size(); uint32_t start = _labels.size(); for (const auto &label: addr) { _labels.emplace_back(label); } - uint32_t end = _labels.size(); - auto [ignore, was_inserted] = _map.insert(std::make_pair(Key(start, end), value)); - assert(was_inserted); + _map.insert(std::make_pair(Key(start, hash), value)); } - void add_mapping(IndirectRef addr) { + + template + void add_mapping(ConstArrayRef addr) { + hash_t h = 0; uint32_t value = _map.size(); uint32_t start = _labels.size(); - for (const auto *label: addr) { - _labels.emplace_back(*label); + for (const auto &label: addr) { + _labels.emplace_back(label); + h = 31 * h + hash_label(_labels.back()); } - uint32_t end = _labels.size(); - auto [ignore, was_inserted] = _map.insert(std::make_pair(Key(start, end), value)); - assert(was_inserted); + _map.insert(std::make_pair(Key(start, h), value)); } - size_t lookup(DirectStr addr) const { - auto pos = _map.find(addr); + + size_t lookup(hash_t hash) const { + auto pos = _map.find(hash); return (pos == _map.end()) ? npos() : pos->second; } - size_t lookup(DirectRef addr) const { - auto pos = _map.find(addr); - return (pos == _map.end()) ? npos() : pos->second; + + template + size_t lookup(ConstArrayRef addr) const { + return lookup(hash_addr(addr)); } - size_t lookup(IndirectRef addr) const { - auto pos = _map.find(addr); - return (pos == _map.end()) ? npos() : pos->second; + + template + void each_map_entry(F &&f) const { + _map.for_each([&](const auto &entry) + { + f(entry.first.start, entry.second, entry.first.hash); + }); } }; diff --git a/eval/src/vespa/eval/eval/simple_value.cpp b/eval/src/vespa/eval/eval/simple_value.cpp index 84ba35404fd..d752549b0f2 100644 --- a/eval/src/vespa/eval/eval/simple_value.cpp +++ b/eval/src/vespa/eval/eval/simple_value.cpp @@ -30,14 +30,14 @@ struct CreateSimpleValueBuilderBase { // look up a full address in the map directly struct LookupView : public Value::Index::View { - const SimpleSparseMap &index; + const SimpleSparseMap ↦ size_t subspace; - LookupView(const SimpleSparseMap &index_in) - : index(index_in), subspace(SimpleSparseMap::npos()) {} + LookupView(const SimpleSparseMap &map_in) + : map(map_in), subspace(SimpleSparseMap::npos()) {} void lookup(ConstArrayRef addr) override { - subspace = index.lookup(addr); + subspace = map.lookup(addr); } bool next_result(ConstArrayRef, size_t &idx_out) override { @@ -55,25 +55,27 @@ struct LookupView : public Value::Index::View { // find matching mappings for a partial address with brute force filtering struct FilterView : public Value::Index::View { - size_t num_mapped_dims; - const std::vector &labels; - std::vector match_dims; - std::vector extract_dims; - std::vector query; - size_t pos; + using Label = SimpleSparseMap::HashedLabel; + + size_t num_mapped_dims; + const std::vector