Fix some inefficiencies in FieldsVisitor (#127688)

Just a couple obvious finds. No need to start from `null` here,
just makes locality worse to so and adds conditionals. Also, we can save some field lookups
and a needless hot allocation of `BytesRef` that isn't guaranteed to be escape analyzed away.

Author: original-brownbear

server/src/main/java/org/elasticsearch/index/fieldvisitor/FieldsVisitor.java

@@ -29,8 +29,6 @@
 import java.util.Set;
 import java.util.function.Function;
 
-import static java.util.Collections.emptyMap;
-
 /**
  * Base {@link StoredFieldVisitor} that retrieves all non-redundant metadata.
  */
@@ -40,9 +38,9 @@ public class FieldsVisitor extends FieldNamesProvidingStoredFieldsVisitor {
     private final boolean loadSource;
     final String sourceFieldName;
     private final Set<String> requiredFields;
-    protected BytesReference source;
-    protected String id;
-    protected Map<String, List<Object>> fieldsValues;
+    private BytesReference source;
+    private String id;
+    private final Map<String, List<Object>> fieldsValues = new HashMap<>();
 
     public FieldsVisitor(boolean loadSource) {
         this(loadSource, SourceFieldMapper.NAME);
@@ -58,24 +56,29 @@ public FieldsVisitor(boolean loadSource, String sourceFieldName) {
 
     @Override
     public Status needsField(FieldInfo fieldInfo) {
-        if (requiredFields.remove(fieldInfo.name)) {
+        final String name = fieldInfo.name;
+        var requiredFields = this.requiredFields;
+        if (requiredFields.remove(name)) {
             return Status.YES;
         }
         // Always load _ignored to be explicit about ignored fields
         // This works because _ignored is added as the first metadata mapper,
         // so its stored fields always appear first in the list.
         // Note that _ignored is also multi-valued, which is why it can't be removed from the set like other fields
-        if (IgnoredFieldMapper.NAME.equals(fieldInfo.name)) {
+        if (IgnoredFieldMapper.NAME.equals(name)) {
             return Status.YES;
         }
+        // All these fields are single-valued so we can stop when the set is empty
+        if (requiredFields.isEmpty()) {
+            return Status.STOP;
+        }
         // support _uid for loading older indices
-        if ("_uid".equals(fieldInfo.name)) {
+        if ("_uid".equals(name)) {
             if (requiredFields.remove(IdFieldMapper.NAME) || requiredFields.remove(LegacyTypeFieldMapper.NAME)) {
                 return Status.YES;
             }
         }
-        // All these fields are single-valued so we can stop when the set is empty
-        return requiredFields.isEmpty() ? Status.STOP : Status.NO;
+        return Status.NO;
     }
 
     @Override
@@ -98,33 +101,31 @@ public final void postProcess(Function<String, MappedFieldType> fieldTypeLookup)
 
     @Override
     public void binaryField(FieldInfo fieldInfo, byte[] value) {
-        binaryField(fieldInfo, new BytesRef(value));
-    }
-
-    private void binaryField(FieldInfo fieldInfo, BytesRef value) {
-        if (sourceFieldName.equals(fieldInfo.name)) {
+        final String name = fieldInfo.name;
+        if (sourceFieldName.equals(name)) {
             source = new BytesArray(value);
-        } else if (IdFieldMapper.NAME.equals(fieldInfo.name)) {
-            id = Uid.decodeId(value.bytes, value.offset, value.length);
+        } else if (IdFieldMapper.NAME.equals(name)) {
+            id = Uid.decodeId(value, 0, value.length);
         } else {
-            addValue(fieldInfo.name, value);
+            addValue(name, new BytesRef(value));
         }
     }
 
     @Override
     public void stringField(FieldInfo fieldInfo, String value) {
-        assert sourceFieldName.equals(fieldInfo.name) == false : "source field must go through binaryField";
-        if ("_uid".equals(fieldInfo.name)) {
+        final String name = fieldInfo.name;
+        assert sourceFieldName.equals(name) == false : "source field must go through binaryField";
+        if ("_uid".equals(name)) {
             // 5.x-only
             int delimiterIndex = value.indexOf('#'); // type is not allowed to have # in it..., ids can
             String type = value.substring(0, delimiterIndex);
             id = value.substring(delimiterIndex + 1);
             addValue(LegacyTypeFieldMapper.NAME, type);
-        } else if (IdFieldMapper.NAME.equals(fieldInfo.name)) {
+        } else if (IdFieldMapper.NAME.equals(name)) {
             // only applies to 5.x indices that have single_type = true
             id = value;
         } else {
-            addValue(fieldInfo.name, value);
+            addValue(name, value);
         }
     }
 
@@ -157,25 +158,20 @@ public String id() {
     }
 
     public String routing() {
-        if (fieldsValues == null) {
-            return null;
-        }
         List<Object> values = fieldsValues.get(RoutingFieldMapper.NAME);
         if (values == null || values.isEmpty()) {
             return null;
         }
         assert values.size() == 1;
-        return values.get(0).toString();
+        return values.getFirst().toString();
     }
 
     public Map<String, List<Object>> fields() {
-        return fieldsValues != null ? fieldsValues : emptyMap();
+        return fieldsValues;
     }
 
     public void reset() {
-        if (fieldsValues != null) {
-            fieldsValues.clear();
-        }
+        fieldsValues.clear();
         source = null;
         id = null;
 
@@ -186,11 +182,6 @@ public void reset() {
     }
 
     void addValue(String name, Object value) {
-        if (fieldsValues == null) {
-            fieldsValues = new HashMap<>();
-        }
-
-        List<Object> values = fieldsValues.computeIfAbsent(name, k -> new ArrayList<>(2));
-        values.add(value);
+        fieldsValues.computeIfAbsent(name, k -> new ArrayList<>(2)).add(value);
     }
 }