Merge pull request #3499 from graphql-java/performance-branch

Get fieldDef improvements from Netflix
graphql-java · Mar 5, 2024 · 6b7f6b7 · 6b7f6b7
2 parents 5077ffb + 1b3e9d4
commit 6b7f6b7
Show file tree

Hide file tree

Showing 6 changed files with 43 additions and 30 deletions.
diff --git a/src/main/java/graphql/execution/ExecutionStrategy.java b/src/main/java/graphql/execution/ExecutionStrategy.java
@@ -348,9 +348,9 @@ protected CompletableFuture<FieldValueInfo> resolveFieldWithInfo(ExecutionContex
                 new InstrumentationFieldParameters(executionContext, executionStepInfo), executionContext.getInstrumentationState()
         ));
 
-        CompletableFuture<FetchedValue> fetchFieldFuture = fetchField(executionContext, parameters);
+        CompletableFuture<FetchedValue> fetchFieldFuture = fetchField(fieldDef, executionContext, parameters);
         CompletableFuture<FieldValueInfo> result = fetchFieldFuture.thenApply((fetchedValue) ->
-                completeField(executionContext, parameters, fetchedValue));
+                completeField(fieldDef, executionContext, parameters, fetchedValue));
 
         CompletableFuture<Object> fieldValueFuture = result.thenCompose(FieldValueInfo::getFieldValueFuture);
 
@@ -377,7 +377,12 @@ protected CompletableFuture<FetchedValue> fetchField(ExecutionContext executionC
         MergedField field = parameters.getField();
         GraphQLObjectType parentType = (GraphQLObjectType) parameters.getExecutionStepInfo().getUnwrappedNonNullType();
         GraphQLFieldDefinition fieldDef = getFieldDef(executionContext.getGraphQLSchema(), parentType, field.getSingleField());
-        GraphQLCodeRegistry codeRegistry = executionContext.getGraphQLSchema().getCodeRegistry();
+        return fetchField(fieldDef, executionContext, parameters);
+    }
+
+    private CompletableFuture<FetchedValue> fetchField(GraphQLFieldDefinition fieldDef, ExecutionContext executionContext, ExecutionStrategyParameters parameters) {
+        MergedField field = parameters.getField();
+        GraphQLObjectType parentType = (GraphQLObjectType) parameters.getExecutionStepInfo().getUnwrappedNonNullType();
 
         // if the DF (like PropertyDataFetcher) does not use the arguments or execution step info then dont build any
 
@@ -412,6 +417,8 @@ protected CompletableFuture<FetchedValue> fetchField(ExecutionContext executionC
                     .queryDirectives(queryDirectives)
                     .build();
         });
+
+        GraphQLCodeRegistry codeRegistry = executionContext.getGraphQLSchema().getCodeRegistry();
         DataFetcher<?> dataFetcher = codeRegistry.getDataFetcher(parentType, fieldDef);
 
         Instrumentation instrumentation = executionContext.getInstrumentation();
@@ -548,6 +555,11 @@ protected FieldValueInfo completeField(ExecutionContext executionContext, Execut
         Field field = parameters.getField().getSingleField();
         GraphQLObjectType parentType = (GraphQLObjectType) parameters.getExecutionStepInfo().getUnwrappedNonNullType();
         GraphQLFieldDefinition fieldDef = getFieldDef(executionContext.getGraphQLSchema(), parentType, field);
+        return completeField(fieldDef, executionContext, parameters, fetchedValue);
+    }
+
+    private FieldValueInfo completeField(GraphQLFieldDefinition fieldDef, ExecutionContext executionContext, ExecutionStrategyParameters parameters, FetchedValue fetchedValue) {
+        GraphQLObjectType parentType = (GraphQLObjectType) parameters.getExecutionStepInfo().getUnwrappedNonNullType();
         ExecutionStepInfo executionStepInfo = createExecutionStepInfo(executionContext, parameters, fieldDef, parentType);
 
         Instrumentation instrumentation = executionContext.getInstrumentation();

diff --git a/src/main/java/graphql/execution/FieldCollector.java b/src/main/java/graphql/execution/FieldCollector.java
@@ -148,10 +148,7 @@ private void collectField(FieldCollectorParameters parameters, Map<String, Merge
                     .addDeferredExecution(deferredExecution))
             );
         } else {
-            fields.put(name, MergedField
-                    .newMergedField(field)
-                    .addDeferredExecution(deferredExecution).build()
-            );
+            fields.put(name, MergedField.newSingletonMergedField(field, deferredExecution));
         }
     }
 

