Check generality correctly on compaction

6 files changed, 165 insertions, 10 deletions

diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/compiled/Binding.java b/container-search/src/main/java/com/yahoo/search/query/profile/compiled/Binding.java
index 3812cdaac3c..8638a99172f 100644
--- a/container-search/src/main/java/com/yahoo/search/query/profile/compiled/Binding.java
+++ b/container-search/src/main/java/com/yahoo/search/query/profile/compiled/Binding.java
@@ -90,12 +90,12 @@ public class Binding implements Comparable<Binding> {
      * Returns whether this binding is a proper generalization of the given binding:
      * Meaning it contains a proper subset of the given bindings.
      */
-    public boolean generalizes(Binding binding) {
-        if ( dimensions.length >= binding.dimensions.length) return false;
-        for (int i = 0; i < dimensions.length; i++) {
-            int indexOfDimension = indexOf(dimensions[i], binding.dimensions);
-            if (indexOfDimension < 0) return false;
-            if ( ! binding.dimensionValues[i].equals(binding.dimensionValues[indexOfDimension])) return false;
+    public boolean generalizes(Binding other) {
+        if ( this.dimensions.length >= other.dimensions.length) return false;
+        for (int i = 0; i < this.dimensions.length; i++) {
+            int otherIndexOfDimension = this.indexOf(dimensions[i], other.dimensions);
+            if (otherIndexOfDimension < 0) return false;
+            if ( ! this.dimensionValues[i].equals(other.dimensionValues[otherIndexOfDimension])) return false;
         }
         return true;
     }
diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/compiled/DimensionalValue.java b/container-search/src/main/java/com/yahoo/search/query/profile/compiled/DimensionalValue.java
index a44ce2034fc..fb62cfca7d3 100644
--- a/container-search/src/main/java/com/yahoo/search/query/profile/compiled/DimensionalValue.java
+++ b/container-search/src/main/java/com/yahoo/search/query/profile/compiled/DimensionalValue.java
@@ -179,12 +179,13 @@ public class DimensionalValue<VALUE> {
             void compact() {
                 Collections.sort(variants);
                 List<Binding> compacted = new ArrayList<>();
+
                 if (variants.get(variants.size() - 1).dimensions().length == 0) { // Shortcut
                     variants = List.of(variants.get(variants.size() - 1));
                 }
                 else {
                     for (int i = variants.size() - 1; i >= 0; i--) {
-                        if (!containsGeneralizationOf(variants.get(i), compacted))
+                        if ( ! containsGeneralizationOf(variants.get(i), compacted))
                             compacted.add(variants.get(i));
                     }
                     Collections.reverse(compacted);
diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/config/QueryProfileXMLReader.java b/container-search/src/main/java/com/yahoo/search/query/profile/config/QueryProfileXMLReader.java
index 1b1cdce5890..db6d0560c5f 100644
--- a/container-search/src/main/java/com/yahoo/search/query/profile/config/QueryProfileXMLReader.java
+++ b/container-search/src/main/java/com/yahoo/search/query/profile/config/QueryProfileXMLReader.java
@@ -45,17 +45,17 @@ public class QueryProfileXMLReader {
 
             for (File file : sortFiles(dir)) {
                 if ( ! file.getName().endsWith(".xml")) continue;
-                queryProfileReaders.add(new NamedReader(file.getName(),new FileReader(file)));
+                queryProfileReaders.add(new NamedReader(file.getName(), new FileReader(file)));
             }
             File typeDir=new File(dir,"types");
             if (typeDir.isDirectory()) {
                 for (File file : sortFiles(typeDir)) {
                     if ( ! file.getName().endsWith(".xml")) continue;
-                    queryProfileTypeReaders.add(new NamedReader(file.getName(),new FileReader(file)));
+                    queryProfileTypeReaders.add(new NamedReader(file.getName(), new FileReader(file)));
                 }
             }
 
-            return read(queryProfileTypeReaders,queryProfileReaders);
+            return read(queryProfileTypeReaders, queryProfileReaders);
         }
         catch (IOException e) {
             throw new IllegalArgumentException("Could not read query profiles from '" + directory + "'", e);
diff --git a/container-search/src/test/java/com/yahoo/search/query/profile/compiled/BindingTestCase.java b/container-search/src/test/java/com/yahoo/search/query/profile/compiled/BindingTestCase.java
new file mode 100644
index 00000000000..621950ebc65
--- /dev/null
+++ b/container-search/src/test/java/com/yahoo/search/query/profile/compiled/BindingTestCase.java
@@ -0,0 +1,56 @@
+// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.search.query.profile.compiled;
+
+import com.yahoo.search.query.profile.DimensionBinding;
+import org.junit.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * @author bratseth
+ */
+public class BindingTestCase {
+
+    @Test
+    public void testGeneralizes() {
+        Map<String, String> m1 = new HashMap<>();
+        m1.put("a", "a1");
+        m1.put("b", "b1");
+        m1.put("c", "c1");
+        m1.put("e", "e1");
+
+        Map<String, String> m2 = new HashMap<>();
+        m2.put("a", "a2");
+        m2.put("b", "b2");
+        m2.put("c", "c2");
+        m2.put("d", "d2");
+        m2.put("e", "e2");
+
+        Map<String, String> m3 = new HashMap<>();
+        m3.put("a", "a1");
+        m3.put("b", "b1");
+        m3.put("c", "c1");
+        m3.put("d", "d1");
+        m3.put("e", "e1");
+
+        Map<String, String> m4 = new HashMap<>();
+        m4.put("a", "a1");
+        m4.put("b", "b1");
+        m4.put("c", "c1");
+        m4.put("d", "d2");
+        m4.put("e", "e1");
+
+        Binding b1 = Binding.createFrom(DimensionBinding.createFrom(m1));
+        Binding b2 = Binding.createFrom(DimensionBinding.createFrom(m2));
+        Binding b3 = Binding.createFrom(DimensionBinding.createFrom(m3));
+        Binding b4 = Binding.createFrom(DimensionBinding.createFrom(m4));
+        assertFalse(b1.generalizes(b2));
+        assertTrue(b1.generalizes(b3));
+        assertTrue(b1.generalizes(b4));
+    }
+
+}
diff --git a/container-search/src/test/java/com/yahoo/search/query/profile/config/test/XmlReadingTestCase.java b/container-search/src/test/java/com/yahoo/search/query/profile/config/test/XmlReadingTestCase.java
index 445073ced3a..fe37aee013d 100644
--- a/container-search/src/test/java/com/yahoo/search/query/profile/config/test/XmlReadingTestCase.java
+++ b/container-search/src/test/java/com/yahoo/search/query/profile/config/test/XmlReadingTestCase.java
@@ -4,6 +4,7 @@ package com.yahoo.search.query.profile.config.test;
 import com.yahoo.jdisc.http.HttpRequest.Method;
 import com.yahoo.container.jdisc.HttpRequest;
 import com.yahoo.processing.request.CompoundName;
+import com.yahoo.search.query.profile.compiled.DimensionalValue;
 import com.yahoo.yolean.Exceptions;
 import com.yahoo.search.Query;
 import com.yahoo.search.query.Properties;
@@ -14,6 +15,7 @@ import com.yahoo.search.query.profile.compiled.CompiledQueryProfileRegistry;
 import com.yahoo.search.query.profile.config.QueryProfileXMLReader;
 import com.yahoo.search.query.profile.types.FieldDescription;
 import com.yahoo.search.query.profile.types.QueryProfileType;
+import org.junit.Ignore;
 import org.junit.Test;
 
 import java.util.HashMap;
@@ -31,6 +33,90 @@ import static org.junit.Assert.fail;
 public class XmlReadingTestCase {
 
     @Test
+    @Ignore
+    public void testTmp2() {
+        // queryProfile=main_single_feed&site=frontpage&lang=zh-Hant-HK&region=HK&device=smartphone&configId=popular&debug=true&format=page&tracelevel=4
+        QueryProfileRegistry registry =
+                new QueryProfileXMLReader().read("/Users/bratseth/development/slingstone/massmedia_serving/homerun/src/main/application/search/flattened");
+        long startTime = System.currentTimeMillis();
+        System.out.println("Compiling ...");
+        CompiledQueryProfileRegistry cRegistry = registry.compile();
+
+        System.out.println("Done in " + (( System.currentTimeMillis() - startTime) / 1000) + " seconds");
+
+        Query q1 = new Query("?queryProfile=main_single_feed&site=frontpage&langReg=zh-Hant-HK&region=HK&device=smartphone&configId=popular&debug=true&format=page&tracelevel=4",
+                             cRegistry.findQueryProfile("main_single_feed"));
+
+        System.out.println("feed.main.rule.slotting.videos.type = " + q1.properties().get("feed.main.rule.slotting.videos.type"));
+    }
+
+    @Test
+    @Ignore
+    public void testTmp() {
+        QueryProfileRegistry registry =
+                new QueryProfileXMLReader().read("/Users/bratseth/development/slingstone/massmedia_serving/homerun/src/main/application/search/flattened");
+        long startTime = System.currentTimeMillis();
+        System.out.println("Compiling ...");
+        CompiledQueryProfileRegistry cRegistry = registry.compile();
+
+        System.out.println("Done in " + (( System.currentTimeMillis() - startTime) / 1000) + " seconds");
+        System.out.println("");
+        CompiledQueryProfile cProfile = cRegistry.getComponent("megastream_unified");
+
+        Query q = new Query("?debug=true&clientId=default&langReg=en-US&format=xml&tracelevel=4&" +
+                            "trace.timestamps=true&site=frontpage&streamBucketId=ga&configId=default&" +
+                            "queryProfile=megastream_unified&lang=ENGLISH&region=US&", cProfile);
+        q.properties().listProperties();
+
+        // About 60 ms with tracelevel=4, a few ms with tracelevel=0
+        startTime = System.currentTimeMillis();
+        System.out.println("Creating query ...");
+        new Query("?debug=true&clientId=default&langReg=en-US&format=xml&tracelevel=0&" +
+                  "trace.timestamps=true&site=frontpage&streamBucketId=ga&configId=default&" +
+                  "queryProfile=megastream_unified&lang=ENGLISH&region=US&", cProfile);
+        System.out.println("Done in " + (System.currentTimeMillis() - startTime) + " ms");
+
+        startTime = System.currentTimeMillis();
+        System.out.println("listProperties() ...");
+        int repeats = 10000;
+        for (int i = 0; i < repeats; i++)
+            q.properties().listProperties();
+
+        System.out.println("Time of " + repeats + " listProperties: " + (System.currentTimeMillis() - startTime) + " ms");
+    }
+
+    @Test
+    @Ignore
+    public void testTmp3() {
+        QueryProfileRegistry registry =
+                new QueryProfileXMLReader().read("/Users/bratseth/development/slingstone/massmedia_serving/homerun/src/main/application/search/flattened");
+        long startTime = System.currentTimeMillis();
+        System.out.println("Compiling ...");
+        CompiledQueryProfileRegistry cRegistry = registry.compile();
+        System.out.println("Done in " + (( System.currentTimeMillis() - startTime) / 1000) + " seconds");
+        System.out.println("");
+        CompiledQueryProfile cProfile = cRegistry.getComponent("megastream_unified");
+
+        Query q = new Query("?debug=true&clientId=default&langReg=en-US&format=xml&tracelevel=4&" +
+                            "trace.timestamps=true&site=frontpage&streamBucketId=ga&configId=default&" +
+                            "queryProfile=megastream_unified&lang=ENGLISH&region=US&", cProfile);
+        q.properties().listProperties();
+
+        // About 60 ms with tracelevel=4, a few ms with tracelevel=0
+        startTime = System.currentTimeMillis();
+        System.out.println("Creating query ...");
+        new Query("?debug=true&clientId=default&langReg=en-US&format=xml&tracelevel=0&" +
+                  "trace.timestamps=true&site=frontpage&streamBucketId=ga&configId=default&" +
+                  "queryProfile=megastream_unified&lang=ENGLISH&region=US&", cProfile);
+        System.out.println("Done in " + (System.currentTimeMillis() - startTime) + " ms");
+
+        startTime = System.currentTimeMillis();
+        System.out.println("listProperties() ...");
+        q.properties().listProperties().forEach((k, v) -> System.out.println("  " + k + "=" + v));
+        System.out.println("Done in " + (System.currentTimeMillis() - startTime) + " ms");
+    }
+
+    @Test
     public void testInheritance() {
         QueryProfileRegistry registry =
                 new QueryProfileXMLReader().read("src/test/java/com/yahoo/search/query/profile/config/test/inheritance");
diff --git a/container-search/src/test/java/com/yahoo/search/query/profile/test/QueryProfileVariantsTestCase.java b/container-search/src/test/java/com/yahoo/search/query/profile/test/QueryProfileVariantsTestCase.java
index 7d3bec7cc9e..0bcae97598b 100644
--- a/container-search/src/test/java/com/yahoo/search/query/profile/test/QueryProfileVariantsTestCase.java
+++ b/container-search/src/test/java/com/yahoo/search/query/profile/test/QueryProfileVariantsTestCase.java
@@ -1249,6 +1249,18 @@ public class QueryProfileVariantsTestCase {
         assertEquals("yahoo/alerts", cAlert.get("vertical.custid", toMap("entry=alert", "intl=us", "lang=en-US")));
     }
 
+    @Test
+    public void testCompactedVariants() {
+        QueryProfile test = new QueryProfile("main_single_feed");
+        test.setDimensions(new String[] {"x"});
+        test.set("InX1Only", "x1", new String[] { "x1" }, null);
+
+        CompiledQueryProfile ctest = test.compile(null);
+        assertEquals("x1", ctest.get("InX1Only", toMap("x=x1")));
+        assertEquals(null, ctest.get("InX1Only", toMap("x=x2")));
+        assertEquals(null, ctest.get("InX1Only"));
+    }
+
     private void assertGet(String expectedValue, String parameter, String[] dimensionValues, QueryProfile profile, CompiledQueryProfile cprofile) {
         Map<String,String> context=toMap(profile,dimensionValues);
         assertEquals("Looking up '" + parameter + "' for '" + Arrays.toString(dimensionValues) + "'",expectedValue,cprofile.get(parameter,context));