Skip to content

Commit

Permalink
Merge pull request #3236 from dfa1/reuse-result-path
Browse files Browse the repository at this point in the history
Minor performance fixes
  • Loading branch information
bbakerman committed Jun 2, 2023
2 parents d6a62a3 + 30db887 commit 7905df0
Show file tree
Hide file tree
Showing 5 changed files with 6 additions and 73 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package graphql.execution;

import com.google.common.collect.Maps;
import graphql.ExecutionResult;
import graphql.ExecutionResultImpl;
import graphql.PublicSpi;
Expand Down Expand Up @@ -28,10 +29,9 @@ protected BiConsumer<List<ExecutionResult>, Throwable> handleResults(ExecutionCo
handleNonNullException(executionContext, overallResult, exception);
return;
}
Map<String, Object> resolvedValuesByField = new LinkedHashMap<>(fieldNames.size());
Map<String, Object> resolvedValuesByField = Maps.newLinkedHashMapWithExpectedSize(fieldNames.size());
int ix = 0;
for (ExecutionResult executionResult : results) {

String fieldName = fieldNames.get(ix++);
resolvedValuesByField.put(fieldName, executionResult.getData());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ public CompletableFuture<ExecutionResult> execute(ExecutionContext executionCont
handleResultsConsumer.accept(null, throwable.getCause());
return;
}

Async.CombinedBuilder<ExecutionResult> executionResultFutures = Async.ofExpectedSize(completeValueInfos.size());
for (FieldValueInfo completeValueInfo : completeValueInfos) {
executionResultFutures.add(completeValueInfo.getFieldValue());
Expand Down
39 changes: 1 addition & 38 deletions src/main/java/graphql/execution/ExecutionStepInfoFactory.java
Original file line number Diff line number Diff line change
@@ -1,52 +1,15 @@
package graphql.execution;

import graphql.Internal;
import graphql.introspection.Introspection;
import graphql.language.Argument;
import graphql.schema.GraphQLCodeRegistry;
import graphql.schema.GraphQLFieldDefinition;
import graphql.schema.GraphQLList;
import graphql.schema.GraphQLObjectType;
import graphql.schema.GraphQLOutputType;
import graphql.util.FpKit;

import java.util.List;
import java.util.Map;
import java.util.function.Supplier;

@Internal
public class ExecutionStepInfoFactory {

public ExecutionStepInfo newExecutionStepInfoForSubField(ExecutionContext executionContext, MergedField mergedField, ExecutionStepInfo parentInfo) {
GraphQLObjectType parentType = (GraphQLObjectType) parentInfo.getUnwrappedNonNullType();
GraphQLFieldDefinition fieldDefinition = Introspection.getFieldDef(executionContext.getGraphQLSchema(), parentType, mergedField.getName());
GraphQLOutputType fieldType = fieldDefinition.getType();
List<Argument> fieldArgs = mergedField.getArguments();
GraphQLCodeRegistry codeRegistry = executionContext.getGraphQLSchema().getCodeRegistry();
Supplier<Map<String, Object>> argumentValuesSupplier = () -> ValuesResolver.getArgumentValues(codeRegistry,
fieldDefinition.getArguments(),
fieldArgs,
executionContext.getCoercedVariables(),
executionContext.getGraphQLContext(),
executionContext.getLocale());
Supplier<Map<String, Object>> argumentValues = FpKit.intraThreadMemoize(argumentValuesSupplier);

ResultPath newPath = parentInfo.getPath().segment(mergedField.getResultKey());

return parentInfo.transform(builder -> builder
.parentInfo(parentInfo)
.type(fieldType)
.fieldDefinition(fieldDefinition)
.fieldContainer(parentType)
.field(mergedField)
.path(newPath)
.arguments(argumentValues));
}

public ExecutionStepInfo newExecutionStepInfoForListElement(ExecutionStepInfo executionInfo, int index) {
public ExecutionStepInfo newExecutionStepInfoForListElement(ExecutionStepInfo executionInfo, ResultPath indexedPath) {
GraphQLList fieldType = (GraphQLList) executionInfo.getUnwrappedNonNullType();
GraphQLOutputType typeInList = (GraphQLOutputType) fieldType.getWrappedType();
ResultPath indexedPath = executionInfo.getPath().segment(index);
return executionInfo.transform(builder -> builder
.parentInfo(executionInfo)
.type(typeInList)
Expand Down
5 changes: 1 addition & 4 deletions src/main/java/graphql/execution/ExecutionStrategy.java
Original file line number Diff line number Diff line change
Expand Up @@ -569,19 +569,16 @@ protected FieldValueInfo completeValueForList(ExecutionContext executionContext,
for (Object item : iterableValues) {
ResultPath indexedPath = parameters.getPath().segment(index);

ExecutionStepInfo stepInfoForListElement = executionStepInfoFactory.newExecutionStepInfoForListElement(executionStepInfo, index);
ExecutionStepInfo stepInfoForListElement = executionStepInfoFactory.newExecutionStepInfoForListElement(executionStepInfo, indexedPath);

NonNullableFieldValidator nonNullableFieldValidator = new NonNullableFieldValidator(executionContext, stepInfoForListElement);

int finalIndex = index;
FetchedValue value = unboxPossibleDataFetcherResult(executionContext, parameters, item);

ExecutionStrategyParameters newParameters = parameters.transform(builder ->
builder.executionStepInfo(stepInfoForListElement)
.nonNullFieldValidator(nonNullableFieldValidator)
.listSize(size.orElse(-1)) // -1 signals that we don't know the size
.localContext(value.getLocalContext())
.currentListIndex(finalIndex)
.path(indexedPath)
.source(value.getFetchedValue())
);
Expand Down
30 changes: 1 addition & 29 deletions src/main/java/graphql/execution/ExecutionStrategyParameters.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ public class ExecutionStrategyParameters {
private final NonNullableFieldValidator nonNullableFieldValidator;
private final ResultPath path;
private final MergedField currentField;
private final int listSize;
private final int currentListIndex;
private final ExecutionStrategyParameters parent;

private ExecutionStrategyParameters(ExecutionStepInfo executionStepInfo,
Expand All @@ -30,8 +28,6 @@ private ExecutionStrategyParameters(ExecutionStepInfo executionStepInfo,
NonNullableFieldValidator nonNullableFieldValidator,
ResultPath path,
MergedField currentField,
int listSize,
int currentListIndex,
ExecutionStrategyParameters parent) {

this.executionStepInfo = assertNotNull(executionStepInfo, () -> "executionStepInfo is null");
Expand All @@ -41,8 +37,6 @@ private ExecutionStrategyParameters(ExecutionStepInfo executionStepInfo,
this.nonNullableFieldValidator = nonNullableFieldValidator;
this.path = path;
this.currentField = currentField;
this.listSize = listSize;
this.currentListIndex = currentListIndex;
this.parent = parent;
}

Expand Down Expand Up @@ -70,14 +64,6 @@ public Object getLocalContext() {
return localContext;
}

public int getListSize() {
return listSize;
}

public int getCurrentListIndex() {
return currentListIndex;
}

public ExecutionStrategyParameters getParent() {
return parent;
}
Expand Down Expand Up @@ -119,8 +105,6 @@ public static class Builder {
NonNullableFieldValidator nonNullableFieldValidator;
ResultPath path = ResultPath.rootPath();
MergedField currentField;
int listSize;
int currentListIndex;
ExecutionStrategyParameters parent;

/**
Expand All @@ -141,8 +125,6 @@ private Builder(ExecutionStrategyParameters oldParameters) {
this.currentField = oldParameters.currentField;
this.path = oldParameters.path;
this.parent = oldParameters.parent;
this.listSize = oldParameters.listSize;
this.currentListIndex = oldParameters.currentListIndex;
}

public Builder executionStepInfo(ExecutionStepInfo executionStepInfo) {
Expand Down Expand Up @@ -185,24 +167,14 @@ public Builder path(ResultPath path) {
return this;
}

public Builder listSize(int listSize) {
this.listSize = listSize;
return this;
}

public Builder currentListIndex(int currentListIndex) {
this.currentListIndex = currentListIndex;
return this;
}

public Builder parent(ExecutionStrategyParameters parent) {
this.parent = parent;
return this;
}


public ExecutionStrategyParameters build() {
return new ExecutionStrategyParameters(executionStepInfo, source, localContext, fields, nonNullableFieldValidator, path, currentField, listSize, currentListIndex, parent);
return new ExecutionStrategyParameters(executionStepInfo, source, localContext, fields, nonNullableFieldValidator, path, currentField, parent);
}
}
}

0 comments on commit 7905df0

Please sign in to comment.