GraphQL Shield response 2times slower on response with large list

[DHoefakker]

Bug report

• [x] I have checked other issues to make sure this is not a duplicate.

Describe the bug

When i return a large document with a array (1000+ elements) it increases response times with almost factor 2 with GraphQL running. When i remove GraphQL shield from the ApplyMiddleware the response time is greatly reduced.

To Reproduce

Have a object with a large list of around 1Mb. Return it with GraphQL shield enabled and disabled and see the differences

Expected behavior

I expect milli second slowdown because of rule check, it looks like every element of the schema is checked. (if so can i disable this is certain types?)

Reproduction

A codesandbox can be found here: https://codesandbox.io/s/gracious-dust-55156?file=/index.js On code line 77 comment out "permissions" use the graphQL playground to see the differences in speed.

use the following query:

{ hello { id start end numbera mainitems{ numbera floata floatb floatc floatd floate items { floataa floatbb numberaa floatcc numberbb timestamp subobj{ floataaa floatbbb } } } } }

[DHoefakker]

After further investigating and going deeper through the issues i ran into issue https://github.com/maticzav/graphql-shield/issues/416 which also describes performance loss, in that issue there is a reference to https://github.com/prisma-labs/graphql-middleware/pull/242

In my code i replaced "applyMiddleware" with "applyMiddlewareToDeclaredResolvers" and there is a huge performance gain.

My question is, is this the correct way? If so maybe it's a good idea to mention it in the documentation, and make clear what the scenarios for both options are ;-)

[DHoefakker]

@maticzav Another related question to the above. It looks like when i use "applyMiddlewareToDeclaredResolvers" then security/roles to types is not working (all types are returned) when i revert back to "applyMiddleware" then types are blocked. Is that normal? Or should it work? If not i'll raise an issue.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[maticzav]

Hey @DHoefakker 👋,

Could you compose a small reproduction sandbox so we can resolve your issue?

[DHoefakker]

Hey @DHoefakker 👋,

Could you compose a small reproduction sandbox so we can resolve your issue?

@maticzav i added a codesandbox in the reproduction section of the initial post. You need more?

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[petrovalex]

Hello, any updates/hints on that? I've used that repo as a base for my benchmark, replaced graphql-yoga with apollo-server-express as that is what we use

My use case is that I would like to restrict everything by default, and allow specific fields like for public use.

I've defined that sample permissions, like

const permissions = shield({ Query: { users: allow }, User: { 'col1': allow, 'col2': allow, 'col5': allow } }, { fallbackRule: deny });

1000 items: without shield = 59ms with shield = 108ms with shield requesting not allowed fields: 3312ms

10000 items: without shield = 463ms with shield = 1521ms with shield requesting not allowed fields: 2mins

[maticzav]

@petrovalex I don't think there's an easy solution to this problem and I don't have the time capacity at the moment to rewrite graphql-shield. Thank you for posting benchmarks. For now, I cannot promise much.

[mindnektar]

If anyone's still bothered by this, I've figured out a way around the problem.

According to the documentation, there are three values you can specify for the caching option: no_cache, contextual and strict. It is suggested that you use the strict option if the resolution of your rule depends on the parent or args parameter. The problem is that when using strict, the cache key is a hash generated from the entire parent and args objects. This is both rather slow for large objects and data sets and too strict, because in my case at least, there are no identical combinations of parent and args, even though the same parent might in fact be referenced, I just define "same" differently.

It turns out there is a fourth caching option: You can pass a function that returns the cache key the rule should use. In my case, I could simply use cache: (parent) => parent.id and that was that. All the permissions I set still work and all my requests are 2-5 times faster.

I hope I could save someone some headache with this.

[tanahel-udem]

If anyone is still experiencing this problem, I found a hacky solution.

I've found that my performance issue was that graph QL shield wrap even defaults resolvers in async functions to handle permissions and errors. In my case since I do not need the error handling and my fallback rule is "allow". I found that keeping only the part of the shield middleware I explicitly define does is faster and still validates the rules I need.

Here is how I removed them: https://codesandbox.io/s/dreamy-brahmagupta-vr8ysw?file=/index.js.

[AccsoSG]

We encountered the same problem in our project. How @tanahel-udem said, the performance problems probably come from wrapping all implicit (default) resolvers, even if no rule is defined for them. In our case, relatively simple queries but larger data sets resulted in over a hundred thousand async functions being executed. This even led to a crash of our API. The solution approach of @tanahel-udem actually helps here, but of course it's a bit hacky. But without this workaround we unfortunately can't use GraphQL Shield.