Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Defer dataloaders #1

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ public CompletableFuture<ExecutionResult> execute(ExecutionContext executionCont
for (FieldValueInfo completeValueInfo : completeValueInfos) {
fieldValuesFutures.addObject(completeValueInfo.getFieldValueObject());
}
// This calls increases the happenedOnFieldValueCalls count - which will trigger the DL dispatch
dataLoaderDispatcherStrategy.executionStrategyOnFieldValuesInfo(completeValueInfos, parameters);
executionStrategyCtx.onFieldValuesInfo(completeValueInfos);
fieldValuesFutures.await().whenComplete(handleResultsConsumer);
Expand Down
1 change: 1 addition & 0 deletions src/main/java/graphql/execution/ExecutionStrategy.java
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@ public static String mkNameForPath(List<Field> currentField) {
}

Async.CombinedBuilder<Object> resultFutures = fieldValuesCombinedBuilder(completeValueInfos);
// Why is this not being called for shops[2]?
dataLoaderDispatcherStrategy.executeObjectOnFieldValuesInfo(completeValueInfos, parameters);
resolveObjectCtx.onFieldValuesInfo(completeValueInfos);
resultFutures.await().whenComplete(handleResultsConsumer);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
public class LevelMap {

// A reasonable default that guarantees no additional allocations for most use cases.
private static final int DEFAULT_INITIAL_SIZE = 16;
private static final int DEFAULT_INITIAL_SIZE = 4;

// this array is mutable in both size and contents.
private int[] countsByLevel;
Expand Down Expand Up @@ -54,13 +54,14 @@ private void maybeResize(int level) {

@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();
// 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();
return Arrays.toString(countsByLevel);
}

public String toString(int level) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ private static class CallStack {
private final Set<Integer> dispatchedLevels = new LinkedHashSet<>();

public CallStack() {
// Why is this initialized to 1?
expectedStrategyCallsPerLevel.set(1, 1);
}

Expand Down Expand Up @@ -99,22 +100,28 @@ public PerLevelDataLoaderDispatchStrategy(ExecutionContext executionContext) {

@Override
public void deferredField(ExecutionContext executionContext, MergedField currentField) {
throw new UnsupportedOperationException("Data Loaders cannot be used to resolve deferred fields");
// TODO: Implement this
System.out.println("deferredField");
}

@Override
public void executionStrategy(ExecutionContext executionContext, ExecutionStrategyParameters parameters) {
System.out.println("executionStrategy");
int curLevel = parameters.getExecutionStepInfo().getPath().getLevel() + 1;
increaseCallCounts(curLevel, parameters);
}

@Override
public void executionStrategyOnFieldValuesInfo(List<FieldValueInfo> fieldValueInfoList, ExecutionStrategyParameters parameters) {
System.out.println("executionStrategyOnFieldValuesInfo");
// Who calls this?
int curLevel = parameters.getPath().getLevel() + 1;
onFieldValuesInfoDispatchIfNeeded(fieldValueInfoList, curLevel, parameters);
}

@Override
public void executionStrategyOnFieldValuesException(Throwable t, ExecutionStrategyParameters executionStrategyParameters) {
System.out.println("executionStrategyOnFieldValuesException");
int curLevel = executionStrategyParameters.getPath().getLevel() + 1;
callStack.lock.runLocked(() ->
callStack.increaseHappenedOnFieldValueCalls(curLevel)
Expand All @@ -124,19 +131,23 @@ public void executionStrategyOnFieldValuesException(Throwable t, ExecutionStrate

@Override
public void executeObject(ExecutionContext executionContext, ExecutionStrategyParameters parameters) {
System.out.println("executeObject");
int curLevel = parameters.getExecutionStepInfo().getPath().getLevel() + 1;
increaseCallCounts(curLevel, parameters);
}

@Override
public void executeObjectOnFieldValuesInfo(List<FieldValueInfo> fieldValueInfoList, ExecutionStrategyParameters parameters) {
System.out.println("executeObjectOnFieldValuesInfo: " + parameters);
// Who calls this?
int curLevel = parameters.getPath().getLevel() + 1;
onFieldValuesInfoDispatchIfNeeded(fieldValueInfoList, curLevel, parameters);
}


@Override
public void executeObjectOnFieldValuesException(Throwable t, ExecutionStrategyParameters parameters) {
System.out.println("executeObjectOnFieldValuesException");
int curLevel = parameters.getPath().getLevel() + 1;
callStack.lock.runLocked(() ->
callStack.increaseHappenedOnFieldValueCalls(curLevel)
Expand Down Expand Up @@ -220,6 +231,21 @@ private boolean levelReady(int level) {
// level 1 is special: there is only one strategy call and that's it
return callStack.allFetchesHappened(1);
}

// StringBuilder sb = new StringBuilder();
// sb.append("fetchCountPerLevel(").append(callStack.fetchCountPerLevel).append(") ");
// sb.append("\n");
// sb.append("expectedFetchCountPerLevel(").append(callStack.expectedFetchCountPerLevel).append(") ");
// sb.append("\n");
// sb.append("happenedStrategyCallsPerLevel(").append(callStack.happenedStrategyCallsPerLevel).append(") ");
// sb.append("\n");
// sb.append("expectedStrategyCallsPerLevel(").append(callStack.expectedStrategyCallsPerLevel).append(") ");
// sb.append("\n");
// sb.append("happenedOnFieldValueCallsPerLevel(").append(callStack.happenedOnFieldValueCallsPerLevel).append(") ");
// sb.append("\n");
// sb.append("-------------------------------");
// System.out.println(sb);

if (levelReady(level - 1) && callStack.allOnFieldCallsHappened(level - 1)
&& callStack.allStrategyCallsHappened(level) && callStack.allFetchesHappened(level)) {

Expand All @@ -230,7 +256,9 @@ private boolean levelReady(int level) {

void dispatch(int level) {
DataLoaderRegistry dataLoaderRegistry = executionContext.getDataLoaderRegistry();
// NOTE: dispatches all
dataLoaderRegistry.dispatchAll();
System.out.println("dispatching level " + level);
}

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -265,13 +265,22 @@ class DataLoaderPerformanceData {

static def deferredQuery = """
query {
shops {
id name
... @defer {
departments {
id name
products {
id name
# 4 shops - 1 null, 3 not
shops {
id # property data fetcher
name
... @defer(if: true) {
# the data fetcher for data loaders doesn't really do anything (returns CF not-resolved)
# Data Loader dispatch is when the actual data gets fetched. Batch loader is then called


# note: data loader also does optimization: duplicated IDs, etc....
departments { # 3 calls - 1 per non-null shop
id
name # data loader can be used on any field (for example: name)
products { # 1 + 2 + 3 = 6 products
id
name
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ class DataLoaderPerformanceWithChainedInstrumentationTest extends Specification
batchCompareDataFetchers.productsForDepartmentsBatchLoaderCounter.get() == 1

where:
incrementalSupport << [true, false]
incrementalSupport << [true]
}

def "chainedInstrumentation: 970 ensure data loader is performant for multiple field with lists using async batch loading"() {
Expand Down Expand Up @@ -141,7 +141,6 @@ class DataLoaderPerformanceWithChainedInstrumentationTest extends Specification
exception.message == "Data Loaders cannot be used to resolve deferred fields"
}

@Ignore("Resolution of deferred fields via Data loaders is not yet supported")
def "chainedInstrumentation: data loader will work with deferred queries"() {

when:
Expand All @@ -152,7 +151,7 @@ class DataLoaderPerformanceWithChainedInstrumentationTest extends Specification
.graphQLContext([(ENABLE_INCREMENTAL_SUPPORT): true])
.build()

IncrementalExecutionResult result = graphQL.execute(executionInput)
IncrementalExecutionResult result = graphQL.execute(executionInput) as IncrementalExecutionResult

then:
result.toSpecification() == expectedInitialDeferredData
Expand Down
Loading