Express-graphql: Performance improvement caching validation

[jamesmoriarty]

I'm currently using GraphQL to transform large documents and I've noticed a significant amount of time in validate. https://github.com/graphql/express-graphql/blob/master/src/index.js#L254

validate(schema, documentAST, validationRules);

The time spent on the first request:

parse: 21.042ms
hash: 3.727ms
validate: 965.500ms <
execute: 331.606ms

If I cache the result of validate. The time spent on the second request:

parse: 49.794ms
hash: 5.105ms
validate: 5.203ms <
execute: 336.634ms

As far as I can tell the schema is static, the query is the same with different arguments, and the validation rules are the same.

The change looks something like:

hash = hashFn(schema, documentAST, validationRules)

if (!cache[hash]) {
   cache[hash] = validate(schema, documentAST, validationRules);
}

const validationErrors = cache[hash];

As it's currently stateless, I'm trying to understand the trade-offs caching the result. I would appreciate any input from the contributors / maintainers. Basically, is this a terrible idea?

[IvanGoncharov]

@jamesmoriarty GraphQL validate functionality was designed with a pretty similar case in mind:
https://facebook.github.io/graphql/June2018/#sec-Validation

Typically validation is performed in the context of a request immediately before execution, however a GraphQL service may execute a request without explicitly validating it if that exact same request is known to have been validated before. For example: the request may be validated during development, provided it does not later change, or a service may validate a request once and memoize the result to avoid validating the same request again in the future. Any client‐side or development‐time tool should report validation errors and not allow the formulation or execution of requests known to be invalid at that given point in time.

Moreover you can optimise it a little bit more by hashing query string before parsing it to AST.
AFAIK Facebook engineer went one step forward and hash queries during development so client sents only hash. Here is a talk with more details: https://youtu.be/G_zipR8Y8Ks

[jamesmoriarty]

Thanks @IvanGoncharov. I really appreciate the response.

Is this a feature the maintainers would be interested in merging? If so is there any guidance on how the maintainers would like it implemented?

app.use('/graphql', graphqlHTTP({
  schema: MyGraphQLSchema,
  graphiql: true,
  validationCache: false <default> | <cache object e.g. {} or LRU>
}));

cc/ @wincent @zpao @kassens

[IvanGoncharov]

Fixed in master.
@jamesmoriarty Thanks for PR 👍

[kaushik143]

@IvanGoncharov Is the proposal for validationCache accepted? I looking to do a similar kind of optimization for improving GraphQL performance