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

Dataloader multilevel dispatch #990

Merged
merged 41 commits into from Jun 5, 2018

Conversation

Projects
None yet
2 participants
@andimarek
Member

andimarek commented Mar 23, 2018

this is an attempt to improve the performance of DataLoader for lists in lists: basically changing the ExecutionStrategy to a breadth deep traversal to batch load the maximum possible

andimarek added some commits Mar 23, 2018

stack.pop();
}
}
// TODO: should be threadsafe

This comment has been minimized.

@bbakerman

bbakerman Mar 27, 2018

Member

To make this threadsafe - invent a new type that encapsulates level + count and make that have read / write locks.

System.out.println(String.format("'%s' has %d depth", key, dataLoaderRegistry.getDataLoader(key).dispatchDepth()));
});
System.out.println("\n\n");

This comment has been minimized.

@bbakerman

bbakerman Mar 27, 2018

Member

Clearly the printfs need to go at some stage!

@bbakerman

This comment has been minimized.

Member

bbakerman commented Apr 4, 2018

@andimarek - I split the logic contained in the instrumentation class out into its own graphql.execution.instrumentation.dataloader.FieldLevelTrackingApproach class

This will allow us to include Sam Bs Braid code as another approach. We can then continue to test with both say and work out which is the better one. We might then retain one or the other or maybe both.

bbakerman and others added some commits May 12, 2018

Merge remote-tracking branch 'upstream/master' into dataloader-multil…
…evel-dispatch

# Conflicts:
#	src/main/java/graphql/util/FpKit.java

bbakerman added some commits Jun 2, 2018

Merge remote-tracking branch 'upstream/master' into dataloader-multil…
…evel-dispatch

# Conflicts:
#	src/main/java/graphql/execution/ExecutionStrategy.java
Renamed the resolveField2 and made the resolveField method delegate t…
…o it. This preserves backwards compatibility
@andimarek

This comment has been minimized.

Member

andimarek commented Jun 3, 2018

breaking change because of the change to the return type of ExecutionStrategyInstrumentationContext.beginExecutionStrategy.

Also because of the changed return values to completeValue etc.: they return now a FieldValueInfo.

andimarek and others added some commits Jun 3, 2018

Merge remote-tracking branch 'upstream/master' into dataloader-multil…
…evel-dispatch

# Conflicts:
#	src/main/java/graphql/execution/AsyncExecutionStrategy.java

@bbakerman bbakerman merged commit ca6f7fa into master Jun 5, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@andimarek andimarek added this to the 9.0 milestone Jun 5, 2018

@andimarek andimarek deleted the dataloader-multilevel-dispatch branch Jun 6, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment