Revert "Bratseth/allocation free hit field traversal"

3 files changed, 71 insertions, 49 deletions

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 2d69a262f15..bae1185d6a9 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,19 +185,28 @@ public final class DefaultRenderer extends AsynchronousSectionedRenderer<Result>
 
     private void renderHitFields(XMLWriter writer, Hit hit) {
         renderSyntheticRelevanceField(writer, hit);
-        hit.forEachField((name, value) -> renderField(writer, name, value));
+        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());
     }
 
-    private void renderField(XMLWriter writer, String name, Object value) {
-        if (name.startsWith("$")) return;
+    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
 
-        writeOpenFieldElement(writer, name);
-        renderFieldContent(writer, value);
+        writeOpenFieldElement(writer, fieldName);
+        renderFieldContent(writer, hit, fieldName);
         writeCloseFieldElement(writer);
     }
 
-    private void renderFieldContent(XMLWriter writer, Object value) {
-        writer.escapedContent(asXML(value), false);
+    private void renderFieldContent(XMLWriter writer, Hit hit, String fieldName) {
+        writer.escapedContent(asXML(hit.getField(fieldName)), 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 6c7018317c3..34b02b1bee8 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.lang.MutableBoolean;
+import com.yahoo.prelude.fastsearch.FastHit;
 import com.yahoo.processing.Response;
 import com.yahoo.processing.execution.Execution.Trace;
 import com.yahoo.processing.rendering.AsynchronousSectionedRenderer;
@@ -49,7 +49,6 @@ 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;
@@ -473,14 +472,14 @@ public class JsonRenderer extends AsynchronousSectionedRenderer<Result> {
         return ! (hit instanceof DefaultErrorHit);
     }
 
-    private void fieldsStart(MutableBoolean hasFieldsField) throws IOException {
-        if (hasFieldsField.get()) return;
+    private boolean fieldsStart(boolean hasFieldsField) throws IOException {
+        if (hasFieldsField) return true;
         generator.writeObjectFieldStart(FIELDS);
-        hasFieldsField.set(true);
+        return true;
     }
 
-    private void fieldsEnd(MutableBoolean hasFieldsField) throws IOException {
-        if ( ! hasFieldsField.get()) return;
+    private void fieldsEnd(boolean hasFieldsField) throws IOException {
+        if (!hasFieldsField) return;
         generator.writeEndObject();
     }
 
@@ -509,37 +508,39 @@ public class JsonRenderer extends AsynchronousSectionedRenderer<Result> {
     }
 
     private void renderAllFields(Hit hit) throws IOException {
-        MutableBoolean hasFieldsField = new MutableBoolean(false);
-        renderTotalHitCount(hit, hasFieldsField);
-        renderStandardFields(hit, hasFieldsField);
+        boolean hasFieldsField = false;
+
+        hasFieldsField |= renderTotalHitCount(hit, hasFieldsField);
+        hasFieldsField |= renderStandardFields(hit, hasFieldsField);
         fieldsEnd(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 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 boolean shouldRender(String name, Object value) {
+    private boolean shouldRender(String fieldName, Hit hit) {
         if (debugRendering) return true;
 
-        if (name.startsWith(VESPA_HIDDEN_FIELD_PREFIX)) return false;
+        if (fieldName.startsWith(VESPA_HIDDEN_FIELD_PREFIX)) return false;
 
-        if (value instanceof CharSequence && ((CharSequence) value).length() == 0) return false;
+        Object field = hit.getField(fieldName);
+
+        if (field instanceof CharSequence && ((CharSequence) field).length() == 0) return false;
 
         // StringFieldValue cannot hold a null, so checking length directly is OK:
-        if (value instanceof StringFieldValue && ((StringFieldValue) value).getString().isEmpty()) return false;
+        if (field instanceof StringFieldValue && ((StringFieldValue) field).getString().isEmpty()) return false;
 
-        if (value instanceof NanNumber) return false;
+        if (field instanceof NanNumber) return false;
 
         return true;
     }
@@ -606,16 +607,17 @@ public class JsonRenderer extends AsynchronousSectionedRenderer<Result> {
         return (id instanceof RawBucketId ? Arrays.toString(((RawBucketId) id).getTo()) : id.getTo()).toString();
     }
 
-    private void renderTotalHitCount(Hit hit, MutableBoolean hasFieldsField) throws IOException {
-        if ( ! (getRecursionLevel() == 1 && hit instanceof HitGroup)) return;
+    private boolean renderTotalHitCount(Hit hit, boolean hasFieldsField) throws IOException {
+        if ( ! (getRecursionLevel() == 1 && hit instanceof HitGroup)) return false;
 
         fieldsStart(hasFieldsField);
         generator.writeNumberField(TOTAL_COUNT, getResult().getTotalHitCount());
+        return true;
     }
 
-    private void renderField(String name, Object value) throws IOException {
-        generator.writeFieldName(name);
-        renderFieldContents(value);
+    private void renderField(String fieldName, Hit hit) throws IOException {
+        generator.writeFieldName(fieldName);
+        renderFieldContents(hit.getField(fieldName));
     }
 
     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 08599540cb1..dc72061e224 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,19 +271,25 @@ public final class SyncDefaultRenderer extends Renderer {
 
     private void renderHitFields(XMLWriter writer, Hit hit) {
         renderSyntheticRelevanceField(writer, hit);
-        hit.forEachField((name, value) -> renderField(writer, name, value));
+        for (Iterator<Map.Entry<String, Object>> it = hit.fieldIterator(); it.hasNext(); ) {
+            renderField(writer, hit, it);
+        }
     }
 
-    private void renderField(XMLWriter writer, String name, Object value) {
-        if (name.startsWith("$")) return;
+    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
 
-        writeOpenFieldElement(writer, name);
-        renderFieldContent(writer, value);
+        writeOpenFieldElement(writer, fieldName);
+        renderFieldContent(writer, hit, fieldName);
         writeCloseFieldElement(writer);
     }
 
-    private void renderFieldContent(XMLWriter writer, Object value) {
-        writer.escapedContent(asXML(value), false);
+    private void renderFieldContent(XMLWriter writer, Hit hit, String fieldName) {
+        writer.escapedContent(asXML(hit.getField(fieldName)), false);
     }
 
     private String asXML(Object value) {
@@ -298,10 +304,10 @@ public final class SyncDefaultRenderer extends Renderer {
     }
 
     private void renderSyntheticRelevanceField(XMLWriter writer, Hit hit) {
-        String relevancyFieldName = "relevancy";
-        Relevance relevance = hit.getRelevance();
+        final String relevancyFieldName = "relevancy";
+        final Relevance relevance = hit.getRelevance();
 
-        if (relevance != null) {
+        if (shouldRenderField(hit, relevancyFieldName) && relevance != null) {
             renderSimpleField(writer, relevancyFieldName, relevance);
         }
     }
@@ -326,6 +332,11 @@ 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) {