Merge pull request #26621 from vespa-engine/balder/add-general-compound-name-cache

Balder/add general compound name cache

5 files changed, 76 insertions, 15 deletions

diff --git a/container-core/src/main/java/com/yahoo/processing/request/CompoundName.java b/container-core/src/main/java/com/yahoo/processing/request/CompoundName.java
index 66750b2943d..ac7b3d24d08 100644
--- a/container-core/src/main/java/com/yahoo/processing/request/CompoundName.java
+++ b/container-core/src/main/java/com/yahoo/processing/request/CompoundName.java
@@ -1,10 +1,13 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.processing.request;
 
+import com.yahoo.concurrent.CopyOnWriteHashMap;
+
 import java.util.AbstractList;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
+import java.util.Map;
 
 import static com.yahoo.text.Lowercase.toLowerCase;
 
@@ -36,7 +39,8 @@ public final class CompoundName {
 
     /** The empty compound */
     public static final CompoundName empty = new CompoundName("");
-
+    private static final Map<String, CompoundName> cache = new CopyOnWriteHashMap<>();
+    private static final int MAX_CACHE_SIZE = 10_000;
     /**
      * Constructs this from a string which may contains dot-separated components
      *
@@ -298,7 +302,20 @@ public final class CompoundName {
         return b.length()==0 ? "" : b.substring(0, b.length()-1);
     }
 
-    public static CompoundName from(String name) { return new CompoundName(name); }
+    /**
+     *  Creates a CompoundName from a string, possibly reusing from cache.
+     *  Prefer over constructing on the fly.
+     **/
+    public static CompoundName from(String name) {
+        CompoundName found = cache.get(name);
+        if (found != null) return found;
+
+        CompoundName compound = new CompoundName(name);
+        if (cache.size() < MAX_CACHE_SIZE) {
+            cache.put(name, compound);
+        }
+        return compound;
+    }
 
     private static class ImmutableArrayList extends AbstractList<String> {
 
diff --git a/container-search/src/main/java/com/yahoo/prelude/searcher/JSONDebugSearcher.java b/container-search/src/main/java/com/yahoo/prelude/searcher/JSONDebugSearcher.java
index 10b2cf0fdc7..409b502f086 100644
--- a/container-search/src/main/java/com/yahoo/prelude/searcher/JSONDebugSearcher.java
+++ b/container-search/src/main/java/com/yahoo/prelude/searcher/JSONDebugSearcher.java
@@ -25,7 +25,7 @@ public class JSONDebugSearcher extends Searcher {
     public static final String STRUCT_FIELD = "Structured data field (as json): ";
     public static final String FEATURE_FIELD = "Feature data field (as json): ";
 
-    private static CompoundName PROPERTYNAME = new CompoundName("dumpjson");
+    private static final CompoundName PROPERTYNAME = new CompoundName("dumpjson");
 
     @Override
     public Result search(Query query, Execution execution) {
diff --git a/container-search/src/main/java/com/yahoo/search/Query.java b/container-search/src/main/java/com/yahoo/search/Query.java
index cff43e07d70..39fd372d2a7 100644
--- a/container-search/src/main/java/com/yahoo/search/Query.java
+++ b/container-search/src/main/java/com/yahoo/search/Query.java
@@ -426,28 +426,36 @@ public class Query extends com.yahoo.processing.Request implements Cloneable {
      * dependent objects for the appropriate subset of the given property values
      */
     private void setFieldsFrom(Properties properties, Map<String, String> context) {
-        setFrom(CompoundName.empty, properties, Query.getArgumentType(), context);
+        setFrom("", properties, Query.getArgumentType(), context);
+    }
+
+    private static String append(String a, String b) {
+        if (a.isEmpty()) return b;
+        if (b.isEmpty()) return a;
+        return a + "." + b;
     }
 
     /**
      * For each field in the given query profile type, take the corresponding value from originalProperties
      * (if any) set it to properties(), recursively.
      */
-    private void setFrom(CompoundName prefix, Properties originalProperties, QueryProfileType arguments, Map<String, String> context) {
-        prefix = prefix.append(getPrefix(arguments));
+    private void setFrom(String prefix, Properties originalProperties, QueryProfileType arguments, Map<String, String> context) {
+        prefix = append(prefix, getPrefix(arguments).toString());
         for (FieldDescription field : arguments.fields().values()) {
 
             if (field.getType() == FieldType.genericQueryProfileType) { // Generic map
-                CompoundName fullName = prefix.append(field.getCompoundName());
-                for (Map.Entry<String, Object> entry : originalProperties.listProperties(fullName, context).entrySet()) {
-                    properties().set(fullName.append(entry.getKey()), entry.getValue(), context);
+                String fullName = append(prefix, field.getCompoundName().toString());
+                for (Map.Entry<String, Object> entry : originalProperties.listProperties(CompoundName.from(fullName), context).entrySet()) {
+                    properties().set(CompoundName.from(append(fullName, entry.getKey())), entry.getValue(), context);
                 }
             }
             else if (field.getType() instanceof QueryProfileFieldType) { // Nested arguments
                 setFrom(prefix, originalProperties, ((QueryProfileFieldType)field.getType()).getQueryProfileType(), context);
             }
             else {
-                CompoundName fullName = prefix.append(field.getCompoundName());
+                CompoundName fullName = prefix.isEmpty()
+                        ? field.getCompoundName()
+                        : CompoundName.from(append(prefix, field.getCompoundName().toString()));
                 Object value = originalProperties.get(fullName, context);
                 if (value != null) {
                     properties().set(fullName, value, context);
@@ -458,14 +466,15 @@ public class Query extends com.yahoo.processing.Request implements Cloneable {
 
     /** Calls properties.set on all entries in requestMap */
     private void setPropertiesFromRequestMap(Map<String, String> requestMap, Properties properties, boolean ignoreSelect) {
-        for (var entry : requestMap.entrySet()) {
+        var entrySet = requestMap.entrySet();
+        for (var entry : entrySet) {
             if (ignoreSelect && entry.getKey().equals(Select.SELECT)) continue;
             if (RankFeatures.isFeatureName(entry.getKey())) continue; // Set these last
-            properties.set(entry.getKey(), entry.getValue(), requestMap);
+            properties.set(CompoundName.from(entry.getKey()), entry.getValue(), requestMap);
         }
-        for (var entry : requestMap.entrySet()) {
+        for (var entry : entrySet) {
             if ( ! RankFeatures.isFeatureName(entry.getKey())) continue;
-            properties.set(entry.getKey(), entry.getValue(), requestMap);
+            properties.set(CompoundName.from(entry.getKey()), entry.getValue(), requestMap);
         }
     }
 
diff --git a/container-search/src/test/java/com/yahoo/search/query/profile/test/QueryProfileTestCase.java b/container-search/src/test/java/com/yahoo/search/query/profile/test/QueryProfileTestCase.java
index c4541fe9f58..cd4cb32df2e 100644
--- a/container-search/src/test/java/com/yahoo/search/query/profile/test/QueryProfileTestCase.java
+++ b/container-search/src/test/java/com/yahoo/search/query/profile/test/QueryProfileTestCase.java
@@ -486,7 +486,8 @@ public class QueryProfileTestCase {
         assertEquals("a.b-value", cp.get("a.b", QueryProfileVariantsTestCase.toMap(p, new String[]{"x1"})));
     }
 
-    public void testSettingNonLeaf4b() {
+    @Test
+    void testSettingNonLeaf4b() {
         QueryProfile p = new QueryProfile("test");
         p.setDimensions(new String[] {"x"});
         p.set("a","a-value", (QueryProfileRegistry)null);
@@ -648,6 +649,32 @@ public class QueryProfileTestCase {
         assertTrue(traceContains("foo: value", query));
     }
 
+    @Test
+    void benchQueryCreation() throws InterruptedException {
+        QueryProfile p = new QueryProfile("test");
+        p.setDimensions(new String[]{"x", "y"});
+        p.set("clustering.something", "bar", null);
+        p.set("clustering.something", "bar", new String[]{"x1", "y1"}, null);
+        p.freeze();
+        CompiledQueryProfile cqp = p.compile(null);
+        var httpRequest = HttpRequest.createTestRequest("?x=x1&y=y1&query=bar&clustering.timeline.kano=tur&" +
+                "clustering.enable=true&clustering.timeline.bucketspec=-" +
+                "7d/3h&clustering.timeline.tophit=false&clustering.timeli" +
+                "ne=true", Method.GET);
+        for (int i = 0; i < 30000; i++) {
+            Query q = new Query(httpRequest, cqp);
+            assertTrue(q.properties().getBoolean(CompoundName.from("clustering.timeline"), false));
+        }
+        Thread.sleep(2000);
+        long start = System.nanoTime();
+        for (int i = 0; i < 100000; i++) {
+            Query q = new Query(httpRequest, cqp);
+            assertTrue(q.properties().getBoolean(CompoundName.from("clustering.timeline"), false));
+        }
+        long now = System.nanoTime();
+        System.out.println("Duration = " + (now - start)/1_000_000 + " ms");
+    }
+
     // NB: NOT RECURSIVE
     private boolean traceContains(String string, Query query) {
         for (TraceNode node : query.getContext(true).getTrace().traceNode().children())
diff --git a/container-search/src/test/java/com/yahoo/search/test/QueryTestCase.java b/container-search/src/test/java/com/yahoo/search/test/QueryTestCase.java
index 559b814a265..748a20801d1 100644
--- a/container-search/src/test/java/com/yahoo/search/test/QueryTestCase.java
+++ b/container-search/src/test/java/com/yahoo/search/test/QueryTestCase.java
@@ -327,6 +327,14 @@ public class QueryTestCase {
     }
 
     @Test
+    void testBooleanParameterNoQueryProfile() {
+        QueryProfile profile = new QueryProfile("myProfile");
+        Query query = new Query("/?query=something&ranking.softtimeout.enable=false");
+        assertFalse(query.properties().getBoolean("ranking.softtimeout.enable"));
+        assertFalse(query.getRanking().getSoftTimeout().getEnable());
+    }
+
+    @Test
     void testQueryProfileSubstitution2() {
         QueryProfile profile = new QueryProfile("myProfile");
         profile.set("model.language", "en-US", null);