Skip to content

Commit 0b887a9

Browse files
bbakermanbbakerman
authored
This is some performance optimisations for NormalisedOperations (graphql-java#2732)
* Tweaks that make NormalisedOperations run faster in benchmarks * Filter set moved back into main Kit class as immutable * Improvement to benchmark but no improvements found
1 parent 83c97eb commit 0b887a9

7 files changed

Lines changed: 80 additions & 53 deletions

File tree

‎src/main/java/graphql/execution/ConditionalNodes.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ public class ConditionalNodes {
1919
ValuesResolver valuesResolver = new ValuesResolver();
2020

2121
public boolean shouldInclude(Map<String, Object> variables, List<Directive> directives) {
22+
// shortcut on no directives
23+
if (directives.isEmpty()) {
24+
return true;
25+
}
2226
boolean skip = getDirectiveResult(variables, directives, SkipDirective.getName(), false);
2327
if (skip) {
2428
return false;

‎src/main/java/graphql/execution/MergedField.java‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,9 @@ public class MergedField {
6363
private final ImmutableList<Field> fields;
6464
private final Field singleField;
6565

66-
private MergedField(List<Field> fields) {
66+
private MergedField(ImmutableList<Field> fields) {
6767
assertNotEmpty(fields);
68-
this.fields = ImmutableList.copyOf(fields);
68+
this.fields = fields;
6969
this.singleField = fields.get(0);
7070
}
7171

@@ -74,7 +74,7 @@ private MergedField(List<Field> fields) {
7474
*
7575
* WARNING: This is not always the key in the execution result, because of possible aliases. See {@link #getResultKey()}
7676
*
77-
* @return the name of of the merged fields.
77+
* @return the name of the merged fields.
7878
*/
7979
public String getName() {
8080
return singleField.getName();
@@ -141,14 +141,13 @@ public MergedField transform(Consumer<Builder> builderConsumer) {
141141

142142
public static class Builder {
143143

144-
private final List<Field> fields;
144+
private final ImmutableList.Builder<Field> fields = new ImmutableList.Builder<>();
145145

146146
private Builder() {
147-
this.fields = new ArrayList<>();
148147
}
149148

150149
private Builder(MergedField existing) {
151-
this.fields = new ArrayList<>(existing.getFields());
150+
fields.addAll(existing.getFields());
152151
}
153152

154153
public Builder fields(List<Field> fields) {
@@ -162,7 +161,7 @@ public Builder addField(Field field) {
162161
}
163162

164163
public MergedField build() {
165-
return new MergedField(fields);
164+
return new MergedField(fields.build());
166165
}
167166

168167
}

‎src/main/java/graphql/execution/ValuesResolver.java‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -146,8 +146,7 @@ public Map<String, NormalizedInputValue> getNormalizedVariableValues(GraphQLSche
146146
public Map<String, Object> getArgumentValues(List<GraphQLArgument> argumentTypes,
147147
List<Argument> arguments,
148148
Map<String, Object> coercedVariables) {
149-
GraphQLCodeRegistry codeRegistry = GraphQLCodeRegistry.newCodeRegistry().fieldVisibility(DEFAULT_FIELD_VISIBILITY).build();
150-
return getArgumentValuesImpl(codeRegistry, argumentTypes, arguments, coercedVariables);
149+
return getArgumentValuesImpl(DEFAULT_FIELD_VISIBILITY, argumentTypes, arguments, coercedVariables);
151150
}
152151

153152
/**
@@ -162,7 +161,6 @@ public Map<String, Object> getArgumentValues(List<GraphQLArgument> argumentTypes
162161
public Map<String, NormalizedInputValue> getNormalizedArgumentValues(List<GraphQLArgument> argumentTypes,
163162
List<Argument> arguments,
164163
Map<String, NormalizedInputValue> normalizedVariables) {
165-
GraphQLCodeRegistry codeRegistry = GraphQLCodeRegistry.newCodeRegistry().fieldVisibility(DEFAULT_FIELD_VISIBILITY).build();
166164
if (argumentTypes.isEmpty()) {
167165
return Collections.emptyMap();
168166
}
@@ -182,7 +180,7 @@ public Map<String, NormalizedInputValue> getNormalizedArgumentValues(List<GraphQ
182180
}
183181

184182
GraphQLInputType argumentType = argumentDefinition.getType();
185-
Object value = literalToNormalizedValue(codeRegistry.getFieldVisibility(), argumentType, argument.getValue(), normalizedVariables);
183+
Object value = literalToNormalizedValue(DEFAULT_FIELD_VISIBILITY, argumentType, argument.getValue(), normalizedVariables);
186184
result.put(argumentName, new NormalizedInputValue(simplePrint(argumentType), value));
187185
}
188186
return result;
@@ -192,7 +190,7 @@ public Map<String, Object> getArgumentValues(GraphQLCodeRegistry codeRegistry,
192190
List<GraphQLArgument> argumentTypes,
193191
List<Argument> arguments,
194192
Map<String, Object> coercedVariables) {
195-
return getArgumentValuesImpl(codeRegistry, argumentTypes, arguments, coercedVariables);
193+
return getArgumentValuesImpl(codeRegistry.getFieldVisibility(), argumentTypes, arguments, coercedVariables);
196194
}
197195

198196
public static Value<?> valueToLiteral(InputValueWithState inputValueWithState, GraphQLType type) {
@@ -427,7 +425,7 @@ private Map<String, Object> externalValueToInternalValueForVariables(GraphQLSche
427425
}
428426

429427

430-
private Map<String, Object> getArgumentValuesImpl(GraphQLCodeRegistry codeRegistry,
428+
private Map<String, Object> getArgumentValuesImpl(GraphqlFieldVisibility fieldVisibility,
431429
List<GraphQLArgument> argumentTypes,
432430
List<Argument> arguments,
433431
Map<String, Object> coercedVariables
@@ -455,7 +453,7 @@ private Map<String, Object> getArgumentValuesImpl(GraphQLCodeRegistry codeRegist
455453
}
456454
if (!hasValue && argumentDefinition.hasSetDefaultValue()) {
457455
Object coercedDefaultValue = defaultValueToInternalValue(
458-
codeRegistry.getFieldVisibility(),
456+
fieldVisibility,
459457
defaultValue,
460458
argumentType);
461459
coercedValues.put(argumentName, coercedDefaultValue);
@@ -467,7 +465,7 @@ private Map<String, Object> getArgumentValuesImpl(GraphQLCodeRegistry codeRegist
467465
} else if (argumentValue instanceof VariableReference) {
468466
coercedValues.put(argumentName, value);
469467
} else {
470-
value = literalToInternalValue(codeRegistry.getFieldVisibility(), argumentType, argument.getValue(), coercedVariables);
468+
value = literalToInternalValue(fieldVisibility, argumentType, argument.getValue(), coercedVariables);
471469
coercedValues.put(argumentName, value);
472470
}
473471
}

‎src/main/java/graphql/normalized/ExecutableNormalizedOperationFactory.java‎

Lines changed: 30 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,10 @@
3737
import java.util.Collection;
3838
import java.util.Collections;
3939
import java.util.LinkedHashMap;
40-
import java.util.LinkedHashSet;
4140
import java.util.List;
4241
import java.util.Map;
4342
import java.util.Set;
43+
import java.util.function.Predicate;
4444

4545
import static graphql.Assert.assertNotNull;
4646
import static graphql.Assert.assertShouldNeverHappen;
@@ -235,41 +235,44 @@ public CollectNFResult collectFromMergedField(FieldCollectorNormalizedQueryParam
235235
possibleObjects
236236
);
237237
}
238+
Map<String, List<CollectedField>> fieldsByName = fieldsByResultKey(collectedFields);
239+
ImmutableList.Builder<ExecutableNormalizedField> resultNFs = ImmutableList.builder();
240+
ImmutableListMultimap.Builder<ExecutableNormalizedField, FieldAndAstParent> normalizedFieldToAstFields = ImmutableListMultimap.builder();
241+
242+
createNFs(resultNFs, parameters, fieldsByName, normalizedFieldToAstFields, level, executableNormalizedField);
243+
244+
return new CollectNFResult(resultNFs.build(), normalizedFieldToAstFields.build());
245+
}
246+
247+
private Map<String, List<CollectedField>> fieldsByResultKey(List<CollectedField> collectedFields) {
238248
Map<String, List<CollectedField>> fieldsByName = new LinkedHashMap<>();
239249
for (CollectedField collectedField : collectedFields) {
240250
fieldsByName.computeIfAbsent(collectedField.field.getResultKey(), ignored -> new ArrayList<>()).add(collectedField);
241251
}
242-
List<ExecutableNormalizedField> resultNFs = new ArrayList<>();
243-
ImmutableListMultimap.Builder<ExecutableNormalizedField, FieldAndAstParent> normalizedFieldToAstFields = ImmutableListMultimap.builder();
244-
245-
createNFs(parameters, fieldsByName, resultNFs, normalizedFieldToAstFields, level, executableNormalizedField);
246-
247-
return new CollectNFResult(resultNFs, normalizedFieldToAstFields.build());
252+
return fieldsByName;
248253
}
249254

250255
public CollectNFResult collectFromOperation(FieldCollectorNormalizedQueryParams parameters,
251256
OperationDefinition operationDefinition,
252257
GraphQLObjectType rootType) {
253-
Set<GraphQLObjectType> possibleObjects = new LinkedHashSet<>();
254-
possibleObjects.add(rootType);
258+
259+
260+
Set<GraphQLObjectType> possibleObjects = ImmutableSet.of(rootType);
255261
List<CollectedField> collectedFields = new ArrayList<>();
256262
collectFromSelectionSet(parameters, operationDefinition.getSelectionSet(), collectedFields, rootType, possibleObjects);
257263
// group by result key
258-
Map<String, List<CollectedField>> fieldsByName = new LinkedHashMap<>();
259-
for (CollectedField collectedField : collectedFields) {
260-
fieldsByName.computeIfAbsent(collectedField.field.getResultKey(), ignored -> new ArrayList<>()).add(collectedField);
261-
}
262-
List<ExecutableNormalizedField> resultNFs = new ArrayList<>();
264+
Map<String, List<CollectedField>> fieldsByName = fieldsByResultKey(collectedFields);
265+
ImmutableList.Builder<ExecutableNormalizedField> resultNFs = ImmutableList.builder();
263266
ImmutableListMultimap.Builder<ExecutableNormalizedField, FieldAndAstParent> normalizedFieldToAstFields = ImmutableListMultimap.builder();
264267

265-
createNFs(parameters, fieldsByName, resultNFs, normalizedFieldToAstFields, 1, null);
268+
createNFs(resultNFs, parameters, fieldsByName, normalizedFieldToAstFields, 1, null);
266269

267-
return new CollectNFResult(resultNFs, normalizedFieldToAstFields.build());
270+
return new CollectNFResult(resultNFs.build(), normalizedFieldToAstFields.build());
268271
}
269272

270-
private void createNFs(FieldCollectorNormalizedQueryParams parameters,
273+
private void createNFs(ImmutableList.Builder<ExecutableNormalizedField> nfListBuilder,
274+
FieldCollectorNormalizedQueryParams parameters,
271275
Map<String, List<CollectedField>> fieldsByName,
272-
List<ExecutableNormalizedField> resultNFs,
273276
ImmutableListMultimap.Builder<ExecutableNormalizedField, FieldAndAstParent> normalizedFieldToAstFields,
274277
int level,
275278
ExecutableNormalizedField parent) {
@@ -284,7 +287,7 @@ private void createNFs(FieldCollectorNormalizedQueryParams parameters,
284287
for (CollectedField collectedField : fieldGroup.fields) {
285288
normalizedFieldToAstFields.put(nf, new FieldAndAstParent(collectedField.field, collectedField.astTypeCondition));
286289
}
287-
resultNFs.add(nf);
290+
nfListBuilder.add(nf);
288291
}
289292
if (commonParentsGroups.size() > 1) {
290293
parameters.addPossibleMergers(parent, resultKey);
@@ -308,7 +311,8 @@ private ExecutableNormalizedField createNF(FieldCollectorNormalizedQueryParams p
308311
normalizedArgumentValues = valuesResolver.getNormalizedArgumentValues(fieldDefinition.getArguments(), field.getArguments(), parameters.getNormalizedVariableValues());
309312
}
310313
ImmutableList<String> objectTypeNames = map(objectTypes, GraphQLObjectType::getName);
311-
ExecutableNormalizedField executableNormalizedField = ExecutableNormalizedField.newNormalizedField()
314+
315+
return ExecutableNormalizedField.newNormalizedField()
312316
.alias(field.getAlias())
313317
.resolvedArguments(argumentValues)
314318
.normalizedArguments(normalizedArgumentValues)
@@ -318,8 +322,6 @@ private ExecutableNormalizedField createNF(FieldCollectorNormalizedQueryParams p
318322
.level(level)
319323
.parent(parent)
320324
.build();
321-
322-
return executableNormalizedField;
323325
}
324326

325327
private static class CollectedFieldGroup {
@@ -333,20 +335,21 @@ public CollectedFieldGroup(Set<CollectedField> fields, Set<GraphQLObjectType> ob
333335
}
334336

335337
private List<CollectedFieldGroup> groupByCommonParents(Collection<CollectedField> fields) {
336-
Set<GraphQLObjectType> allRelevantObjects = new LinkedHashSet<>();
338+
ImmutableSet.Builder<GraphQLObjectType> objectTypes = ImmutableSet.builder();
337339
for (CollectedField collectedField : fields) {
338-
allRelevantObjects.addAll(collectedField.objectTypes);
340+
objectTypes.addAll(collectedField.objectTypes);
339341
}
342+
Set<GraphQLObjectType> allRelevantObjects = objectTypes.build();
340343
Map<GraphQLType, ImmutableList<CollectedField>> groupByAstParent = groupingBy(fields, fieldAndType -> fieldAndType.astTypeCondition);
341344
if (groupByAstParent.size() == 1) {
342-
return singletonList(new CollectedFieldGroup(new LinkedHashSet<>(fields), allRelevantObjects));
345+
return singletonList(new CollectedFieldGroup(ImmutableSet.copyOf(fields), allRelevantObjects));
343346
}
344-
List<CollectedFieldGroup> result = new ArrayList<>();
347+
ImmutableList.Builder<CollectedFieldGroup> result = ImmutableList.builder();
345348
for (GraphQLObjectType objectType : allRelevantObjects) {
346349
Set<CollectedField> relevantFields = filterSet(fields, field -> field.objectTypes.contains(objectType));
347350
result.add(new CollectedFieldGroup(relevantFields, singleton(objectType)));
348351
}
349-
return result;
352+
return result.build();
350353
}
351354

352355

‎src/main/java/graphql/util/FpKit.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33

44
import com.google.common.collect.ImmutableList;
5+
import com.google.common.collect.ImmutableSet;
56
import graphql.Internal;
67

78
import java.lang.reflect.Array;
@@ -279,13 +280,13 @@ public static <T> List<T> filterList(Collection<T> list, Predicate<T> filter) {
279280
}
280281

281282
public static <T> Set<T> filterSet(Collection<T> input, Predicate<T> filter) {
282-
LinkedHashSet<T> result = new LinkedHashSet<>();
283+
ImmutableSet.Builder<T> result = ImmutableSet.builder();
283284
for (T t : input) {
284285
if (filter.test(t)) {
285286
result.add(t);
286287
}
287288
}
288-
return result;
289+
return result.build();
289290
}
290291

291292
/**

src/test/java/benchmark/NQBenchmark.java renamed to src/test/java/benchmark/NQBenchmark1.java

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,11 @@
1919
import org.openjdk.jmh.annotations.State;
2020
import org.openjdk.jmh.annotations.Threads;
2121
import org.openjdk.jmh.annotations.Warmup;
22+
import org.openjdk.jmh.infra.Blackhole;
2223

2324
import java.io.IOException;
2425
import java.net.URL;
2526
import java.util.Collections;
26-
import java.util.concurrent.ExecutionException;
2727
import java.util.concurrent.TimeUnit;
2828

2929
import static com.google.common.io.Resources.getResource;
@@ -32,9 +32,9 @@
3232
@BenchmarkMode(Mode.Throughput)
3333
@Warmup(iterations = 2)
3434
@Measurement(iterations = 2, timeUnit = TimeUnit.NANOSECONDS)
35-
public class NQBenchmark {
35+
public class NQBenchmark1 {
3636

37-
@org.openjdk.jmh.annotations.State(Scope.Benchmark)
37+
@State(Scope.Benchmark)
3838
public static class MyState {
3939

4040
GraphQLSchema schema;
@@ -62,15 +62,29 @@ private String readFromClasspath(String file) throws IOException {
6262

6363
@Benchmark
6464
@Warmup(iterations = 2)
65-
@Measurement(iterations = 100, time = 10)
65+
@Measurement(iterations = 3, time = 10)
6666
@Threads(1)
6767
@Fork(3)
6868
@BenchmarkMode(Mode.AverageTime)
6969
@OutputTimeUnit(TimeUnit.MILLISECONDS)
70-
public ExecutableNormalizedOperation benchMarkAvgTime(MyState myState) throws ExecutionException, InterruptedException {
70+
public void benchMarkAvgTime(MyState myState, Blackhole blackhole ) {
71+
runImpl(myState, blackhole);
72+
}
73+
74+
@Benchmark
75+
@Warmup(iterations = 2)
76+
@Measurement(iterations = 3, time = 10)
77+
@Threads(1)
78+
@Fork(3)
79+
@BenchmarkMode(Mode.Throughput)
80+
@OutputTimeUnit(TimeUnit.SECONDS)
81+
public void benchMarkThroughput(MyState myState, Blackhole blackhole ) {
82+
runImpl(myState, blackhole);
83+
}
84+
85+
private void runImpl(MyState myState, Blackhole blackhole) {
7186
ExecutableNormalizedOperation executableNormalizedOperation = ExecutableNormalizedOperationFactory.createExecutableNormalizedOperation(myState.schema, myState.document, null, Collections.emptyMap());
72-
// System.out.println("fields size:" + normalizedQuery.getFieldToNormalizedField().size());
73-
return executableNormalizedOperation;
87+
blackhole.consume(executableNormalizedOperation);
7488
}
7589

7690

‎src/test/java/benchmark/OverlappingFieldValidationBenchmark.java‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import org.openjdk.jmh.annotations.State;
2626
import org.openjdk.jmh.annotations.Threads;
2727
import org.openjdk.jmh.annotations.Warmup;
28+
import org.openjdk.jmh.infra.Blackhole;
2829

2930
import java.io.IOException;
3031
import java.net.URL;
@@ -75,8 +76,15 @@ private String readFromClasspath(String file) throws IOException {
7576
}
7677

7778
@Benchmark
78-
public List<ValidationError> overlappingFieldValidation(MyState myState) {
79-
return validateQuery(myState.schema, myState.document);
79+
public void overlappingFieldValidationAbgTime(MyState myState, Blackhole blackhole) {
80+
blackhole.consume(validateQuery(myState.schema, myState.document));
81+
}
82+
83+
@Benchmark
84+
@BenchmarkMode(Mode.Throughput)
85+
@OutputTimeUnit(TimeUnit.SECONDS)
86+
public void overlappingFieldValidationThroughput(MyState myState, Blackhole blackhole) {
87+
blackhole.consume(validateQuery(myState.schema, myState.document));
8088
}
8189

8290

0 commit comments

Comments
 (0)