Skip to content

Conversation

@samuelAndalon
Copy link
Contributor

No description provided.

samuelAndalon and others added 2 commits April 14, 2025 11:06
…tcher per property per graphql operation (#2079)

### 📝 Description

Inspired by graphql-java/graphql-java#3754.
Currently, graphql-kotlin, through the
[KotlinDataFetcherFactoryProvider](https://github.com/ExpediaGroup/graphql-kotlin/blob/master/generator/graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/generator/execution/KotlinDataFetcherFactoryProvider.kt#L62)
creates a
[PropertyDataFetcher](https://github.com/ExpediaGroup/graphql-kotlin/blob/master/generator/graphql-kotlin-schema-generator/src/main/kotlin/com/expediagroup/graphql/generator/execution/PropertyDataFetcher.kt)
per source's property.

This instance is created [every single time
](https://github.com/graphql-java/graphql-java/blob/master/src/main/java/graphql/schema/GraphQLCodeRegistry.java#L100)the
graphql-java
[DataFetcherFactory](https://github.com/graphql-java/graphql-java/blob/master/src/main/java/graphql/schema/DataFetcherFactory.java)
is invoked, which happens to be on runtime per property per
graphql-operation.

This PR will introduce a new object class `SingletonPropertyDataFetcher`
which will host its own singleton factory that will always return
`SingletonPropertyDataFetcher` which will store all
`KProperty.Getter<*>`s in a ConcurrentHashMap.

Instead of just replacing the SimpleKotlinDataFetcherFactoryProvider, I
am creating a new one, to avoid breaking changes, and to allow users to
decide what they want, this switch might come with a cost, we are
avoiding object allocations, in favor of a singleton that will possibly
hold thousands of `KProperty.Getter<*>`s.

---------

Co-authored-by: Samuel Vazquez <[email protected]>
@samuelAndalon samuelAndalon merged commit 713b696 into 8.x.x Apr 14, 2025
1 check passed
@samuelAndalon samuelAndalon deleted the 8.2.1/singleton-property-data-fetcher branch April 14, 2025 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants