Skip to content

Use a queue for execution #5389

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

Open
wants to merge 21 commits into
base: master
Choose a base branch
from
Open

Use a queue for execution #5389

wants to merge 21 commits into from

Conversation

rmosolgo
Copy link
Owner

@rmosolgo rmosolgo commented Jun 20, 2025

I'm feeling really inspired by @gmac's proof-of-concept over at https://github.com/gmac/graphql-cardinal/, so I thought I'd try again at refactoring execution to use a queue instead of recursive method calls. I have tried before (#4967, #4968, somewhat #3998, #4935) and given up along the way, but I'm going to try again 😅

TODO:

  • Extract some refactors from this branch
  • Make Lazy and Dataloader work
  • Consider steps for continue_value, resolve_list_item
  • Use steps for call_method_on_directives
  • Merge ResolveTypeStep into ResultHash
  • Merge directive checks into steps too?
  • Merge authorized_new lazy usage into ResultHash
  • Look for other after_lazy usages and rebuild them using queue steps
  • Refactor ResultArray and ResultHash into state machines
  • Merge ResultHash into Schema::Object
  • Fix dataloaded arguments

@rmosolgo
Copy link
Owner Author

There's still a lot to do here for compatibility and performance, but the initial benchmark results are surprisingly good (even with the terrible FieldResolutionStep implementation which is hogging memory). This is using a modified version of the benchmark from graphql-cardinal:

  ruby 3.4.1 (2024-12-25 revision 48d4efcb85) +PRISM [x86_64-darwin22]
  Calculating -------------------------------------
  graphql-ruby: 142 resolvers
-                         399.549 (± 8.5%) i/s    (2.50 ms/i) -      2.000k in   5.048521s
+                         538.453 (± 3.9%) i/s    (1.86 ms/i) -      2.695k in   5.013324s

  graphql-ruby: 140002 resolvers
-                           0.517 (± 0.0%) i/s     (1.93 s/i) -      3.000 in   5.797333s
+                           0.761 (± 0.0%) i/s     (1.31 s/i) -      4.000 in   5.272811s

- Total allocated: 10149304 bytes (70353 objects)
+ Total allocated: 13431224 bytes (97389 objects)

So that's a 34% or 47% speedup. It's using 30% more memory, because it's currently the Simplest Thing That Could Possibly Work. Presumably refactoring it to avoid those repeated allocations will make it even faster!

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.

1 participant