-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Removing some of the Optional.map() and .stream() for performance reasons #3930
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
base: master
Are you sure you want to change the base?
Conversation
Optional.ofNullable(executionContext.getGraphQLContext()) | ||
.map(graphqlContext -> graphqlContext.getBoolean(ExperimentalApi.ENABLE_INCREMENTAL_SUPPORT)) | ||
.orElse(false) | ||
executionContext.hasIncrementalSupport() |
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.
This is critical path stuff and is used in a number of places
Optional.ofNullable(executionContext.getGraphQLContext()) | ||
.map(graphqlContext -> graphqlContext.getBoolean(ExperimentalApi.ENABLE_INCREMENTAL_SUPPORT)) | ||
.orElse(false) | ||
executionContext.hasIncrementalSupport() |
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.
This is more critical path - avoids two Optional allocations
count++; | ||
} | ||
} | ||
int nonDeferredFieldCount = count; |
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.
This is more critical path when defer is on
for (IncrementalPayload incrementalItem : incrementalItems) { | ||
list.add(incrementalItem.toSpecification()); | ||
} | ||
result.put("incremental", list); |
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.
critical path when defer is on
Test Results 314 files 314 suites 53s ⏱️ Results for commit 35eb00e. ♻️ This comment has been updated with latest results. |
One thing: is Collectors.ToList really the same as ArrayList and should we initialize ArrayList with the right size if we know it? |
Some of these are on a critical path so will help a smidge to be more performant.
Others are not and are mostly done via IDEA "rewriting" the .stream() to a direct call