Merge pull request #5844 from vespa-engine/bratseth/allocation-free-hit-field-traversal

Bratseth/allocation free hit field traversal

18 files changed, 181 insertions, 166 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 298bd839393..4310124037e 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,6 +19,7 @@ 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
@@ -217,7 +218,13 @@ public class FastHit extends Hit {
         return getSummaryValue(name);
     }
 
-    /** Returns the fields of this as a read-only map. This is more costly than fieldIterator() */
+    @Override
+    public void forEachField(BiConsumer<String, Object> consumer) {
+        super.forEachField(consumer);
+        for (SummaryData summaryData : summaries)
+            summaryData.forEachField(consumer);
+    }
+
     @Override
     public Map<String, Object> fields() {
         Map<String, Object> fields = new HashMap<>();
@@ -228,14 +235,23 @@ 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 */
-    Iterator<String> fieldNameIterator() {
+    private Iterator<String> fieldNameIterator() {
         return new FieldNameIterator(this, super.fieldKeys().iterator());
     }
 
@@ -268,16 +284,6 @@ 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();
     }
@@ -520,25 +526,36 @@ public class FastHit extends Hit {
             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));
-                }
-            }
+        void forEachField(BiConsumer<String, Object> consumer) {
+            data.traverse((ObjectTraverser)(name, value) -> {
+                if ( ! shadowed(name) && ! removed(name))
+                    consumer.accept(name, value);
+            });
         }
 
