Description

bbakerman

opened on Apr 29, 2025

Issue body actions

Discussed in #3912

^{Originally posted by samuelAndalon April 11, 2025}
Hello from graphql-kotlin!! first of that wanted to thank you all for this library, the reason I am reaching out is because we noticed this PR #3754 and the performance that will provide!, we created something very similar in graphql-kotlin
ExpediaGroup/graphql-kotlin#2079

while I was working on it I thought it might be a good addition to have a DataFetcherFactory with no DataFetcherFactoryEnvironment as argument, or maybe with a Supplier, similar to the LightDataFetcher.

The reason to ask for this is because graphql-kotlin is code first, so we don't actually need the DataFetcherFactoryEnvironment when creating data fetchers, we create them with reflection, so instantiating a DataFetcherFactoryEnvironment per data fetcher per field per query could be a source of memory bottleneck as is not used for Singleton Factories

private static final DataFetcherFactory<?> SINGLETON_FETCHER_FACTORY = environment -> SINGLETON_FETCHER;
```</div>

Activity

1. bbakerman commented on Apr 29, 2025

   

   bbakerman

   on Apr 29, 2025

   MemberAuthor

   More actions

   I created an issue from the discussion since we tend to follow up in issuer better and this needs investigation

   @samuelAndalon sorry for not getting back to you on this. Its been Easter and i have been away a fair bit.

   I thought it might be a good addition to have a DataFetcherFactory with no DataFetcherFactoryEnvironment as argument

   I think that PR adds exactly what you want (unless I dont understand what you want properly which is entirely likely)

   public interface DataFetcherFactory<T> {
   
       /**
        * Returns a {@link graphql.schema.DataFetcher}
        *
        * @param environment the environment that needs the data fetcher
        *
        * @return a data fetcher
        *
        * @deprecated This method will go away at some point and {@link DataFetcherFactory#get(GraphQLFieldDefinition)} will be used
        */
       @Deprecated(since = "2024-11-26")
       DataFetcher<T> get(DataFetcherFactoryEnvironment environment);
   
       /**
        * Returns a {@link graphql.schema.DataFetcher} given the field definition
        * which is cheaper in object allocation terms.
        *
        * @param fieldDefinition the field that needs the data fetcher
        *
        * @return a data fetcher
        */
   
       default DataFetcher<T> get(GraphQLFieldDefinition fieldDefinition) {
           return null;
       }
   }

   The .get with GraphQLFieldDefinition gets called first in the code registry and then the older method

   Let me look into this more because perhaps its not working in the way I think it is
2. bbakerman

   added

   performancework that is primarily targeted as performance improvementswork that is primarily targeted as performance improvements

   on Apr 29, 2025
3. github-actions commented on Jun 29, 2025

   

   github-actionsBot

   on Jun 29, 2025 – with GitHub Actions

   Contributor

   More actions

   Hello, this issue has been inactive for 60 days, so we're marking it as stale. If you would like to continue this discussion, please comment within the next 30 days or we'll close the issue.
4. github-actions

   added

   Stale

   on Jun 29, 2025
5. dondonz

   added

   keep-openTells Stale Bot to keep PRs and issues openTells Stale Bot to keep PRs and issues open

   on Jul 6, 2025
6. dondonz commented on Aug 9, 2025

   

   dondonz

   on Aug 9, 2025

   Member

   More actions

   Checking in, is there any more to do on this issue?
7. bbakerman commented on Aug 11, 2025

   

   bbakerman

   on Aug 11, 2025

   MemberAuthor

   More actions

   All done
8. bbakerman

   closed this as completedon Aug 11, 2025

Issue actions