Skip to content

Conversation

@rmosolgo
Copy link
Owner

@rmosolgo rmosolgo commented Aug 28, 2017

For each key in the response, we create one Context and one-or-zero FieldResult/SelectionResult. The leads to a lot of extra GC work, see this benchmark which highlights graphql overhead:

#861 (comment)

We can remove this overhead by using Context#value/Context#value= to manage results.

Another advantage is that you can backtrack through the results using context.parent.value, which will fix #881.

Also fixes #882.

Finally, this lays the groundwork for streaming execution. Since ctx objects track their underlying objects, they can be used to resume execution at any time. This could reduce TTFB with graphql-client.

TODO:

  • Address todos in code, smooth out the ad hoc conditionals which were scattered around to make the tests pass
  • simplify query/context.rb:
    • (as much as i could) Merge Query::Context & Query::Context::FieldResolutionContext
    • Move to a new class which describes its current role May as well leave it, break less stuff
    • Move Context.flatten to somewhere that makes sense (inside Execution?)
  • Rename variables in execution code to match the kind of objects they are
  • Add a test for backtracking context

@rmosolgo rmosolgo changed the base branch from master to 1.7.x August 30, 2017 13:18
@rmosolgo rmosolgo merged commit 7c74fed into 1.7.x Sep 2, 2017
@rmosolgo rmosolgo deleted the use-context-for-results branch September 2, 2017 18:58
@rmosolgo rmosolgo mentioned this pull request Sep 14, 2017
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.

2 participants