-        private Iterator<Map.Entry<String, Object>> fieldIterator() {
-            return new SummaryDataFieldIterator(hit, type, data.fields().iterator(), index);
+        Iterator<Map.Entry<String, Object>> fieldIterator() {
+            return new SummaryDataFieldIterator(this, type, data.fields().iterator());
+        }
+
+        Iterator<String> fieldNameIterator() {
+            return new SummaryDataFieldNameIterator(this, data.fields().iterator());
+        }
+
+        /**
+         * 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<String> fieldNameIterator() {
-            return new SummaryDataFieldNameIterator(hit, data.fields().iterator(), index);
+        private boolean removed(String fieldName) {
+            return hit.removedFields != null && hit.removedFields.contains(fieldName);
         }
 
         /**
@@ -547,19 +564,15 @@ public class FastHit extends Hit {
          */
         private static abstract class SummaryDataIterator<VALUE> implements Iterator<VALUE> {
 
-            private final FastHit hit;
+            private final SummaryData summaryData;
             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(FastHit hit,
-                                Iterator<Map.Entry<String, Inspector>> fieldIterator,
-                                int index) {
-                this.hit = hit;
+            SummaryDataIterator(SummaryData summaryData, Iterator<Map.Entry<String, Inspector>> fieldIterator) {
+                this.summaryData = summaryData;
                 this.fieldIterator = fieldIterator;
-                this.index = index;
             }
 
             @Override
@@ -583,40 +596,23 @@ public class FastHit extends Hit {
                     Map.Entry<String, Inspector> nextEntry = fieldIterator.next();
                     String fieldName = nextEntry.getKey();
                     next = toValue(nextEntry);
-                    if ( next != null &&
-                         ! hit.hasField(fieldName) &&
-                         ! isRemoved(fieldName) &&
-                         ! isAlreadyReturned(fieldName))
+                    if ( next != null && ! summaryData.shadowed(fieldName) && ! summaryData.removed(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(FastHit hit,
+            SummaryDataFieldIterator(SummaryData summaryData,
                                      DocsumDefinition type,
-                                     Iterator<Map.Entry<String, Inspector>> fieldIterator,
-                                     int index) {
-                super(hit, fieldIterator, index);
+                                     Iterator<Map.Entry<String, Inspector>> fieldIterator) {
+                super(summaryData, fieldIterator);
                 this.type = type;
                 advanceNext();
             }
@@ -654,10 +650,9 @@ public class FastHit extends Hit {
         /** Iterator over the field names in a SummaryData instance. Read only. */
         private static class SummaryDataFieldNameIterator extends SummaryDataIterator<String> {
 
-            SummaryDataFieldNameIterator(FastHit hit,
-                                         Iterator<Map.Entry<String, Inspector>> fieldIterator,
-                                         int index) {
-                super(hit, fieldIterator, index);
+            SummaryDataFieldNameIterator(SummaryData summaryData,
+                                         Iterator<Map.Entry<String, Inspector>> fieldIterator) {
+                super(summaryData, fieldIterator);
                 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 164ff9a993b..415ebd7871c 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 : (Set) fullHit.fields().keySet()) {
+            for (Object propertyKeyObject : 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 9e1808bb08e..ca87c0c1d46 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,6 +107,12 @@ 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 7ff5b1b1a0e..d4cad7f1246 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  <a href="mailto:steinar@yahoo-inc.com">Steinar Knutsen</a>
+ * @author Steinar Knutsen
  */
 public class QuotingSearcher extends Searcher {
 
diff --git a/container-search/src/main/java/com/yahoo/search/grouping/result/HitRenderer.java b/container-search/src/main/java/com/yahoo/search/grouping/result/HitRenderer.java
index 259b219b181..37906c8012f 100644
--- a/container-search/src/main/java/com/yahoo/search/grouping/result/HitRenderer.java
+++ b/container-search/src/main/java/com/yahoo/search/grouping/result/HitRenderer.java
@@ -13,7 +13,7 @@ import java.util.Map;
 /**
  * This is a helper class for rendering grouping results.
  *
- * @author <a href="mailto:simon@yahoo-inc.com">Simon Thoresen</a>
+ * @author Simon Thoresen
  */
 public abstract class HitRenderer {
 
@@ -50,9 +50,7 @@ public abstract class HitRenderer {
             if (hit instanceof RootGroup) {
                 renderContinuation(Continuation.THIS_PAGE, ((RootGroup)hit).continuation(), writer);
             }
-            for (String label : hit.fieldKeys()) {
-                writer.openTag(TAG_OUTPUT).attribute(ATR_LABEL, label).content(hit.getField(label), false).closeTag();
-            }
+            hit.forEachField((name, value) -> writer.openTag(TAG_OUTPUT).attribute(ATR_LABEL, name).content(value, false).closeTag());
         } else if (hit instanceof HitList) {
             writer.openTag(TAG_HIT_LIST).attribute(ATR_LABEL, ((HitList)hit).getLabel());
             renderContinuations(((HitList)hit).continuations(), writer);
diff --git a/container-search/src/main/java/com/yahoo/search/pagetemplates/engine/Organizer.java b/container-search/src/main/java/com/yahoo/search/pagetemplates/engine/Organizer.java
index 97a5bdd72ca..3e6e82a5584 100644
--- a/container-search/src/main/java/com/yahoo/search/pagetemplates/engine/Organizer.java
+++ b/container-search/src/main/java/com/yahoo/search/pagetemplates/engine/Organizer.java
@@ -37,11 +37,7 @@ public class Organizer {
         sectionGroup.setQuery(result.hits().getQuery());
         if (errors!=null && errors instanceof DefaultErrorHit)
             sectionGroup.add((DefaultErrorHit)errors);
-        for (Iterator<Map.Entry<String, Object>> it = result.hits().fieldIterator(); it.hasNext(); ) {
-            Map.Entry<String, Object> field = it.next();
-            sectionGroup.setField(field.getKey(), field.getValue());
-        }
-
+        result.hits().forEachField((name, value) -> sectionGroup.setField(name, value));
         result.setHits(sectionGroup);
     }
 
diff --git a/container-search/src/main/java/com/yahoo/search/querytransform/NGramSearcher.java b/container-search/src/main/java/com/yahoo/search/querytransform/NGramSearcher.java
index c64927072e2..2768a546cd0 100644
--- a/container-search/src/main/java/com/yahoo/search/querytransform/NGramSearcher.java
+++ b/container-search/src/main/java/com/yahoo/search/querytransform/NGramSearcher.java
@@ -160,7 +160,7 @@ public class NGramSearcher extends Searcher {
             if (hit.isMeta()) continue;
             Object sddocname = hit.getField(Hit.SDDOCNAME_FIELD);
             if (sddocname == null) return;
-            for (String fieldName : hit.fieldKeys()) {
+            for (String fieldName : hit.fieldKeys()) { // TODO: Iterate over indexes instead
                 Index index = session.getIndex(fieldName, sddocname.toString());
                 if (index.isNGram() && (index.getHighlightSummary() || index.getDynamicSummary())) {
                     hit.setField(fieldName, recombineNGramsField(hit.getField(fieldName), index.getGramSize()));
diff --git a/container-search/src/main/java/com/yahoo/search/rendering/DefaultRenderer.java b/container-search/src/main/java/com/yahoo/search/rendering/DefaultRenderer.java
index bae1185d6a9..2d69a262f15 100644
--- a/container-search/src/main/java/com/yahoo/search/rendering/DefaultRenderer.java
+++ b/container-search/src/main/java/com/yahoo/search/rendering/DefaultRenderer.java
@@ -185,28 +185,19 @@ public final class DefaultRenderer extends AsynchronousSectionedRenderer<Result>
 
     private void renderHitFields(XMLWriter writer, Hit hit) {
         renderSyntheticRelevanceField(writer, hit);
-        for (Iterator<Map.Entry<String, Object>> it = hit.fieldIterator(); it.hasNext(); ) {
-            renderField(writer, hit, it);
-        }
-    }
-
-    private void renderField(XMLWriter writer, Hit hit, Iterator<Map.Entry<String, Object>> it) {
-        renderGenericField(writer, hit, it.next());
+        hit.forEachField((name, value) -> renderField(writer, name, value));
     }
 
-    private void renderGenericField(XMLWriter writer, Hit hit, Map.Entry<String, Object> entry) {
-        String fieldName = entry.getKey();
-
-        // skip depending on hit type
-        if (fieldName.startsWith("$")) return; // Don't render fields that start with $ // TODO: Move to should render
+    private void renderField(XMLWriter writer, String name, Object value) {
+        if (name.startsWith("$")) return;
 
-        writeOpenFieldElement(writer, fieldName);
-        renderFieldContent(writer, hit, fieldName);
+        writeOpenFieldElement(writer, name);
+        renderFieldContent(writer, value);
         writeCloseFieldElement(writer);
     }
 
-    private void renderFieldContent(XMLWriter writer, Hit hit, String fieldName) {
-        writer.escapedContent(asXML(hit.getField(fieldName)), false);
+    private void renderFieldContent(XMLWriter writer, Object value) {
+        writer.escapedContent(asXML(value), false);
     }
 
     private String asXML(Object value) {
diff --git a/container-search/src/main/java/com/yahoo/search/rendering/JsonRenderer.java b/container-search/src/main/java/com/yahoo/search/rendering/JsonRenderer.java
index 34b02b1bee8..6c7018317c3 100644
--- a/container-search/src/main/java/com/yahoo/search/rendering/JsonRenderer.java
+++ b/container-search/src/main/java/com/yahoo/search/rendering/JsonRenderer.java
@@ -14,7 +14,7 @@ import com.yahoo.document.datatypes.FieldValue;
 import com.yahoo.document.datatypes.StringFieldValue;
 import com.yahoo.document.datatypes.TensorFieldValue;
 import com.yahoo.document.json.JsonWriter;
-import com.yahoo.prelude.fastsearch.FastHit;
+import com.yahoo.lang.MutableBoolean;
 import com.yahoo.processing.Response;
 import com.yahoo.processing.execution.Execution.Trace;
 import com.yahoo.processing.rendering.AsynchronousSectionedRenderer;
@@ -49,6 +49,7 @@ import java.io.IOException;
 import java.io.OutputStream;
 import java.io.PrintWriter;
 import java.io.StringWriter;
+import java.io.UncheckedIOException;
 import java.math.BigDecimal;
 import java.math.BigInteger;
 import java.nio.charset.StandardCharsets;
@@ -472,14 +473,14 @@ public class JsonRenderer extends AsynchronousSectionedRenderer<Result> {
         return ! (hit instanceof DefaultErrorHit);
     }
 
-    private boolean fieldsStart(boolean hasFieldsField) throws IOException {
-        if (hasFieldsField) return true;
+    private void fieldsStart(MutableBoolean hasFieldsField) throws IOException {
+        if (hasFieldsField.get()) return;
         generator.writeObjectFieldStart(FIELDS);
-        return true;
+        hasFieldsField.set(true);
     }
 
-    private void fieldsEnd(boolean hasFieldsField) throws IOException {
-        if (!hasFieldsField) return;
+    private void fieldsEnd(MutableBoolean hasFieldsField) throws IOException {
+        if ( ! hasFieldsField.get()) return;
         generator.writeEndObject();
     }
 
@@ -508,39 +509,37 @@ public class JsonRenderer extends AsynchronousSectionedRenderer<Result> {
     }
 
     private void renderAllFields(Hit hit) throws IOException {
-        boolean hasFieldsField = false;
-
-        hasFieldsField |= renderTotalHitCount(hit, hasFieldsField);
-        hasFieldsField |= renderStandardFields(hit, hasFieldsField);
+        MutableBoolean hasFieldsField = new MutableBoolean(false);
+        renderTotalHitCount(hit, hasFieldsField);
+        renderStandardFields(hit, hasFieldsField);
         fieldsEnd(hasFieldsField);
     }
 
-    private boolean renderStandardFields(Hit hit, boolean initialHasFieldsField) throws IOException {
-        boolean hasFieldsField = initialHasFieldsField;
-        for (String fieldName : hit.fieldKeys()) {
-            if (!shouldRender(fieldName, hit)) continue;
-
-            // We can't look at the size of fieldKeys() and know whether we need
-            // the fields object, as all fields may be hidden.
-            hasFieldsField |= fieldsStart(hasFieldsField);
-            renderField(fieldName, hit);
-        }
-        return hasFieldsField;
+    private void renderStandardFields(Hit hit, MutableBoolean hasFieldsField) {
+        hit.forEachField((name, value) -> {
+            try {
+                if (shouldRender(name, value)) {
+                    fieldsStart(hasFieldsField);
+                    renderField(name, value);
+                }
+            }
+            catch (IOException e) {
+                throw new UncheckedIOException(e);
+            }
+        });
     }
 
-    private boolean shouldRender(String fieldName, Hit hit) {
+    private boolean shouldRender(String name, Object value) {
         if (debugRendering) return true;
 
-        if (fieldName.startsWith(VESPA_HIDDEN_FIELD_PREFIX)) return false;
+        if (name.startsWith(VESPA_HIDDEN_FIELD_PREFIX)) return false;
 
-        Object field = hit.getField(fieldName);
-
-        if (field instanceof CharSequence && ((CharSequence) field).length() == 0) return false;
+        if (value instanceof CharSequence && ((CharSequence) value).length() == 0) return false;
 
         // StringFieldValue cannot hold a null, so checking length directly is OK:
-        if (field instanceof StringFieldValue && ((StringFieldValue) field).getString().isEmpty()) return false;
+        if (value instanceof StringFieldValue && ((StringFieldValue) value).getString().isEmpty()) return false;
 
-        if (field instanceof NanNumber) return false;
+        if (value instanceof NanNumber) return false;
 
         return true;
     }
@@ -607,17 +606,16 @@ public class JsonRenderer extends AsynchronousSectionedRenderer<Result> {
         return (id instanceof RawBucketId ? Arrays.toString(((RawBucketId) id).getTo()) : id.getTo()).toString();
     }
 
-    private boolean renderTotalHitCount(Hit hit, boolean hasFieldsField) throws IOException {
-        if ( ! (getRecursionLevel() == 1 && hit instanceof HitGroup)) return false;
+    private void renderTotalHitCount(Hit hit, MutableBoolean hasFieldsField) throws IOException {
+        if ( ! (getRecursionLevel() == 1 && hit instanceof HitGroup)) return;
 
         fieldsStart(hasFieldsField);
         generator.writeNumberField(TOTAL_COUNT, getResult().getTotalHitCount());
-        return true;
     }
 
-    private void renderField(String fieldName, Hit hit) throws IOException {
-        generator.writeFieldName(fieldName);
-        renderFieldContents(hit.getField(fieldName));
+    private void renderField(String name, Object value) throws IOException {
+        generator.writeFieldName(name);
+        renderFieldContents(value);
     }
 
     private void renderFieldContents(Object field) throws IOException {
diff --git a/container-search/src/main/java/com/yahoo/search/rendering/SyncDefaultRenderer.java b/container-search/src/main/java/com/yahoo/search/rendering/SyncDefaultRenderer.java
index dc72061e224..08599540cb1 100644
--- a/container-search/src/main/java/com/yahoo/search/rendering/SyncDefaultRenderer.java
+++ b/container-search/src/main/java/com/yahoo/search/rendering/SyncDefaultRenderer.java
@@ -271,25 +271,19 @@ public final class SyncDefaultRenderer extends Renderer {
 
     private void renderHitFields(XMLWriter writer, Hit hit) {
         renderSyntheticRelevanceField(writer, hit);
-        for (Iterator<Map.Entry<String, Object>> it = hit.fieldIterator(); it.hasNext(); ) {
-            renderField(writer, hit, it);
-        }
+        hit.forEachField((name, value) -> renderField(writer, name, value));
     }
 
-    private void renderField(XMLWriter writer, Hit hit, Iterator<Map.Entry<String, Object>> it) {
-        Map.Entry<String, Object> entry = it.next();
-        String fieldName = entry.getKey();
-
-        if ( ! shouldRenderField(hit, fieldName)) return;
-        if (fieldName.startsWith("$")) return; // Don't render fields that start with $ // TODO: Move to should render
+    private void renderField(XMLWriter writer, String name, Object value) {
+        if (name.startsWith("$")) return;
 
-        writeOpenFieldElement(writer, fieldName);
-        renderFieldContent(writer, hit, fieldName);
+        writeOpenFieldElement(writer, name);
+        renderFieldContent(writer, value);
         writeCloseFieldElement(writer);
     }
 
-    private void renderFieldContent(XMLWriter writer, Hit hit, String fieldName) {
-        writer.escapedContent(asXML(hit.getField(fieldName)), false);
+    private void renderFieldContent(XMLWriter writer, Object value) {
+        writer.escapedContent(asXML(value), false);
     }
 
     private String asXML(Object value) {
@@ -304,10 +298,10 @@ public final class SyncDefaultRenderer extends Renderer {
     }
 
     private void renderSyntheticRelevanceField(XMLWriter writer, Hit hit) {
-        final String relevancyFieldName = "relevancy";
-        final Relevance relevance = hit.getRelevance();
+        String relevancyFieldName = "relevancy";
+        Relevance relevance = hit.getRelevance();
 
-        if (shouldRenderField(hit, relevancyFieldName) && relevance != null) {
+        if (relevance != null) {
             renderSimpleField(writer, relevancyFieldName, relevance);
         }
     }
@@ -332,11 +326,6 @@ public final class SyncDefaultRenderer extends Renderer {
         writer.closeStartTag();
     }
 
-    private boolean shouldRenderField(Hit hit, String relevancyFieldName) {
-        // skip depending on hit type
-        return true;
-    }
-
     private void renderHitAttributes(XMLWriter writer, Hit hit) {
         writer.attribute(TYPE, hit.types().stream().collect(Collectors.joining(" ")));
         if (hit.getRelevance() != null) {
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 15c148b7db7..f68916c8a68 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
@@ -21,6 +21,7 @@ import java.util.LinkedHashSet;
 import java.util.Map;
 import java.util.Set;
 import java.util.TreeMap;
+import java.util.function.BiConsumer;
 import java.util.stream.Collectors;
 
 /**
@@ -401,10 +402,30 @@ public class Hit extends ListenableFreezableClass implements Data, Comparable<Hi
     /** Returns the name of the source creating this hit */
     public String getSource() { return source; }
 
+    /**
+     * Receive a callback on the given object for each field in this hit.
+     * This is the most resource efficient way of traversing all the fields of a hit.
+     */
+    public void forEachField(BiConsumer<String, Object> consumer) {
+        if (fields == null) return;
+        fields.forEach(consumer);
+    }
+
     /** Returns the fields of this as a read-only map. This is more costly than fieldIterator() */
-    // TODO Should it be deprecated ?
     public Map<String, Object> fields() { return getUnmodifiableFieldMap(); }
 
+    /** Returns a modifiable iterator over the fields of this */
+    public Iterator<Map.Entry<String, Object>> fieldIterator() { return getFieldMap().entrySet().iterator(); }
+
+    /**
+     * 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.
+     */
+    public Set<String> fieldKeys() {
+        return getFieldMap().keySet();
+    }
+
     /** Allocate room for the given number of fields to avoid resizing. */
     public void reserve(int minSize) {
         getFieldMap(minSize);
@@ -419,9 +440,6 @@ public class Hit extends ListenableFreezableClass implements Data, Comparable<Hi
         return getFieldMap().put(key, value);
     }
 
-    /** Returns a modifiable iterator over the fields of this */
-    public Iterator<Map.Entry<String, Object>> fieldIterator() { return getFieldMap().entrySet().iterator(); }
-
     /** Returns a field value or null if not present */
     public Object getField(String value) { return fields != null ? fields.get(value) : null; }
 
@@ -439,15 +457,6 @@ public class Hit extends ListenableFreezableClass implements Data, Comparable<Hi
         return getFieldMap().remove(field);
     }
 
-    /**
-     * 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.
-     */
-    public Set<String> fieldKeys() {
-        return getFieldMap().keySet();
-    }
-
     protected boolean hasField(String name) {
         return fields != null && fields.containsKey(name);
     }
diff --git a/container-search/src/main/java/com/yahoo/search/yql/FieldFilter.java b/container-search/src/main/java/com/yahoo/search/yql/FieldFilter.java
index 75c2865d0a5..14dce2f6342 100644
--- a/container-search/src/main/java/com/yahoo/search/yql/FieldFilter.java
+++ b/container-search/src/main/java/com/yahoo/search/yql/FieldFilter.java
@@ -52,12 +52,7 @@ public class FieldFilter extends Searcher {
         for (Iterator<Hit> i = result.hits().unorderedDeepIterator(); i.hasNext();) {
             Hit h = i.next();
             if (h.isMeta()) continue;
-            for (Iterator<Entry<String, Object>> fields = h.fieldIterator(); fields.hasNext();) {
-                Entry<String, Object> field = fields.next();
-                if ( ! requestedFields.contains(field.getKey()))
-                    fields.remove();
-            }
-
+            h.fieldKeys().retainAll(requestedFields);
         }
     }
 
diff --git a/container-search/src/test/java/com/yahoo/prelude/fastsearch/SlimeSummaryTestCase.java b/container-search/src/test/java/com/yahoo/prelude/fastsearch/SlimeSummaryTestCase.java
index 83871b559b4..1b74a6c5b29 100644
--- a/container-search/src/test/java/com/yahoo/prelude/fastsearch/SlimeSummaryTestCase.java
+++ b/container-search/src/test/java/com/yahoo/prelude/fastsearch/SlimeSummaryTestCase.java
@@ -31,6 +31,7 @@ import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 public class SlimeSummaryTestCase {
 
@@ -286,6 +287,14 @@ public class SlimeSummaryTestCase {
             assertEquals(field.getValue(), expected.get(field.getKey()));
         }
         assertEquals(expected.size(), fieldIteratorFieldCount);
+        // field traverser
+        Map<String, Object> traversed = new HashMap<>();
+        hit.forEachField((name, value) -> {
+            if (traversed.containsKey(name))
+                fail("Multiple callbacks for " + name);
+            traversed.put(name, value);
+        });
+        assertEquals(expected, traversed);
     }
 
     private byte[] emptySummary() {
diff --git a/container-search/src/test/java/com/yahoo/search/rendering/JsonRendererTestCase.java b/container-search/src/test/java/com/yahoo/search/rendering/JsonRendererTestCase.java
index defabc7463b..4a0075c0cf1 100644
--- a/container-search/src/test/java/com/yahoo/search/rendering/JsonRendererTestCase.java
+++ b/container-search/src/test/java/com/yahoo/search/rendering/JsonRendererTestCase.java
@@ -1069,7 +1069,6 @@ public class JsonRendererTestCase {
         r.getElapsedTime().add(t);
         renderer.setTimeSource(() -> 8L);
         String summary = render(r);
-        System.out.println(summary);
         assertEqualJson(expected, summary);
     }
 
diff --git a/container-search/src/test/java/com/yahoo/search/yql/FieldFilterTestCase.java b/container-search/src/test/java/com/yahoo/search/yql/FieldFilterTestCase.java
index f5097aec6e1..50313b630ea 100644
--- a/container-search/src/test/java/com/yahoo/search/yql/FieldFilterTestCase.java
+++ b/container-search/src/test/java/com/yahoo/search/yql/FieldFilterTestCase.java
@@ -20,9 +20,10 @@ import com.yahoo.search.searchchain.testutil.DocumentSourceSearcher;
 /**
  * Smoketest that we remove fields in a sane manner.
  *
- * @author <a href="mailto:steinar@yahoo-inc.com">Steinar Knutsen</a>
+ * @author Steinar Knutsen
  */
 public class FieldFilterTestCase {
+
     private static final String FIELD_C = "c";
     private static final String FIELD_B = "b";
     private static final String FIELD_A = "a";
@@ -41,8 +42,7 @@ public class FieldFilterTestCase {
         DocumentSourceSearcher mockBackend = new DocumentSourceSearcher();
         mockBackend.addResult(query, result);
 
-        searchChain = new Chain<Searcher>(new FieldFilter(),
-                mockBackend);
+        searchChain = new Chain<Searcher>(new FieldFilter(), mockBackend);
         context = Execution.Context.createContextStub(null);
         execution = new Execution(searchChain, context);
 
diff --git a/vespajlib/src/main/java/com/yahoo/lang/MutableBoolean.java b/vespajlib/src/main/java/com/yahoo/lang/MutableBoolean.java
new file mode 100644
index 00000000000..b009ffdd2b6
--- /dev/null
+++ b/vespajlib/src/main/java/com/yahoo/lang/MutableBoolean.java
@@ -0,0 +1,24 @@
+// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.lang;
+
+/**
+ * A mutable boolean
+ *
+ * @author bratseth
+ */
+public class MutableBoolean {
+
+    private boolean value;
+
+    public MutableBoolean(boolean value) {
+        this.value = value;
+    }
+
+    public boolean get() { return value; }
+
+    public void set(boolean value) { this.value = value; }
+
+    @Override
+    public String toString() { return Boolean.toString(value); }
+
+}
diff --git a/vespajlib/src/main/java/com/yahoo/lang/MutableInteger.java b/vespajlib/src/main/java/com/yahoo/lang/MutableInteger.java
index 34e928db674..a988a3f6fa2 100644
--- a/vespajlib/src/main/java/com/yahoo/lang/MutableInteger.java
+++ b/vespajlib/src/main/java/com/yahoo/lang/MutableInteger.java
@@ -30,4 +30,7 @@ public class MutableInteger {
         return value;
     }
 
+    @Override
+    public String toString() { return Integer.toString(value); }
+
 }
diff --git a/vespajlib/src/main/java/com/yahoo/lang/MutableLong.java b/vespajlib/src/main/java/com/yahoo/lang/MutableLong.java
index e0e4a0828a9..36d3a8a6343 100644
--- a/vespajlib/src/main/java/com/yahoo/lang/MutableLong.java
+++ b/vespajlib/src/main/java/com/yahoo/lang/MutableLong.java
@@ -30,4 +30,7 @@ public class MutableLong {
         return value;
     }
 
+    @Override
+    public String toString() { return Long.toString(value); }
+
 }