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

ExecutionResult are still being created per field #3458

Closed
bbakerman opened this issue Feb 19, 2024 · 1 comment
Closed

ExecutionResult are still being created per field #3458

bbakerman opened this issue Feb 19, 2024 · 1 comment

Comments

@bbakerman
Copy link
Member

The beginField instrumentation method was deprecated during this version but for backwards compat reasons it got delegated back to and hence it was not a breaking change

default InstrumentationContext<ExecutionResult> beginField(InstrumentationFieldParameters parameters, InstrumentationState state) {

and adapter was used

    default InstrumentationContext<Object> beginFieldExecution(InstrumentationFieldParameters parameters, InstrumentationState state) {
        InstrumentationContext<ExecutionResult> ic = beginField(parameters, state);
        return ic == null ? null : new ExecutionResultInstrumentationContextAdapter(ic);
    }

but this adapter does thgis

        CompletableFuture<ExecutionResult> future = result.thenApply(obj -> ExecutionResultImpl.newExecutionResult().data(obj).build());
        delegate.onDispatched(future);
        //
        // when the mapped future is completed, then call onCompleted on the delegate
        future.whenComplete(delegate::onCompleted);

that is it creates an Er for backwards compat reasons - so in fact we are creating a ER for each field value anyway.

I think we have to bite the bullet here and NOT call the old method (backwards compat breaking change) in order to get the efficiency we want.

This means that people need to update to the new method in Instrumentation

@bbakerman
Copy link
Member Author

fixed in v22

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants
@bbakerman and others