Skip to content

Avoid allocating in GraphQLInputObjectType.getFieldDefinitions #3797

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

Conversation

kilink
Copy link
Contributor

@kilink kilink commented Jan 23, 2025

Avoid copying / allocating a new List in GraphQLInputObjectType's getFieldDefinitions method. the field definitions are already stored as values in an ImmutableMap, and the default Guava implementation of the values method returns an ImmutableList; the issue arises due to the returned collection being a "partial view", which ImmutableList.copyOf uses as a signal that it should create a defensive copy of the passed in collection. In this particular case we just don't care if it's a partial view, since the owning ImmutableMap will be retained regardless.

@kilink kilink force-pushed the graphql-input-object-type-field-definitions-allocations branch from c67e972 to 49f455b Compare January 23, 2025 17:24
Avoid copying / allocating a new List in GraphQLInputObjectType's getFieldDefinitions method.
the field definitions are already stored as values in an ImmutableMap, and the default Guava
implementation of the values method returns an ImmutableList; the issue arises due to the
returned collection being a "partial view", which ImmutableList.copyOf uses as a signal that
it should create a defensive copy of the passed in collection. In this particular case we
just don't care if it's a partial view, since the owning ImmutableMap will be retained
regardless.
@kilink kilink force-pushed the graphql-input-object-type-field-definitions-allocations branch from 49f455b to 6f9ef4c Compare January 24, 2025 00:13
@dondonz dondonz added the performance work that is primarily targeted as performance improvements label Jan 25, 2025
@dondonz dondonz added this to the 23.x breaking changes milestone Jan 25, 2025
Copy link
Member

@bbakerman bbakerman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this

@bbakerman bbakerman merged commit 97381d6 into graphql-java:master Jan 27, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance work that is primarily targeted as performance improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants