Speed up synthetic source again

server/src/main/java/org/elasticsearch/index/mapper/ObjectMapper.java

@@ -604,19 +604,34 @@ public DocValuesLoader docValuesLoader(LeafReader leafReader, int[] docIdsInLeaf
                     loaders.add(loader);
                 }
             }
-            return docId -> {
+            if (loaders.isEmpty()) {
+                return null;
+            }
+            return new ObjectDocValuesLoader(loaders);
+        }
+
+        private class ObjectDocValuesLoader implements DocValuesLoader {
+            private final List<SourceLoader.SyntheticFieldLoader.DocValuesLoader> loaders;
+
+            private ObjectDocValuesLoader(List<DocValuesLoader> loaders) {
+                this.loaders = loaders;
+            }
+
+            @Override
+            public boolean advanceToDoc(int docId) throws IOException {
+                boolean anyLeafHasDocValues = false;
                 for (SourceLoader.SyntheticFieldLoader.DocValuesLoader docValueLoader : loaders) {
                     boolean leafHasValue = docValueLoader.advanceToDoc(docId);
-                    hasValue |= leafHasValue;
+                    anyLeafHasDocValues |= leafHasValue;
                 }
-                /*
-                 * Important and kind of sneaky note: this will return true
-                 * if there were any values loaded from stored fields as
-                 * well. That *is* how we "wake up" objects that contain just
-                 * stored field.
-                 */
-                return hasValue;
-            };
+                hasValue |= anyLeafHasDocValues;
+                return anyLeafHasDocValues;
+            }
+        }
+
+        @Override
+        public boolean hasValue() {
+            return hasValue;
         }
 
         @Override
@@ -626,7 +641,9 @@ public void write(XContentBuilder b) throws IOException {
             }
             startSyntheticField(b);
             for (SourceLoader.SyntheticFieldLoader field : fields) {
-                field.write(b);
+                if (field.hasValue()) {
+                    field.write(b);
+                }
             }
             b.endObject();
             hasValue = false;

server/src/main/java/org/elasticsearch/index/mapper/SortedNumericDocValuesSyntheticFieldLoader.java

@@ -63,6 +63,11 @@ public DocValuesLoader docValuesLoader(LeafReader reader, int[] docIdsInLeaf) th
         return loader;
     }
 
