-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Minor performance fixes #3236
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
Minor performance fixes #3236
Conversation
| public ExecutionStepInfo newExecutionStepInfoForListElement(ExecutionStepInfo executionInfo, ResultPath indexedPath) { | ||
| GraphQLList fieldType = (GraphQLList) executionInfo.getUnwrappedNonNullType(); | ||
| GraphQLOutputType typeInList = (GraphQLOutputType) fieldType.getWrappedType(); | ||
| ResultPath indexedPath = executionInfo.getPath().segment(index); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
according to my memory profiler, this is dropping from 3-4% of total heap to just 1-2% (as expected)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to explain to others - the indexedPath was allocated out side this method and then reallocated again here. This double allocation has been removed
|
master branch 110afe0 this branch: |
This is a breaking change because it is removal from a public API. However, the fields are not used.
| @Internal | ||
| public class ExecutionStepInfoFactory { | ||
|
|
||
| public ExecutionStepInfo newExecutionStepInfoForSubField(ExecutionContext executionContext, MergedField mergedField, ExecutionStepInfo parentInfo) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just checked - code was never used
| public int getCurrentListIndex() { | ||
| return currentListIndex; | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are not used anywhere and are informational
|
Thanks |
hello @bbakerman @andimarek :)
another batch of small fixes... please have a look when you have time :)
D.