Lazy summary decoding

5 files changed, 533 insertions, 40 deletions

diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/DocsumDefinition.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/DocsumDefinition.java
index d50006fb82c..161d00b49bb 100644
--- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/DocsumDefinition.java
+++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/DocsumDefinition.java
@@ -66,9 +66,11 @@ public class DocsumDefinition {
         return fields.get(fieldIndex);
     }
 
-    /** Returns the index of a field name */
-    public Integer getFieldIndex(String fieldName) {
-        return fieldNameToIndex.get(fieldName);
+    /** Returns the field with this name, or null if none */
+    public DocsumField getField(String fieldName) {
+        Integer index = fieldNameToIndex.get(fieldName);
+        if (index == null) return null;
+        return getField(index);
     }
 
     @Override
diff --git a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastHit.java b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastHit.java
index f1f5e2b7403..b167ad2124e 100644
--- a/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastHit.java
+++ b/container-search/src/main/java/com/yahoo/prelude/fastsearch/FastHit.java
@@ -1,6 +1,7 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.prelude.fastsearch;
 
+import com.yahoo.data.access.ObjectTraverser;
 import com.yahoo.document.GlobalId;
 import com.yahoo.fs4.QueryPacketData;
 import com.yahoo.net.URI;
@@ -8,6 +9,17 @@ import com.yahoo.search.result.Hit;
 import com.yahoo.search.result.Relevance;
 import com.yahoo.data.access.Inspector;
 
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.NoSuchElementException;
+import java.util.Set;
+
 /**
  * A regular hit from a Vespa backend
  *
@@ -34,6 +46,24 @@ public class FastHit extends Hit {
     private transient CacheKey cacheKey = null;
 
     /**
+     * Summaries added to this hit which are not yet decoded into fields.
+     * Fields are resolved by returning the first non-null value found by
+     * 1) the field value from the Map of fields in the Hit supertype, and
+     * 2) each of the summaries, in the order of the list (which is the add order).
+     * This ensures that values set from code overwrites any value received as
+     * summary data.
+     *
+     * The reason we keep this rather than eagerly decoding into a the field map
+     * is to reduce garbage collection and decoding cost, with the assumption
+     * that most fields passes through the container with no processing most
+     * of the time.
+     */
+    private List<SummaryData> summaries = new ArrayList<>(1);
+
+    /** Removed field values, which should therefore not be returned if present in summary data */
+    private Set<String> removedFields = null;
+
+    /**
      * Creates an empty and temporarily invalid summary hit
      */
     public FastHit() { }
@@ -128,16 +158,7 @@ public class FastHit extends Hit {
 
     /** For internal use */
     public void addSummary(DocsumDefinition docsumDef, Inspector value) {
-        reserve(docsumDef.getFieldCount());
-        for (DocsumField field : docsumDef.getFields()) {
-            String fieldName = field.getName();
-            Inspector f = value.field(fieldName);
-            if (field.getEmulConfig().forceFillEmptyFields() || f.valid()) {
-                if (super.getField(fieldName) == null) {
-                    setField(fieldName, field.convert(f));
-                }
-            }
-        }
+        summaries.add(new SummaryData(this, docsumDef, value));
     }
 
     /**
@@ -172,8 +193,117 @@ public class FastHit extends Hit {
      * </ul>
      */
     @Override
-    public Object getField(String key) {
-        return super.getField(key);
+    public Object getField(String name) {
+        Object value = super.getField(name);
+        if (value != null) return value;
+        value = getSummaryValue(name);
+        if (value != null)
+            super.setField(name, value);
+        return value;
+    }
+
+    /** Returns the fields of this as a read-only map. This is more costly than fieldIterator() */
+    @Override
+    public Map<String, Object> fields() {
+        Map<String, Object> fields = new HashMap<>();
+        for (Iterator<Map.Entry<String, Object>> i = fieldIterator(); i.hasNext(); ) {
+            Map.Entry<String, Object> field = i.next();
+            fields.put(field.getKey(), field.getValue());
+        }
+        return fields;
+    }
+
+    /** Returns a modifiable iterator over the fields of this */
+    @Override
+    public Iterator<Map.Entry<String, Object>> fieldIterator() {
+        return new FieldIterator(this, super.fieldIterator());
+    }
+
+    /** Returns a modifiable iterator over the field names of this */
+    Iterator<String> fieldNameIterator() {
+        return new FieldNameIterator(this, super.fieldKeys().iterator());
+    }
+
+    /** Removes all fields of this */
+    @Override
+    public void clearFields() {
+        summaries.clear();
+        super.clearFields();
+    }
+
+    /**
+     * Removes a field from this
+     *
+     * @return the removed value of the field, or null if none
+     */
+    @Override
+    public Object removeField(String name) {
+        Object removedValue = super.removeField(name);
+        if (removedValue == null)
+            removedValue = getSummaryValue(name);
+
+        if (removedValue != null) {
+            if (removedFields == null)
+                removedFields = new HashSet<>(2);
+            removedFields.add(name);
+        }
+
+        return removedValue;
+    }
+
+    /**
+     * Returns the keys of the fields of this hit as a modifiable view.
+     * This follows the rules of key sets returned from maps: Key removals are reflected
+     * in the map, add and addAll is not supported.
+     */
+    @Override
+    public Set<String> fieldKeys() {
+        return new FieldSet(this);
+    }
+
+    private Set<String> mapFieldKeys() {
+        return super.fieldKeys();
+    }
+
+    /** Returns whether this field is present <b>in the field map in the parent hit</b> */
+    // Note: If this is made public it must be changed to also check the summary data
+    //       (and internal usage must change to another method).
+    @Override
+    protected boolean hasField(String name) {
+        return super.hasField(name);
+    }
+
+    /** Returns whether any fields are present <b>in the field map in the parent hit</b> */
+    // Note: If this is made public it must be changed to also check the summary data
+    //       (and internal usage must change to another method).
+    @Override
+    protected boolean hasFields() {
+        return super.hasFields();
+    }
+
+    /**
+     * Changes the key under which a value is found. This is useful because it allows keys to be changed
+     * without accessing the value (which may be lazily created).
+     *
+     * @deprecated do not use
+     */
+    @Deprecated
+    @Override
+    @SuppressWarnings("deprecation")
+    public void changeFieldKey(String oldKey, String newKey) {
+        Object value = removeField(oldKey);
+        if (value != null)
+            setField(newKey, value);
+    }
+
+    private Object getSummaryValue(String name) {
+        if (removedFields != null && removedFields.contains(name))
+            return null;
+        for (SummaryData summaryData : summaries) {
+            Object value = summaryData.getField(name);
+            if (value != null) return value;
+        }
+        return null;
     }
 
     @Override
@@ -192,6 +322,8 @@ public class FastHit extends Hit {
         }
     }
 
+    /** @deprecated do not use */
+    @Deprecated // TODO: Make private on Vespa 7
     public static String asHexString(GlobalId gid) {
         StringBuilder sb = new StringBuilder();
         byte[] rawGid = gid.getRawId();
@@ -205,4 +337,364 @@ public class FastHit extends Hit {
         return sb.toString();
     }
 
+    /** A set view of all the field names in this hit. Add/addAll is not supported but remove is. */
+    private static class FieldSet implements Set<String> {
+
+        // A set implementation which tries to avoid creating an actual set when possible.
+        // With more work this could be optimized to avoid creating the set in additional cases.
+
+        private final FastHit hit;
+
+        /** The computed set of fields. Lazily created as it is not always needed. */
+        private Set<String> fieldSet = null;
+
+        public FieldSet(FastHit hit) {
+            this.hit = hit;
+        }
+
+        @Override
+        public int size() {
+            return createSet().size();
+        }
+
+        @Override
+        public boolean isEmpty() {
+            if ( ! hit.hasFields() && hit.summaries.isEmpty()) return true;
+            return createSet().isEmpty();
+        }
+
+        @Override
+        public boolean contains(Object o) {
+            String field = (String)o;
+            if (hit.hasField(field)) return true;
+            return createSet().contains(field);
+        }
+
+        @Override
+        public Object[] toArray() {
+            return createSet().toArray();
+        }
+
+        @Override
+        public <T> T[] toArray(T[] a) {
+            return createSet().toArray(a);
+        }
+
+        @Override
+        public boolean add(String s) {
+            throw new UnsupportedOperationException();
+        }
+
+        @Override
+        public boolean remove(Object o) {
+            String field = (String)o;
+            boolean removed = hit.removeField(field) != null;
+            if (fieldSet != null)
+                fieldSet.remove(field);
+            return removed;
+        }
+
+        @Override
+        public boolean containsAll(Collection<?> c) {
+            for (Object field : c)
+                if ( ! contains(field))
+                    return false;
+            return true;
+        }
+
+        @Override
+        public boolean addAll(Collection<? extends String> c) {
+            throw new UnsupportedOperationException();
+        }
+
+        @Override
+        public boolean retainAll(Collection<?> c) {
+            Set<?> toRetain = c instanceof Set<?> ? (Set<?>)c : new HashSet<Object>(c);
+            boolean anyRemoved = false;
+            for (Iterator<String> i = iterator(); i.hasNext(); ) {
+                String field = i.next();
+                if (toRetain.contains(field)) continue;
+
+                i.remove();
+                anyRemoved = true;
+            }
+            return anyRemoved;
+        }
+
+        @Override
+        public boolean removeAll(Collection<?> c) {
+            boolean anyRemoved = false;
+            for (Object field : c)
+                if (remove(field))
+                    anyRemoved = true;
+            return anyRemoved;
+        }
+
+        @Override
+        public void clear() {
+            hit.clearFields();
+            fieldSet = null;
+        }
+
+        @Override
+        public Iterator<String> iterator() {
+            return hit.fieldNameIterator();
+        }
+
+        private Set<String> createSet() {
+            if ( ! hit.hasFields() && hit.summaries.isEmpty()) return Collections.emptySet(); // shortcut
+
+            Set<String> fields = new HashSet<>();
+            if (hit.hasFields())
+                fields.addAll(hit.mapFieldKeys());
+
+            for (SummaryData summaryData : hit.summaries)
+                summaryData.data.traverse((ObjectTraverser)(name, __) -> fields.add(name));
+            fields.removeAll(hit.removedFields);
+
+            this.fieldSet = fields;
+
+            return fields;
+        }
+
+    }
+
+    /** Summary data (values of a number of fields) received for this hit */
+    private static class SummaryData {
+
+        private final FastHit hit;
+        private final DocsumDefinition type;
+        private final Inspector data;
+
+        SummaryData(FastHit hit, DocsumDefinition type, Inspector data) {
+            this.hit = hit;
+            this.type = type;
+            this.data = data;
+        }
+
+        Object getField(String name) {
+            DocsumField fieldType = type.getField(name);
+            if (fieldType == null) return null;
+            Inspector fieldValue = data.field(name);
+            if ( ! fieldValue.valid() && ! fieldType.getEmulConfig().forceFillEmptyFields()) return null;
+            return fieldType.convert(fieldValue);
+        }
+
+        /** Decodes the given summary into the field map in the parent */
+        private void decodeInto(FastHit hit) {
+            hit.reserve(type.getFieldCount());
+            for (DocsumField field : type.getFields()) {
+                String fieldName = field.getName();
+                Inspector f = data.field(fieldName);
+                if (field.getEmulConfig().forceFillEmptyFields() || f.valid()) {
+                    if (hit.getField(fieldName) == null)
+                        hit.setField(fieldName, field.convert(f));
+                }
+            }
+        }
+
+        private Iterator<Map.Entry<String, Object>> fieldIterator() {
+            return new SummaryFieldDataIterator(hit, type, data.fields().iterator());
+        }
+
+        private Iterator<String> fieldNameIterator() {
+            return new SummaryFieldNameDataIterator(hit, data.fields().iterator());
+        }
+
+        /** A wrapper of a field iterator which skips removed fields. Read only. */
+        private static abstract class SummaryDataIterator<VALUE> implements Iterator<VALUE> {
+
+            private final FastHit hit;
+            private final Iterator<Map.Entry<String, Inspector>> fieldIterator;
+
+            /**
+             * The next value or null if none, eagerly read because we need to skip removed and overwritten values
+             */
+            private Map.Entry<String, Inspector> next;
+
+            SummaryDataIterator(FastHit hit, Iterator<Map.Entry<String, Inspector>> fieldIterator) {
+                this.hit = hit;
+                this.fieldIterator = fieldIterator;
+            }
+
+            @Override
+            public boolean hasNext() {
+                return next != null;
+            }
+
+            @Override
+            public VALUE next() {
+                if (next == null) throw new NoSuchElementException();
+
+                VALUE returnValue = toValue(next);
+                advanceNext();
+                return returnValue;
+            }
+
+            protected abstract VALUE toValue(Map.Entry<String, Inspector> field);
+
+            private void advanceNext() {
+                while (fieldIterator.hasNext()) {
+                    next = fieldIterator.next();
+                    if (!hit.hasField(next.getKey()) && !hit.removedFields.contains(next.getKey()))
+                        break;
+                }
+                next = null;
+            }
+        }
+
+        private static class SummaryFieldDataIterator extends SummaryDataIterator<Map.Entry<String, Object>> {
+
+            private final DocsumDefinition type;
+
+            /** The next value or null if none, eagerly read because we need to skip removed and overwritten values */
+            private Map.Entry<String, Inspector> next;
+
+            SummaryFieldDataIterator(FastHit hit, DocsumDefinition type, Iterator<Map.Entry<String, Inspector>> fieldIterator) {
+                super(hit, fieldIterator);
+                this.type = type;
+            }
+
+            @Override
+            protected Map.Entry<String, Object> toValue(Map.Entry<String, Inspector> field) {
+                return new SummaryFieldEntry(field.getKey(), type.getField(field.getKey()).convert(field.getValue()));
+            }
+
+            private static final class SummaryFieldEntry implements Map.Entry<String, Object> {
+
+                private final String key;
+                private final Object value;
+
+                public SummaryFieldEntry(String key, Object value) {
+                    this.key = key;
+                    this.value = value;
+                }
+
+                @Override
+                public String getKey() { return key; }
+
+                @Override
+                public Object getValue() { return value; }
+
+                @Override
+                public Object setValue(Object value) { throw new UnsupportedOperationException(); }
+
+            }
+
+        }
+
+        /** A wrapper of a field iterator which converts to the expected field types. Read only. */
+        private static class SummaryFieldNameDataIterator extends SummaryDataIterator<String> {
+
+            /** The next value or null if none, eagerly read because we need to skip removed and overwritten values */
+            private Map.Entry<String, Inspector> next;
+
+            SummaryFieldNameDataIterator(FastHit hit, Iterator<Map.Entry<String, Inspector>> fieldIterator) {
+                super(hit, fieldIterator);
+            }
+
+            @Override
+            protected String toValue(Map.Entry<String, Inspector> field) {
+                return field.toString();
+            }
+
+        }
+    }
+
+    private static abstract class SummaryIterator<VALUE> implements Iterator<VALUE> {
+
+        private final FastHit hit;
+
+        /** -1 means that the current iterator is the map iterator of the parent hit, not any summary data iterator */
+        private int currentSummaryDataIndex = -1;
+        private Iterator<VALUE> currentIterator;
+        private VALUE previousReturned = null;
+
+        public SummaryIterator(FastHit hit, Iterator<VALUE> mapFieldsIterator) {
+            this.hit = hit;
+            this.currentIterator = mapFieldsIterator;
+        }
+
+        @Override
+        public boolean hasNext() {
+            if (currentIterator.hasNext()) return true;
+            return nextIterator();
+        }
+
+        @Override
+        public VALUE next() {
+            if (currentIterator.hasNext() || nextIterator()) return previousReturned = currentIterator.next();
+            throw new NoSuchElementException();
+        }
+
+        @Override
+        public void remove() {
+            if (previousReturned == null)
+                throw new IllegalStateException();
+            if ( ! ( currentIterator instanceof SummaryData.SummaryDataIterator))
+                currentIterator.remove(); // remove from the map
+            hit.removedFields.add(nameOf(previousReturned));
+            previousReturned = null;
+        }
+
+        protected abstract String nameOf(VALUE value);
+        protected abstract Iterator<VALUE> iteratorFor(SummaryData summary);
+
+        /** Advanced to the next non-empty iterator, if any */
+        private boolean nextIterator() {
+            while (++currentSummaryDataIndex < hit.summaries.size()) {
+                currentIterator = iteratorFor(hit.summaries.get(currentSummaryDataIndex));
+                if (currentIterator.hasNext())
+                    return true;
+            }
+            return false;
+        }
+
+    }
+
+    private static class FieldIterator extends SummaryIterator<Map.Entry<String, Object>> {
+
+        public FieldIterator(FastHit hit, Iterator<Map.Entry<String, Object>> mapFieldsIterator) {
+            super(hit, mapFieldsIterator);
+        }
+
+        @Override
+        protected String nameOf(Map.Entry<String, Object> value) {
+            return value.getKey();
+        }
+
+        @Override
+        protected Iterator<Map.Entry<String, Object>> iteratorFor(SummaryData summary) {
+            return summary.fieldIterator();
+        }
+
+    }
+
+
+    private static class FieldNameIterator extends SummaryIterator<String> {
+
+        public FieldNameIterator(FastHit hit, Iterator<String> mapFieldNamesIterator) {
+            super(hit, mapFieldNamesIterator);
+        }
+
+        @Override
+        protected String nameOf(String value) {
+            return value;
+        }
+
+        @Override
+        protected Iterator<String> iteratorFor(SummaryData summary) {
+            return summary.fieldNameIterator();
+        }
+
+    }
+
+    // TODO: Things we need to check that we test:
+    // - removing, then adding a field
+    // - removing from field and field name iterators
+    // - removing fields, both summary and map, then iterating
+    // - removing all fields in some summary, then iterating
+    // - adding a field from the iterator
+    // - iterating with fields set and multiple summaries, where some summaries have overlapping fields (TODO: Or ensure no overlapping fields?)
+
 }
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 0fed379d446..f13fb2e88f4 100644
--- a/container-search/src/main/java/com/yahoo/search/Query.java
+++ b/container-search/src/main/java/com/yahoo/search/Query.java
@@ -151,11 +151,7 @@ public class Query extends com.yahoo.processing.Request implements Cloneable {
 
     //--------------  Generic property containers --------------------------------
 
-    /**
-     * The synchronous view of the JDisc request causing this query.
-     *
-     * @since 5.1
-     */
+    /** The synchronous view of the JDisc request causing this query */
     private final HttpRequest httpRequest;
 
     /** The context, or null if there is no context */
@@ -195,7 +191,7 @@ public class Query extends com.yahoo.processing.Request implements Cloneable {
 
     private static QueryProfileType argumentType;
     static {
-        argumentType=new QueryProfileType("native");
+        argumentType = new QueryProfileType("native");
         argumentType.setBuiltin(true);
 
         argumentType.addField(new FieldDescription(OFFSET.toString(), "integer", "offset start"));
@@ -622,7 +618,7 @@ public class Query extends com.yahoo.processing.Request implements Cloneable {
      *
      * @deprecated this is ignored
      */
-    @Deprecated
+    @Deprecated // TODO: Remove on Vespa 7
     public void setCompress(boolean ignored) { }
 
     /**
@@ -630,7 +626,7 @@ public class Query extends com.yahoo.processing.Request implements Cloneable {
      *
      * @deprecated this always returns false
      */
-    @Deprecated
+    @Deprecated // TODO: Remove on Vespa 7
     public boolean getCompress() { return false; }
 
     /** Returns a string describing this query */
diff --git a/container-search/src/main/java/com/yahoo/search/result/Hit.java b/container-search/src/main/java/com/yahoo/search/result/Hit.java
index 0664720c7d7..15c148b7db7 100644
--- a/container-search/src/main/java/com/yahoo/search/result/Hit.java
+++ b/container-search/src/main/java/com/yahoo/search/result/Hit.java
@@ -401,14 +401,9 @@ public class Hit extends ListenableFreezableClass implements Data, Comparable<Hi
     /** Returns the name of the source creating this hit */
     public String getSource() { return source; }
 
-    /**
-     * Returns the fields of this as a read-only map. This is more costly than the preferred iterator(), as
-     * it uses Collections.unmodifiableMap()
-     *
-     * @return An readonly map of the fields
-     */
+    /** Returns the fields of this as a read-only map. This is more costly than fieldIterator() */
     // TODO Should it be deprecated ?
-    public final Map<String, Object> fields() { return getUnmodifiableFieldMap(); }
+    public Map<String, Object> fields() { return getUnmodifiableFieldMap(); }
 
     /** Allocate room for the given number of fields to avoid resizing. */
     public void reserve(int minSize) {
@@ -424,14 +419,10 @@ public class Hit extends ListenableFreezableClass implements Data, Comparable<Hi
         return getFieldMap().put(key, value);
     }
 
-    /**
-     * Returns an iterator over the fields of this
-     * 
-     * @return an iterator for traversing the fields of this hit
-     */
-    public final Iterator<Map.Entry<String,Object>> fieldIterator() { return getFieldMap().entrySet().iterator(); }
+    /** Returns a modifiable iterator over the fields of this */
+    public Iterator<Map.Entry<String, Object>> fieldIterator() { return getFieldMap().entrySet().iterator(); }
 
-    /** Returns a field value */
+    /** Returns a field value or null if not present */
     public Object getField(String value) { return fields != null ? fields.get(value) : null; }
 
     /** Removes all fields of this */
@@ -457,10 +448,22 @@ public class Hit extends ListenableFreezableClass implements Data, Comparable<Hi
         return getFieldMap().keySet();
     }
 
+    protected boolean hasField(String name) {
+        return fields != null && fields.containsKey(name);
+    }
+
+    /** Returns whether any fields are set in this */
+    protected boolean hasFields() {
+        return fields != null && ! fields.isEmpty();
+    }
+
     /**
      * Changes the key under which a value is found. This is useful because it allows keys to be changed
      * without accessing the value (which may be lazily created).
+     *
+     * @deprecated do not use
      */
+    @Deprecated // TODO: Remove on Vespa 7
     public void changeFieldKey(String oldKey, String newKey) {
         Map<String,Object> fieldMap = getFieldMap();
         Object value = fieldMap.remove(oldKey);
@@ -468,7 +471,7 @@ public class Hit extends ListenableFreezableClass implements Data, Comparable<Hi
     }
 
     private Map<String, Object> getFieldMap() {
-        return getFieldMap(16);
+        return getFieldMap(2);
     }
 
     private Map<String, Object> getFieldMap(int minSize) {
diff --git a/container-search/src/main/java/com/yahoo/search/yql/MinimalQueryInserter.java b/container-search/src/main/java/com/yahoo/search/yql/MinimalQueryInserter.java
index eb1b3a68219..a20949e3dfd 100644
--- a/container-search/src/main/java/com/yahoo/search/yql/MinimalQueryInserter.java
+++ b/container-search/src/main/java/com/yahoo/search/yql/MinimalQueryInserter.java
@@ -21,14 +21,14 @@ import com.yahoo.yolean.chain.Provides;
 /**
  * Minimal combinator for YQL+ syntax and heuristically parsed user queries.
  *
- * @author <a href="mailto:steinar@yahoo-inc.com">Steinar Knutsen</a>
- * @since 5.1.28
+ * @author Steinar Knutsen
  */
 @Beta
 @Provides(MinimalQueryInserter.EXTERNAL_YQL)
 @Before(PhaseNames.TRANSFORMED_QUERY)
 @After("com.yahoo.prelude.statistics.StatisticsSearcher")
 public class MinimalQueryInserter extends Searcher {
+
     public static final String EXTERNAL_YQL = "ExternalYql";
 
     public static final CompoundName YQL = new CompoundName("yql");