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

A New GraphQL Runtime #1730

Closed
5 tasks done
rmosolgo opened this issue Aug 2, 2018 · 1 comment
Closed
5 tasks done

A New GraphQL Runtime #1730

rmosolgo opened this issue Aug 2, 2018 · 1 comment
Assignees

Comments

@rmosolgo
Copy link
Owner

rmosolgo commented Aug 2, 2018

(some of these issues were also described in #1394)

I have a goal for this year to rewrite GraphQL execution, here's why:

Problems with the Current Runtime

  • Performance: InternalRepresentation::Rewrite is not worth it for big schemas:
    • The time it takes is exponential (I think) to the number of types who implement an interface, and for the Node interface, this can be quite big
    • Because @skip/@include is implemented during Rewrite, its output can't be cached and it must be recalculated each time a static query is run
  • Complex code: The rewrite step is unapproachable to new devs
    • It was added to support the max_complexity feature, but we should implement that in a way that is familiar and hackable to people
  • Hackability: The execution routine isn't open to hacking, which we need from time to time, let's be honest. The old SerialExecution classes were better in this regard, since they could be extended and overridden. But they were not efficient enough, because of the object overhead.

So, what could be better?

Proposed Solution

I want to try an AST interpreter (again, like SerialExecution). I think it will have to be a multi-pass interpreter in order to support something like query analysis. The interpreter should be designed in such a way that:

  • the core execution methods can be overridden
  • custom directives can be added (perhaps a few will be implemented this way out of the box: @skip, @include, @export, and the server-side equivalent of @skip, something like @skipIfServerCondition which skips if a flag is set on the server)
  • the code kinda does what you expect
  • all preparation work can be front-loaded for static queries

To this end I've started reading Essentials of Programming Languages, which centers around writing a type-checked interpreter. (Last time I was working on execution, I was doing the Stanford compilers course, can you tell 😆.)

Measuring Success

Besides the design goals posted above, I want this interpreter to perform better on the benchmarks in this repo (measured by wall clock time).

Especially relevant is the current worst-case scenario involving lots of nested ... on Node spreads, which is dreadfully slow when Node has lots of implementers. I would accept a little decrease in other areas to get that case back on track. (Since GitHub has a big schema, and we use static queries with graphql-client, we pay this cost a lot!)

Migration Path

I want to entice people to use class-based schema definition, and if the new interpreter works, I'd like to use it a "carrot." So, the new interpreter will work with class-based types and schemas only.

In the same vein, it will live alongside existing execution code for a while, and it won't be the default at first.

Timeline

There's still a lot to do before this can land:

So, maybe this new execution code will be available on an opt-in basis in 1.10.

Your Thoughts?

I opened this issue because this goal has been on my mind for a long time and I thought I'd share it. If you have opinions about the issues raised here or the proposed solutions, please add them here!

@rmosolgo
Copy link
Owner Author

The new interpreter has been running in GitHub since mid-December 🍻

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

No branches or pull requests

1 participant