drupal-graphql / graphql

GraphQL integration for Drupal 9/10
288 stars 201 forks source link

Benchmarking and performance improvements #948

Open klausi opened 4 years ago

klausi commented 4 years ago

I played around with benchmarking at https://github.com/klausi/graphql_bench (see result timings in README)

The good news: GraphQL module is as fast/slow as JSON:API module in an uncached scenario.

The bad news: JSON:API is able to leverage page caching (GET requests) and even Drupal's dynamic page cache. That way it can outperform GraphQL which relies on POST requests.

Can we somehow build a similar page cache when a query does not contain a mutation?

Out of curiosity I also built a primitive Rust GraphQL server against Drupal's database and ... it delivers in 0.5ms :-O . Of course that can be considered heavy cheating since it completely bypasses Drupal's access controls. Still worth a look for people that want to offload the hottest GraphQL queries on a dedicated Rust endpoint with raw queries.

pmelab commented 4 years ago

Thanks! That's interesting. GraphQL should cache at least the whole query on processor level, even with POST request. Was this part of the evaluation.

Other than that, partial response caching would be the biggest missing piece of performance optimization I can think of.f

klausi commented 4 years ago

Caching: yes, this is enabled on the test server config. Not sure how that works on the processor level.

I also took a look at profiling with blackfire and found that 33% of the request time is spent in graphql schema parsing and executing the query. The 66% rest is invoking lots of drupal services and composer autoloading. Would be cool if we could cache the whole built graphql schema, but that is not serializable because of anonymous functions.

rthideaway commented 4 years ago

@pmelab regarding

GraphQL should cache at least the whole query on processor level

Do you think you could check it as well? Tried to check it myself, but could not found any indication that this is happening. The query result is cached for query types. But not the processed query.

This maybe would be possible by using persisted queries? Which also does not seem to be supported by graphql module atm.

Btw we are talking here about graphql version 4.x, just to make sure we are on the same line

sebas5384 commented 4 years ago

GraphQL server should support GET queries, and at least with the 3.x you can. In 4.x, there's any requirement which obligates you to use POST instead of GET for queries.

If you can use GET in 4.x then is a matter of configuring you client, what are you using? in case of using Apollo Client it's as simple as setting a flag useGETForQueries: true, check de docs here.

There's a caveat though, the GET request can get very big hitting limits on proxies like Nginx or Varnish, and there's when the persisted queries saves the day.

Kingdutch commented 4 years ago

but that is not serializable because of anonymous functions.

Would it be weird to disallow anonymous functions? While working with the module I find that creating a small data producer is relatively painless. If caching the GraphQL schema would provide a double-digit-percentage performance improvement on every request that would be well worth it in my opinion.

Looking at persisted queries for a solution feels like a step backwards because those lose all the flexibility that GraphQL provides (if I understand them correctly).

$0.02

klausi commented 2 years ago

Yes, I think we should do something like that. Maybe we can build this in a compatible way, so that a schema with anonymous functions still works but just does not give the caching benefit? Can we mark a schema as uncachable if it contains an anonymous function? We could then even display a warning in the UI like "Your schema is uncachable because of an anonymous function. Consider creating a data producer plugin for it instead".