diff --git a/src/jmh/java/benchmark/IntMapBenchmark.java b/src/jmh/java/benchmark/IntMapBenchmark.java deleted file mode 100644 index b5b5272e41..0000000000 --- a/src/jmh/java/benchmark/IntMapBenchmark.java +++ /dev/null @@ -1,43 +0,0 @@ -package benchmark; - -import graphql.execution.instrumentation.dataloader.LevelMap; -import org.openjdk.jmh.annotations.Benchmark; -import org.openjdk.jmh.annotations.Fork; -import org.openjdk.jmh.annotations.Measurement; -import org.openjdk.jmh.annotations.Scope; -import org.openjdk.jmh.annotations.State; -import org.openjdk.jmh.annotations.Warmup; -import org.openjdk.jmh.infra.Blackhole; - -import java.util.LinkedHashMap; -import java.util.Map; - -@State(Scope.Benchmark) -@Warmup(iterations = 2, time = 5) -@Measurement(iterations = 3) -@Fork(2) -public class IntMapBenchmark { - - @Benchmark - public void benchmarkLinkedHashMap(Blackhole blackhole) { - Map result = new LinkedHashMap<>(); - for (int i = 0; i < 30; i++) { - int level = i % 10; - int count = i * 2; - result.put(level, result.getOrDefault(level, 0) + count); - blackhole.consume(result.get(level)); - } - } - - @Benchmark - public void benchmarkIntMap(Blackhole blackhole) { - LevelMap result = new LevelMap(16); - for (int i = 0; i < 30; i++) { - int level = i % 10; - int count = i * 2; - result.increment(level, count); - blackhole.consume(result.get(level)); - } - } -} - diff --git a/src/jmh/java/performance/DataLoaderPerformance.java b/src/jmh/java/performance/DataLoaderPerformance.java index 20e144abd8..6f81408d6a 100644 --- a/src/jmh/java/performance/DataLoaderPerformance.java +++ b/src/jmh/java/performance/DataLoaderPerformance.java @@ -4,6 +4,7 @@ import graphql.ExecutionInput; import graphql.ExecutionResult; import graphql.GraphQL; +import graphql.execution.instrumentation.dataloader.DataLoaderDispatchingContextKeys; import graphql.schema.DataFetcher; import graphql.schema.GraphQLSchema; import graphql.schema.idl.RuntimeWiring; @@ -14,16 +15,28 @@ import org.dataloader.DataLoader; import org.dataloader.DataLoaderFactory; import org.dataloader.DataLoaderRegistry; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Fork; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.OutputTimeUnit; import org.openjdk.jmh.annotations.Scope; import org.openjdk.jmh.annotations.Setup; import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Warmup; import org.openjdk.jmh.infra.Blackhole; import java.util.List; import java.util.Map; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; +@State(Scope.Benchmark) +@Warmup(iterations = 2, time = 5) +@Measurement(iterations = 3) +@Fork(2) public class DataLoaderPerformance { static Owner o1 = new Owner("O-1", "Andi", List.of("P-1", "P-2", "P-3")); @@ -560,6 +573,9 @@ public void setup() { } + @Benchmark + @BenchmarkMode(Mode.AverageTime) + @OutputTimeUnit(TimeUnit.MILLISECONDS) public void executeRequestWithDataLoaders(MyState myState, Blackhole blackhole) { DataLoader ownerDL = DataLoaderFactory.newDataLoader(ownerBatchLoader); DataLoader petDL = DataLoaderFactory.newDataLoader(petBatchLoader); @@ -571,7 +587,7 @@ public void executeRequestWithDataLoaders(MyState myState, Blackhole blackhole) .dataLoaderRegistry(registry) // .profileExecution(true) .build(); -// executionInput.getGraphQLContext().put(DataLoaderDispatchingContextKeys.ENABLE_DATA_LOADER_CHAINING, true); + executionInput.getGraphQLContext().put(DataLoaderDispatchingContextKeys.ENABLE_DATA_LOADER_CHAINING, true); ExecutionResult execute = myState.graphQL.execute(executionInput); // ProfilerResult profilerResult = executionInput.getGraphQLContext().get(ProfilerResult.PROFILER_CONTEXT_KEY); // System.out.println("execute: " + execute); diff --git a/src/main/java/graphql/execution/instrumentation/dataloader/LevelMap.java b/src/main/java/graphql/execution/instrumentation/dataloader/LevelMap.java deleted file mode 100644 index 45fdca37b9..0000000000 --- a/src/main/java/graphql/execution/instrumentation/dataloader/LevelMap.java +++ /dev/null @@ -1,79 +0,0 @@ -package graphql.execution.instrumentation.dataloader; - -import graphql.Internal; - -import java.util.Arrays; - -/** - * This data structure tracks the number of expected calls on a given level - */ -@Internal -public class LevelMap { - - // A reasonable default that guarantees no additional allocations for most use cases. - private static final int DEFAULT_INITIAL_SIZE = 16; - - // this array is mutable in both size and contents. - private int[] countsByLevel; - - public LevelMap(int initialSize) { - if (initialSize < 0) { - throw new IllegalArgumentException("negative size " + initialSize); - } - countsByLevel = new int[initialSize]; - } - - public LevelMap() { - this(DEFAULT_INITIAL_SIZE); - } - - public int get(int level) { - maybeResize(level); - return countsByLevel[level]; - } - - public void increment(int level, int by) { - maybeResize(level); - countsByLevel[level] += by; - } - - public void set(int level, int newValue) { - maybeResize(level); - countsByLevel[level] = newValue; - } - - private void maybeResize(int level) { - if (level < 0) { - throw new IllegalArgumentException("negative level " + level); - } - if (level + 1 > countsByLevel.length) { - int newSize = level == 0 ? 1 : level * 2; - countsByLevel = Arrays.copyOf(countsByLevel, newSize); - } - } - - @Override - public String toString() { - StringBuilder result = new StringBuilder(); - result.append("IntMap["); - for (int i = 0; i < countsByLevel.length; i++) { - result.append("[level=").append(i).append(",count=").append(countsByLevel[i]).append("] "); - } - result.append("]"); - return result.toString(); - } - - public String toString(int level) { - StringBuilder result = new StringBuilder(); - result.append("IntMap["); - for (int i = 1; i <= level; i++) { - result.append("level=").append(i).append(",count=").append(countsByLevel[i]).append(" "); - } - result.append("]"); - return result.toString(); - } - - public void clear() { - Arrays.fill(countsByLevel, 0); - } -} diff --git a/src/main/java/graphql/execution/instrumentation/dataloader/PerLevelDataLoaderDispatchStrategy.java b/src/main/java/graphql/execution/instrumentation/dataloader/PerLevelDataLoaderDispatchStrategy.java index 943bae4ffd..ae95166133 100644 --- a/src/main/java/graphql/execution/instrumentation/dataloader/PerLevelDataLoaderDispatchStrategy.java +++ b/src/main/java/graphql/execution/instrumentation/dataloader/PerLevelDataLoaderDispatchStrategy.java @@ -72,9 +72,16 @@ public StateForLevel(@Nullable DataLoaderInvocation dataLoaderInvocation, StateForLevel currentState = currentStateRef.get(); - boolean dispatchingStarted = currentState != null && currentState.dispatchingStarted; - boolean dispatchingFinished = currentState != null && currentState.dispatchingFinished; - boolean currentlyDelayedDispatching = currentState != null && currentState.currentlyDelayedDispatching; + boolean dispatchingStarted = false; + boolean dispatchingFinished = false; + boolean currentlyDelayedDispatching = false; + + if (currentState != null) { + dispatchingStarted = currentState.dispatchingStarted; + dispatchingFinished = currentState.dispatchingFinished; + currentlyDelayedDispatching = currentState.currentlyDelayedDispatching; + + } if (!chained) { if (normalDispatchOrDelayed) { @@ -107,10 +114,16 @@ public boolean newDataLoaderInvocation(DataLoaderInvocation dataLoaderInvocation while (true) { StateForLevel currentState = currentStateRef.get(); + boolean dispatchingStarted = false; + boolean dispatchingFinished = false; + boolean currentlyDelayedDispatching = false; - boolean dispatchingStarted = currentState != null && currentState.dispatchingStarted; - boolean dispatchingFinished = currentState != null && currentState.dispatchingFinished; - boolean currentlyDelayedDispatching = currentState != null && currentState.currentlyDelayedDispatching; + if (currentState != null) { + dispatchingStarted = currentState.dispatchingStarted; + dispatchingFinished = currentState.dispatchingFinished; + currentlyDelayedDispatching = currentState.currentlyDelayedDispatching; + + } // we need to start a new delayed dispatching if // the normal dispatching is finished and there is no currently delayed dispatching for this level @@ -135,6 +148,35 @@ public void clear() { private static class CallStack { + /** + * We track three things per level: + * - the number of execute object calls + * - the number of object completion calls + * - if the level is already dispatched + *

+ * The number of execute object calls is the number of times the execution + * of a field with sub selection (meaning it is an object) started. + *

+ * For each execute object call there will be one matching object completion call, + * indicating that the all fields in the sub selection have been fetched AND completed. + * Completion implies the fetched value is "resolved" (CompletableFuture is completed if it was a CF) + * and it the engine has processed it and called any needed subsequent execute object calls (if the result + * was none null and of Object of [Object] (or [[Object]] etc). + *

+ * Together we know a that a level is ready for dispatch if: + * - the parent was dispatched + * - the #executeObject == #completionFinished in the grandparent level. + *

+ * The second condition implies that all execute object calls in the parent level happened + * which again implies that all fetch fields in the current level have happened. + *

+ * For the first level we track only if all expected fetched field calls have happened. + */ + + /** + * The whole algo is impleted lock free and relies purely on CAS methods to handle concurrency. + */ + static class StateForLevel { private final int happenedCompletionFinishedCount; private final int happenedExecuteObjectCalls; diff --git a/src/test/groovy/graphql/execution/instrumentation/dataloader/LevelMapTest.groovy b/src/test/groovy/graphql/execution/instrumentation/dataloader/LevelMapTest.groovy deleted file mode 100644 index 1f0069fb19..0000000000 --- a/src/test/groovy/graphql/execution/instrumentation/dataloader/LevelMapTest.groovy +++ /dev/null @@ -1,113 +0,0 @@ -package graphql.execution.instrumentation.dataloader - -import spock.lang.Specification - -class LevelMapTest extends Specification { - - def "increase adds levels"() { - given: - LevelMap sut = new LevelMap(0) // starts empty - - when: - sut.increment(2, 42) // level 2 has count 42 - - then: - sut.get(0) == 0 - sut.get(1) == 0 - sut.get(2) == 42 - } - - def "increase count by 10 for every level"() { - given: - LevelMap sut = new LevelMap(0) - - when: - 5.times {Integer level -> - sut.increment(level, 10) - } - - then: - 5.times { Integer level -> - sut.get(level) == 10 - } - } - def "increase yields new count"() { - given: - LevelMap sut = new LevelMap(0) - - when: - sut.increment(1, 0) - - then: - sut.get(1) == 0 - - when: - sut.increment(1, 1) - - then: - sut.get(1) == 1 - - when: - sut.increment(1, 100) - - then: - sut.get(1) == 101 - } - - def "set yields new value"() { - given: - LevelMap sut = new LevelMap(0) - - when: - sut.set(1, 1) - - then: - sut.get(1) == 1 - - when: - sut.increment(1, 100) - - then: - sut.get(1) == 101 - - when: - sut.set(1, 666) - - then: - sut.get(1) == 666 - } - - def "toString() is important for debugging"() { - given: - LevelMap sut = new LevelMap(0) - - when: - sut.toString() - - then: - sut.toString() == "IntMap[]" - - when: - sut.increment(0, 42) - - then: - sut.toString() == "IntMap[[level=0,count=42] ]" - - when: - sut.increment(1, 1) - - then: - sut.toString() == "IntMap[[level=0,count=42] [level=1,count=1] ]" - } - - def "can get outside of its size"() { - given: - LevelMap sut = new LevelMap(0) - - when: - sut.get(1) - - then: - sut.get(1) == 0 - } -}