Revert "Bratseth/allocation free hit field traversal"

4 files changed, 62 insertions, 63 deletions

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 4310124037e..298bd839393 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
@@ -19,7 +19,6 @@ import java.util.List;
 import java.util.Map;
 import java.util.NoSuchElementException;
 import java.util.Set;
-import java.util.function.BiConsumer;
 
 /**
  * A regular hit from a Vespa backend
@@ -218,13 +217,7 @@ public class FastHit extends Hit {
         return getSummaryValue(name);
     }
 
-    @Override
-    public void forEachField(BiConsumer<String, Object> consumer) {
-        super.forEachField(consumer);
-        for (SummaryData summaryData : summaries)
-            summaryData.forEachField(consumer);
-    }
-
+    /** 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<>();
@@ -235,23 +228,14 @@ public class FastHit extends Hit {
         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 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);
-    }
-
     /** Returns a modifiable iterator over the field names of this */
-    private Iterator<String> fieldNameIterator() {
+    Iterator<String> fieldNameIterator() {
         return new FieldNameIterator(this, super.fieldKeys().iterator());
     }
 
@@ -284,6 +268,16 @@ public class FastHit extends Hit {
         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();
     }
@@ -526,36 +520,25 @@ public class FastHit extends Hit {
             return fieldType.convert(fieldValue);
         }
 
-        void forEachField(BiConsumer<String, Object> consumer) {
-            data.traverse((ObjectTraverser)(name, value) -> {
-                if ( ! shadowed(name) && ! removed(name))
-                    consumer.accept(name, value);
-            });
-        }
-
-        Iterator<Map.Entry<String, Object>> fieldIterator() {
-            return new SummaryDataFieldIterator(this, type, data.fields().iterator());
-        }
-
-        Iterator<String> fieldNameIterator() {
-            return new SummaryDataFieldNameIterator(this, data.fields().iterator());
+        /** 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));
+                }
+            }
         }
 
-        /**
-         * Returns whether this field is present in the map properties
-         * or an earlier (lower index) summary in this hit
-         */
-        private boolean shadowed(String name) {
-            if (hit.hasField(name)) return true;
-            for (int i = 0; i < index; i++) {
-                if (hit.summaries.get(i).type.fieldNames().contains(name))
-                    return true;
-            }
-            return false;
+        private Iterator<Map.Entry<String, Object>> fieldIterator() {
+            return new SummaryDataFieldIterator(hit, type, data.fields().iterator(), index);
         }
 
-        private boolean removed(String fieldName) {
-            return hit.removedFields != null && hit.removedFields.contains(fieldName);
+        private Iterator<String> fieldNameIterator() {
+            return new SummaryDataFieldNameIterator(hit, data.fields().iterator(), index);
         }
 
         /**
@@ -564,15 +547,19 @@ public class FastHit extends Hit {
          */
         private static abstract class SummaryDataIterator<VALUE> implements Iterator<VALUE> {
 
-            private final SummaryData summaryData;
+            private final FastHit hit;
             private final Iterator<Map.Entry<String, Inspector>> fieldIterator;
+            private final int index;
 
             /** The next value or null if none, eagerly read because we need to skip removed and overwritten values */
             private VALUE next;
 
-            SummaryDataIterator(SummaryData summaryData, Iterator<Map.Entry<String, Inspector>> fieldIterator) {
-                this.summaryData = summaryData;
+            SummaryDataIterator(FastHit hit,
+                                Iterator<Map.Entry<String, Inspector>> fieldIterator,
+                                int index) {
+                this.hit = hit;
                 this.fieldIterator = fieldIterator;
+                this.index = index;
             }
 
             @Override
@@ -596,23 +583,40 @@ public class FastHit extends Hit {
                     Map.Entry<String, Inspector> nextEntry = fieldIterator.next();
                     String fieldName = nextEntry.getKey();
                     next = toValue(nextEntry);
-                    if ( next != null && ! summaryData.shadowed(fieldName) && ! summaryData.removed(fieldName))
+                    if ( next != null &&
+                         ! hit.hasField(fieldName) &&
+                         ! isRemoved(fieldName) &&
+                         ! isAlreadyReturned(fieldName))
                         return;
                 }
                 next = null;
             }
 
+            private boolean isRemoved(String fieldName) {
+                return hit.removedFields != null && hit.removedFields.contains(fieldName);
+            }
+
+            private boolean isAlreadyReturned(String fieldName) {
+                for (int i = 0; i < index; i++) {
+                    if (hit.summaries.get(i).type.fieldNames().contains(fieldName))
+                        return true;
+                }
+                return false;
+            }
+
         }
 
+
         /** Iterator over the fields in a SummaryData instance. Read only. */
         private static class SummaryDataFieldIterator extends SummaryDataIterator<Map.Entry<String, Object>> {
 
             private final DocsumDefinition type;
 
-            SummaryDataFieldIterator(SummaryData summaryData,
+            SummaryDataFieldIterator(FastHit hit,
                                      DocsumDefinition type,
-                                     Iterator<Map.Entry<String, Inspector>> fieldIterator) {
-                super(summaryData, fieldIterator);
+                                     Iterator<Map.Entry<String, Inspector>> fieldIterator,
+                                     int index) {
+                super(hit, fieldIterator, index);
                 this.type = type;
                 advanceNext();
             }
@@ -650,9 +654,10 @@ public class FastHit extends Hit {
         /** Iterator over the field names in a SummaryData instance. Read only. */
         private static class SummaryDataFieldNameIterator extends SummaryDataIterator<String> {
 
-            SummaryDataFieldNameIterator(SummaryData summaryData,
-                                         Iterator<Map.Entry<String, Inspector>> fieldIterator) {
-                super(summaryData, fieldIterator);
+            SummaryDataFieldNameIterator(FastHit hit,
+                                         Iterator<Map.Entry<String, Inspector>> fieldIterator,
+                                         int index) {
+                super(hit, fieldIterator, index);
                 advanceNext();
             }
 
diff --git a/container-search/src/main/java/com/yahoo/prelude/searcher/DocumentSourceSearcher.java b/container-search/src/main/java/com/yahoo/prelude/searcher/DocumentSourceSearcher.java
index 415ebd7871c..164ff9a993b 100644
--- a/container-search/src/main/java/com/yahoo/prelude/searcher/DocumentSourceSearcher.java
+++ b/container-search/src/main/java/com/yahoo/prelude/searcher/DocumentSourceSearcher.java
@@ -74,7 +74,7 @@ public class DocumentSourceSearcher extends Searcher {
             removePropertiesNotStartingByA(attributeHit);
             attributeHit.setFillable();
             attributeHit.setRelevance(fullHit.getRelevance());
-            for (Object propertyKeyObject : fullHit.fields().keySet()) {
+            for (Object propertyKeyObject : (Set) fullHit.fields().keySet()) {
                 String propertyKey=propertyKeyObject.toString();
                 if (propertyKey.startsWith("attribute"))
                     attributeHit.setField(propertyKey, fullHit.getField(propertyKey));
diff --git a/container-search/src/main/java/com/yahoo/prelude/searcher/JuniperSearcher.java b/container-search/src/main/java/com/yahoo/prelude/searcher/JuniperSearcher.java
index ca87c0c1d46..9e1808bb08e 100644
--- a/container-search/src/main/java/com/yahoo/prelude/searcher/JuniperSearcher.java
+++ b/container-search/src/main/java/com/yahoo/prelude/searcher/JuniperSearcher.java
@@ -107,12 +107,6 @@ public class JuniperSearcher extends Searcher {
             if (searchDefinitionField == null) continue;
             String searchDefinitionName = searchDefinitionField.toString();
 
-            // TODO: Switch to iterate over indexes in the outer loop:
-            //for (Index index : indexFacts.getIndexes(searchDefinitionName())) {
-            //    if (index.getDynamicSummary() || index.getHighlightSummary()) {
-            //        insertTags(hit.buildHitField(index.getName(), true, true), bolding, index.getDynamicSummary());
-            //    }
-            //}
             for (String fieldName : hit.fields().keySet()) {
                 Index index = indexFacts.getIndex(fieldName, searchDefinitionName);
                 if (index.getDynamicSummary() || index.getHighlightSummary())
diff --git a/container-search/src/main/java/com/yahoo/prelude/searcher/QuotingSearcher.java b/container-search/src/main/java/com/yahoo/prelude/searcher/QuotingSearcher.java
index d4cad7f1246..7ff5b1b1a0e 100644
--- a/container-search/src/main/java/com/yahoo/prelude/searcher/QuotingSearcher.java
+++ b/container-search/src/main/java/com/yahoo/prelude/searcher/QuotingSearcher.java
@@ -20,7 +20,7 @@ import com.yahoo.search.searchchain.Execution;
  *
  * May be extended to do quoting template sensitive.
  *
- * @author Steinar Knutsen
+ * @author  <a href="mailto:steinar@yahoo-inc.com">Steinar Knutsen</a>
  */
 public class QuotingSearcher extends Searcher {