adding Profiler #3976
adding Profiler #3976
Conversation
Test Results 324 files 324 suites 4m 16s ⏱️ Results for commit 572090e. ♻️ This comment has been updated with latest results. |
|
|
||
| } | ||
|
|
||
|
|
# Conflicts: # src/main/java/graphql/execution/instrumentation/dataloader/PerLevelDataLoaderDispatchStrategy.java # src/main/java/graphql/schema/DataFetchingEnvironmentImpl.java
# Conflicts: # src/main/java/graphql/EngineRunningState.java # src/main/java/graphql/ExecutionInput.java # src/main/java/graphql/GraphQL.java # src/main/java/graphql/execution/ExecutionContext.java # src/test/groovy/graphql/execution/AsyncExecutionStrategyTest.groovy # src/test/groovy/graphql/execution/AsyncSerialExecutionStrategyTest.groovy # src/test/groovy/graphql/execution/ExecutionStrategyTest.groovy # src/test/groovy/graphql/execution/instrumentation/fieldvalidation/FieldValidationTest.groovy
| public Builder profileExecution(boolean profileExecution) { | ||
| this.profileExecution = profileExecution; | ||
| return this; | ||
| } |
There was a problem hiding this comment.
For another PR, not this one
Can you add the toggle to GraphQLUnusualConfiguration instead?
|
|
||
| @Override | ||
| public void chainedStrategyDispatching(int level) { | ||
| profilerResult.chainedStrategyDispatching(level); |
There was a problem hiding this comment.
To make it more obvious to a user, can comment on this method (or interface method) to indicate that this refers to the option DataLoaderDispatchingContextKeys.ENABLE_DATA_LOADER_CHAINING
I think to us, we know what Chained DataLoaders are but this is not a well known phrase
I'll also make a note to mention in the documentation, this is a way to measure the impact of Chained DataLoaders
|
|
||
| @Internal | ||
| @NullMarked | ||
| public interface Profiler { |
There was a problem hiding this comment.
Add a quick JavaDoc explaining what this does
A Profiler monitors important execution timings and counters. Enable this via ExecutionInput/new Unusual Configuration method
Of course 1 sentence isn't enough, let's add a documentation link here when it's written
There was a problem hiding this comment.
This is an internal class btw
| import java.util.List; | ||
| import java.util.concurrent.CompletableFuture; | ||
| import java.util.concurrent.atomic.AtomicLong; | ||
|
|
There was a problem hiding this comment.
Not for this PR, but in the future, with more Profiler features you might want to introduce options to toggle sets of tracking on and off
e.g. toggle DataLoader related tracking on and off
|
|
||
| } | ||
|
|
||
| default <V> void manualDispatch(String dataLoaderName, int level, int count) { |
While we recently added a Java agent to profile a GraphQL Java application the following things became clear:
Java agents are not the future of the JVM: soon a dynamic loading will be forbidden. See https://openjdk.org/jeps/451
We added the ability to track the engine running state
We added the ability to chain DataLoaders, which as a side effect allow for tracking which dataloaders are used by which field / DataFetcher
Together this means Java agents are not really future proof + they are not needed anymore, as we have all the abilities now inside GraphQL Java directly itself.
The goal is to remove the Java agent eventually and offer a better alternative in form of this PRs: a built-in Profiler in GraphQL Java.
A Profiler is enabled per execution (request) and will collect with very minimal overhead all relevant informations to understand the performance and execution. The result will be made available via GraphQLContext.