diff --git a/src/main/java/graphql/execution/ExecutionContext.java b/src/main/java/graphql/execution/ExecutionContext.java index 0c406c6954..0a8fd6b144 100644 --- a/src/main/java/graphql/execution/ExecutionContext.java +++ b/src/main/java/graphql/execution/ExecutionContext.java @@ -15,8 +15,10 @@ import org.dataloader.DataLoaderRegistry; import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.CopyOnWriteArrayList; import java.util.function.Consumer; @@ -38,6 +40,7 @@ public class ExecutionContext { private final Object context; private final Instrumentation instrumentation; private final List errors = new CopyOnWriteArrayList<>(); + private final Set errorPaths = new HashSet<>(); private final DataLoaderRegistry dataLoaderRegistry; private final CacheControl cacheControl; private final DeferSupport deferSupport = new DeferSupport(); @@ -128,13 +131,8 @@ public void addError(GraphQLError error, ExecutionPath fieldPath) { // field errors should be handled - ie only once per field if its already there for nullability // but unclear if its not that error path // - for (GraphQLError graphQLError : errors) { - List path = graphQLError.getPath(); - if (path != null) { - if (fieldPath.equals(ExecutionPath.fromList(path))) { - return; - } - } + if (!errorPaths.add(fieldPath)) { + return; } this.errors.add(error); } @@ -149,6 +147,9 @@ public void addError(GraphQLError error) { // see https://github.com/graphql-java/graphql-java/issues/888 on how the spec is unclear // on how exactly multiple errors should be handled - ie only once per field or not outside the nullability // aspect. + if (error.getPath() != null) { + this.errorPaths.add(ExecutionPath.fromList(error.getPath())); + } this.errors.add(error); } diff --git a/src/test/java/benchmark/AddError.java b/src/test/java/benchmark/AddError.java new file mode 100644 index 0000000000..efd123acde --- /dev/null +++ b/src/test/java/benchmark/AddError.java @@ -0,0 +1,38 @@ +package benchmark; + +import graphql.execution.ExecutionContext; +import graphql.execution.ExecutionContextBuilder; +import graphql.execution.ExecutionId; +import graphql.execution.ExecutionPath; +import graphql.schema.idl.errors.SchemaMissingError; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Warmup; + +import java.util.Collections; + +@State(Scope.Benchmark) +public class AddError { + + private ExecutionContext context = new ExecutionContextBuilder() + .executionId(ExecutionId.generate()) + .build(); + + private volatile int x = 0; + + @Benchmark + @BenchmarkMode(Mode.SingleShotTime) + @Warmup(iterations = 1, batchSize = 50000) + @Measurement(iterations = 1, batchSize = 5000) + public ExecutionContext benchMarkAddError() { + context.addError( + new SchemaMissingError(), + ExecutionPath.fromList(Collections.singletonList(x++)) + ); + return context; + } +}