Merge pull request #12902 from vespa-engine/bratseth/faster-qp-compiling

Bratseth/faster qp compiling

9 files changed, 61 insertions, 105 deletions

diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/DimensionBinding.java b/container-search/src/main/java/com/yahoo/search/query/profile/DimensionBinding.java
index 50bd2c58da8..0cbfdc5dca0 100644
--- a/container-search/src/main/java/com/yahoo/search/query/profile/DimensionBinding.java
+++ b/container-search/src/main/java/com/yahoo/search/query/profile/DimensionBinding.java
@@ -3,7 +3,6 @@ package com.yahoo.search.query.profile;
 
 import java.util.ArrayList;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
@@ -22,7 +21,7 @@ public class DimensionBinding {
     private DimensionValues values;
 
     /** The binding from those dimensions to values, and possibly other values */
-    private Map<String, String> context; // TODO: This is not needed any more
+    private Map<String, String> context;
 
     public static final DimensionBinding nullBinding =
         new DimensionBinding(Collections.unmodifiableList(Collections.emptyList()), DimensionValues.empty, null);
@@ -34,13 +33,13 @@ public class DimensionBinding {
     private boolean containsAllNulls;
 
     // NOTE: Map must be ordered
-    public static DimensionBinding createFrom(Map<String,String> values) {
+    public static DimensionBinding createFrom(Map<String, String> values) {
         return createFrom(new ArrayList<>(values.keySet()), values);
     }
 
     /** Creates a binding from a variant and a context. Any of the arguments may be null. */
     // NOTE: Map must be ordered
-    public static DimensionBinding createFrom(List<String> dimensions, Map<String,String> context) {
+    public static DimensionBinding createFrom(List<String> dimensions, Map<String, String> context) {
         if (dimensions == null || dimensions.size() == 0) {
             if (context == null) return nullBinding;
             if (dimensions == null) return new DimensionBinding(null, DimensionValues.empty, context); // Null, but must preserve context
@@ -51,7 +50,7 @@ public class DimensionBinding {
 
     /** Creates a binding from a variant and a context. Any of the arguments may be null. */
     public static DimensionBinding createFrom(List<String> dimensions, DimensionValues dimensionValues) {
-        if (dimensionValues==null || dimensionValues == DimensionValues.empty) return nullBinding;
+        if (dimensionValues == null || dimensionValues == DimensionValues.empty) return nullBinding;
 
         // If null, preserve raw material for creating a context later (in createFor)
         if (dimensions == null) return new DimensionBinding(null, dimensionValues, null);
@@ -61,14 +60,13 @@ public class DimensionBinding {
 
     /** Returns a binding for a (possibly) new set of variants. Variants may be null, but not bindings */
     public DimensionBinding createFor(List<String> newDimensions) {
-        if (newDimensions==null) return this; // Note: Not necessarily null - if no new variants then keep the existing binding
-        // if (this.context==null && values.length==0) return nullBinding; // No data from which to create a non-null binding
-        if (this.dimensions==newDimensions) return this; // Avoid creating a new object if the dimensions are the same
-
-        Map<String,String> context=this.context;
-        if (context==null)
-            context=this.values.asContext(this.dimensions !=null ? this.dimensions : newDimensions);
-        return new DimensionBinding(newDimensions,extractDimensionValues(newDimensions,context),context);
+        if (newDimensions == null) return this; // Note: Not necessarily null - if no new variants then keep the existing binding
+        if (this.dimensions == newDimensions) return this; // Avoid creating a new object if the dimensions are the same
+
+        Map<String,String> context = this.context;
+        if (context == null)
+            context = this.values.asContext(this.dimensions != null ? this.dimensions : newDimensions);
+        return new DimensionBinding(newDimensions, extractDimensionValues(newDimensions, context), context);
     }
 
     /**
@@ -76,20 +74,20 @@ public class DimensionBinding {
      * The array will not be modified. The context is needed in order to convert this binding to another
      * given another set of variant dimensions.
      */
-    private DimensionBinding(List<String> dimensions, DimensionValues values, Map<String,String> context) {
-        this.dimensions=dimensions;
-        this.values=values;
+    private DimensionBinding(List<String> dimensions, DimensionValues values, Map<String, String> context) {
+        this.dimensions = dimensions;
+        this.values = values;
         this.context = context;
-        containsAllNulls=values.isEmpty();
+        containsAllNulls = values.isEmpty();
     }
 
     /** Returns a read-only list of the dimensions of this. This value is undefined if this isNull() */
     public List<String> getDimensions() { return dimensions; }
 
     /** Returns a context created from the dimensions and values of this */
-    public Map<String,String> getContext() {
-        if (context !=null) return context;
-        context =values.asContext(dimensions);
+    public Map<String, String> getContext() {
+        if (context != null) return context;
+        context = values.asContext(dimensions);
         return context;
     }
 
@@ -102,7 +100,7 @@ public class DimensionBinding {
     public DimensionValues getValues() { return values; }
 
     /** Returns true only if this binding is null (contains no values for its dimensions (if any) */
-    public boolean isNull() { return dimensions==null || containsAllNulls; }
+    public boolean isNull() { return dimensions == null || containsAllNulls; }
 
     /**
      * Returns an array of the dimension values corresponding to the dimensions of this from the given context,
@@ -110,10 +108,10 @@ public class DimensionBinding {
      * Dimensions which are not set in this context get a null value.
      */
     private static DimensionValues extractDimensionValues(List<String> dimensions, Map<String,String> context) {
-        String[] dimensionValues=new String[dimensions.size()];
-        if (context==null || context.size()==0) return DimensionValues.createFrom(dimensionValues);
-        for (int i=0; i<dimensions.size(); i++)
-            dimensionValues[i]=context.get(dimensions.get(i));
+        String[] dimensionValues = new String[dimensions.size()];
+        if (context == null || context.size() == 0) return DimensionValues.createFrom(dimensionValues);
+        for (int i = 0; i < dimensions.size(); i++)
+            dimensionValues[i] = context.get(dimensions.get(i));
         return DimensionValues.createFrom(dimensionValues);
     }
 
@@ -138,16 +136,6 @@ public class DimensionBinding {
         return DimensionBinding.createFrom(combinedDimensions, combinedValues);
     }
 
-    /** Returns the binding of this (dimension->value) as a map */
-    private Map<String, String> asMap() {
-        Map<String, String> map = new LinkedHashMap<>();
-        for (int i = 0; i < Math.min(dimensions.size(), values.size()); i++) {
-            if (values.getValues()[i] != null)
-                map.put(dimensions.get(i), values.getValues()[i]);
-        }
-        return map;
-    }
-
     /**
      * Returns a combined list of dimensions from two separate lists,
      * or null if they are incompatible.
@@ -155,8 +143,12 @@ public class DimensionBinding {
      * (or return null if impossible).
      */
     private List<String> combineDimensions(List<String> d1, List<String> d2) {
+        if (d1.equals(d2)) return d1;
+        if (d1.isEmpty()) return d2;
+        if (d2.isEmpty()) return d1;
+
         List<String> combined = new ArrayList<>();
-        int d1Index = 0, d2Index=0;
+        int d1Index = 0, d2Index = 0;
         while (d1Index < d1.size() && d2Index < d2.size()) {
             if (d1.get(d1Index).equals(d2.get(d2Index))) { // agreement on next element
                 combined.add(d1.get(d1Index));
@@ -186,27 +178,22 @@ public class DimensionBinding {
      * or null if they are incompatible.
      */
     private Map<String, String> combineValues(Map<String, String> m1, Map<String, String> m2) {
-        Map<String, String> combinedValues = new LinkedHashMap<>(m1);
+        if (m1.isEmpty()) return m2;
+        if (m2.isEmpty()) return m1;
+        Map<String, String> combinedValues = null;
         for (Map.Entry<String, String> m2Entry : m2.entrySet()) {
             if (m2Entry.getValue() == null) continue;
             String m1Value = m1.get(m2Entry.getKey());
             if (m1Value != null && ! m1Value.equals(m2Entry.getValue()))
                 return null; // conflicting values of a key
+            if (combinedValues == null)
+                combinedValues = new LinkedHashMap<>(m1);
             combinedValues.put(m2Entry.getKey(), m2Entry.getValue());
         }
-        return combinedValues;
+        return combinedValues == null ? m1 : combinedValues;
     }
 
-    private boolean intersects(List<String> l1, List<String> l2) {
-        for (String l1Item : l1)
-            if (l2.contains(l1Item))
-                return true;
-        return false;
-    }
-
-    /**
-     * Returns true if <code>this == invalidBinding</code>
-     */
+    /** Returns true if this == invalidBinding */
     public boolean isInvalid() { return this == invalidBinding; }
 
     @Override
@@ -226,7 +213,7 @@ public class DimensionBinding {
     /** Two bindings are equal if they contain the same dimensions and the same non-null values */
     @Override
     public boolean equals(Object o) {
-        if (o==this) return true;
+        if (o == this) return true;
         if (! (o instanceof DimensionBinding)) return false;
         DimensionBinding other = (DimensionBinding)o;
         if ( ! this.dimensions.equals(other.dimensions)) return false;
diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfile.java b/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfile.java
index 7ae18f96d86..ab0f129f1e9 100644
--- a/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfile.java
+++ b/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfile.java
@@ -564,7 +564,7 @@ public class QueryProfile extends FreezableSimpleComponent implements Cloneable
         if (visitor.isDone()) return;
 
         if (allowContent) {
-            visitContent(visitor,dimensionBinding);
+            visitContent(visitor, dimensionBinding);
             if (visitor.isDone()) return;
         }
 
@@ -601,7 +601,7 @@ public class QueryProfile extends FreezableSimpleComponent implements Cloneable
             visitor.acceptValue(contentKey, getContent(contentKey), dimensionBinding, this, null);
         }
         else { // get all content in this
-            for (Map.Entry<String,Object> entry : getContent().entrySet()) {
+            for (Map.Entry<String, Object> entry : getContent().entrySet()) {
                 visitor.acceptValue(entry.getKey(), entry.getValue(), dimensionBinding, this, null);
                 if (visitor.isDone()) return;
             }
@@ -614,7 +614,7 @@ public class QueryProfile extends FreezableSimpleComponent implements Cloneable
     }
 
     /** Returns all the content from this as an unmodifiable map */
-    protected Map<String,Object> getContent() {
+    protected Map<String, Object> getContent() {
         return content.unmodifiableMap();
     }
 
diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfileCompiler.java b/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfileCompiler.java
index cffe941b912..f1fc90dee09 100644
--- a/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfileCompiler.java
+++ b/container-search/src/main/java/com/yahoo/search/query/profile/QueryProfileCompiler.java
@@ -71,6 +71,7 @@ public class QueryProfileCompiler {
      */
     private static Set<DimensionBindingForPath> collectVariants(CompoundName path, QueryProfile profile, DimensionBinding currentVariant) {
         Set<DimensionBindingForPath> variants = new HashSet<>();
+
         variants.addAll(collectVariantsFromValues(path, profile.getContent(), currentVariant));
         variants.addAll(collectVariantsInThis(path, profile, currentVariant));
         if (profile instanceof BackedOverridableQueryProfile)
@@ -157,6 +158,7 @@ public class QueryProfileCompiler {
 
                 if (combinedVariant.isInvalid()) continue; // values at this point in the graph are unreachable
 
+                variants.add(new DimensionBindingForPath(combinedVariant, path));
                 variants.addAll(collectVariantsFromValues(path, variant.values(), combinedVariant));
                 for (QueryProfile variantInheritedProfile : variant.inherited())
                     variants.addAll(collectVariants(path, variantInheritedProfile, combinedVariant));
@@ -169,9 +171,6 @@ public class QueryProfileCompiler {
                                                                            Map<String, Object> values,
                                                                            DimensionBinding currentVariant) {
         Set<DimensionBindingForPath> variants = new HashSet<>();
-        if ( ! values.isEmpty())
-            variants.add(new DimensionBindingForPath(currentVariant, path)); // there are actual values for this variant
-
         for (Map.Entry<String, Object> entry : values.entrySet()) {
             if (entry.getValue() instanceof QueryProfile)
                 variants.addAll(collectVariants(path.append(entry.getKey()), (QueryProfile)entry.getValue(), currentVariant));
@@ -202,7 +201,7 @@ public class QueryProfileCompiler {
 
         @Override
         public int hashCode() {
-           return binding.hashCode() + 17*path.hashCode();
+           return binding.hashCode() + 17 * path.hashCode();
         }
 
         @Override
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 2774bd4ebf2..88014eef46d 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
@@ -106,7 +106,7 @@ public class Binding implements Comparable<Binding> {
      * Returns true if all the dimension values in this have the same values
      * in the given context.
      */
-    public boolean matches(Map<String,String> context) {
+    public boolean matches(Map<String, String> context) {
         for (int i = 0; i < dimensions.length; i++) {
             if ( ! dimensionValues[i].equals(context.get(dimensions[i]))) return false;
         }
diff --git a/container-search/src/main/java/com/yahoo/search/query/profile/compiled/CompiledQueryProfile.java b/container-search/src/main/java/com/yahoo/search/query/profile/compiled/CompiledQueryProfile.java
index 94f4e4747a0..1c7a7cf3e97 100644
--- a/container-search/src/main/java/com/yahoo/search/query/profile/compiled/CompiledQueryProfile.java
+++ b/container-search/src/main/java/com/yahoo/search/query/profile/compiled/CompiledQueryProfile.java
@@ -102,7 +102,7 @@ public class CompiledQueryProfile extends AbstractComponent implements Cloneable
      * For example, if {a.d =&gt; "a.d-value" ,a.e =&gt; "a.e-value", b.d =&gt; "b.d-value", then calling listValues("a")
      * will return {"d" =&gt; "a.d-value","e" =&gt; "a.e-value"}
      */
-    public final Map<String, Object> listValues(CompoundName prefix) {  return listValues(prefix, Collections.<String,String>emptyMap()); }
+    public final Map<String, Object> listValues(CompoundName prefix) {  return listValues(prefix, Collections.emptyMap()); }
     public final Map<String, Object> listValues(String prefix) { return listValues(new CompoundName(prefix)); }
 
     /**
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 b5481059ac0..0137b848dac 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
@@ -96,7 +96,7 @@ public class DimensionalValue<VALUE> {
         private VALUE value = null;
 
         /** The minimal binding this holds for */
-        private Binding binding = null;
+        private Binding binding;
 
         public Value(VALUE value, Binding binding) {
             this.value = value;
diff --git a/container-search/src/main/java/com/yahoo/search/query/properties/PropertyAliases.java b/container-search/src/main/java/com/yahoo/search/query/properties/PropertyAliases.java
index a4a82d27f8e..83e8dd530ad 100644
--- a/container-search/src/main/java/com/yahoo/search/query/properties/PropertyAliases.java
+++ b/container-search/src/main/java/com/yahoo/search/query/properties/PropertyAliases.java
@@ -37,6 +37,8 @@ public class PropertyAliases extends Properties {
      * @return the real name if an alias or the input name itself
      */
     protected CompoundName unalias(CompoundName nameOrAlias) {
+        if (aliases.isEmpty()) return nameOrAlias;
+        if (nameOrAlias.size() > 1) return nameOrAlias; // aliases are simple names
         CompoundName properName = aliases.get(nameOrAlias.getLowerCasedName());
         return (properName != null) ? properName : nameOrAlias;
     }
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 e9f7ff24d42..6d7d6f38e99 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
@@ -299,8 +299,6 @@ public class XmlReadingTestCase {
         String queryString="tiled?query=india&queryProfile=myprofile&source.common.intl=tw&source.common.mode=adv";
 
         Query query=new Query(HttpRequest.createTestRequest(queryString, Method.GET), registry.getComponent("myprofile"));
-        for (Map.Entry e : query.properties().listProperties().entrySet())
-            System.out.println(e);
         assertEquals("news",query.properties().listProperties().get("source.common.provider"));
         assertEquals("news",query.properties().get("source.common.provider"));
     }
diff --git a/processing/src/main/java/com/yahoo/processing/request/CompoundName.java b/processing/src/main/java/com/yahoo/processing/request/CompoundName.java
index 976fb3e2796..2185aac7adc 100644
--- a/processing/src/main/java/com/yahoo/processing/request/CompoundName.java
+++ b/processing/src/main/java/com/yahoo/processing/request/CompoundName.java
@@ -20,12 +20,6 @@ import static com.yahoo.text.Lowercase.toLowerCase;
  */
 public final class CompoundName {
 
-    /**
-     * The string name of this compound.
-     */
-    private final String name;
-    private final String lowerCasedName;
-
     private final ImmutableList<String> compounds;
 
     /** A hashcode which is always derived from the compounds (NEVER the string) */
@@ -43,7 +37,7 @@ public final class CompoundName {
      * @throws NullPointerException if name is null
      */
     public CompoundName(String name) {
-        this(name, parse(name));
+        this(parse(name));
     }
 
     /** Constructs this from an array of name components which are assumed not to contain dots */
@@ -53,21 +47,6 @@ public final class CompoundName {
 
     /** Constructs this from a list of compounds. */
     public CompoundName(List<String> compounds) {
-        this(toCompoundString(compounds), compounds);
-    }
-
-    /**
-     * Constructs this from a name with already parsed compounds.
-     * Private to avoid creating names with inconsistencies.
-     *
-     * @param name the string representation of the compounds
-     * @param compounds the compounds of this name
-     */
-    private CompoundName(String name, List<String> compounds) {
-        if (name == null) throw new NullPointerException("Name can not be null");
-
-        this.name = name;
-        this.lowerCasedName = toLowerCase(name);
         if (compounds.size()==1 && compounds.get(0).isEmpty())
             this.compounds = ImmutableList.of();
         else
@@ -111,7 +90,7 @@ public final class CompoundName {
         if (isEmpty()) return new CompoundName(name);
         List<String> newCompounds = new ArrayList<>(compounds);
         newCompounds.addAll(parse(name));
-        return new CompoundName(concat(this.name, name), newCompounds);
+        return new CompoundName(newCompounds);
     }
 
     /**
@@ -124,11 +103,7 @@ public final class CompoundName {
         if (isEmpty()) return name;
         List<String> newCompounds = new ArrayList<>(compounds);
         newCompounds.addAll(name.compounds);
-        return new CompoundName(concat(this.name, name.name), newCompounds);
-    }
-
-    private String concat(String name1, String name2) {
-        return name1 + "." + name2;
+        return new CompoundName(newCompounds);
     }
 
     /**
@@ -242,14 +217,11 @@ public final class CompoundName {
     public boolean hasPrefix(CompoundName prefix) {
         if (prefix.size() > this.size()) return false;
 
-        int prefixLength = prefix.name.length();
-        if (prefixLength == 0)
-            return true;
-
-        if (name.length() > prefixLength && name.charAt(prefixLength) != '.')
-            return false;
-
-        return name.startsWith(prefix.name);
+        for (int i = 0; i < prefix.size(); i++) {
+            if ( ! this.compounds.get(i).equals(prefix.get(i)))
+                return false;
+        }
+        return true;
     }
 
     /**
@@ -267,24 +239,22 @@ public final class CompoundName {
         if (o == this) return true;
         if ( ! (o instanceof CompoundName)) return false;
         CompoundName other = (CompoundName)o;
-        return this.name.equals(other.name);
+        return this.compounds.equals(other.compounds);
     }
 
     /**
      * Returns the string representation of this - all the name components in order separated by dots.
      */
     @Override
-    public String toString() { return name; }
-
-    public String getLowerCasedName() {
-        return lowerCasedName;
-    }
-
-    private static String toCompoundString(List<String> compounds) {
+    public String toString() {
         StringBuilder b = new StringBuilder();
         for (String compound : compounds)
             b.append(compound).append(".");
         return b.length()==0 ? "" : b.substring(0, b.length()-1);
     }
 
+    public String getLowerCasedName() {
+        return toString().toLowerCase();
+    }
+
 }