The Wayback Machine - http://web.archive.org/web/20201210210205/https://github.com/graphql/graphql-js/issues/2822
Skip to content
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

Optionally skip `collectReferencedTypes` during `buildSchema`? #2822

Open
xuorig opened this issue Oct 15, 2020 · 4 comments
Open

Optionally skip `collectReferencedTypes` during `buildSchema`? #2822

xuorig opened this issue Oct 15, 2020 · 4 comments

Comments

@xuorig
Copy link

@xuorig xuorig commented Oct 15, 2020

buildSchema can be quite slow given a large schema. Using GitHub's public SDL on my local machine, It can take around 200ms to build a schema from an SDL string.

This time is improved by a lot if we assume we're dealing with a valid schema. This brings down the same SDL to about 120ms.

const schema = buildSchema(sdl, { assumeValid: true });

Even when skipping validation, I noticed a significant portion of the time is spent in collectReferencedTypes.

Screen Shot 2020-10-15 at 1 05 00 PM

In the case of buildSchema, it seems like the types that are collected at part time could be considered as the full set of types for the schema. From my understanding, it looks like this collectReferencedTypes is mainly used to support the use case of programmatically creating a schema by providing only the root types:

const schema = new GraphQLSchema({
  query: new GraphQLObjectType({
     name: 'Query',
     fields: {
       hero: { type: characterInterface, ... },
    }
  }),
  types: [humanType, droidType],
})

When it comes to buildSchema however, I believe this is work that could easily be skipped. Correct me if I am wrong here! Additionally, it's hard to extend buildSchema or GraphQLSchema in user land if someone did want to skip this processing. The fact this is done in the constructor makes this particularly difficult.

Would you be open to a configuration option, or another entry point (refactoring the collect part away from Schema's constructor possibly) so that this work can be skipped?

My use case involves parsing and using buildSchema dynamically at request time, which is why these optimizations are needed. Let me know if I've missed anything here 🙇

@yaacovCR
Copy link

@yaacovCR yaacovCR commented Oct 15, 2020

Off topic, but I'd love to hear more about that use case and why you are doing that... Do you have a link you could share?

@xuorig
Copy link
Author

@xuorig xuorig commented Oct 15, 2020

@yaacovCR nothing really magical, I've got an API that sends out SDL strings, and a React application that uses buildSchema to render information about multiple schemas. Can't share much about the project but that's pretty much it!

@IvanGoncharov
Copy link
Member

@IvanGoncharov IvanGoncharov commented Oct 19, 2020

@xuorig I think it's a totally valid concern.
As a solution, we can allow types to be either array or ObjMap and if ObjMap is passed we don't do a collection.
What do you think?

@xuorig
Copy link
Author

@xuorig xuorig commented Oct 19, 2020

I like it @IvanGoncharov! 👍 Let me know if you'd like me to work on a PR this week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.