Avoid an allocation of a chained context in the most common case #2979
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
Author
|
Starting benchmark : After this change So it seems to be an ok improvement |
…xt_allocation_if_one_instrumentation # Conflicts: # src/test/groovy/graphql/execution/AsyncExecutionStrategyTest.groovy # src/test/groovy/graphql/execution/ExecutionStrategyExceptionHandlingEquivalenceTest.groovy # src/test/groovy/graphql/execution/ExecutionTest.groovy # src/test/groovy/graphql/execution/instrumentation/dataloader/DataLoaderDispatcherInstrumentationTest.groovy
bbakerman
requested review from
andimarek and
dondonz
and removed request for
dondonz
dondonz
approved these changes
Oct 29, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This changes the ChainedInstrumentation so that if we have 1 instrumentation in the list (the most common case) then no object allocation need be done for that case. We handle zero instrumentations as well (albeit we dont expect that case often) and otherwise we do what we do today
This also creates a "performant"
SimplePerformantInstrumentationwhich does NOT delegate back to the deprecated methods and hence does not incude a "newState" object allocation of the parameters.eg its avoid the above where a new parameters object is allocated every time
So the bulk of the change is updating the existing instrumentations to use the new more performant base class. I have deprecated the old
SimpleInstrumentation