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

Added ability to use a custom executor such as graphql-jit #7981

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

andrewmcgivery
Copy link
Contributor

@andrewmcgivery andrewmcgivery commented Nov 5, 2024

Added ability to use a custom executor such as graphql-jit. This is a notable performance improvement.

Why?

These changes are to optimize general execution time and especially execution time for larger payloads.

Here is a comparison of the response time with a increasing response size. You can see that compared to the Apollo baseline, using Apollo + graphql-jit + staticOnly cache control mode results in a more than twice as fast response time when getting to larger response sizes. Even with smaller response sizes, there is a notable difference in response time.

Screenshot 2024-11-04 at 2 20 19 PM

How?

Version 4 of Apollo Server removed the executor argument. This was due to it being a "legacy" option that used to be used for the Gateway. However, in the process of removing it, it also removed the option to drop in something like graphql-JIT.

GraphQL JIT is a drop in replacement for graphql-js when executing. It is pretty significantly faster when looking at benchmarks.

This change introduces an argument called customExecutor which allows it to be dropped in. E.g.:

const executor = (schema: GraphQLSchema, cacheSize = 2014, compilerOpts = {}) => {
  const cache = lru(cacheSize);
  return async ({ contextValue, document, operationName, request, queryHash }) => {
    const prefix = operationName || 'NotParametrized';
    const cacheKey = `${prefix}-${queryHash}`;
    let compiledQuery = cache.get(cacheKey);
    if (!compiledQuery) {
      const compilationResult = compileQuery(schema, document, operationName || undefined, compilerOpts);
      if (isCompiledQuery(compilationResult)) {
        compiledQuery = compilationResult;
        cache.set(cacheKey, compiledQuery);
      } else {
        // ...is ExecutionResult
        return compilationResult;
      }
    }

    return compiledQuery.query(undefined, contextValue, request.variables || {});
  };
};

const schema = buildSubgraphSchema([{ typeDefs, resolvers }]);

const server = new ApolloServer<BaseContext>({
  schema,
  customExecutor: executor(schema),
});

@svc-apollo-docs
Copy link
Collaborator

svc-apollo-docs commented Nov 5, 2024

🚫 Docs Preview Denied

You must have approval from an Apollo team member to request a docs preview. If you are a team member, please comment !docs preview.

File Changes

0 new, 1 changed, 1 removed
* (developer-tools)/apollo-server/(latest)/api/apollo-server.mdx
- (developer-tools)/apollo-server/(latest)/performance/custom-executor.mdx

Copy link

codesandbox-ci bot commented Nov 5, 2024

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

@apollographql apollographql deleted a comment from glasser Nov 5, 2024
@andrewmcgivery andrewmcgivery marked this pull request as ready for review November 5, 2024 23:14
import { compileQuery, isCompiledQuery } from 'graphql-jit';
import { lru } from 'tiny-lru';

const executor = (schema: GraphQLSchema, cacheSize = 2014, compilerOpts = {}) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a schema on GraphQLRequestContext too — can we just use that one instead of having to specify it both to this function and to the server? I guess perhaps we then have to include it in the cache key?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you suggesting using schema from request instead of passing it into the executor?

I suppose that could work assuming its the same schema object.

@@ -121,3 +121,15 @@ export type GraphQLRequestContextDidEncounterSubsequentErrors<
export type GraphQLRequestContextWillSendSubsequentPayload<
TContext extends BaseContext,
> = GraphQLRequestContextWillSendResponse<TContext>;

export type GraphQLExecutionResult = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need a new type here, or can we just use ExecutionResult from graphql (which is the type of singleResult where we assign it below)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call. Updated!

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

Successfully merging this pull request may close these issues.

3 participants