From 00e7d63e41842231528343a6e80ede595d997ff5 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Thu, 1 Dec 2022 07:36:44 +0100 Subject: - Reduce usage of guava. - Ensure that tests relying on order are determinsitic. --- zkfacade/pom.xml | 5 ----- 1 file changed, 5 deletions(-) (limited to 'zkfacade') diff --git a/zkfacade/pom.xml b/zkfacade/pom.xml index 9c5ed23636f..d9a9cb51c89 100644 --- a/zkfacade/pom.xml +++ b/zkfacade/pom.xml @@ -57,11 +57,6 @@ - - com.google.guava - guava - provided - org.apache.zookeeper zookeeper -- cgit v1.2.3 From f578da98634e6c148a360a9ac4ec2313ba1a3033 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Thu, 1 Dec 2022 09:25:04 +0100 Subject: Revert "- Reduce usage of guava." --- clustercontroller-utils/pom.xml | 5 ++ config-application-package/pom.xml | 5 ++ config-provisioning/pom.xml | 11 +++++ docproc/pom.xml | 6 +++ documentapi/pom.xml | 2 +- .../filedistribution/FileReferenceCompressor.java | 26 +++++++---- model-evaluation/pom.xml | 5 ++ .../vespa/models/evaluation/FunctionEvaluator.java | 8 ++-- .../vespa/models/evaluation/LazyArrayContext.java | 53 ++++++++++++---------- .../java/ai/vespa/models/evaluation/Model.java | 30 ++++++------ .../vespa/models/evaluation/ModelsEvaluator.java | 5 +- .../models/handler/ModelsEvaluationHandler.java | 5 +- .../models/evaluation/ModelsEvaluatorTest.java | 11 +---- .../ai/vespa/models/handler/HandlerTester.java | 9 ++-- .../handler/ModelsEvaluationHandlerTest.java | 22 ++------- model-integration/pom.xml | 23 +++++----- .../rankingexpression/importer/ImportedModel.java | 22 +++++---- .../importer/vespa/VespaImportTestCase.java | 12 ++--- opennlp-linguistics/pom.xml | 6 +++ .../ranking/features/fieldmatch/Field.java | 13 ++++-- .../rankingexpression/ExpressionFunction.java | 12 +++-- .../evaluation/AbstractArrayContext.java | 17 ++++--- .../rankingexpression/rule/Arguments.java | 15 ++++-- .../rankingexpression/rule/LambdaFunctionNode.java | 34 ++++++++------ .../rankingexpression/rule/SetMembershipNode.java | 3 +- .../fieldmatch/test/FieldMatchMetricsTestCase.java | 8 ++-- service-monitor/pom.xml | 6 +++ vdslib/pom.xml | 5 ++ zkfacade/pom.xml | 5 ++ 29 files changed, 225 insertions(+), 159 deletions(-) (limited to 'zkfacade') diff --git a/clustercontroller-utils/pom.xml b/clustercontroller-utils/pom.xml index 381a4c88946..c534794ad0a 100644 --- a/clustercontroller-utils/pom.xml +++ b/clustercontroller-utils/pom.xml @@ -39,6 +39,11 @@ junit-jupiter-engine test + + com.google.guava + guava + provided + diff --git a/config-application-package/pom.xml b/config-application-package/pom.xml index 41e65f19b79..3d471f1c94b 100644 --- a/config-application-package/pom.xml +++ b/config-application-package/pom.xml @@ -44,6 +44,11 @@ ${project.version} provided + + com.google.guava + guava + provided + com.yahoo.vespa defaults diff --git a/config-provisioning/pom.xml b/config-provisioning/pom.xml index b447cb792a6..b190bca06a5 100644 --- a/config-provisioning/pom.xml +++ b/config-provisioning/pom.xml @@ -45,6 +45,17 @@ ${project.version} provided + + com.google.guava + guava + provided + + + com.google.inject + guice + provided + no_aop + com.yahoo.vespa testutil diff --git a/docproc/pom.xml b/docproc/pom.xml index dce6dafafeb..325e198c59a 100644 --- a/docproc/pom.xml +++ b/docproc/pom.xml @@ -16,6 +16,12 @@ 8-SNAPSHOT + + com.google.inject + guice + no_aop + provided + com.yahoo.vespa component diff --git a/documentapi/pom.xml b/documentapi/pom.xml index 271a984ea30..9b690f345ae 100644 --- a/documentapi/pom.xml +++ b/documentapi/pom.xml @@ -27,8 +27,8 @@ com.google.guava guava - test + junit junit diff --git a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReferenceCompressor.java b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReferenceCompressor.java index 0c3e69141e9..b6f84b979f5 100644 --- a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReferenceCompressor.java +++ b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReferenceCompressor.java @@ -121,11 +121,14 @@ public class FileReferenceCompressor { switch (type) { case compressed: log.log(Level.FINE, () -> "Compressing with compression type " + compressionType); - return switch (compressionType) { - case gzip -> new GZIPOutputStream(new FileOutputStream(outputFile)); - case lz4 -> new LZ4BlockOutputStream(new FileOutputStream(outputFile)); - default -> throw new RuntimeException("Unknown compression type " + compressionType); - }; + switch (compressionType) { + case gzip: + return new GZIPOutputStream(new FileOutputStream(outputFile)); + case lz4: + return new LZ4BlockOutputStream(new FileOutputStream(outputFile)); + default: + throw new RuntimeException("Unknown compression type " + compressionType); + } case file: return new FileOutputStream(outputFile); default: @@ -137,11 +140,14 @@ public class FileReferenceCompressor { switch (type) { case compressed: log.log(Level.FINE, () -> "Decompressing with compression type " + compressionType); - return switch (compressionType) { - case gzip -> new GZIPInputStream(new FileInputStream(inputFile)); - case lz4 -> new LZ4BlockInputStream(new FileInputStream(inputFile)); - default -> throw new RuntimeException("Unknown compression type " + compressionType); - }; + switch (compressionType) { + case gzip: + return new GZIPInputStream(new FileInputStream(inputFile)); + case lz4: + return new LZ4BlockInputStream(new FileInputStream(inputFile)); + default: + throw new RuntimeException("Unknown compression type " + compressionType); + } case file: return new FileInputStream(inputFile); default: diff --git a/model-evaluation/pom.xml b/model-evaluation/pom.xml index c0600872666..caf28199c3d 100644 --- a/model-evaluation/pom.xml +++ b/model-evaluation/pom.xml @@ -73,6 +73,11 @@ ${project.version} provided + + com.google.guava + guava + provided + org.lz4 lz4-java diff --git a/model-evaluation/src/main/java/ai/vespa/models/evaluation/FunctionEvaluator.java b/model-evaluation/src/main/java/ai/vespa/models/evaluation/FunctionEvaluator.java index 1d3da73a509..6af33e29e62 100644 --- a/model-evaluation/src/main/java/ai/vespa/models/evaluation/FunctionEvaluator.java +++ b/model-evaluation/src/main/java/ai/vespa/models/evaluation/FunctionEvaluator.java @@ -101,11 +101,9 @@ public class FunctionEvaluator { } public Tensor evaluate() { - function.argumentTypes().keySet().stream().sorted() - .forEach(name -> { - var type = function.argumentTypes().get(name); - checkArgument(name, type); - }); + for (Map.Entry argument : function.argumentTypes().entrySet()) { + checkArgument(argument.getKey(), argument.getValue()); + } evaluated = true; evaluateOnnxModels(); return function.getBody().evaluate(context).asTensor(); diff --git a/model-evaluation/src/main/java/ai/vespa/models/evaluation/LazyArrayContext.java b/model-evaluation/src/main/java/ai/vespa/models/evaluation/LazyArrayContext.java index 81325740218..d030108a17a 100644 --- a/model-evaluation/src/main/java/ai/vespa/models/evaluation/LazyArrayContext.java +++ b/model-evaluation/src/main/java/ai/vespa/models/evaluation/LazyArrayContext.java @@ -1,7 +1,8 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package ai.vespa.models.evaluation; -import com.yahoo.lang.MutableInteger; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.yahoo.searchlib.rankingexpression.ExpressionFunction; import com.yahoo.searchlib.rankingexpression.Reference; import com.yahoo.searchlib.rankingexpression.evaluation.Context; @@ -13,7 +14,6 @@ import com.yahoo.searchlib.rankingexpression.rule.CompositeNode; import com.yahoo.searchlib.rankingexpression.rule.ConstantNode; import com.yahoo.searchlib.rankingexpression.rule.ExpressionNode; import com.yahoo.searchlib.rankingexpression.rule.ReferenceNode; -import com.yahoo.stream.CustomCollectors; import com.yahoo.tensor.Tensor; import com.yahoo.tensor.TensorType; @@ -24,7 +24,6 @@ import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; -import java.util.stream.Collectors; /** * An array context supporting functions invocations implemented as lazy values. @@ -152,16 +151,16 @@ public final class LazyArrayContext extends Context implements ContextIndex { private static class IndexedBindings { /** The mapping from variable name to index */ - private final Map nameToIndex; + private final ImmutableMap nameToIndex; /** The names which needs to be bound externally when invoking this (i.e not constant or invocation */ - private final Set arguments; + private final ImmutableSet arguments; /** The current values set */ private final Value[] values; /** ONNX models indexed by rank feature that calls them */ - private final Map onnxModels; + private final ImmutableMap onnxModels; /** The object instance which encodes "no value is set". The actual value of this is never used. */ private static final Value missing = new DoubleValue(Double.NaN).freeze(); @@ -170,14 +169,14 @@ public final class LazyArrayContext extends Context implements ContextIndex { private Value missingValue = new DoubleValue(Double.NaN).freeze(); - private IndexedBindings(Map nameToIndex, + private IndexedBindings(ImmutableMap nameToIndex, Value[] values, - Set arguments, - Map onnxModels) { - this.nameToIndex = Map.copyOf(nameToIndex); + ImmutableSet arguments, + ImmutableMap onnxModels) { + this.nameToIndex = nameToIndex; this.values = values; this.arguments = arguments; - this.onnxModels = Map.copyOf(onnxModels); + this.onnxModels = onnxModels; } /** @@ -196,14 +195,16 @@ public final class LazyArrayContext extends Context implements ContextIndex { Map onnxModelsInUse = new HashMap<>(); extractBindTargets(function.getBody().getRoot(), referencedFunctions, bindTargets, arguments, onnxModels, onnxModelsInUse); - this.onnxModels = Map.copyOf(onnxModelsInUse); - this.arguments = Set.copyOf(arguments); + this.onnxModels = ImmutableMap.copyOf(onnxModelsInUse); + this.arguments = ImmutableSet.copyOf(arguments); values = new Value[bindTargets.size()]; Arrays.fill(values, missing); - MutableInteger nextIndex = new MutableInteger(0); - nameToIndex = Map.copyOf(bindTargets.stream() - .collect(CustomCollectors.toLinkedMap(name -> name, name -> nextIndex.next()))); + int i = 0; + ImmutableMap.Builder nameToIndexBuilder = new ImmutableMap.Builder<>(); + for (String variable : bindTargets) + nameToIndexBuilder.put(variable, i++); + nameToIndex = nameToIndexBuilder.build(); // 2. Bind the bind targets for (Constant constant : constants) { @@ -251,7 +252,8 @@ public final class LazyArrayContext extends Context implements ContextIndex { bindTargets.add(node.toString()); arguments.add(node.toString()); } - else if (node instanceof CompositeNode cNode) { + else if (node instanceof CompositeNode) { + CompositeNode cNode = (CompositeNode)node; for (ExpressionNode child : cNode.children()) extractBindTargets(child, functions, bindTargets, arguments, onnxModels, onnxModelsInUse); } @@ -289,14 +291,16 @@ public final class LazyArrayContext extends Context implements ContextIndex { } private Optional getArgument(ExpressionNode node) { - if (node instanceof ReferenceNode reference) { + if (node instanceof ReferenceNode) { + ReferenceNode reference = (ReferenceNode) node; if (reference.getArguments().size() > 0) { if (reference.getArguments().expressions().get(0) instanceof ConstantNode) { ExpressionNode constantNode = reference.getArguments().expressions().get(0); return Optional.of(stripQuotes(constantNode.toString())); } - if (reference.getArguments().expressions().get(0) instanceof ReferenceNode refNode) { - return Optional.of(refNode.getName()); + if (reference.getArguments().expressions().get(0) instanceof ReferenceNode) { + ReferenceNode referenceNode = (ReferenceNode) reference.getArguments().expressions().get(0); + return Optional.of(referenceNode.getName()); } } } @@ -312,17 +316,20 @@ public final class LazyArrayContext extends Context implements ContextIndex { } private boolean isFunctionReference(ExpressionNode node) { - if ( ! (node instanceof ReferenceNode reference)) return false; + if ( ! (node instanceof ReferenceNode)) return false; + ReferenceNode reference = (ReferenceNode)node; return reference.getName().equals("rankingExpression") && reference.getArguments().size() == 1; } private boolean isOnnx(ExpressionNode node) { - if ( ! (node instanceof ReferenceNode reference)) return false; + if ( ! (node instanceof ReferenceNode)) return false; + ReferenceNode reference = (ReferenceNode) node; return reference.getName().equals("onnx") || reference.getName().equals("onnxModel"); } private boolean isConstant(ExpressionNode node) { - if ( ! (node instanceof ReferenceNode reference)) return false; + if ( ! (node instanceof ReferenceNode)) return false; + ReferenceNode reference = (ReferenceNode)node; return reference.getName().equals("constant") && reference.getArguments().size() == 1; } diff --git a/model-evaluation/src/main/java/ai/vespa/models/evaluation/Model.java b/model-evaluation/src/main/java/ai/vespa/models/evaluation/Model.java index ffcfb5e9379..1ecec4108a3 100644 --- a/model-evaluation/src/main/java/ai/vespa/models/evaluation/Model.java +++ b/model-evaluation/src/main/java/ai/vespa/models/evaluation/Model.java @@ -2,16 +2,15 @@ package ai.vespa.models.evaluation; import com.yahoo.api.annotations.Beta; +import com.google.common.collect.ImmutableMap; import com.yahoo.searchlib.rankingexpression.ExpressionFunction; import com.yahoo.searchlib.rankingexpression.evaluation.ContextIndex; import com.yahoo.searchlib.rankingexpression.evaluation.ExpressionOptimizer; -import com.yahoo.stream.CustomCollectors; import com.yahoo.tensor.TensorType; import java.util.Arrays; import java.util.Collection; -import java.util.HashMap; -import java.util.LinkedHashMap; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.stream.Collectors; @@ -36,10 +35,10 @@ public class Model { private final List publicFunctions; /** Instances of each usage of the above function, where variables (if any) are replaced by their bindings */ - private final Map referencedFunctions; + private final ImmutableMap referencedFunctions; /** Context prototypes, indexed by function name (as all invocations of the same function share the same context prototype) */ - private final Map contextPrototypes; + private final ImmutableMap contextPrototypes; private final ExpressionOptimizer expressionOptimizer = new ExpressionOptimizer(); @@ -47,9 +46,9 @@ public class Model { public Model(String name, Collection functions) { this(name, functions.stream().collect(Collectors.toMap(f -> FunctionReference.fromName(f.getName()), f -> f)), - Map.of(), - List.of(), - List.of()); + Collections.emptyMap(), + Collections.emptyList(), + Collections.emptyList()); } Model(String name, @@ -60,7 +59,7 @@ public class Model { this.name = name; // Build context and add missing function arguments (missing because it is legal to omit scalar type arguments) - Map contextBuilder = new LinkedHashMap<>(); + ImmutableMap.Builder contextBuilder = new ImmutableMap.Builder<>(); for (Map.Entry function : functions.entrySet()) { try { LazyArrayContext context = new LazyArrayContext(function.getValue(), referencedFunctions, constants, onnxModels, this); @@ -93,14 +92,19 @@ public class Model { throw new IllegalArgumentException("Could not prepare an evaluation context for " + function, e); } } - this.contextPrototypes = Map.copyOf(contextBuilder); + this.contextPrototypes = contextBuilder.build(); this.functions = List.copyOf(functions.values()); this.publicFunctions = functions.values().stream() .filter(f -> !f.getName().startsWith(INTERMEDIATE_OPERATION_FUNCTION_PREFIX)).toList(); // Optimize functions - this.referencedFunctions = Map.copyOf(referencedFunctions.entrySet().stream() - .collect(CustomCollectors.toLinkedMap(f -> f.getKey(), f -> optimize(f.getValue(), contextPrototypes.get(f.getKey().functionName()))))); + ImmutableMap.Builder functionsBuilder = new ImmutableMap.Builder<>(); + for (Map.Entry function : referencedFunctions.entrySet()) { + ExpressionFunction optimizedFunction = optimize(function.getValue(), + contextPrototypes.get(function.getKey().functionName())); + functionsBuilder.put(function.getKey(), optimizedFunction); + } + this.referencedFunctions = functionsBuilder.build(); } /** Returns an optimized version of the given function */ @@ -138,7 +142,7 @@ public class Model { } /** Returns an immutable map of the referenced function instances of this */ - Map referencedFunctions() { return Map.copyOf(referencedFunctions); } + Map referencedFunctions() { return referencedFunctions; } /** Returns the given referred function, or throws a IllegalArgumentException if it does not exist */ ExpressionFunction requireReferencedFunction(FunctionReference reference) { diff --git a/model-evaluation/src/main/java/ai/vespa/models/evaluation/ModelsEvaluator.java b/model-evaluation/src/main/java/ai/vespa/models/evaluation/ModelsEvaluator.java index 40a503e0212..88843fd99ab 100644 --- a/model-evaluation/src/main/java/ai/vespa/models/evaluation/ModelsEvaluator.java +++ b/model-evaluation/src/main/java/ai/vespa/models/evaluation/ModelsEvaluator.java @@ -2,6 +2,7 @@ package ai.vespa.models.evaluation; import com.yahoo.api.annotations.Beta; +import com.google.common.collect.ImmutableMap; import com.yahoo.component.annotation.Inject; import com.yahoo.component.AbstractComponent; import com.yahoo.filedistribution.fileacquirer.FileAcquirer; @@ -22,7 +23,7 @@ import java.util.Map; @Beta public class ModelsEvaluator extends AbstractComponent { - private final Map models; + private final ImmutableMap models; @Inject public ModelsEvaluator(RankProfilesConfig config, @@ -42,7 +43,7 @@ public class ModelsEvaluator extends AbstractComponent { } public ModelsEvaluator(Map models) { - this.models = Map.copyOf(models); + this.models = ImmutableMap.copyOf(models); } /** Returns the models of this as an immutable map */ diff --git a/model-evaluation/src/main/java/ai/vespa/models/handler/ModelsEvaluationHandler.java b/model-evaluation/src/main/java/ai/vespa/models/handler/ModelsEvaluationHandler.java index 78addf0328a..2661b9c2eb2 100644 --- a/model-evaluation/src/main/java/ai/vespa/models/handler/ModelsEvaluationHandler.java +++ b/model-evaluation/src/main/java/ai/vespa/models/handler/ModelsEvaluationHandler.java @@ -102,8 +102,9 @@ public class ModelsEvaluationHandler extends ThreadedHttpRequestHandler { private HttpResponse listAllModels(HttpRequest request) { Slime slime = new Slime(); Cursor root = slime.setObject(); - modelsEvaluator.models().keySet().stream().sorted() - .forEach(name -> root.setString(name, baseUrl(request) + name)); + for (String modelName: modelsEvaluator.models().keySet()) { + root.setString(modelName, baseUrl(request) + modelName); + } return new Response(200, com.yahoo.slime.JsonFormat.toJsonBytes(slime)); } diff --git a/model-evaluation/src/test/java/ai/vespa/models/evaluation/ModelsEvaluatorTest.java b/model-evaluation/src/test/java/ai/vespa/models/evaluation/ModelsEvaluatorTest.java index f09bac63085..c4e859bec9f 100644 --- a/model-evaluation/src/test/java/ai/vespa/models/evaluation/ModelsEvaluatorTest.java +++ b/model-evaluation/src/test/java/ai/vespa/models/evaluation/ModelsEvaluatorTest.java @@ -75,7 +75,7 @@ public class ModelsEvaluatorTest { evaluator.evaluate(); } catch (IllegalStateException e) { - assertEquals("Argument 'arg1' must be bound to a value of type tensor(d0[1])", + assertEquals("Argument 'arg2' must be bound to a value of type tensor(d1{})", Exceptions.toMessageString(e)); } @@ -88,15 +88,6 @@ public class ModelsEvaluatorTest { assertEquals("Argument 'arg1' must be bound to a value of type tensor(d0[1])", Exceptions.toMessageString(e)); } - try { // Just the other binding - FunctionEvaluator evaluator = model.evaluatorOf("test"); - evaluator.bind("arg1", Tensor.from(TensorType.fromSpec("tensor(d0[1])"), "{{d0:0}:0.1}")); - evaluator.evaluate(); - } - catch (IllegalStateException e) { - assertEquals("Argument 'arg2' must be bound to a value of type tensor(d1{})", - Exceptions.toMessageString(e)); - } try { // Wrong binding argument FunctionEvaluator evaluator = model.evaluatorOf("test"); diff --git a/model-evaluation/src/test/java/ai/vespa/models/handler/HandlerTester.java b/model-evaluation/src/test/java/ai/vespa/models/handler/HandlerTester.java index 3b16be311a0..fc05a9936a9 100644 --- a/model-evaluation/src/test/java/ai/vespa/models/handler/HandlerTester.java +++ b/model-evaluation/src/test/java/ai/vespa/models/handler/HandlerTester.java @@ -10,6 +10,7 @@ import com.yahoo.tensor.serialization.JsonFormat; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.nio.charset.StandardCharsets; +import java.util.Collections; import java.util.Map; import java.util.concurrent.Executors; @@ -24,19 +25,19 @@ class HandlerTester { } void assertResponse(String url, int expectedCode) { - assertResponse(url, Map.of(), expectedCode, (String)null); + assertResponse(url, Collections.emptyMap(), expectedCode, (String)null); } void assertResponse(String url, int expectedCode, String expectedResult) { - assertResponse(url, Map.of(), expectedCode, expectedResult); + assertResponse(url, Collections.emptyMap(), expectedCode, expectedResult); } void assertResponse(String url, int expectedCode, String expectedResult, Map headers) { - assertResponse(url, Map.of(), expectedCode, expectedResult, headers); + assertResponse(url, Collections.emptyMap(), expectedCode, expectedResult, headers); } void assertResponse(String url, Map properties, int expectedCode, String expectedResult) { - assertResponse(url, properties, expectedCode, expectedResult, Map.of()); + assertResponse(url, properties, expectedCode, expectedResult, Collections.emptyMap()); } void assertResponse(String url, Map properties, int expectedCode, String expectedResult, Map headers) { diff --git a/model-evaluation/src/test/java/ai/vespa/models/handler/ModelsEvaluationHandlerTest.java b/model-evaluation/src/test/java/ai/vespa/models/handler/ModelsEvaluationHandlerTest.java index d804e50c67d..0de8ce5f061 100644 --- a/model-evaluation/src/test/java/ai/vespa/models/handler/ModelsEvaluationHandlerTest.java +++ b/model-evaluation/src/test/java/ai/vespa/models/handler/ModelsEvaluationHandlerTest.java @@ -49,30 +49,16 @@ public class ModelsEvaluationHandlerTest { @Test public void testListModels() { String url = "http://localhost/model-evaluation/v1"; - String expected = "{" + - "\"lightgbm_regression\":\"http://localhost/model-evaluation/v1/lightgbm_regression\"," + - "\"mnist_saved\":\"http://localhost/model-evaluation/v1/mnist_saved\"," + - "\"mnist_softmax\":\"http://localhost/model-evaluation/v1/mnist_softmax\"," + - "\"mnist_softmax_saved\":\"http://localhost/model-evaluation/v1/mnist_softmax_saved\"," + - "\"vespa_model\":\"http://localhost/model-evaluation/v1/vespa_model\"," + - "\"xgboost_2_2\":\"http://localhost/model-evaluation/v1/xgboost_2_2\"," + - "\"xgboost_non_standalone\":\"http://localhost/model-evaluation/v1/xgboost_non_standalone\"" + - "}"; + String expected = + "{\"mnist_softmax\":\"http://localhost/model-evaluation/v1/mnist_softmax\",\"xgboost_non_standalone\":\"http://localhost/model-evaluation/v1/xgboost_non_standalone\",\"mnist_saved\":\"http://localhost/model-evaluation/v1/mnist_saved\",\"mnist_softmax_saved\":\"http://localhost/model-evaluation/v1/mnist_softmax_saved\",\"vespa_model\":\"http://localhost/model-evaluation/v1/vespa_model\",\"xgboost_2_2\":\"http://localhost/model-evaluation/v1/xgboost_2_2\",\"lightgbm_regression\":\"http://localhost/model-evaluation/v1/lightgbm_regression\"}"; handler.assertResponse(url, 200, expected); } @Test public void testListModelsWithDifferentHost() { String url = "http://localhost/model-evaluation/v1"; - String expected = "{" + - "\"lightgbm_regression\":\"http://localhost:8088/model-evaluation/v1/lightgbm_regression\"," + - "\"mnist_saved\":\"http://localhost:8088/model-evaluation/v1/mnist_saved\"," + - "\"mnist_softmax\":\"http://localhost:8088/model-evaluation/v1/mnist_softmax\"," + - "\"mnist_softmax_saved\":\"http://localhost:8088/model-evaluation/v1/mnist_softmax_saved\"," + - "\"vespa_model\":\"http://localhost:8088/model-evaluation/v1/vespa_model\"," + - "\"xgboost_2_2\":\"http://localhost:8088/model-evaluation/v1/xgboost_2_2\"," + - "\"xgboost_non_standalone\":\"http://localhost:8088/model-evaluation/v1/xgboost_non_standalone\"" + - "}"; + String expected = + "{\"mnist_softmax\":\"http://localhost:8088/model-evaluation/v1/mnist_softmax\",\"xgboost_non_standalone\":\"http://localhost:8088/model-evaluation/v1/xgboost_non_standalone\",\"mnist_saved\":\"http://localhost:8088/model-evaluation/v1/mnist_saved\",\"mnist_softmax_saved\":\"http://localhost:8088/model-evaluation/v1/mnist_softmax_saved\",\"vespa_model\":\"http://localhost:8088/model-evaluation/v1/vespa_model\",\"xgboost_2_2\":\"http://localhost:8088/model-evaluation/v1/xgboost_2_2\",\"lightgbm_regression\":\"http://localhost:8088/model-evaluation/v1/lightgbm_regression\"}"; handler.assertResponse(url, 200, expected, Map.of("Host", "localhost:8088")); } diff --git a/model-integration/pom.xml b/model-integration/pom.xml index 63232b61106..7d3ab3f7a5f 100644 --- a/model-integration/pom.xml +++ b/model-integration/pom.xml @@ -15,6 +15,12 @@ 8-SNAPSHOT container-plugin + + junit + junit + test + + com.yahoo.vespa annotations @@ -52,6 +58,12 @@ provided + + com.google.guava + guava + provided + + com.microsoft.onnxruntime onnxruntime @@ -60,17 +72,6 @@ com.google.protobuf protobuf-java - - - junit - junit - test - - - com.google.guava - guava - test - diff --git a/model-integration/src/main/java/ai/vespa/rankingexpression/importer/ImportedModel.java b/model-integration/src/main/java/ai/vespa/rankingexpression/importer/ImportedModel.java index 8c55e6793c0..35c409a637c 100644 --- a/model-integration/src/main/java/ai/vespa/rankingexpression/importer/ImportedModel.java +++ b/model-integration/src/main/java/ai/vespa/rankingexpression/importer/ImportedModel.java @@ -1,19 +1,20 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package ai.vespa.rankingexpression.importer; +import com.google.common.collect.ImmutableMap; import ai.vespa.rankingexpression.importer.configmodelview.ImportedMlFunction; import ai.vespa.rankingexpression.importer.configmodelview.ImportedMlModel; import com.yahoo.config.application.api.ApplicationPackage; import com.yahoo.io.IOUtils; import com.yahoo.searchlib.rankingexpression.RankingExpression; import com.yahoo.searchlib.rankingexpression.parser.ParseException; -import com.yahoo.stream.CustomCollectors; import com.yahoo.tensor.Tensor; import com.yahoo.tensor.TensorType; import java.io.File; import java.io.IOException; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; @@ -72,7 +73,7 @@ public class ImportedModel implements ImportedMlModel { public String toString() { return "imported model '" + name + "' from " + source; } /** Returns an immutable map of the inputs of this */ - public Map inputs() { return Map.copyOf(inputs); } + public Map inputs() { return Collections.unmodifiableMap(inputs); } @Override public Optional inputTypeSpec(String input) { @@ -120,7 +121,7 @@ public class ImportedModel implements ImportedMlModel { * which are not Inputs/Placeholders or Variables (which instead become respectively inputs and constants). * Note that only nodes recursively referenced by a placeholder/input are added. */ - public Map expressions() { return Map.copyOf(expressions); } + public Map expressions() { return Collections.unmodifiableMap(expressions); } /** * Returns an immutable map of the functions that are part of this model. @@ -129,7 +130,7 @@ public class ImportedModel implements ImportedMlModel { public Map functions() { return asExpressionStrings(functions); } /** Returns an immutable map of the signatures of this */ - public Map signatures() { return Map.copyOf(signatures); } + public Map signatures() { return Collections.unmodifiableMap(signatures); } /** Returns the given signature. If it does not already exist it is added to this. */ public Signature signature(String name) { @@ -269,29 +270,30 @@ public class ImportedModel implements ImportedMlModel { * Returns an immutable map of the inputs (evaluation context) of this. This is a map from input name * in this signature to input name in the owning model */ - public Map inputs() { return Map.copyOf(inputs); } + public Map inputs() { return Collections.unmodifiableMap(inputs); } /** Returns the name and type of all inputs in this signature as an immutable map */ Map inputMap() { + ImmutableMap.Builder inputs = new ImmutableMap.Builder<>(); // Note: We're naming inputs by their actual name (used in the expression, given by what the input maps *to* // in the model, as these are the names which must actually be bound, if we are to avoid creating an // "input mapping" to accommodate this complexity - return Map.copyOf(inputs.entrySet() - .stream() - .collect(CustomCollectors.toLinkedMap(Map.Entry::getValue, e -> owner().inputs.get(e.getValue())))); + for (Map.Entry inputEntry : inputs().entrySet()) + inputs.put(inputEntry.getValue(), owner().inputs().get(inputEntry.getValue())); + return inputs.build(); } /** Returns the type of the input this input references */ public TensorType inputArgument(String inputName) { return owner().inputs().get(inputs.get(inputName)); } /** Returns an immutable list of the expression names of this */ - public Map outputs() { return Map.copyOf(outputs); } + public Map outputs() { return Collections.unmodifiableMap(outputs); } /** * Returns an immutable list of the outputs of this which could not be imported, * with a string detailing the reason for each */ - public Map skippedOutputs() { return Map.copyOf(skippedOutputs); } + public Map skippedOutputs() { return Collections.unmodifiableMap(skippedOutputs); } /** Returns the expression this output references as an imported function */ public ImportedMlFunction outputFunction(String outputName, String functionName) { diff --git a/model-integration/src/test/java/ai/vespa/rankingexpression/importer/vespa/VespaImportTestCase.java b/model-integration/src/test/java/ai/vespa/rankingexpression/importer/vespa/VespaImportTestCase.java index 75e31d66e5b..d9c7e67c946 100644 --- a/model-integration/src/test/java/ai/vespa/rankingexpression/importer/vespa/VespaImportTestCase.java +++ b/model-integration/src/test/java/ai/vespa/rankingexpression/importer/vespa/VespaImportTestCase.java @@ -10,8 +10,7 @@ import com.yahoo.searchlib.rankingexpression.parser.ParseException; import com.yahoo.tensor.Tensor; import org.junit.Test; -import java.util.Map; -import java.util.stream.Collectors; +import java.util.List; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; @@ -52,12 +51,9 @@ public class VespaImportTestCase { assertEquals("reduce(reduce(input1 * input2, sum, name) * constant(constant1asLarge), max, x) * constant2", model.expressions().get("foo2").getRoot().toString()); - Map byName = model.outputExpressions().stream() - .collect(Collectors.toUnmodifiableMap(ImportedMlFunction::name, f -> f)); - assertEquals(2, byName.size()); - assertTrue(byName.containsKey("foo1")); - assertTrue(byName.containsKey("foo2")); - ImportedMlFunction foo1Function = byName.get("foo1"); + List functions = model.outputExpressions(); + assertEquals(2, functions.size()); + ImportedMlFunction foo1Function = functions.get(0); assertEquals("foo1", foo1Function.name()); assertEquals("reduce(reduce(input1 * input2, sum, name) * constant1, max, x) * constant2", foo1Function.expression()); assertEquals("tensor():{202.5}", evaluate(foo1Function, "{{name:a, x:0}: 1, {name:a, x:1}: 2, {name:a, x:2}: 3}").toString()); diff --git a/opennlp-linguistics/pom.xml b/opennlp-linguistics/pom.xml index a7907ba212f..40f1e95f4f4 100644 --- a/opennlp-linguistics/pom.xml +++ b/opennlp-linguistics/pom.xml @@ -52,6 +52,12 @@ linguistics ${project.version} + + com.google.inject + guice + provided + no_aop + org.apache.opennlp opennlp-tools diff --git a/searchlib/src/main/java/com/yahoo/searchlib/ranking/features/fieldmatch/Field.java b/searchlib/src/main/java/com/yahoo/searchlib/ranking/features/fieldmatch/Field.java index 3f711df4fdd..9492cebc608 100644 --- a/searchlib/src/main/java/com/yahoo/searchlib/ranking/features/fieldmatch/Field.java +++ b/searchlib/src/main/java/com/yahoo/searchlib/ranking/features/fieldmatch/Field.java @@ -1,7 +1,8 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.searchlib.ranking.features.fieldmatch; -import java.util.Arrays; +import com.google.common.collect.ImmutableList; + import java.util.List; /** @@ -11,17 +12,19 @@ import java.util.List; */ public class Field { - private final List terms; + private final ImmutableList terms; /** Creates a field from a space-separated string */ public Field(String fieldString) { - terms = Arrays.stream(fieldString.split(" ")).map(Term::new).toList(); - + ImmutableList.Builder list = new ImmutableList.Builder<>(); + for (String term : fieldString.split(" ")) + list.add(new Term(term)); + this.terms = list.build(); } /** Creates a field from a list of terms */ public Field(List terms) { - this.terms = List.copyOf(terms); + this.terms = ImmutableList.copyOf(terms); } /** Returns an immutable list of the terms in this */ diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/ExpressionFunction.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/ExpressionFunction.java index 241a53fb458..eac87ff2f12 100755 --- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/ExpressionFunction.java +++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/ExpressionFunction.java @@ -1,6 +1,8 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.searchlib.rankingexpression; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.yahoo.searchlib.rankingexpression.rule.ConstantNode; import com.yahoo.searchlib.rankingexpression.rule.ExpressionNode; import com.yahoo.searchlib.rankingexpression.rule.FunctionNode; @@ -34,10 +36,10 @@ import java.util.Optional; public class ExpressionFunction { private final String name; - private final List arguments; + private final ImmutableList arguments; /** Types of the inputs, if known. The keys here is any subset (including empty and identity) of the argument list */ - private final Map argumentTypes; + private final ImmutableMap argumentTypes; private final RankingExpression body; private final Optional returnType; @@ -60,17 +62,17 @@ public class ExpressionFunction { * @param body the ranking expression that defines this function */ public ExpressionFunction(String name, List arguments, RankingExpression body) { - this(name, arguments, body, Map.of(), Optional.empty()); + this(name, arguments, body, ImmutableMap.of(), Optional.empty()); } public ExpressionFunction(String name, List arguments, RankingExpression body, Map argumentTypes, Optional returnType) { this.name = Objects.requireNonNull(name, "name cannot be null"); - this.arguments = arguments==null ? List.of() : List.copyOf(arguments); + this.arguments = arguments==null ? ImmutableList.of() : ImmutableList.copyOf(arguments); this.body = Objects.requireNonNull(body, "body cannot be null"); if ( ! this.arguments.containsAll(argumentTypes.keySet())) throw new IllegalArgumentException("Argument type keys must be a subset of the argument keys"); - this.argumentTypes = Map.copyOf(argumentTypes); + this.argumentTypes = ImmutableMap.copyOf(argumentTypes); this.returnType = Objects.requireNonNull(returnType, "returnType cannot be null"); } diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/AbstractArrayContext.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/AbstractArrayContext.java index 4d6ac0104c7..340556b7e2d 100644 --- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/AbstractArrayContext.java +++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/AbstractArrayContext.java @@ -1,12 +1,11 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.searchlib.rankingexpression.evaluation; -import com.yahoo.lang.MutableInteger; +import com.google.common.collect.ImmutableMap; import com.yahoo.searchlib.rankingexpression.RankingExpression; import com.yahoo.searchlib.rankingexpression.rule.CompositeNode; import com.yahoo.searchlib.rankingexpression.rule.ExpressionNode; import com.yahoo.searchlib.rankingexpression.rule.ReferenceNode; -import com.yahoo.stream.CustomCollectors; import java.util.BitSet; import java.util.LinkedHashSet; @@ -117,7 +116,7 @@ public abstract class AbstractArrayContext extends Context implements Cloneable, private static class IndexedBindings implements Cloneable { /** The mapping from variable name to index */ - private final Map nameToIndex; + private final ImmutableMap nameToIndex; /** The current values set, pre-converted to doubles */ private double[] doubleValues; @@ -126,7 +125,7 @@ public abstract class AbstractArrayContext extends Context implements Cloneable, private BitSet setValues; /** Value to return if value is missing. */ - private final double missingValue; + private double missingValue; public IndexedBindings(RankingExpression expression, Value missingValue) { Set bindTargets = new LinkedHashSet<>(); @@ -139,8 +138,11 @@ public abstract class AbstractArrayContext extends Context implements Cloneable, doubleValues[i] = this.missingValue; } - MutableInteger index = new MutableInteger(0); - nameToIndex = bindTargets.stream().collect(CustomCollectors.toLinkedMap(name -> name, name -> index.next())); + int i = 0; + ImmutableMap.Builder nameToIndexBuilder = new ImmutableMap.Builder<>(); + for (String variable : bindTargets) + nameToIndexBuilder.put(variable,i++); + nameToIndex = nameToIndexBuilder.build(); } private void extractBindTargets(ExpressionNode node, Set bindTargets) { @@ -150,7 +152,8 @@ public abstract class AbstractArrayContext extends Context implements Cloneable, ": Array lookup is not supported with features having arguments)"); bindTargets.add(node.toString()); } - else if (node instanceof CompositeNode cNode) { + else if (node instanceof CompositeNode) { + CompositeNode cNode = (CompositeNode)node; for (ExpressionNode child : cNode.children()) extractBindTargets(child, bindTargets); } diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/Arguments.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/Arguments.java index b716b693011..e770e6ac038 100644 --- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/Arguments.java +++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/Arguments.java @@ -1,6 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.searchlib.rankingexpression.rule; +import com.google.common.collect.ImmutableList; import com.yahoo.searchlib.rankingexpression.evaluation.Context; import com.yahoo.searchlib.rankingexpression.evaluation.Value; @@ -17,23 +18,27 @@ public final class Arguments implements Serializable { public static final Arguments EMPTY = new Arguments(); - private final List expressions; + private final ImmutableList expressions; public Arguments() { - this(List.of()); + this(ImmutableList.of()); } public Arguments(ExpressionNode singleArgument) { - this(List.of(singleArgument)); + this(ImmutableList.of(singleArgument)); } public Arguments(List expressions) { if (expressions == null) { - this.expressions = List.of(); + this.expressions = ImmutableList.of(); return; } - this.expressions = List.copyOf(expressions); + // Build in a roundabout way because java generics and lists + ImmutableList.Builder b = ImmutableList.builder(); + for (ExpressionNode node : expressions) + b.add(node); + this.expressions = b.build(); } /** Returns an unmodifiable list of the expressions in this, never null */ diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/LambdaFunctionNode.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/LambdaFunctionNode.java index 9c0c0e46804..97e9a74f9c8 100644 --- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/LambdaFunctionNode.java +++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/LambdaFunctionNode.java @@ -1,6 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.searchlib.rankingexpression.rule; +import com.google.common.collect.ImmutableList; import com.yahoo.searchlib.rankingexpression.Reference; import com.yahoo.searchlib.rankingexpression.evaluation.Context; import com.yahoo.searchlib.rankingexpression.evaluation.MapContext; @@ -26,7 +27,7 @@ import java.util.stream.Collectors; */ public class LambdaFunctionNode extends CompositeNode { - private final List arguments; + private final ImmutableList arguments; private final ExpressionNode functionExpression; public LambdaFunctionNode(List arguments, ExpressionNode functionExpression) { @@ -36,7 +37,7 @@ public class LambdaFunctionNode extends CompositeNode { .filter(f -> ! arguments.contains(f)) .collect(Collectors.joining(", "))); } - this.arguments = List.copyOf(arguments); + this.arguments = ImmutableList.copyOf(arguments); this.functionExpression = functionExpression; } @@ -105,12 +106,15 @@ public class LambdaFunctionNode extends CompositeNode { } private Optional getDirectEvaluator() { - if ( ! (functionExpression instanceof OperationNode node)) { + if ( ! (functionExpression instanceof OperationNode)) { return Optional.empty(); } - if ( ! (node.children().get(0) instanceof ReferenceNode lhs) || ! (node.children().get(1) instanceof ReferenceNode rhs)) { + OperationNode node = (OperationNode) functionExpression; + if ( ! (node.children().get(0) instanceof ReferenceNode) || ! (node.children().get(1) instanceof ReferenceNode)) { return Optional.empty(); } + var lhs = (ReferenceNode) node.children().get(0); + var rhs = (ReferenceNode) node.children().get(1); if (! lhs.getName().equals(arguments.get(0)) || ! rhs.getName().equals(arguments.get(1))) { return Optional.empty(); } @@ -118,17 +122,17 @@ public class LambdaFunctionNode extends CompositeNode { return Optional.empty(); } Operator operator = node.operators().get(0); - return switch (operator) { - case or -> asFunctionExpression((left, right) -> ((left != 0.0) || (right != 0.0)) ? 1.0 : 0.0); - case and -> asFunctionExpression((left, right) -> ((left != 0.0) && (right != 0.0)) ? 1.0 : 0.0); - case plus -> asFunctionExpression((left, right) -> left + right); - case minus -> asFunctionExpression((left, right) -> left - right); - case multiply -> asFunctionExpression((left, right) -> left * right); - case divide -> asFunctionExpression((left, right) -> left / right); - case modulo -> asFunctionExpression((left, right) -> left % right); - case power -> asFunctionExpression(Math::pow); - default -> Optional.empty(); - }; + switch (operator) { + case or: return asFunctionExpression((left, right) -> ((left != 0.0) || (right != 0.0)) ? 1.0 : 0.0); + case and: return asFunctionExpression((left, right) -> ((left != 0.0) && (right != 0.0)) ? 1.0 : 0.0); + case plus: return asFunctionExpression((left, right) -> left + right); + case minus: return asFunctionExpression((left, right) -> left - right); + case multiply: return asFunctionExpression((left, right) -> left * right); + case divide: return asFunctionExpression((left, right) -> left / right); + case modulo: return asFunctionExpression((left, right) -> left % right); + case power: return asFunctionExpression(Math::pow); + } + return Optional.empty(); } private Optional asFunctionExpression(DoubleBinaryOperator operator) { diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/SetMembershipNode.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/SetMembershipNode.java index 6b87f75d884..5da2fbfe624 100644 --- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/SetMembershipNode.java +++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/SetMembershipNode.java @@ -1,6 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.searchlib.rankingexpression.rule; +import com.google.common.collect.ImmutableList; import com.yahoo.searchlib.rankingexpression.Reference; import com.yahoo.searchlib.rankingexpression.evaluation.BooleanValue; import com.yahoo.searchlib.rankingexpression.evaluation.Context; @@ -29,7 +30,7 @@ public class SetMembershipNode extends BooleanNode { public SetMembershipNode(ExpressionNode testValue, List setValues) { this.testValue = testValue; - this.setValues = List.copyOf(setValues); + this.setValues = ImmutableList.copyOf(setValues); } /** The value to check for membership in the set */ diff --git a/searchlib/src/test/java/com/yahoo/searchlib/ranking/features/fieldmatch/test/FieldMatchMetricsTestCase.java b/searchlib/src/test/java/com/yahoo/searchlib/ranking/features/fieldmatch/test/FieldMatchMetricsTestCase.java index 41a6cd69878..f4a003868f8 100644 --- a/searchlib/src/test/java/com/yahoo/searchlib/ranking/features/fieldmatch/test/FieldMatchMetricsTestCase.java +++ b/searchlib/src/test/java/com/yahoo/searchlib/ranking/features/fieldmatch/test/FieldMatchMetricsTestCase.java @@ -1,6 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.searchlib.ranking.features.fieldmatch.test; +import com.google.common.collect.ImmutableList; import com.yahoo.searchlib.ranking.features.fieldmatch.Field; import com.yahoo.searchlib.ranking.features.fieldmatch.FieldMatchMetrics; import com.yahoo.searchlib.ranking.features.fieldmatch.FieldMatchMetricsComputer; @@ -9,7 +10,6 @@ import com.yahoo.searchlib.ranking.features.fieldmatch.QueryTerm; import com.yahoo.searchlib.ranking.features.fieldmatch.Query; import org.junit.Test; -import java.util.ArrayList; import java.util.List; import static org.junit.Assert.assertEquals; @@ -776,9 +776,9 @@ public class FieldMatchMetricsTestCase { } private Field toField(String fieldString) { - if (fieldString.length() == 0) return new Field(List.of()); + if (fieldString.length() == 0) return new Field(ImmutableList.of()); - List terms = new ArrayList<>(); + ImmutableList.Builder terms = new ImmutableList.Builder<>(); for (String termString : fieldString.split(" ")) { String[] colonSplit = termString.split(":"); if (colonSplit.length > 1) @@ -786,7 +786,7 @@ public class FieldMatchMetricsTestCase { else terms.add(new Field.Term(colonSplit[0])); } - return new Field(List.copyOf(terms)); + return new Field(terms.build()); } } diff --git a/service-monitor/pom.xml b/service-monitor/pom.xml index 2378d63d457..6a46838a0ce 100644 --- a/service-monitor/pom.xml +++ b/service-monitor/pom.xml @@ -90,6 +90,12 @@ ${project.version} provided + + com.google.inject + guice + provided + no_aop + com.fasterxml.jackson.core jackson-annotations diff --git a/vdslib/pom.xml b/vdslib/pom.xml index 4067fc9d8d4..c534b2024ed 100644 --- a/vdslib/pom.xml +++ b/vdslib/pom.xml @@ -14,6 +14,11 @@ vdslib + + com.google.guava + guava + provided + junit junit diff --git a/zkfacade/pom.xml b/zkfacade/pom.xml index d9a9cb51c89..9c5ed23636f 100644 --- a/zkfacade/pom.xml +++ b/zkfacade/pom.xml @@ -57,6 +57,11 @@ + + com.google.guava + guava + provided + org.apache.zookeeper zookeeper -- cgit v1.2.3 From 1eb22cc4a24973f52b344c3033cff394c724cbe4 Mon Sep 17 00:00:00 2001 From: Henning Baldersheim Date: Thu, 1 Dec 2022 09:32:05 +0100 Subject: Revert "Revert "- Reduce usage of guava."" --- clustercontroller-utils/pom.xml | 5 -- config-application-package/pom.xml | 5 -- config-provisioning/pom.xml | 11 ----- docproc/pom.xml | 6 --- documentapi/pom.xml | 2 +- .../filedistribution/FileReferenceCompressor.java | 26 ++++------- model-evaluation/pom.xml | 5 -- .../vespa/models/evaluation/FunctionEvaluator.java | 8 ++-- .../vespa/models/evaluation/LazyArrayContext.java | 53 ++++++++++------------ .../java/ai/vespa/models/evaluation/Model.java | 30 ++++++------ .../vespa/models/evaluation/ModelsEvaluator.java | 5 +- .../models/handler/ModelsEvaluationHandler.java | 5 +- .../models/evaluation/ModelsEvaluatorTest.java | 11 ++++- .../ai/vespa/models/handler/HandlerTester.java | 9 ++-- .../handler/ModelsEvaluationHandlerTest.java | 22 +++++++-- model-integration/pom.xml | 23 +++++----- .../rankingexpression/importer/ImportedModel.java | 22 ++++----- .../importer/vespa/VespaImportTestCase.java | 12 +++-- opennlp-linguistics/pom.xml | 6 --- .../ranking/features/fieldmatch/Field.java | 13 ++---- .../rankingexpression/ExpressionFunction.java | 12 ++--- .../evaluation/AbstractArrayContext.java | 17 +++---- .../rankingexpression/rule/Arguments.java | 15 ++---- .../rankingexpression/rule/LambdaFunctionNode.java | 34 ++++++-------- .../rankingexpression/rule/SetMembershipNode.java | 3 +- .../fieldmatch/test/FieldMatchMetricsTestCase.java | 8 ++-- service-monitor/pom.xml | 6 --- vdslib/pom.xml | 5 -- zkfacade/pom.xml | 5 -- 29 files changed, 159 insertions(+), 225 deletions(-) (limited to 'zkfacade') diff --git a/clustercontroller-utils/pom.xml b/clustercontroller-utils/pom.xml index c534794ad0a..381a4c88946 100644 --- a/clustercontroller-utils/pom.xml +++ b/clustercontroller-utils/pom.xml @@ -39,11 +39,6 @@ junit-jupiter-engine test - - com.google.guava - guava - provided - diff --git a/config-application-package/pom.xml b/config-application-package/pom.xml index 3d471f1c94b..41e65f19b79 100644 --- a/config-application-package/pom.xml +++ b/config-application-package/pom.xml @@ -44,11 +44,6 @@ ${project.version} provided - - com.google.guava - guava - provided - com.yahoo.vespa defaults diff --git a/config-provisioning/pom.xml b/config-provisioning/pom.xml index b190bca06a5..b447cb792a6 100644 --- a/config-provisioning/pom.xml +++ b/config-provisioning/pom.xml @@ -45,17 +45,6 @@ ${project.version} provided - - com.google.guava - guava - provided - - - com.google.inject - guice - provided - no_aop - com.yahoo.vespa testutil diff --git a/docproc/pom.xml b/docproc/pom.xml index 325e198c59a..dce6dafafeb 100644 --- a/docproc/pom.xml +++ b/docproc/pom.xml @@ -16,12 +16,6 @@ 8-SNAPSHOT - - com.google.inject - guice - no_aop - provided - com.yahoo.vespa component diff --git a/documentapi/pom.xml b/documentapi/pom.xml index 9b690f345ae..271a984ea30 100644 --- a/documentapi/pom.xml +++ b/documentapi/pom.xml @@ -27,8 +27,8 @@ com.google.guava guava + test - junit junit diff --git a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReferenceCompressor.java b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReferenceCompressor.java index b6f84b979f5..0c3e69141e9 100644 --- a/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReferenceCompressor.java +++ b/filedistribution/src/main/java/com/yahoo/vespa/filedistribution/FileReferenceCompressor.java @@ -121,14 +121,11 @@ public class FileReferenceCompressor { switch (type) { case compressed: log.log(Level.FINE, () -> "Compressing with compression type " + compressionType); - switch (compressionType) { - case gzip: - return new GZIPOutputStream(new FileOutputStream(outputFile)); - case lz4: - return new LZ4BlockOutputStream(new FileOutputStream(outputFile)); - default: - throw new RuntimeException("Unknown compression type " + compressionType); - } + return switch (compressionType) { + case gzip -> new GZIPOutputStream(new FileOutputStream(outputFile)); + case lz4 -> new LZ4BlockOutputStream(new FileOutputStream(outputFile)); + default -> throw new RuntimeException("Unknown compression type " + compressionType); + }; case file: return new FileOutputStream(outputFile); default: @@ -140,14 +137,11 @@ public class FileReferenceCompressor { switch (type) { case compressed: log.log(Level.FINE, () -> "Decompressing with compression type " + compressionType); - switch (compressionType) { - case gzip: - return new GZIPInputStream(new FileInputStream(inputFile)); - case lz4: - return new LZ4BlockInputStream(new FileInputStream(inputFile)); - default: - throw new RuntimeException("Unknown compression type " + compressionType); - } + return switch (compressionType) { + case gzip -> new GZIPInputStream(new FileInputStream(inputFile)); + case lz4 -> new LZ4BlockInputStream(new FileInputStream(inputFile)); + default -> throw new RuntimeException("Unknown compression type " + compressionType); + }; case file: return new FileInputStream(inputFile); default: diff --git a/model-evaluation/pom.xml b/model-evaluation/pom.xml index caf28199c3d..c0600872666 100644 --- a/model-evaluation/pom.xml +++ b/model-evaluation/pom.xml @@ -73,11 +73,6 @@ ${project.version} provided - - com.google.guava - guava - provided - org.lz4 lz4-java diff --git a/model-evaluation/src/main/java/ai/vespa/models/evaluation/FunctionEvaluator.java b/model-evaluation/src/main/java/ai/vespa/models/evaluation/FunctionEvaluator.java index 6af33e29e62..1d3da73a509 100644 --- a/model-evaluation/src/main/java/ai/vespa/models/evaluation/FunctionEvaluator.java +++ b/model-evaluation/src/main/java/ai/vespa/models/evaluation/FunctionEvaluator.java @@ -101,9 +101,11 @@ public class FunctionEvaluator { } public Tensor evaluate() { - for (Map.Entry argument : function.argumentTypes().entrySet()) { - checkArgument(argument.getKey(), argument.getValue()); - } + function.argumentTypes().keySet().stream().sorted() + .forEach(name -> { + var type = function.argumentTypes().get(name); + checkArgument(name, type); + }); evaluated = true; evaluateOnnxModels(); return function.getBody().evaluate(context).asTensor(); diff --git a/model-evaluation/src/main/java/ai/vespa/models/evaluation/LazyArrayContext.java b/model-evaluation/src/main/java/ai/vespa/models/evaluation/LazyArrayContext.java index d030108a17a..81325740218 100644 --- a/model-evaluation/src/main/java/ai/vespa/models/evaluation/LazyArrayContext.java +++ b/model-evaluation/src/main/java/ai/vespa/models/evaluation/LazyArrayContext.java @@ -1,8 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package ai.vespa.models.evaluation; -import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSet; +import com.yahoo.lang.MutableInteger; import com.yahoo.searchlib.rankingexpression.ExpressionFunction; import com.yahoo.searchlib.rankingexpression.Reference; import com.yahoo.searchlib.rankingexpression.evaluation.Context; @@ -14,6 +13,7 @@ import com.yahoo.searchlib.rankingexpression.rule.CompositeNode; import com.yahoo.searchlib.rankingexpression.rule.ConstantNode; import com.yahoo.searchlib.rankingexpression.rule.ExpressionNode; import com.yahoo.searchlib.rankingexpression.rule.ReferenceNode; +import com.yahoo.stream.CustomCollectors; import com.yahoo.tensor.Tensor; import com.yahoo.tensor.TensorType; @@ -24,6 +24,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.stream.Collectors; /** * An array context supporting functions invocations implemented as lazy values. @@ -151,16 +152,16 @@ public final class LazyArrayContext extends Context implements ContextIndex { private static class IndexedBindings { /** The mapping from variable name to index */ - private final ImmutableMap nameToIndex; + private final Map nameToIndex; /** The names which needs to be bound externally when invoking this (i.e not constant or invocation */ - private final ImmutableSet arguments; + private final Set arguments; /** The current values set */ private final Value[] values; /** ONNX models indexed by rank feature that calls them */ - private final ImmutableMap onnxModels; + private final Map onnxModels; /** The object instance which encodes "no value is set". The actual value of this is never used. */ private static final Value missing = new DoubleValue(Double.NaN).freeze(); @@ -169,14 +170,14 @@ public final class LazyArrayContext extends Context implements ContextIndex { private Value missingValue = new DoubleValue(Double.NaN).freeze(); - private IndexedBindings(ImmutableMap nameToIndex, + private IndexedBindings(Map nameToIndex, Value[] values, - ImmutableSet arguments, - ImmutableMap onnxModels) { - this.nameToIndex = nameToIndex; + Set arguments, + Map onnxModels) { + this.nameToIndex = Map.copyOf(nameToIndex); this.values = values; this.arguments = arguments; - this.onnxModels = onnxModels; + this.onnxModels = Map.copyOf(onnxModels); } /** @@ -195,16 +196,14 @@ public final class LazyArrayContext extends Context implements ContextIndex { Map onnxModelsInUse = new HashMap<>(); extractBindTargets(function.getBody().getRoot(), referencedFunctions, bindTargets, arguments, onnxModels, onnxModelsInUse); - this.onnxModels = ImmutableMap.copyOf(onnxModelsInUse); - this.arguments = ImmutableSet.copyOf(arguments); + this.onnxModels = Map.copyOf(onnxModelsInUse); + this.arguments = Set.copyOf(arguments); values = new Value[bindTargets.size()]; Arrays.fill(values, missing); - int i = 0; - ImmutableMap.Builder nameToIndexBuilder = new ImmutableMap.Builder<>(); - for (String variable : bindTargets) - nameToIndexBuilder.put(variable, i++); - nameToIndex = nameToIndexBuilder.build(); + MutableInteger nextIndex = new MutableInteger(0); + nameToIndex = Map.copyOf(bindTargets.stream() + .collect(CustomCollectors.toLinkedMap(name -> name, name -> nextIndex.next()))); // 2. Bind the bind targets for (Constant constant : constants) { @@ -252,8 +251,7 @@ public final class LazyArrayContext extends Context implements ContextIndex { bindTargets.add(node.toString()); arguments.add(node.toString()); } - else if (node instanceof CompositeNode) { - CompositeNode cNode = (CompositeNode)node; + else if (node instanceof CompositeNode cNode) { for (ExpressionNode child : cNode.children()) extractBindTargets(child, functions, bindTargets, arguments, onnxModels, onnxModelsInUse); } @@ -291,16 +289,14 @@ public final class LazyArrayContext extends Context implements ContextIndex { } private Optional getArgument(ExpressionNode node) { - if (node instanceof ReferenceNode) { - ReferenceNode reference = (ReferenceNode) node; + if (node instanceof ReferenceNode reference) { if (reference.getArguments().size() > 0) { if (reference.getArguments().expressions().get(0) instanceof ConstantNode) { ExpressionNode constantNode = reference.getArguments().expressions().get(0); return Optional.of(stripQuotes(constantNode.toString())); } - if (reference.getArguments().expressions().get(0) instanceof ReferenceNode) { - ReferenceNode referenceNode = (ReferenceNode) reference.getArguments().expressions().get(0); - return Optional.of(referenceNode.getName()); + if (reference.getArguments().expressions().get(0) instanceof ReferenceNode refNode) { + return Optional.of(refNode.getName()); } } } @@ -316,20 +312,17 @@ public final class LazyArrayContext extends Context implements ContextIndex { } private boolean isFunctionReference(ExpressionNode node) { - if ( ! (node instanceof ReferenceNode)) return false; - ReferenceNode reference = (ReferenceNode)node; + if ( ! (node instanceof ReferenceNode reference)) return false; return reference.getName().equals("rankingExpression") && reference.getArguments().size() == 1; } private boolean isOnnx(ExpressionNode node) { - if ( ! (node instanceof ReferenceNode)) return false; - ReferenceNode reference = (ReferenceNode) node; + if ( ! (node instanceof ReferenceNode reference)) return false; return reference.getName().equals("onnx") || reference.getName().equals("onnxModel"); } private boolean isConstant(ExpressionNode node) { - if ( ! (node instanceof ReferenceNode)) return false; - ReferenceNode reference = (ReferenceNode)node; + if ( ! (node instanceof ReferenceNode reference)) return false; return reference.getName().equals("constant") && reference.getArguments().size() == 1; } diff --git a/model-evaluation/src/main/java/ai/vespa/models/evaluation/Model.java b/model-evaluation/src/main/java/ai/vespa/models/evaluation/Model.java index 1ecec4108a3..ffcfb5e9379 100644 --- a/model-evaluation/src/main/java/ai/vespa/models/evaluation/Model.java +++ b/model-evaluation/src/main/java/ai/vespa/models/evaluation/Model.java @@ -2,15 +2,16 @@ package ai.vespa.models.evaluation; import com.yahoo.api.annotations.Beta; -import com.google.common.collect.ImmutableMap; import com.yahoo.searchlib.rankingexpression.ExpressionFunction; import com.yahoo.searchlib.rankingexpression.evaluation.ContextIndex; import com.yahoo.searchlib.rankingexpression.evaluation.ExpressionOptimizer; +import com.yahoo.stream.CustomCollectors; import com.yahoo.tensor.TensorType; import java.util.Arrays; import java.util.Collection; -import java.util.Collections; +import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.stream.Collectors; @@ -35,10 +36,10 @@ public class Model { private final List publicFunctions; /** Instances of each usage of the above function, where variables (if any) are replaced by their bindings */ - private final ImmutableMap referencedFunctions; + private final Map referencedFunctions; /** Context prototypes, indexed by function name (as all invocations of the same function share the same context prototype) */ - private final ImmutableMap contextPrototypes; + private final Map contextPrototypes; private final ExpressionOptimizer expressionOptimizer = new ExpressionOptimizer(); @@ -46,9 +47,9 @@ public class Model { public Model(String name, Collection functions) { this(name, functions.stream().collect(Collectors.toMap(f -> FunctionReference.fromName(f.getName()), f -> f)), - Collections.emptyMap(), - Collections.emptyList(), - Collections.emptyList()); + Map.of(), + List.of(), + List.of()); } Model(String name, @@ -59,7 +60,7 @@ public class Model { this.name = name; // Build context and add missing function arguments (missing because it is legal to omit scalar type arguments) - ImmutableMap.Builder contextBuilder = new ImmutableMap.Builder<>(); + Map contextBuilder = new LinkedHashMap<>(); for (Map.Entry function : functions.entrySet()) { try { LazyArrayContext context = new LazyArrayContext(function.getValue(), referencedFunctions, constants, onnxModels, this); @@ -92,19 +93,14 @@ public class Model { throw new IllegalArgumentException("Could not prepare an evaluation context for " + function, e); } } - this.contextPrototypes = contextBuilder.build(); + this.contextPrototypes = Map.copyOf(contextBuilder); this.functions = List.copyOf(functions.values()); this.publicFunctions = functions.values().stream() .filter(f -> !f.getName().startsWith(INTERMEDIATE_OPERATION_FUNCTION_PREFIX)).toList(); // Optimize functions - ImmutableMap.Builder functionsBuilder = new ImmutableMap.Builder<>(); - for (Map.Entry function : referencedFunctions.entrySet()) { - ExpressionFunction optimizedFunction = optimize(function.getValue(), - contextPrototypes.get(function.getKey().functionName())); - functionsBuilder.put(function.getKey(), optimizedFunction); - } - this.referencedFunctions = functionsBuilder.build(); + this.referencedFunctions = Map.copyOf(referencedFunctions.entrySet().stream() + .collect(CustomCollectors.toLinkedMap(f -> f.getKey(), f -> optimize(f.getValue(), contextPrototypes.get(f.getKey().functionName()))))); } /** Returns an optimized version of the given function */ @@ -142,7 +138,7 @@ public class Model { } /** Returns an immutable map of the referenced function instances of this */ - Map referencedFunctions() { return referencedFunctions; } + Map referencedFunctions() { return Map.copyOf(referencedFunctions); } /** Returns the given referred function, or throws a IllegalArgumentException if it does not exist */ ExpressionFunction requireReferencedFunction(FunctionReference reference) { diff --git a/model-evaluation/src/main/java/ai/vespa/models/evaluation/ModelsEvaluator.java b/model-evaluation/src/main/java/ai/vespa/models/evaluation/ModelsEvaluator.java index 88843fd99ab..40a503e0212 100644 --- a/model-evaluation/src/main/java/ai/vespa/models/evaluation/ModelsEvaluator.java +++ b/model-evaluation/src/main/java/ai/vespa/models/evaluation/ModelsEvaluator.java @@ -2,7 +2,6 @@ package ai.vespa.models.evaluation; import com.yahoo.api.annotations.Beta; -import com.google.common.collect.ImmutableMap; import com.yahoo.component.annotation.Inject; import com.yahoo.component.AbstractComponent; import com.yahoo.filedistribution.fileacquirer.FileAcquirer; @@ -23,7 +22,7 @@ import java.util.Map; @Beta public class ModelsEvaluator extends AbstractComponent { - private final ImmutableMap models; + private final Map models; @Inject public ModelsEvaluator(RankProfilesConfig config, @@ -43,7 +42,7 @@ public class ModelsEvaluator extends AbstractComponent { } public ModelsEvaluator(Map models) { - this.models = ImmutableMap.copyOf(models); + this.models = Map.copyOf(models); } /** Returns the models of this as an immutable map */ diff --git a/model-evaluation/src/main/java/ai/vespa/models/handler/ModelsEvaluationHandler.java b/model-evaluation/src/main/java/ai/vespa/models/handler/ModelsEvaluationHandler.java index 2661b9c2eb2..78addf0328a 100644 --- a/model-evaluation/src/main/java/ai/vespa/models/handler/ModelsEvaluationHandler.java +++ b/model-evaluation/src/main/java/ai/vespa/models/handler/ModelsEvaluationHandler.java @@ -102,9 +102,8 @@ public class ModelsEvaluationHandler extends ThreadedHttpRequestHandler { private HttpResponse listAllModels(HttpRequest request) { Slime slime = new Slime(); Cursor root = slime.setObject(); - for (String modelName: modelsEvaluator.models().keySet()) { - root.setString(modelName, baseUrl(request) + modelName); - } + modelsEvaluator.models().keySet().stream().sorted() + .forEach(name -> root.setString(name, baseUrl(request) + name)); return new Response(200, com.yahoo.slime.JsonFormat.toJsonBytes(slime)); } diff --git a/model-evaluation/src/test/java/ai/vespa/models/evaluation/ModelsEvaluatorTest.java b/model-evaluation/src/test/java/ai/vespa/models/evaluation/ModelsEvaluatorTest.java index c4e859bec9f..f09bac63085 100644 --- a/model-evaluation/src/test/java/ai/vespa/models/evaluation/ModelsEvaluatorTest.java +++ b/model-evaluation/src/test/java/ai/vespa/models/evaluation/ModelsEvaluatorTest.java @@ -75,7 +75,7 @@ public class ModelsEvaluatorTest { evaluator.evaluate(); } catch (IllegalStateException e) { - assertEquals("Argument 'arg2' must be bound to a value of type tensor(d1{})", + assertEquals("Argument 'arg1' must be bound to a value of type tensor(d0[1])", Exceptions.toMessageString(e)); } @@ -88,6 +88,15 @@ public class ModelsEvaluatorTest { assertEquals("Argument 'arg1' must be bound to a value of type tensor(d0[1])", Exceptions.toMessageString(e)); } + try { // Just the other binding + FunctionEvaluator evaluator = model.evaluatorOf("test"); + evaluator.bind("arg1", Tensor.from(TensorType.fromSpec("tensor(d0[1])"), "{{d0:0}:0.1}")); + evaluator.evaluate(); + } + catch (IllegalStateException e) { + assertEquals("Argument 'arg2' must be bound to a value of type tensor(d1{})", + Exceptions.toMessageString(e)); + } try { // Wrong binding argument FunctionEvaluator evaluator = model.evaluatorOf("test"); diff --git a/model-evaluation/src/test/java/ai/vespa/models/handler/HandlerTester.java b/model-evaluation/src/test/java/ai/vespa/models/handler/HandlerTester.java index fc05a9936a9..3b16be311a0 100644 --- a/model-evaluation/src/test/java/ai/vespa/models/handler/HandlerTester.java +++ b/model-evaluation/src/test/java/ai/vespa/models/handler/HandlerTester.java @@ -10,7 +10,6 @@ import com.yahoo.tensor.serialization.JsonFormat; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.nio.charset.StandardCharsets; -import java.util.Collections; import java.util.Map; import java.util.concurrent.Executors; @@ -25,19 +24,19 @@ class HandlerTester { } void assertResponse(String url, int expectedCode) { - assertResponse(url, Collections.emptyMap(), expectedCode, (String)null); + assertResponse(url, Map.of(), expectedCode, (String)null); } void assertResponse(String url, int expectedCode, String expectedResult) { - assertResponse(url, Collections.emptyMap(), expectedCode, expectedResult); + assertResponse(url, Map.of(), expectedCode, expectedResult); } void assertResponse(String url, int expectedCode, String expectedResult, Map headers) { - assertResponse(url, Collections.emptyMap(), expectedCode, expectedResult, headers); + assertResponse(url, Map.of(), expectedCode, expectedResult, headers); } void assertResponse(String url, Map properties, int expectedCode, String expectedResult) { - assertResponse(url, properties, expectedCode, expectedResult, Collections.emptyMap()); + assertResponse(url, properties, expectedCode, expectedResult, Map.of()); } void assertResponse(String url, Map properties, int expectedCode, String expectedResult, Map headers) { diff --git a/model-evaluation/src/test/java/ai/vespa/models/handler/ModelsEvaluationHandlerTest.java b/model-evaluation/src/test/java/ai/vespa/models/handler/ModelsEvaluationHandlerTest.java index 0de8ce5f061..d804e50c67d 100644 --- a/model-evaluation/src/test/java/ai/vespa/models/handler/ModelsEvaluationHandlerTest.java +++ b/model-evaluation/src/test/java/ai/vespa/models/handler/ModelsEvaluationHandlerTest.java @@ -49,16 +49,30 @@ public class ModelsEvaluationHandlerTest { @Test public void testListModels() { String url = "http://localhost/model-evaluation/v1"; - String expected = - "{\"mnist_softmax\":\"http://localhost/model-evaluation/v1/mnist_softmax\",\"xgboost_non_standalone\":\"http://localhost/model-evaluation/v1/xgboost_non_standalone\",\"mnist_saved\":\"http://localhost/model-evaluation/v1/mnist_saved\",\"mnist_softmax_saved\":\"http://localhost/model-evaluation/v1/mnist_softmax_saved\",\"vespa_model\":\"http://localhost/model-evaluation/v1/vespa_model\",\"xgboost_2_2\":\"http://localhost/model-evaluation/v1/xgboost_2_2\",\"lightgbm_regression\":\"http://localhost/model-evaluation/v1/lightgbm_regression\"}"; + String expected = "{" + + "\"lightgbm_regression\":\"http://localhost/model-evaluation/v1/lightgbm_regression\"," + + "\"mnist_saved\":\"http://localhost/model-evaluation/v1/mnist_saved\"," + + "\"mnist_softmax\":\"http://localhost/model-evaluation/v1/mnist_softmax\"," + + "\"mnist_softmax_saved\":\"http://localhost/model-evaluation/v1/mnist_softmax_saved\"," + + "\"vespa_model\":\"http://localhost/model-evaluation/v1/vespa_model\"," + + "\"xgboost_2_2\":\"http://localhost/model-evaluation/v1/xgboost_2_2\"," + + "\"xgboost_non_standalone\":\"http://localhost/model-evaluation/v1/xgboost_non_standalone\"" + + "}"; handler.assertResponse(url, 200, expected); } @Test public void testListModelsWithDifferentHost() { String url = "http://localhost/model-evaluation/v1"; - String expected = - "{\"mnist_softmax\":\"http://localhost:8088/model-evaluation/v1/mnist_softmax\",\"xgboost_non_standalone\":\"http://localhost:8088/model-evaluation/v1/xgboost_non_standalone\",\"mnist_saved\":\"http://localhost:8088/model-evaluation/v1/mnist_saved\",\"mnist_softmax_saved\":\"http://localhost:8088/model-evaluation/v1/mnist_softmax_saved\",\"vespa_model\":\"http://localhost:8088/model-evaluation/v1/vespa_model\",\"xgboost_2_2\":\"http://localhost:8088/model-evaluation/v1/xgboost_2_2\",\"lightgbm_regression\":\"http://localhost:8088/model-evaluation/v1/lightgbm_regression\"}"; + String expected = "{" + + "\"lightgbm_regression\":\"http://localhost:8088/model-evaluation/v1/lightgbm_regression\"," + + "\"mnist_saved\":\"http://localhost:8088/model-evaluation/v1/mnist_saved\"," + + "\"mnist_softmax\":\"http://localhost:8088/model-evaluation/v1/mnist_softmax\"," + + "\"mnist_softmax_saved\":\"http://localhost:8088/model-evaluation/v1/mnist_softmax_saved\"," + + "\"vespa_model\":\"http://localhost:8088/model-evaluation/v1/vespa_model\"," + + "\"xgboost_2_2\":\"http://localhost:8088/model-evaluation/v1/xgboost_2_2\"," + + "\"xgboost_non_standalone\":\"http://localhost:8088/model-evaluation/v1/xgboost_non_standalone\"" + + "}"; handler.assertResponse(url, 200, expected, Map.of("Host", "localhost:8088")); } diff --git a/model-integration/pom.xml b/model-integration/pom.xml index 7d3ab3f7a5f..63232b61106 100644 --- a/model-integration/pom.xml +++ b/model-integration/pom.xml @@ -15,12 +15,6 @@ 8-SNAPSHOT container-plugin - - junit - junit - test - - com.yahoo.vespa annotations @@ -58,12 +52,6 @@ provided - - com.google.guava - guava - provided - - com.microsoft.onnxruntime onnxruntime @@ -72,6 +60,17 @@ com.google.protobuf protobuf-java + + + junit + junit + test + + + com.google.guava + guava + test + diff --git a/model-integration/src/main/java/ai/vespa/rankingexpression/importer/ImportedModel.java b/model-integration/src/main/java/ai/vespa/rankingexpression/importer/ImportedModel.java index 35c409a637c..8c55e6793c0 100644 --- a/model-integration/src/main/java/ai/vespa/rankingexpression/importer/ImportedModel.java +++ b/model-integration/src/main/java/ai/vespa/rankingexpression/importer/ImportedModel.java @@ -1,20 +1,19 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package ai.vespa.rankingexpression.importer; -import com.google.common.collect.ImmutableMap; import ai.vespa.rankingexpression.importer.configmodelview.ImportedMlFunction; import ai.vespa.rankingexpression.importer.configmodelview.ImportedMlModel; import com.yahoo.config.application.api.ApplicationPackage; import com.yahoo.io.IOUtils; import com.yahoo.searchlib.rankingexpression.RankingExpression; import com.yahoo.searchlib.rankingexpression.parser.ParseException; +import com.yahoo.stream.CustomCollectors; import com.yahoo.tensor.Tensor; import com.yahoo.tensor.TensorType; import java.io.File; import java.io.IOException; import java.util.ArrayList; -import java.util.Collections; import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; @@ -73,7 +72,7 @@ public class ImportedModel implements ImportedMlModel { public String toString() { return "imported model '" + name + "' from " + source; } /** Returns an immutable map of the inputs of this */ - public Map inputs() { return Collections.unmodifiableMap(inputs); } + public Map inputs() { return Map.copyOf(inputs); } @Override public Optional inputTypeSpec(String input) { @@ -121,7 +120,7 @@ public class ImportedModel implements ImportedMlModel { * which are not Inputs/Placeholders or Variables (which instead become respectively inputs and constants). * Note that only nodes recursively referenced by a placeholder/input are added. */ - public Map expressions() { return Collections.unmodifiableMap(expressions); } + public Map expressions() { return Map.copyOf(expressions); } /** * Returns an immutable map of the functions that are part of this model. @@ -130,7 +129,7 @@ public class ImportedModel implements ImportedMlModel { public Map functions() { return asExpressionStrings(functions); } /** Returns an immutable map of the signatures of this */ - public Map signatures() { return Collections.unmodifiableMap(signatures); } + public Map signatures() { return Map.copyOf(signatures); } /** Returns the given signature. If it does not already exist it is added to this. */ public Signature signature(String name) { @@ -270,30 +269,29 @@ public class ImportedModel implements ImportedMlModel { * Returns an immutable map of the inputs (evaluation context) of this. This is a map from input name * in this signature to input name in the owning model */ - public Map inputs() { return Collections.unmodifiableMap(inputs); } + public Map inputs() { return Map.copyOf(inputs); } /** Returns the name and type of all inputs in this signature as an immutable map */ Map inputMap() { - ImmutableMap.Builder inputs = new ImmutableMap.Builder<>(); // Note: We're naming inputs by their actual name (used in the expression, given by what the input maps *to* // in the model, as these are the names which must actually be bound, if we are to avoid creating an // "input mapping" to accommodate this complexity - for (Map.Entry inputEntry : inputs().entrySet()) - inputs.put(inputEntry.getValue(), owner().inputs().get(inputEntry.getValue())); - return inputs.build(); + return Map.copyOf(inputs.entrySet() + .stream() + .collect(CustomCollectors.toLinkedMap(Map.Entry::getValue, e -> owner().inputs.get(e.getValue())))); } /** Returns the type of the input this input references */ public TensorType inputArgument(String inputName) { return owner().inputs().get(inputs.get(inputName)); } /** Returns an immutable list of the expression names of this */ - public Map outputs() { return Collections.unmodifiableMap(outputs); } + public Map outputs() { return Map.copyOf(outputs); } /** * Returns an immutable list of the outputs of this which could not be imported, * with a string detailing the reason for each */ - public Map skippedOutputs() { return Collections.unmodifiableMap(skippedOutputs); } + public Map skippedOutputs() { return Map.copyOf(skippedOutputs); } /** Returns the expression this output references as an imported function */ public ImportedMlFunction outputFunction(String outputName, String functionName) { diff --git a/model-integration/src/test/java/ai/vespa/rankingexpression/importer/vespa/VespaImportTestCase.java b/model-integration/src/test/java/ai/vespa/rankingexpression/importer/vespa/VespaImportTestCase.java index d9c7e67c946..75e31d66e5b 100644 --- a/model-integration/src/test/java/ai/vespa/rankingexpression/importer/vespa/VespaImportTestCase.java +++ b/model-integration/src/test/java/ai/vespa/rankingexpression/importer/vespa/VespaImportTestCase.java @@ -10,7 +10,8 @@ import com.yahoo.searchlib.rankingexpression.parser.ParseException; import com.yahoo.tensor.Tensor; import org.junit.Test; -import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; @@ -51,9 +52,12 @@ public class VespaImportTestCase { assertEquals("reduce(reduce(input1 * input2, sum, name) * constant(constant1asLarge), max, x) * constant2", model.expressions().get("foo2").getRoot().toString()); - List functions = model.outputExpressions(); - assertEquals(2, functions.size()); - ImportedMlFunction foo1Function = functions.get(0); + Map byName = model.outputExpressions().stream() + .collect(Collectors.toUnmodifiableMap(ImportedMlFunction::name, f -> f)); + assertEquals(2, byName.size()); + assertTrue(byName.containsKey("foo1")); + assertTrue(byName.containsKey("foo2")); + ImportedMlFunction foo1Function = byName.get("foo1"); assertEquals("foo1", foo1Function.name()); assertEquals("reduce(reduce(input1 * input2, sum, name) * constant1, max, x) * constant2", foo1Function.expression()); assertEquals("tensor():{202.5}", evaluate(foo1Function, "{{name:a, x:0}: 1, {name:a, x:1}: 2, {name:a, x:2}: 3}").toString()); diff --git a/opennlp-linguistics/pom.xml b/opennlp-linguistics/pom.xml index 40f1e95f4f4..a7907ba212f 100644 --- a/opennlp-linguistics/pom.xml +++ b/opennlp-linguistics/pom.xml @@ -52,12 +52,6 @@ linguistics ${project.version} - - com.google.inject - guice - provided - no_aop - org.apache.opennlp opennlp-tools diff --git a/searchlib/src/main/java/com/yahoo/searchlib/ranking/features/fieldmatch/Field.java b/searchlib/src/main/java/com/yahoo/searchlib/ranking/features/fieldmatch/Field.java index 9492cebc608..3f711df4fdd 100644 --- a/searchlib/src/main/java/com/yahoo/searchlib/ranking/features/fieldmatch/Field.java +++ b/searchlib/src/main/java/com/yahoo/searchlib/ranking/features/fieldmatch/Field.java @@ -1,8 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.searchlib.ranking.features.fieldmatch; -import com.google.common.collect.ImmutableList; - +import java.util.Arrays; import java.util.List; /** @@ -12,19 +11,17 @@ import java.util.List; */ public class Field { - private final ImmutableList terms; + private final List terms; /** Creates a field from a space-separated string */ public Field(String fieldString) { - ImmutableList.Builder list = new ImmutableList.Builder<>(); - for (String term : fieldString.split(" ")) - list.add(new Term(term)); - this.terms = list.build(); + terms = Arrays.stream(fieldString.split(" ")).map(Term::new).toList(); + } /** Creates a field from a list of terms */ public Field(List terms) { - this.terms = ImmutableList.copyOf(terms); + this.terms = List.copyOf(terms); } /** Returns an immutable list of the terms in this */ diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/ExpressionFunction.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/ExpressionFunction.java index eac87ff2f12..241a53fb458 100755 --- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/ExpressionFunction.java +++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/ExpressionFunction.java @@ -1,8 +1,6 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.searchlib.rankingexpression; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; import com.yahoo.searchlib.rankingexpression.rule.ConstantNode; import com.yahoo.searchlib.rankingexpression.rule.ExpressionNode; import com.yahoo.searchlib.rankingexpression.rule.FunctionNode; @@ -36,10 +34,10 @@ import java.util.Optional; public class ExpressionFunction { private final String name; - private final ImmutableList arguments; + private final List arguments; /** Types of the inputs, if known. The keys here is any subset (including empty and identity) of the argument list */ - private final ImmutableMap argumentTypes; + private final Map argumentTypes; private final RankingExpression body; private final Optional returnType; @@ -62,17 +60,17 @@ public class ExpressionFunction { * @param body the ranking expression that defines this function */ public ExpressionFunction(String name, List arguments, RankingExpression body) { - this(name, arguments, body, ImmutableMap.of(), Optional.empty()); + this(name, arguments, body, Map.of(), Optional.empty()); } public ExpressionFunction(String name, List arguments, RankingExpression body, Map argumentTypes, Optional returnType) { this.name = Objects.requireNonNull(name, "name cannot be null"); - this.arguments = arguments==null ? ImmutableList.of() : ImmutableList.copyOf(arguments); + this.arguments = arguments==null ? List.of() : List.copyOf(arguments); this.body = Objects.requireNonNull(body, "body cannot be null"); if ( ! this.arguments.containsAll(argumentTypes.keySet())) throw new IllegalArgumentException("Argument type keys must be a subset of the argument keys"); - this.argumentTypes = ImmutableMap.copyOf(argumentTypes); + this.argumentTypes = Map.copyOf(argumentTypes); this.returnType = Objects.requireNonNull(returnType, "returnType cannot be null"); } diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/AbstractArrayContext.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/AbstractArrayContext.java index 340556b7e2d..4d6ac0104c7 100644 --- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/AbstractArrayContext.java +++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/AbstractArrayContext.java @@ -1,11 +1,12 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.searchlib.rankingexpression.evaluation; -import com.google.common.collect.ImmutableMap; +import com.yahoo.lang.MutableInteger; import com.yahoo.searchlib.rankingexpression.RankingExpression; import com.yahoo.searchlib.rankingexpression.rule.CompositeNode; import com.yahoo.searchlib.rankingexpression.rule.ExpressionNode; import com.yahoo.searchlib.rankingexpression.rule.ReferenceNode; +import com.yahoo.stream.CustomCollectors; import java.util.BitSet; import java.util.LinkedHashSet; @@ -116,7 +117,7 @@ public abstract class AbstractArrayContext extends Context implements Cloneable, private static class IndexedBindings implements Cloneable { /** The mapping from variable name to index */ - private final ImmutableMap nameToIndex; + private final Map nameToIndex; /** The current values set, pre-converted to doubles */ private double[] doubleValues; @@ -125,7 +126,7 @@ public abstract class AbstractArrayContext extends Context implements Cloneable, private BitSet setValues; /** Value to return if value is missing. */ - private double missingValue; + private final double missingValue; public IndexedBindings(RankingExpression expression, Value missingValue) { Set bindTargets = new LinkedHashSet<>(); @@ -138,11 +139,8 @@ public abstract class AbstractArrayContext extends Context implements Cloneable, doubleValues[i] = this.missingValue; } - int i = 0; - ImmutableMap.Builder nameToIndexBuilder = new ImmutableMap.Builder<>(); - for (String variable : bindTargets) - nameToIndexBuilder.put(variable,i++); - nameToIndex = nameToIndexBuilder.build(); + MutableInteger index = new MutableInteger(0); + nameToIndex = bindTargets.stream().collect(CustomCollectors.toLinkedMap(name -> name, name -> index.next())); } private void extractBindTargets(ExpressionNode node, Set bindTargets) { @@ -152,8 +150,7 @@ public abstract class AbstractArrayContext extends Context implements Cloneable, ": Array lookup is not supported with features having arguments)"); bindTargets.add(node.toString()); } - else if (node instanceof CompositeNode) { - CompositeNode cNode = (CompositeNode)node; + else if (node instanceof CompositeNode cNode) { for (ExpressionNode child : cNode.children()) extractBindTargets(child, bindTargets); } diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/Arguments.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/Arguments.java index e770e6ac038..b716b693011 100644 --- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/Arguments.java +++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/Arguments.java @@ -1,7 +1,6 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.searchlib.rankingexpression.rule; -import com.google.common.collect.ImmutableList; import com.yahoo.searchlib.rankingexpression.evaluation.Context; import com.yahoo.searchlib.rankingexpression.evaluation.Value; @@ -18,27 +17,23 @@ public final class Arguments implements Serializable { public static final Arguments EMPTY = new Arguments(); - private final ImmutableList expressions; + private final List expressions; public Arguments() { - this(ImmutableList.of()); + this(List.of()); } public Arguments(ExpressionNode singleArgument) { - this(ImmutableList.of(singleArgument)); + this(List.of(singleArgument)); } public Arguments(List expressions) { if (expressions == null) { - this.expressions = ImmutableList.of(); + this.expressions = List.of(); return; } - // Build in a roundabout way because java generics and lists - ImmutableList.Builder b = ImmutableList.builder(); - for (ExpressionNode node : expressions) - b.add(node); - this.expressions = b.build(); + this.expressions = List.copyOf(expressions); } /** Returns an unmodifiable list of the expressions in this, never null */ diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/LambdaFunctionNode.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/LambdaFunctionNode.java index 97e9a74f9c8..9c0c0e46804 100644 --- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/LambdaFunctionNode.java +++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/LambdaFunctionNode.java @@ -1,7 +1,6 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.searchlib.rankingexpression.rule; -import com.google.common.collect.ImmutableList; import com.yahoo.searchlib.rankingexpression.Reference; import com.yahoo.searchlib.rankingexpression.evaluation.Context; import com.yahoo.searchlib.rankingexpression.evaluation.MapContext; @@ -27,7 +26,7 @@ import java.util.stream.Collectors; */ public class LambdaFunctionNode extends CompositeNode { - private final ImmutableList arguments; + private final List arguments; private final ExpressionNode functionExpression; public LambdaFunctionNode(List arguments, ExpressionNode functionExpression) { @@ -37,7 +36,7 @@ public class LambdaFunctionNode extends CompositeNode { .filter(f -> ! arguments.contains(f)) .collect(Collectors.joining(", "))); } - this.arguments = ImmutableList.copyOf(arguments); + this.arguments = List.copyOf(arguments); this.functionExpression = functionExpression; } @@ -106,15 +105,12 @@ public class LambdaFunctionNode extends CompositeNode { } private Optional getDirectEvaluator() { - if ( ! (functionExpression instanceof OperationNode)) { + if ( ! (functionExpression instanceof OperationNode node)) { return Optional.empty(); } - OperationNode node = (OperationNode) functionExpression; - if ( ! (node.children().get(0) instanceof ReferenceNode) || ! (node.children().get(1) instanceof ReferenceNode)) { + if ( ! (node.children().get(0) instanceof ReferenceNode lhs) || ! (node.children().get(1) instanceof ReferenceNode rhs)) { return Optional.empty(); } - var lhs = (ReferenceNode) node.children().get(0); - var rhs = (ReferenceNode) node.children().get(1); if (! lhs.getName().equals(arguments.get(0)) || ! rhs.getName().equals(arguments.get(1))) { return Optional.empty(); } @@ -122,17 +118,17 @@ public class LambdaFunctionNode extends CompositeNode { return Optional.empty(); } Operator operator = node.operators().get(0); - switch (operator) { - case or: return asFunctionExpression((left, right) -> ((left != 0.0) || (right != 0.0)) ? 1.0 : 0.0); - case and: return asFunctionExpression((left, right) -> ((left != 0.0) && (right != 0.0)) ? 1.0 : 0.0); - case plus: return asFunctionExpression((left, right) -> left + right); - case minus: return asFunctionExpression((left, right) -> left - right); - case multiply: return asFunctionExpression((left, right) -> left * right); - case divide: return asFunctionExpression((left, right) -> left / right); - case modulo: return asFunctionExpression((left, right) -> left % right); - case power: return asFunctionExpression(Math::pow); - } - return Optional.empty(); + return switch (operator) { + case or -> asFunctionExpression((left, right) -> ((left != 0.0) || (right != 0.0)) ? 1.0 : 0.0); + case and -> asFunctionExpression((left, right) -> ((left != 0.0) && (right != 0.0)) ? 1.0 : 0.0); + case plus -> asFunctionExpression((left, right) -> left + right); + case minus -> asFunctionExpression((left, right) -> left - right); + case multiply -> asFunctionExpression((left, right) -> left * right); + case divide -> asFunctionExpression((left, right) -> left / right); + case modulo -> asFunctionExpression((left, right) -> left % right); + case power -> asFunctionExpression(Math::pow); + default -> Optional.empty(); + }; } private Optional asFunctionExpression(DoubleBinaryOperator operator) { diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/SetMembershipNode.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/SetMembershipNode.java index 5da2fbfe624..6b87f75d884 100644 --- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/SetMembershipNode.java +++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/rule/SetMembershipNode.java @@ -1,7 +1,6 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.searchlib.rankingexpression.rule; -import com.google.common.collect.ImmutableList; import com.yahoo.searchlib.rankingexpression.Reference; import com.yahoo.searchlib.rankingexpression.evaluation.BooleanValue; import com.yahoo.searchlib.rankingexpression.evaluation.Context; @@ -30,7 +29,7 @@ public class SetMembershipNode extends BooleanNode { public SetMembershipNode(ExpressionNode testValue, List setValues) { this.testValue = testValue; - this.setValues = ImmutableList.copyOf(setValues); + this.setValues = List.copyOf(setValues); } /** The value to check for membership in the set */ diff --git a/searchlib/src/test/java/com/yahoo/searchlib/ranking/features/fieldmatch/test/FieldMatchMetricsTestCase.java b/searchlib/src/test/java/com/yahoo/searchlib/ranking/features/fieldmatch/test/FieldMatchMetricsTestCase.java index f4a003868f8..41a6cd69878 100644 --- a/searchlib/src/test/java/com/yahoo/searchlib/ranking/features/fieldmatch/test/FieldMatchMetricsTestCase.java +++ b/searchlib/src/test/java/com/yahoo/searchlib/ranking/features/fieldmatch/test/FieldMatchMetricsTestCase.java @@ -1,7 +1,6 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.searchlib.ranking.features.fieldmatch.test; -import com.google.common.collect.ImmutableList; import com.yahoo.searchlib.ranking.features.fieldmatch.Field; import com.yahoo.searchlib.ranking.features.fieldmatch.FieldMatchMetrics; import com.yahoo.searchlib.ranking.features.fieldmatch.FieldMatchMetricsComputer; @@ -10,6 +9,7 @@ import com.yahoo.searchlib.ranking.features.fieldmatch.QueryTerm; import com.yahoo.searchlib.ranking.features.fieldmatch.Query; import org.junit.Test; +import java.util.ArrayList; import java.util.List; import static org.junit.Assert.assertEquals; @@ -776,9 +776,9 @@ public class FieldMatchMetricsTestCase { } private Field toField(String fieldString) { - if (fieldString.length() == 0) return new Field(ImmutableList.of()); + if (fieldString.length() == 0) return new Field(List.of()); - ImmutableList.Builder terms = new ImmutableList.Builder<>(); + List terms = new ArrayList<>(); for (String termString : fieldString.split(" ")) { String[] colonSplit = termString.split(":"); if (colonSplit.length > 1) @@ -786,7 +786,7 @@ public class FieldMatchMetricsTestCase { else terms.add(new Field.Term(colonSplit[0])); } - return new Field(terms.build()); + return new Field(List.copyOf(terms)); } } diff --git a/service-monitor/pom.xml b/service-monitor/pom.xml index 6a46838a0ce..2378d63d457 100644 --- a/service-monitor/pom.xml +++ b/service-monitor/pom.xml @@ -90,12 +90,6 @@ ${project.version} provided - - com.google.inject - guice - provided - no_aop - com.fasterxml.jackson.core jackson-annotations diff --git a/vdslib/pom.xml b/vdslib/pom.xml index c534b2024ed..4067fc9d8d4 100644 --- a/vdslib/pom.xml +++ b/vdslib/pom.xml @@ -14,11 +14,6 @@ vdslib - - com.google.guava - guava - provided - junit junit diff --git a/zkfacade/pom.xml b/zkfacade/pom.xml index 9c5ed23636f..d9a9cb51c89 100644 --- a/zkfacade/pom.xml +++ b/zkfacade/pom.xml @@ -57,11 +57,6 @@ - - com.google.guava - guava - provided - org.apache.zookeeper zookeeper -- cgit v1.2.3 From 42859fd78955b9fbdfd550db27b9f06157690f61 Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Mon, 5 Dec 2022 14:21:03 +0100 Subject: Fewer retries in curator client retry policy --- zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'zkfacade') diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java index bb81135dae3..80159624eed 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java @@ -68,7 +68,7 @@ public class Curator extends AbstractComponent implements AutoCloseable { private static final Duration ZK_CONNECTION_TIMEOUT = Duration.ofSeconds(30); private static final Duration BASE_SLEEP_TIME = Duration.ofSeconds(1); - private static final int MAX_RETRIES = 10; + private static final int MAX_RETRIES = 4; private static final RetryPolicy DEFAULT_RETRY_POLICY = new ExponentialBackoffRetry((int) BASE_SLEEP_TIME.toMillis(), MAX_RETRIES); // Default value taken from ZookeeperServerConfig static final long defaultJuteMaxBuffer = Long.parseLong(System.getProperty("jute.maxbuffer", "52428800")); -- cgit v1.2.3 From 146cfc7d87fa26b5ad54103602a1c60908100e1f Mon Sep 17 00:00:00 2001 From: jonmv Date: Mon, 2 Jan 2023 13:34:21 +0100 Subject: Ensure lock is cleared when lease is invalidated --- .../java/com/yahoo/vespa/curator/SingletonManager.java | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) (limited to 'zkfacade') diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/SingletonManager.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/SingletonManager.java index 8eda57b0476..f4074f9df77 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/SingletonManager.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/SingletonManager.java @@ -336,14 +336,16 @@ class SingletonManager { shouldBeActive = false; } } - if (active && ! shouldBeActive) { + if ( ! shouldBeActive) { logger.log(FINE, () -> "Doom value is " + doom); - try { - if ( ! singletons.isEmpty()) metrics.deactivation(singletons.peek()::deactivate); - active = false; - } - catch (RuntimeException e) { - logger.log(WARNING, "Failed to deactivate " + singletons.peek(), e); + if (active) { + try { + if (!singletons.isEmpty()) metrics.deactivation(singletons.peek()::deactivate); + active = false; + } + catch (RuntimeException e) { + logger.log(WARNING, "Failed to deactivate " + singletons.peek(), e); + } } unlock(); } -- cgit v1.2.3 From 5d094f89ee2f93b423888376bde1c20a4869e4e6 Mon Sep 17 00:00:00 2001 From: jonmv Date: Tue, 3 Jan 2023 09:59:08 +0100 Subject: Compute prerequisite tests for a given prod deployment --- .../controller/deployment/DeploymentStatus.java | 27 +++++++++++++++++++--- .../controller/deployment/DeploymentTrigger.java | 11 ++++----- .../vespa/hosted/controller/deployment/Run.java | 1 + .../com/yahoo/vespa/curator/SingletonManager.java | 2 +- 4 files changed, 31 insertions(+), 10 deletions(-) (limited to 'zkfacade') diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java index ef4ee16217e..e90d6adc2f0 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java @@ -33,14 +33,15 @@ import com.yahoo.vespa.hosted.controller.versions.VespaVersion.Confidence; import java.time.Duration; import java.time.Instant; import java.time.temporal.ChronoUnit; +import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Collections; +import java.util.Deque; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import java.util.Map.Entry; import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -67,7 +68,6 @@ import static java.util.stream.Collectors.mapping; import static java.util.stream.Collectors.toList; import static java.util.stream.Collectors.toMap; import static java.util.stream.Collectors.toSet; -import static java.util.stream.Collectors.toUnmodifiableList; /** * Status of the deployment jobs of an {@link Application}. @@ -444,7 +444,7 @@ public class DeploymentStatus { * which does not downgrade any deployments in the instance, * which is not already rolling out to the instance, and * which causes at least one job to run if deployed to the instance. - * For the "exclusive" revision upgrade policy it is the oldest such revision; otherwise, it is the latest. + * For the "next" revision target policy it is the oldest such revision; otherwise, it is the latest. */ public Change outstandingChange(InstanceName instance) { StepStatus status = instanceSteps().get(instance); @@ -749,6 +749,27 @@ public class DeploymentStatus { return first; } + /** + * Returns set of declared tests directly reachable from the given production job, or the first declared (or implicit) test. + * A test in instance {@code I} is directly reachable from a job in instance {@code K} if a chain of instances {@code I, J, ..., K} + * exists, such that only {@code I} has a declared test of the particular type. + * These are the declared tests that should be OK before we proceed with the corresponding production deployment. + * If no such tests exist, the first declared test, or a test in the first declared instance, is used instead. + */ + private List prerequisiteTests(JobId prodJob, JobType testType) { + List tests = new ArrayList<>(); + Deque instances = new ArrayDeque<>(); + instances.add(prodJob.application().instance()); + while ( ! instances.isEmpty()) { + InstanceName instance = instances.poll(); + Optional test = declaredTest(application().id().instance(instance), testType); + if (test.isPresent()) tests.add(test.get()); + else instances.addAll(instanceSteps().get(instance).dependencies().stream().map(StepStatus::instance).toList()); + } + if (tests.isEmpty()) tests.add(firstDeclaredOrElseImplicitTest(testType)); + return tests; + } + /** Adds the primitive steps contained in the given step, which depend on the given previous primitives, to the dependency graph. */ private List fillStep(Map dependencies, List allSteps, DeploymentSpec.Step step, List previous, InstanceName instance, Function jobs, diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java index 169cde8437a..e905b60687e 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java @@ -452,12 +452,11 @@ public class DeploymentTrigger { Predicate revisionFilter = spec.revisionTarget() == DeploymentSpec.RevisionTarget.next ? failing -> status.application().require(instance).change().revision().get().compareTo(failing) == 0 : failing -> revision.compareTo(failing) > 0; - switch (spec.revisionChange()) { - case whenClear: return ! isChangingRevision; - case whenFailing: return ! isChangingRevision || status.hasFailures(revisionFilter); - case always: return true; - default: throw new IllegalStateException("Unknown revision upgrade policy"); - } + return switch (spec.revisionChange()) { + case whenClear -> ! isChangingRevision; + case whenFailing -> ! isChangingRevision || status.hasFailures(revisionFilter); + case always -> true; + }; } private Instance withRemainingChange(Instance instance, Change change, DeploymentStatus status, boolean allowOutdatedPlatform) { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Run.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Run.java index 7ffaaabb1a7..71ff28c47e6 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Run.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Run.java @@ -14,6 +14,7 @@ import java.util.stream.Collectors; import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.aborted; import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.noTests; +import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.reset; import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.running; import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.success; import static com.yahoo.vespa.hosted.controller.deployment.Step.Status.succeeded; diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/SingletonManager.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/SingletonManager.java index f4074f9df77..0b6d1611563 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/SingletonManager.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/SingletonManager.java @@ -340,7 +340,7 @@ class SingletonManager { logger.log(FINE, () -> "Doom value is " + doom); if (active) { try { - if (!singletons.isEmpty()) metrics.deactivation(singletons.peek()::deactivate); + if ( ! singletons.isEmpty()) metrics.deactivation(singletons.peek()::deactivate); active = false; } catch (RuntimeException e) { -- cgit v1.2.3 From 27ab9028091207ee4546f884afc6146fa75c7c36 Mon Sep 17 00:00:00 2001 From: jonmv Date: Tue, 14 Feb 2023 07:45:56 +0100 Subject: When lock was closed, unbeknownst to us, release it internally --- .../com/yahoo/vespa/curator/SingletonManager.java | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) (limited to 'zkfacade') diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/SingletonManager.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/SingletonManager.java index 0b6d1611563..9cdcb1dfb74 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/SingletonManager.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/SingletonManager.java @@ -139,6 +139,7 @@ class SingletonManager { final Path path; final MetricHelper metrics; Lock lock = null; + Lock toClose = null; boolean active; Janitor(String id) { @@ -153,13 +154,19 @@ class SingletonManager { public void unlock() { doom.set(null); - if (lock != null) try { + if (lock != null) toClose = lock; + lock = null; + if (toClose != null) try { logger.log(INFO, "Relinquishing lease for " + id); - lock.close(); - lock = null; + toClose.close(); + toClose = null; + } + catch (IllegalMonitorStateException e) { + toClose = null; + logger.log(WARNING, "Failed closing " + lock + ", already closed", e); } catch (Exception e) { - logger.log(WARNING, "Failed closing " + lock, e); + logger.log(WARNING, "Failed closing " + lock + ", will retry", e); } } @@ -272,6 +279,10 @@ class SingletonManager { * If lock is held, or acquired, ping the ZK cluster to extend our deadline. */ private void renewLease() { + if (toClose != null) { + logger.log(INFO, "Need to close the old lock before attempting a new one"); + return; + } if (doom.get() == INVALID) { logger.log(INFO, "Lease invalidated"); doom.set(null); -- cgit v1.2.3 From ebdfe8a67e349eb7f3472352e03d67dea2cb95f4 Mon Sep 17 00:00:00 2001 From: jonmv Date: Tue, 14 Feb 2023 09:15:44 +0100 Subject: Make state more explicit --- .../com/yahoo/vespa/curator/SingletonManager.java | 33 ++++++++++------------ 1 file changed, 15 insertions(+), 18 deletions(-) (limited to 'zkfacade') diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/SingletonManager.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/SingletonManager.java index 9cdcb1dfb74..67ca661b28f 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/SingletonManager.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/SingletonManager.java @@ -128,18 +128,19 @@ class SingletonManager { } - private static final Instant INVALID = Instant.ofEpochMilli(-1); + private static final Instant PENDING = Instant.ofEpochMilli(-1); + private static final Instant INVALID = Instant.ofEpochMilli(-2); + private static final Instant CLOSING = Instant.ofEpochMilli(-3); final BlockingDeque tasks = new LinkedBlockingDeque<>(); final Deque singletons = new ArrayDeque<>(2); - final AtomicReference doom = new AtomicReference<>(); + final AtomicReference doom = new AtomicReference<>(PENDING); final AtomicBoolean shutdown = new AtomicBoolean(); final Thread worker; final String id; final Path path; final MetricHelper metrics; Lock lock = null; - Lock toClose = null; boolean active; Janitor(String id) { @@ -153,21 +154,19 @@ class SingletonManager { } public void unlock() { - doom.set(null); - if (lock != null) toClose = lock; - lock = null; - if (toClose != null) try { + if (lock != null) try { logger.log(INFO, "Relinquishing lease for " + id); - toClose.close(); - toClose = null; + lock.close(); } catch (IllegalMonitorStateException e) { - toClose = null; logger.log(WARNING, "Failed closing " + lock + ", already closed", e); } catch (Exception e) { logger.log(WARNING, "Failed closing " + lock + ", will retry", e); + return; } + doom.set(PENDING); + lock = null; } private void run() { @@ -279,13 +278,11 @@ class SingletonManager { * If lock is held, or acquired, ping the ZK cluster to extend our deadline. */ private void renewLease() { - if (toClose != null) { - logger.log(INFO, "Need to close the old lock before attempting a new one"); - return; - } - if (doom.get() == INVALID) { + if (doom.compareAndSet(INVALID, CLOSING)) { logger.log(INFO, "Lease invalidated"); - doom.set(null); + } + if (doom.get() == CLOSING) { + logger.log(INFO, "Must close the lock before attempting to renew lease"); return; // Skip to updateStatus, deactivation, and release the lock. } // Witness value to detect if invalidation occurs between here and successful ping. @@ -313,7 +310,7 @@ class SingletonManager { return; } if ( ! doom.compareAndSet(ourDoom, start.plus(curator.sessionTimeout().multipliedBy(9).dividedBy(10)))) { - logger.log(FINE, "Deadline changed, current lease renewal is void"); + logger.log(INFO, "Deadline changed from " + ourDoom + " to " + doom.get() + "; current lease renewal is void"); } } @@ -336,7 +333,7 @@ class SingletonManager { */ private void updateStatus() { Instant ourDoom = doom.get(); - boolean shouldBeActive = ourDoom != null && ourDoom != INVALID && ! clock.instant().isAfter(ourDoom); + boolean shouldBeActive = ourDoom.isAfter(Instant.EPOCH) && ! clock.instant().isAfter(ourDoom); if ( ! active && shouldBeActive) { try { active = true; -- cgit v1.2.3 From a8a9d7fde0b9c94f42ab205835b433311157c2f0 Mon Sep 17 00:00:00 2001 From: jonmv Date: Tue, 14 Feb 2023 09:48:31 +0100 Subject: Simplify, since lock will only throw IMSE after first close attempt --- .../com/yahoo/vespa/curator/SingletonManager.java | 24 ++++++++-------------- 1 file changed, 9 insertions(+), 15 deletions(-) (limited to 'zkfacade') diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/SingletonManager.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/SingletonManager.java index 67ca661b28f..079641f0068 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/SingletonManager.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/SingletonManager.java @@ -128,13 +128,12 @@ class SingletonManager { } - private static final Instant PENDING = Instant.ofEpochMilli(-1); + private static final Instant EMPTY = Instant.ofEpochMilli(-1); private static final Instant INVALID = Instant.ofEpochMilli(-2); - private static final Instant CLOSING = Instant.ofEpochMilli(-3); final BlockingDeque tasks = new LinkedBlockingDeque<>(); final Deque singletons = new ArrayDeque<>(2); - final AtomicReference doom = new AtomicReference<>(PENDING); + final AtomicReference doom = new AtomicReference<>(EMPTY); final AtomicBoolean shutdown = new AtomicBoolean(); final Thread worker; final String id; @@ -162,10 +161,9 @@ class SingletonManager { logger.log(WARNING, "Failed closing " + lock + ", already closed", e); } catch (Exception e) { - logger.log(WARNING, "Failed closing " + lock + ", will retry", e); - return; + logger.log(WARNING, "Failed closing " + lock + ", will let it expire", e); } - doom.set(PENDING); + doom.set(EMPTY); lock = null; } @@ -278,15 +276,12 @@ class SingletonManager { * If lock is held, or acquired, ping the ZK cluster to extend our deadline. */ private void renewLease() { - if (doom.compareAndSet(INVALID, CLOSING)) { + // Witness value to detect if invalidation occurs between here and successful ping. + Instant ourDoom = doom.get(); + if (ourDoom == INVALID) { logger.log(INFO, "Lease invalidated"); - } - if (doom.get() == CLOSING) { - logger.log(INFO, "Must close the lock before attempting to renew lease"); return; // Skip to updateStatus, deactivation, and release the lock. } - // Witness value to detect if invalidation occurs between here and successful ping. - Instant ourDoom = doom.get(); Instant start = clock.instant(); if (lock == null) try { lock = curator.lock(path.append("lock"), tickTimeout); @@ -332,8 +327,7 @@ class SingletonManager { * If activation fails, we release the lock, so a different container may acquire it. */ private void updateStatus() { - Instant ourDoom = doom.get(); - boolean shouldBeActive = ourDoom.isAfter(Instant.EPOCH) && ! clock.instant().isAfter(ourDoom); + boolean shouldBeActive = doom.get().isAfter(clock.instant()); if ( ! active && shouldBeActive) { try { active = true; @@ -345,7 +339,7 @@ class SingletonManager { } } if ( ! shouldBeActive) { - logger.log(FINE, () -> "Doom value is " + doom); + logger.log(FINE, () -> "Doom value is " + doom.get()); if (active) { try { if ( ! singletons.isEmpty()) metrics.deactivation(singletons.peek()::deactivate); -- cgit v1.2.3 From b77f19bedcda698065b34a0c4eb19503ec93f8fa Mon Sep 17 00:00:00 2001 From: jonmv Date: Tue, 14 Feb 2023 09:50:28 +0100 Subject: Unify catch-and-log branches --- zkfacade/src/main/java/com/yahoo/vespa/curator/SingletonManager.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) (limited to 'zkfacade') diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/SingletonManager.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/SingletonManager.java index 079641f0068..e37d561ca1b 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/SingletonManager.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/SingletonManager.java @@ -157,11 +157,8 @@ class SingletonManager { logger.log(INFO, "Relinquishing lease for " + id); lock.close(); } - catch (IllegalMonitorStateException e) { - logger.log(WARNING, "Failed closing " + lock + ", already closed", e); - } catch (Exception e) { - logger.log(WARNING, "Failed closing " + lock + ", will let it expire", e); + logger.log(WARNING, "Failed closing " + lock, e); } doom.set(EMPTY); lock = null; -- cgit v1.2.3