diff --git a/src/main/java/graphql/execution/MergedField.java b/src/main/java/graphql/execution/MergedField.java
@@ -73,6 +73,10 @@ private MergedField(ImmutableList<Field> fields, ImmutableList<DeferredExecution
         this.deferredExecutions = deferredExecutions;
     }
 
+    private MergedField(Field field, DeferredExecution deferredExecution) {
+        this(ImmutableList.of(field), deferredExecution == null ? ImmutableList.of() : ImmutableList.of(deferredExecution));
+    }
+
     /**
      * All merged fields have the same name.
      *
@@ -131,11 +135,10 @@ public List<Field> getFields() {
      * @return all defer executions.
      */
     @ExperimentalApi
-    public ImmutableList<DeferredExecution> getDeferredExecutions() {
+    public List<DeferredExecution> getDeferredExecutions() {
         return deferredExecutions;
     }
 
-
     public static Builder newMergedField() {
         return new Builder();
     }
@@ -148,6 +151,10 @@ public static Builder newMergedField(List<Field> fields) {
         return new Builder().fields(fields);
     }
 
+    static MergedField newSingletonMergedField(Field field, DeferredExecution deferredExecution) {
+        return new MergedField(field, deferredExecution);
+    }
+
     public MergedField transform(Consumer<Builder> builderConsumer) {
         Builder builder = new Builder(this);
         builderConsumer.accept(builder);

diff --git a/src/main/java/graphql/execution/MergedSelectionSet.java b/src/main/java/graphql/execution/MergedSelectionSet.java
@@ -13,10 +13,12 @@
 @PublicApi
 public class MergedSelectionSet {
 
-    private final ImmutableMap<String, MergedField> subFields;
+    private final Map<String, MergedField> subFields;
+    private final List<String> keys;
 
     protected MergedSelectionSet(Map<String, MergedField> subFields) {
-        this.subFields = ImmutableMap.copyOf(Assert.assertNotNull(subFields));
+        this.subFields = subFields == null ? ImmutableMap.of() : subFields;
+        this.keys =  ImmutableList.copyOf(this.subFields.keySet());
     }
 
     public Map<String, MergedField> getSubFields() {
@@ -40,7 +42,7 @@ public MergedField getSubField(String key) {
     }
 
     public List<String> getKeys() {
-        return ImmutableList.copyOf(keySet());
+        return keys;
     }
 
     public boolean isEmpty() {
@@ -52,10 +54,10 @@ public static Builder newMergedSelectionSet() {
     }
 
     public static class Builder {
-        private Map<String, MergedField> subFields = ImmutableMap.of();
 
-        private Builder() {
+        private Map<String, MergedField> subFields;
 
+        private Builder() {
         }
 
         public Builder subFields(Map<String, MergedField> subFields) {

diff --git a/src/main/java/graphql/schema/GraphQLInterfaceType.java b/src/main/java/graphql/schema/GraphQLInterfaceType.java
@@ -2,12 +2,12 @@
 
 import com.google.common.collect.ImmutableList;
 import graphql.Assert;
-import graphql.AssertException;
 import graphql.DirectivesUtil;
 import graphql.Internal;
 import graphql.PublicApi;
 import graphql.language.InterfaceTypeDefinition;
 import graphql.language.InterfaceTypeExtensionDefinition;
+import graphql.util.FpKit;
 import graphql.util.TraversalControl;
 import graphql.util.TraverserContext;
 
@@ -20,12 +20,12 @@
 import java.util.function.UnaryOperator;
 
 import static graphql.Assert.assertNotNull;
+import static graphql.Assert.assertShouldNeverHappen;
 import static graphql.Assert.assertValidName;
 import static graphql.collect.ImmutableKit.emptyList;
 import static graphql.schema.GraphqlTypeComparators.sortTypes;
 import static graphql.util.FpKit.getByName;
 import static graphql.util.FpKit.valuesToList;
-import static java.lang.String.format;
 
 /**
  * In graphql, an interface is an abstract type that defines the set of fields that a type must include to
@@ -41,7 +41,7 @@ public class GraphQLInterfaceType implements GraphQLNamedType, GraphQLCompositeT
 
     private final String name;
     private final String description;
-    private final Map<String, GraphQLFieldDefinition> fieldDefinitionsByName = new LinkedHashMap<>();
+    private final Map<String, GraphQLFieldDefinition> fieldDefinitionsByName;
     private final TypeResolver typeResolver;
     private final InterfaceTypeDefinition definition;
     private final ImmutableList<InterfaceTypeExtensionDefinition> extensionDefinitions;
@@ -78,17 +78,12 @@ private GraphQLInterfaceType(String name,
         this.originalInterfaces = ImmutableList.copyOf(sortTypes(interfaceComparator, interfaces));
         this.extensionDefinitions = ImmutableList.copyOf(extensionDefinitions);
         this.directivesHolder = new DirectivesUtil.DirectivesHolder(directives, appliedDirectives);
-        buildDefinitionMap(fieldDefinitions);
+        this.fieldDefinitionsByName = buildDefinitionMap(fieldDefinitions);
     }
 
-    private void buildDefinitionMap(List<GraphQLFieldDefinition> fieldDefinitions) {
-        for (GraphQLFieldDefinition fieldDefinition : fieldDefinitions) {
-            String name = fieldDefinition.getName();
-            if (fieldDefinitionsByName.containsKey(name)) {
-                throw new AssertException(format("Duplicated definition for field '%s' in interface '%s'", name, this.name));
-            }
-            fieldDefinitionsByName.put(name, fieldDefinition);
-        }
+    private Map<String, GraphQLFieldDefinition> buildDefinitionMap(List<GraphQLFieldDefinition> fieldDefinitions) {
+        return FpKit.getByName(fieldDefinitions, GraphQLFieldDefinition::getName,
+                (fld1, fld2) -> assertShouldNeverHappen("Duplicated definition for field '%s' in interface '%s'", fld1.getName(), this.name));
     }
 
     @Override

diff --git a/src/main/java/graphql/schema/GraphQLObjectType.java b/src/main/java/graphql/schema/GraphQLObjectType.java
@@ -44,7 +44,7 @@ public class GraphQLObjectType implements GraphQLNamedOutputType, GraphQLComposi
     private final String name;
     private final String description;
     private final Comparator<? super GraphQLSchemaElement> interfaceComparator;
-    private final ImmutableMap<String, GraphQLFieldDefinition> fieldDefinitionsByName;
+    private final Map<String, GraphQLFieldDefinition> fieldDefinitionsByName;
     private final ImmutableList<GraphQLNamedOutputType> originalInterfaces;
     private final DirectivesUtil.DirectivesHolder directivesHolder;
     private final ObjectTypeDefinition definition;
@@ -82,9 +82,9 @@ void replaceInterfaces(List<GraphQLNamedOutputType> interfaces) {
         this.replacedInterfaces = ImmutableList.copyOf(sortTypes(interfaceComparator, interfaces));
     }
 
-    private ImmutableMap<String, GraphQLFieldDefinition> buildDefinitionMap(List<GraphQLFieldDefinition> fieldDefinitions) {
-        return ImmutableMap.copyOf(FpKit.getByName(fieldDefinitions, GraphQLFieldDefinition::getName,
-                (fld1, fld2) -> assertShouldNeverHappen("Duplicated definition for field '%s' in type '%s'", fld1.getName(), this.name)));
+    private Map<String, GraphQLFieldDefinition> buildDefinitionMap(List<GraphQLFieldDefinition> fieldDefinitions) {
+        return FpKit.getByName(fieldDefinitions, GraphQLFieldDefinition::getName,
+                (fld1, fld2) -> assertShouldNeverHappen("Duplicated definition for field '%s' in type '%s'", fld1.getName(), this.name));
     }
 
     @Override