+    @Override
+    public boolean hasValue() {
+        return values.count() > 0;
+    }
+
     @Override
     public void write(XContentBuilder b) throws IOException {
         switch (values.count()) {

server/src/main/java/org/elasticsearch/index/mapper/SortedSetDocValuesSyntheticFieldLoader.java

@@ -67,6 +67,11 @@ public DocValuesLoader docValuesLoader(LeafReader reader, int[] docIdsInLeaf) th
         return loader;
     }
 
+    @Override
+    public boolean hasValue() {
+        return values.count() > 0;
+    }
+
     @Override
     public void write(XContentBuilder b) throws IOException {
         switch (values.count()) {

server/src/main/java/org/elasticsearch/index/mapper/SourceLoader.java

@@ -106,8 +106,18 @@ public Set<String> requiredStoredFields() {
 
         @Override
         public Leaf leaf(LeafReader reader, int[] docIdsInLeaf) throws IOException {
-            SyntheticFieldLoader.DocValuesLoader leaf = loader.docValuesLoader(reader, docIdsInLeaf);
-            return (fieldsVisitor, docId) -> {
+            return new SyntheticLeaf(loader.docValuesLoader(reader, docIdsInLeaf));
+        }
+
+        private class SyntheticLeaf implements Leaf {
+            private final SyntheticFieldLoader.DocValuesLoader docValuesLoader;
+
+            private SyntheticLeaf(SyntheticFieldLoader.DocValuesLoader docValuesLoader) {
+                this.docValuesLoader = docValuesLoader;
+            }
+
+            @Override
+            public BytesReference source(FieldsVisitor fieldsVisitor, int docId) throws IOException {
                 if (fieldsVisitor != null) {
                     for (Map.Entry<String, List<Object>> e : fieldsVisitor.fields().entrySet()) {
                         SyntheticFieldLoader.StoredFieldLoader loader = storedFieldLoaders.get(e.getKey());
@@ -116,16 +126,19 @@ public Leaf leaf(LeafReader reader, int[] docIdsInLeaf) throws IOException {
                         }
                     }
                 }
+                if (docValuesLoader != null) {
+                    docValuesLoader.advanceToDoc(docId);
+                }
                 // TODO accept a requested xcontent type
                 try (XContentBuilder b = new XContentBuilder(JsonXContent.jsonXContent, new ByteArrayOutputStream())) {
-                    if (leaf.advanceToDoc(docId)) {
+                    if (loader.hasValue()) {
                         loader.write(b);
                     } else {
                         b.startObject().endObject();
                     }
                     return BytesReference.bytes(b);
                 }
-            };
+            }
         }
     }
 
@@ -175,6 +188,11 @@ public DocValuesLoader docValuesLoader(LeafReader leafReader, int[] docIdsInLeaf
                 return null;
             }
 
+            @Override
+            public boolean hasValue() {
+                return false;
+            }
+
             @Override
             public void write(XContentBuilder b) {}
         };
@@ -192,6 +210,8 @@ public void write(XContentBuilder b) {}
          */
         DocValuesLoader docValuesLoader(LeafReader leafReader, int[] docIdsInLeaf) throws IOException;
 
+        boolean hasValue();
+
         /**
          * Write values for this document.
          */

server/src/main/java/org/elasticsearch/index/mapper/StringStoredFieldFieldLoader.java

@@ -39,6 +39,11 @@ public void load(List<Object> values) {
         this.values = values;
     }
 
+    @Override
+    public boolean hasValue() {
+        return values != null && values.isEmpty() == false;
+    }
+
     @Override
     public void write(XContentBuilder b) throws IOException {
         if (values == null || values.isEmpty()) {

server/src/test/java/org/elasticsearch/index/mapper/ObjectMapperTests.java

@@ -23,6 +23,7 @@
 
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.instanceOf;
+import static org.hamcrest.Matchers.nullValue;
 
 public class ObjectMapperTests extends MapperServiceTestCase {
 
@@ -516,4 +517,43 @@ public void testSubobjectsCannotBeUpdatedOnRoot() throws IOException {
         );
         assertEquals("the [subobjects] parameter can't be updated for the object mapping [_doc]", exception.getMessage());
     }
+
+    /**
+     * Makes sure that an empty object mapper returns {@code null} from
+     * {@link SourceLoader.SyntheticFieldLoader#docValuesLoader}. This
+     * is important because it allows us to skip whole chains of empty
+     * fields when loading synthetic {@code _source}.
+     */
+    public void testSyntheticSourceDocValuesEmpty() throws IOException {
+        DocumentMapper mapper = createDocumentMapper(mapping(b -> b.startObject("o").field("type", "object").endObject()));
+        ObjectMapper o = (ObjectMapper) mapper.mapping().getRoot().getMapper("o");
+        assertThat(o.syntheticFieldLoader().docValuesLoader(null, null), nullValue());
+        assertThat(mapper.mapping().getRoot().syntheticFieldLoader().docValuesLoader(null, null), nullValue());
+    }
+
+    /**
+     * Makes sure that an object mapper containing only fields without
+     * doc values returns {@code null} from
+     * {@link SourceLoader.SyntheticFieldLoader#docValuesLoader}. This
+     * is important because it allows us to skip whole chains of empty
+     * fields when loading synthetic {@code _source}.
+     */
+    public void testSyntheticSourceDocValuesFieldWithout() throws IOException {
+        DocumentMapper mapper = createDocumentMapper(mapping(b -> {
+            b.startObject("o").startObject("properties");
+            {
+                b.startObject("kwd");
+                {
+                    b.field("type", "keyword");
+                    b.field("doc_values", false);
+                    b.field("store", true);
+                }
+                b.endObject();
+            }
+            b.endObject().endObject();
+        }));
+        ObjectMapper o = (ObjectMapper) mapper.mapping().getRoot().getMapper("o");
+        assertThat(o.syntheticFieldLoader().docValuesLoader(null, null), nullValue());
+        assertThat(mapper.mapping().getRoot().syntheticFieldLoader().docValuesLoader(null, null), nullValue());
+    }
 }

x-pack/plugin/mapper-aggregate-metric/src/main/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapper.java

@@ -725,6 +725,11 @@ public DocValuesLoader docValuesLoader(LeafReader reader, int[] docIdsInLeaf) th
             return new AggregateDocValuesLoader();
         }
 
+        @Override
+        public boolean hasValue() {
+            return metricHasValue.isEmpty() == false;
+        }
+
         @Override
         public void write(XContentBuilder b) throws IOException {
             if (metricHasValue.isEmpty()) {

x-pack/plugin/mapper-constant-keyword/src/main/java/org/elasticsearch/xpack/constantkeyword/mapper/ConstantKeywordFieldMapper.java

@@ -311,6 +311,11 @@ protected String contentType() {
 
     @Override
     public SourceLoader.SyntheticFieldLoader syntheticFieldLoader() {
+        String value = fieldType().value();
+        ;
+        if (value == null) {
+            return SourceLoader.SyntheticFieldLoader.NOTHING;
+        }
         return new SourceLoader.SyntheticFieldLoader() {
             @Override
             public Stream<Map.Entry<String, StoredFieldLoader>> storedFieldLoaders() {
@@ -319,19 +324,14 @@ public Stream<Map.Entry<String, StoredFieldLoader>> storedFieldLoaders() {
 
             @Override
             public DocValuesLoader docValuesLoader(LeafReader reader, int[] docIdsInLeaf) {
-                /*
-                 * If there is a value we need to enable objects containing these
-                 * fields. We could build something special for fields that are
-                 * always "on", but constant_keyword fields are rare enough that
-                 * having an extra doc values loader that always returns `true`
-                 * isn't a big performance hit and gets the job done.
-                 */
-                if (fieldType().value == null) {
-                    return null;
-                }
                 return docId -> true;
             }
 
+            @Override
+            public boolean hasValue() {
+                return true;
+            }
+
             @Override
             public void write(XContentBuilder b) throws IOException {
                 if (fieldType().value != null) {