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

Move runtime state into result objects #4935

Merged
merged 9 commits into from May 3, 2024
Merged

Conversation

rmosolgo
Copy link
Owner

This is an experiment in managing runtime state.

We already have a lot of per-field metadata in current_runtime_state, and most of it is derived from GraphQLResult objects. I think it should be possible to:

  • Move per-object state to GraphQLResult objects (not everything -- can't be per-field since leaf values don't get GraphQLResult objects)
  • Remove per-object state values from runtime method signatures, since they're accessible from the passed-in GraphQLResult
  • Update current_runtime_state to use GraphQLResult in more cases (it already does for path, at least)

If that works, then I'm hoping that GraphQLResult can be the basis for a run-and-return (trampoline) -style execution instead of a recursive one, where the GraphQLResult itself contains information about what needs to be done to it next.

@rmosolgo rmosolgo marked this pull request as ready for review May 3, 2024 15:00
@rmosolgo rmosolgo added this to the 2.3.3 milestone May 3, 2024
@rmosolgo
Copy link
Owner Author

rmosolgo commented May 3, 2024

I checked the benchmarks to make sure there weren't any surprises. The only thing is that Query results now retain references to their underlying GraphQL object type instances. That's the same number of allocations, they just might live a little longer.

@rmosolgo rmosolgo merged commit c40070f into master May 3, 2024
12 checks passed
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.

None yet

1 participant