Explain and test

Author: nik9000

server/src/main/java/org/elasticsearch/search/fetch/StoredFieldsSpec.java

@@ -44,6 +44,10 @@ public StoredFieldsSpec merge(StoredFieldsSpec other) {
         }
         Set<String> mergedFields;
         if (other.requiredStoredFields.isEmpty()) {
+            /*
+             * In the very very common case that we don't need new stored fields
+             * let's not clone the existing array.
+             */
             mergedFields = this.requiredStoredFields;
         } else {
             mergedFields = new HashSet<>(this.requiredStoredFields);

server/src/test/java/org/elasticsearch/search/fetch/StoredFieldsSpecTests.java

@@ -18,7 +18,10 @@
 import org.elasticsearch.search.internal.SearchContext;
 import org.elasticsearch.test.ESTestCase;
 
+import java.util.Set;
+
 import static org.hamcrest.Matchers.hasSize;
+import static org.hamcrest.Matchers.sameInstance;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -67,6 +70,18 @@ public void testScriptFieldsEnableMetadata() {
         assertTrue(spec.requiresMetadata());
     }
 
+    public void testNoCloneOnMerge() {
+        StoredFieldsSpec spec = StoredFieldsSpec.NO_REQUIREMENTS;
+        spec = spec.merge(StoredFieldsSpec.NEEDS_SOURCE);
+        assertThat(spec.requiredStoredFields(), sameInstance(StoredFieldsSpec.NO_REQUIREMENTS.requiredStoredFields()));
+
+        StoredFieldsSpec needsCat = new StoredFieldsSpec(false, false, Set.of("cat"));
+        StoredFieldsSpec withCat = spec.merge(needsCat);
+        spec = withCat.merge(StoredFieldsSpec.NO_REQUIREMENTS);
+        assertThat(spec.requiredStoredFields(), sameInstance(withCat.requiredStoredFields()));
+    }
+
+
     private static SearchContext searchContext(SearchSourceBuilder sourceBuilder) {
         SearchContext sc = mock(SearchContext.class);
         when(sc.fetchSourceContext()).thenReturn(sourceBuilder.fetchSource());