Skip to content

WIP: Remove an object allocation from instrumentation decorators which interact with fields. #2742

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.

Sign up for GitHub

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

Closed
treverj wants to merge 2 commits into from
Closed

WIP: Remove an object allocation from instrumentation decorators which interact with fields. #2742

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3592e79
Remove an object allocation from instrumentation decorators which int…
treverj Feb 25, 2022
609701d
remove accidental includes
treverj Mar 1, 2022
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
8 changes: 4 additions & 4 deletions src/main/java/graphql/execution/ExecutionStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ protected CompletableFuture<FieldValueInfo> resolveFieldWithInfo(ExecutionContex

Instrumentation instrumentation = executionContext.getInstrumentation();
InstrumentationContext<ExecutionResult> fieldCtx = instrumentation.beginField(
new InstrumentationFieldParameters(executionContext, executionStepInfo)
new InstrumentationFieldParameters(executionContext, executionStepInfo), executionContext.getInstrumentationState()
);

CompletableFuture<FetchedValue> fetchFieldFuture = fetchField(executionContext, parameters);
Expand Down Expand Up @@ -269,11 +269,11 @@ protected CompletableFuture<FetchedValue> fetchField(ExecutionContext executionC

Instrumentation instrumentation = executionContext.getInstrumentation();

InstrumentationFieldFetchParameters instrumentationFieldFetchParams = new InstrumentationFieldFetchParameters(executionContext, environment, parameters, dataFetcher instanceof TrivialDataFetcher);
InstrumentationContext<Object> fetchCtx = instrumentation.beginFieldFetch(instrumentationFieldFetchParams);
InstrumentationFieldFetchParameters instrumentationFieldFetchParams = new InstrumentationFieldFetchParameters(executionContext, environment, dataFetcher instanceof TrivialDataFetcher);
InstrumentationContext<Object> fetchCtx = instrumentation.beginFieldFetch(instrumentationFieldFetchParams, executionContext.getInstrumentationState());

CompletableFuture<Object> fetchedValue;
dataFetcher = instrumentation.instrumentDataFetcher(dataFetcher, instrumentationFieldFetchParams);
dataFetcher = instrumentation.instrumentDataFetcher(dataFetcher, instrumentationFieldFetchParams, executionContext.getInstrumentationState());
ExecutionId executionId = executionContext.getExecutionId();
try {
Object fetchedValueRaw = dataFetcher.get(environment);
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/graphql/execution/SubscriptionExecutionStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ private CompletableFuture<ExecutionResult> executeSubscriptionEvent(ExecutionCon
ExecutionStepInfo subscribedFieldStepInfo = createSubscribedFieldStepInfo(executionContext, newParameters);

InstrumentationFieldParameters i13nFieldParameters = new InstrumentationFieldParameters(executionContext, () -> subscribedFieldStepInfo);
InstrumentationContext<ExecutionResult> subscribedFieldCtx = instrumentation.beginSubscribedFieldEvent(i13nFieldParameters);
InstrumentationContext<ExecutionResult> subscribedFieldCtx = instrumentation.beginSubscribedFieldEvent(i13nFieldParameters, executionContext.getInstrumentationState());

FetchedValue fetchedValue = unboxPossibleDataFetcherResult(newExecutionContext, parameters, eventPayload);
FieldValueInfo fieldValueInfo = completeField(newExecutionContext, newParameters, fetchedValue);
Expand Down
24 changes: 12 additions & 12 deletions src/main/java/graphql/execution/instrumentation/ChainedInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,26 +111,26 @@ public ExecutionStrategyInstrumentationContext beginExecutionStrategy(Instrument


@Override
public InstrumentationContext<ExecutionResult> beginSubscribedFieldEvent(InstrumentationFieldParameters parameters) {
public InstrumentationContext<ExecutionResult> beginSubscribedFieldEvent(InstrumentationFieldParameters parameters, InstrumentationState state) {
return new ChainedInstrumentationContext<>(map(instrumentations, instrumentation -> {
InstrumentationState state = getState(instrumentation, parameters.getInstrumentationState());
return instrumentation.beginSubscribedFieldEvent(parameters.withNewState(state));
InstrumentationState newState = getState(instrumentation, state);
return instrumentation.beginSubscribedFieldEvent(parameters, newState);
}));
}

@Override
public InstrumentationContext<ExecutionResult> beginField(InstrumentationFieldParameters parameters) {
public InstrumentationContext<ExecutionResult> beginField(InstrumentationFieldParameters parameters, InstrumentationState state) {
return new ChainedInstrumentationContext<>(map(instrumentations, instrumentation -> {
InstrumentationState state = getState(instrumentation, parameters.getInstrumentationState());
return instrumentation.beginField(parameters.withNewState(state));
InstrumentationState newState = getState(instrumentation, state);
return instrumentation.beginField(parameters, newState);
}));
}

@Override
public InstrumentationContext<Object> beginFieldFetch(InstrumentationFieldFetchParameters parameters) {
public InstrumentationContext<Object> beginFieldFetch(InstrumentationFieldFetchParameters parameters, InstrumentationState state) {
return new ChainedInstrumentationContext<>(map(instrumentations, instrumentation -> {
InstrumentationState state = getState(instrumentation, parameters.getInstrumentationState());
return instrumentation.beginFieldFetch(parameters.withNewState(state));
InstrumentationState newState = getState(instrumentation, state);
return instrumentation.beginFieldFetch(parameters, newState);
}));
}

Expand Down Expand Up @@ -187,10 +187,10 @@ public ExecutionContext instrumentExecutionContext(ExecutionContext executionCon
}

@Override
public DataFetcher<?> instrumentDataFetcher(DataFetcher<?> dataFetcher, InstrumentationFieldFetchParameters parameters) {
public DataFetcher<?> instrumentDataFetcher(DataFetcher<?> dataFetcher, InstrumentationFieldFetchParameters parameters, InstrumentationState parametersState) {
for (Instrumentation instrumentation : instrumentations) {
InstrumentationState state = getState(instrumentation, parameters.getInstrumentationState());
dataFetcher = instrumentation.instrumentDataFetcher(dataFetcher, parameters.withNewState(state));
InstrumentationState state = getState(instrumentation, parametersState);
dataFetcher = instrumentation.instrumentDataFetcher(dataFetcher, parameters, state);
}
return dataFetcher;
}
Expand Down
12 changes: 8 additions & 4 deletions src/main/java/graphql/execution/instrumentation/Instrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,30 +111,33 @@ default InstrumentationState createState(InstrumentationCreateStateParameters pa
* This is called each time a subscription field produces a new reactive stream event value and it needs to be mapped over via the graphql field subselection.
*
* @param parameters the parameters to this step
* @param state the current state associated with these parameters
*
* @return a non null {@link InstrumentationContext} object that will be called back when the step ends
*/
default InstrumentationContext<ExecutionResult> beginSubscribedFieldEvent(InstrumentationFieldParameters parameters) {
default InstrumentationContext<ExecutionResult> beginSubscribedFieldEvent(InstrumentationFieldParameters parameters, InstrumentationState state) {
return noOp();
}

/**
* This is called just before a field is resolved into a value.
*
* @param parameters the parameters to this step
* @param state the current state associated with these parameters
*
* @return a non null {@link InstrumentationContext} object that will be called back when the step ends
*/
InstrumentationContext<ExecutionResult> beginField(InstrumentationFieldParameters parameters);
InstrumentationContext<ExecutionResult> beginField(InstrumentationFieldParameters parameters, InstrumentationState state);

/**
* This is called just before a field {@link DataFetcher} is invoked.
*
* @param parameters the parameters to this step
* @param state the current state associated with these parameters
*
* @return a non null {@link InstrumentationContext} object that will be called back when the step ends
*/
InstrumentationContext<Object> beginFieldFetch(InstrumentationFieldFetchParameters parameters);
InstrumentationContext<Object> beginFieldFetch(InstrumentationFieldFetchParameters parameters, InstrumentationState state);


/**
Expand Down Expand Up @@ -219,10 +222,11 @@ default ExecutionContext instrumentExecutionContext(ExecutionContext executionCo
*
* @param dataFetcher the data fetcher about to be used
* @param parameters the parameters describing the field to be fetched
* @param state the current state associated with these parameters
*
* @return a non null instrumented DataFetcher, the default is to return to the same object
*/
default DataFetcher<?> instrumentDataFetcher(DataFetcher<?> dataFetcher, InstrumentationFieldFetchParameters parameters) {
default DataFetcher<?> instrumentDataFetcher(DataFetcher<?> dataFetcher, InstrumentationFieldFetchParameters parameters, InstrumentationState state) {
return dataFetcher;
}

Expand Down
4 changes: 2 additions & 2 deletions src/main/java/graphql/execution/instrumentation/SimpleInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,12 @@ public InstrumentationContext<ExecutionResult> beginExecuteOperation(Instrumenta
}

@Override
public InstrumentationContext<ExecutionResult> beginField(InstrumentationFieldParameters parameters) {
public InstrumentationContext<ExecutionResult> beginField(InstrumentationFieldParameters parameters, InstrumentationState state) {
return SimpleInstrumentationContext.noOp();
}

@Override
public InstrumentationContext<Object> beginFieldFetch(InstrumentationFieldFetchParameters parameters) {
public InstrumentationContext<Object> beginFieldFetch(InstrumentationFieldFetchParameters parameters, InstrumentationState state) {
return SimpleInstrumentationContext.noOp();
}
}
10 changes: 5 additions & 5 deletions ...ava/graphql/execution/instrumentation/dataloader/DataLoaderDispatcherInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ public InstrumentationState createState(InstrumentationCreateStateParameters par
}

@Override
public DataFetcher<?> instrumentDataFetcher(DataFetcher<?> dataFetcher, InstrumentationFieldFetchParameters parameters) {
DataLoaderDispatcherInstrumentationState state = parameters.getInstrumentationState();
public DataFetcher<?> instrumentDataFetcher(DataFetcher<?> dataFetcher, InstrumentationFieldFetchParameters parameters, InstrumentationState parametersState) {
DataLoaderDispatcherInstrumentationState state = (DataLoaderDispatcherInstrumentationState) parametersState;
if (state.isAggressivelyBatching()) {
return dataFetcher;
}
Expand Down Expand Up @@ -142,15 +142,15 @@ public void onCompleted(ExecutionResult result, Throwable t) {


@Override
public InstrumentationContext<Object> beginFieldFetch(InstrumentationFieldFetchParameters parameters) {
DataLoaderDispatcherInstrumentationState state = parameters.getInstrumentationState();
public InstrumentationContext<Object> beginFieldFetch(InstrumentationFieldFetchParameters parameters, InstrumentationState parametersState) {
DataLoaderDispatcherInstrumentationState state = (DataLoaderDispatcherInstrumentationState) parametersState;
//
// if there are no data loaders, there is nothing to do
//
if (state.hasNoDataLoaders()) {
return new SimpleInstrumentationContext<>();
}
return state.getApproach().beginFieldFetch(parameters.withNewState(state.getState()));
return state.getApproach().beginFieldFetch(parameters, state.getState());
}

@Override
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/graphql/execution/instrumentation/dataloader/FieldLevelTrackingApproach.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,8 @@ private int getCountForList(FieldValueInfo fieldValueInfo) {
}


public InstrumentationContext<Object> beginFieldFetch(InstrumentationFieldFetchParameters parameters) {
CallStack callStack = parameters.getInstrumentationState();
public InstrumentationContext<Object> beginFieldFetch(InstrumentationFieldFetchParameters parameters, InstrumentationState state) {
CallStack callStack = (CallStack) state;
ResultPath path = parameters.getEnvironment().getExecutionStepInfo().getPath();
int level = path.getLevel();
return new InstrumentationContext<Object>() {
Expand Down
26 changes: 1 addition & 25 deletions ...ava/graphql/execution/instrumentation/parameters/InstrumentationFieldFetchParameters.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,38 +13,14 @@
@PublicApi
public class InstrumentationFieldFetchParameters extends InstrumentationFieldParameters {
private final DataFetchingEnvironment environment;
private final ExecutionStrategyParameters executionStrategyParameters;
private final boolean trivialDataFetcher;

public InstrumentationFieldFetchParameters(ExecutionContext getExecutionContext, DataFetchingEnvironment environment, ExecutionStrategyParameters executionStrategyParameters, boolean trivialDataFetcher) {
public InstrumentationFieldFetchParameters(ExecutionContext getExecutionContext, DataFetchingEnvironment environment, boolean trivialDataFetcher) {
super(getExecutionContext, environment::getExecutionStepInfo);
this.environment = environment;
this.executionStrategyParameters = executionStrategyParameters;
this.trivialDataFetcher = trivialDataFetcher;
}

private InstrumentationFieldFetchParameters(ExecutionContext getExecutionContext, DataFetchingEnvironment environment, InstrumentationState instrumentationState, ExecutionStrategyParameters executionStrategyParameters, boolean trivialDataFetcher) {
super(getExecutionContext, environment::getExecutionStepInfo, instrumentationState);
this.environment = environment;
this.executionStrategyParameters = executionStrategyParameters;
this.trivialDataFetcher = trivialDataFetcher;
}

/**
* Returns a cloned parameters object with the new state
*
* @param instrumentationState the new state for this parameters object
*
* @return a new parameters object with the new state
*/
@Override
public InstrumentationFieldFetchParameters withNewState(InstrumentationState instrumentationState) {
return new InstrumentationFieldFetchParameters(
this.getExecutionContext(), this.getEnvironment(),
instrumentationState, executionStrategyParameters, trivialDataFetcher);
}


public DataFetchingEnvironment getEnvironment() {
return environment;
}
Expand Down
25 changes: 0 additions & 25 deletions ...ain/java/graphql/execution/instrumentation/parameters/InstrumentationFieldParameters.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import graphql.execution.ExecutionContext;
import graphql.execution.ExecutionStepInfo;
import graphql.execution.instrumentation.Instrumentation;
import graphql.execution.instrumentation.InstrumentationState;
import graphql.schema.GraphQLFieldDefinition;

import java.util.function.Supplier;
Expand All @@ -16,30 +15,12 @@
public class InstrumentationFieldParameters {
private final ExecutionContext executionContext;
private final Supplier<ExecutionStepInfo> executionStepInfo;
private final InstrumentationState instrumentationState;

public InstrumentationFieldParameters(ExecutionContext executionContext, Supplier<ExecutionStepInfo> executionStepInfo) {
this(executionContext, executionStepInfo, executionContext.getInstrumentationState());
}

InstrumentationFieldParameters(ExecutionContext executionContext, Supplier<ExecutionStepInfo> executionStepInfo, InstrumentationState instrumentationState) {
this.executionContext = executionContext;
this.executionStepInfo = executionStepInfo;
this.instrumentationState = instrumentationState;
}

/**
* Returns a cloned parameters object with the new state
*
* @param instrumentationState the new state for this parameters object
* @return a new parameters object with the new state
*/
public InstrumentationFieldParameters withNewState(InstrumentationState instrumentationState) {
return new InstrumentationFieldParameters(
this.executionContext, this.executionStepInfo, instrumentationState);
}


public ExecutionContext getExecutionContext() {
return executionContext;
}
Expand All @@ -51,10 +32,4 @@ public GraphQLFieldDefinition getField() {
public ExecutionStepInfo getExecutionStepInfo() {
return executionStepInfo.get();
}

@SuppressWarnings("TypeParameterUnusedInFormals")
public <T extends InstrumentationState> T getInstrumentationState() {
//noinspection unchecked
return (T) instrumentationState;
}
}
5 changes: 3 additions & 2 deletions src/main/java/graphql/execution/instrumentation/threadpools/ExecutorInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import graphql.Internal;
import graphql.TrivialDataFetcher;
import graphql.execution.Async;
import graphql.execution.instrumentation.InstrumentationState;
import graphql.execution.instrumentation.SimpleInstrumentation;
import graphql.execution.instrumentation.parameters.InstrumentationFieldFetchParameters;
import graphql.schema.DataFetcher;
Expand All @@ -29,7 +30,7 @@
* An IO oriented thread pool is typically a multiple of {@link Runtime#availableProcessors()} while a CPU oriented thread pool
* is typically no more than {@link Runtime#availableProcessors()}.
* <p>
* The instrumentation will use the {@link graphql.execution.instrumentation.Instrumentation#instrumentDataFetcher(DataFetcher, InstrumentationFieldFetchParameters)}
* The instrumentation will use the {@link graphql.execution.instrumentation.Instrumentation#instrumentDataFetcher(DataFetcher, InstrumentationFieldFetchParameters, InstrumentationState)}
* method to change your data fetchers so they are executed on a thread pool dedicated to fetching (if you provide one).
* <p>
* Once the data fetcher value is returns it will transfer control back to a processing thread pool (if you provide one).
Expand Down Expand Up @@ -106,7 +107,7 @@ public ExecutorInstrumentation build() {
}

@Override
public DataFetcher<?> instrumentDataFetcher(DataFetcher<?> originalDataFetcher, InstrumentationFieldFetchParameters parameters) {
public DataFetcher<?> instrumentDataFetcher(DataFetcher<?> originalDataFetcher, InstrumentationFieldFetchParameters parameters, InstrumentationState parametersState) {
if (originalDataFetcher instanceof TrivialDataFetcher) {
return originalDataFetcher;
}
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/graphql/execution/instrumentation/tracing/TracingInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,8 @@ public CompletableFuture<ExecutionResult> instrumentExecutionResult(ExecutionRes
}

@Override
public InstrumentationContext<Object> beginFieldFetch(InstrumentationFieldFetchParameters parameters) {
TracingSupport tracingSupport = parameters.getInstrumentationState();
public InstrumentationContext<Object> beginFieldFetch(InstrumentationFieldFetchParameters parameters, InstrumentationState state) {
TracingSupport tracingSupport = (TracingSupport) state;
TracingSupport.TracingContext ctx = tracingSupport.beginField(parameters.getEnvironment(), parameters.isTrivialDataFetcher());
return whenCompleted((result, t) -> ctx.onEnd());
}
Expand Down
3 changes: 2 additions & 1 deletion src/test/groovy/graphql/execution/ExecutionStrategyExceptionHandlingEquivalenceTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import graphql.ExecutionInput
import graphql.GraphQL
import graphql.StarWarsSchema
import graphql.execution.instrumentation.InstrumentationContext
import graphql.execution.instrumentation.InstrumentationState
import graphql.execution.instrumentation.SimpleInstrumentation
import graphql.execution.instrumentation.parameters.InstrumentationFieldFetchParameters
import graphql.validation.ValidationError
Expand All @@ -16,7 +17,7 @@ class ExecutionStrategyExceptionHandlingEquivalenceTest extends Specification {
class TestInstrumentation extends SimpleInstrumentation {

@Override
InstrumentationContext<Object> beginFieldFetch(InstrumentationFieldFetchParameters parameters) {
InstrumentationContext<Object> beginFieldFetch(InstrumentationFieldFetchParameters parameters, InstrumentationState state) {
throw new AbortExecutionException([new ValidationError(ValidationErrorType.UnknownType)])
}
}
Expand Down
Loading