Is your feature request related to a problem? Please describe.
While the caching of the AST does provide pretty good performance, there are issues when
dealing with really large schemas.
Describe the solution you'd like
The basic idea is summed up pretty nicely in this comment.
We have to parse the whole schema initially to do schema manipulation, such as adding additional types for @paginate, and merging type extensions. Once we have our final schema AST, we can split it up.
Challenges
We have to solve the issue of orphaned types for Introspection, see the discussion here
I think this would be a great idea. Especially when queries/mutations have a lot of arguments result in a significant slowdown when building a schema. This happens just after the cached AST has been retrieved, Lighthouse loops over all the queries and their arguments. If I'm only performing one query, it seems pretty ineffecient to parse all other queries even though we already know they will not be called anyway.
You could also check if the user is trying to perform a query or a mutation before unserializing and parsing the whole schema. Since it's not possible to perform a mutation and a query in the same call, this would mean you could completely ignore the other part. That could potentially also save a lot of parsing time.
I would aim to do the schema building lazily as well. webonyx/graphql-php allows to build the schema with a type loader. We basically need a function with the following signature:
function(string $typeName): Type
In there, we just do roughly the following:
$typeDefinition = $this->cache->get($typeName);
$resolvedType = $this->nodeFactory->handle($typeDefinition);
No need to differentiate between Queries/Mutations, webonyx has got us covered and will only load the types that are needed for the exact query that was given.
@spawnia I'd like to have a go at this myself, not sure if I'll get anywhere but the conceptually it shouldn't be too hard to get a working version. I'd like to know if I'm working on the right section or not.
I'm currently thinking that for a proof of concept, I could rework the build function of SchemaBuilder. I was thinking I should pass the request and try to decide which typeDefinitions to load/register. Would that give me a significant improvement or do I need to do this earlier in the Schema parsing process?
If I can't find a way to speed up schema parser, we might have to drop Lighthouse for our project.
No need to figure out which types to load. The only ones that always need to be present in the executable schema are the root types. webonyx/graphql-php accepts the incoming query and loads in nested types as it needs them. The only thing we need to implement is the function i described above.
I think the only bottleneck of Lighthouse is the parsing performance.
Auto generated schema can grows very fast, in my new project already 10000 lines of schema was generated… really, the parsing process is a big pain for me.
It's a BIG win to get rid of the parsing performance issue! Looking forward for this!!!
@spawnia Do you have any plan to solve this before long? because it really slows down the response time here.. If not I guess I'm going to send a PR for this issue.
Feel free to dig in, have not got around to doing it. Wish you the best of luck 👍
@spawnia here is my rough idea to optimize the performance.
Second round:
Do you think this is the proper approach to solve the problem?
@yaquawa webonyx/graphql-php already has a mechanism for lazy type loading, which basically does the query analysis/minimum type loading. We just have to find a way to use it properly.
Storing the astNode with the constructed types is a great step forward, as this means we can look at the directives without actually loading the whole DocumentAST. We basically have to eliminate all parts from the resolve process where the DocumentAST is currently used.
Your approach seems like it might work if there is a relatively small number of different queries. However the dynamic nature of GraphQL makes it more likely that we have many different queries.
@spawnia I saw that before, haven't tried yet, but if it will lazy load scalar and input object type and all other kind of types, the process can be as easy as just cache all kinds of types individually and take them from the cache on demand.
@yaquawa ... which is exactly what is described here https://github.com/webonyx/graphql-php/issues/229#issuecomment-359704382 as mentioned in the description.
Would love it if you can give it a shot after #441 is through.
This definitely needs to be addressed. I am currently using Folkloreatelier/laravel-graphql and I've been comparing the two packages within my app as I was about to fully switch. If I query the same thing ({users{id}}) the difference is a whopping ~600ms on my local even with cache mode on.
Most helpful comment
I think this would be a great idea. Especially when queries/mutations have a lot of arguments result in a significant slowdown when building a schema. This happens just after the cached AST has been retrieved, Lighthouse loops over all the queries and their arguments. If I'm only performing one query, it seems pretty ineffecient to parse all other queries even though we already know they will not be called anyway.
You could also check if the user is trying to perform a query or a mutation before unserializing and parsing the whole schema. Since it's not possible to perform a mutation and a query in the same call, this would mean you could completely ignore the other part. That could potentially also save a lot of parsing time.