Inline ordered selections

README.md

@@ -2,17 +2,17 @@
 
 **An (experimental) breadth-first GraphQL executor for Ruby**
 
-Depth-first execution resolves every object field descending down a response tree, while breadth-first visits every _selection position_ once with an aggregated set of objects. A breadth-first approach makes resolver overhead dramatically cheaper when resolvers only scale by the size of the request document rather than the size of the response.
+Depth-first execution resolves every object field descending down a response tree, while breadth-first visits every _selection position_ once with an aggregated set of objects. The breadth-first approach tends to be much faster due to fewer resolver calls and intermediary promises.
 
 ```shell
 graphql-ruby: 140002 resolvers
-    1.159 (± 0.0%) i/s  (862.55 ms/i) - 6.000 in 5.182856s
+   1.087 (± 0.0%) i/s  (919.76 ms/i) -  6.000 in  5.526807s
 graphql-cardinal 140002 resolvers
-    19.251 (±10.4%) i/s   (51.95 ms/i) - 95.000 in 5.007853s
+   21.314 (± 9.4%) i/s   (46.92 ms/i) -  108.000 in  5.095015s
 
 Comparison:
-graphql-cardinal 140002 resolvers:     19.3 i/s
-graphql-ruby: 140002 resolvers:     1.2 i/s - 16.60x  slower
+graphql-cardinal 140002 resolvers:   21.3 i/s
+graphql-ruby: 140002 resolvers:   1.1 i/s - 19.60x  slower
 ```
 
 ### Depth vs. Breadth
@@ -23,7 +23,7 @@ GraphQL requests have two dimensions: _depth_ and _breadth_. The depth dimension
 
 ### Depth-first execution
 
-Depth-first execution (the conventional GraphQL execution strategy) resolves every field in the response by descending down the selection tree of every object. This overhead scales linearly as the response size grows, and balloons quickly with added field tracing and instrumentation.
+Depth-first execution (the conventional GraphQL execution strategy) resolves every field in the response by descending down the selection tree of every object. This overhead scales as the response size grows, and balloons quickly with added field tracing and instrumentation.
 
 ![Depth](./images/depth-first.png)
 
@@ -41,7 +41,7 @@ Breadth-first then runs a single resolver per document selection, and coalesces
 
 ![Breadth](./images/breadth-first.png)
 
-While bigger responses will always take longer to process, the workload is almost entirely your own business logic rather than GraphQL execution overhead. Other advantages:
+While bigger responses will always take longer to process, the workload is your own business logic with very little GraphQL execution overhead. Other advantages:
 
 * Eliminates the need for DataLoader promises, because resolvers are inherently batched.
 * Executes via flat queuing without deep recursion and huge call stacks.

lib/graphql/cardinal/executor.rb

@@ -30,7 +30,6 @@ def initialize(schema, resolvers, document, root_object)
         @data = {}
         @errors = []
         @inline_errors = false
-        @unordered_keys = false
         @path = []
         @exec_queue = []
         @exec_count = 0
@@ -72,7 +71,7 @@ def perform
         end
 
         response = {
-          "data" => @inline_errors || @unordered_keys ? shape_response(@data) : @data,
+          "data" => @inline_errors ? shape_response(@data) : @data,
         }
         response["errors"] = @errors.map(&:to_h) unless @errors.empty?
         response
@@ -82,6 +81,7 @@ def perform
 
       def execute_scope(exec_scope)
         unless exec_scope.fields
+          lazy_field_keys = []
           exec_scope.fields = execution_fields_by_key(exec_scope.parent_type, exec_scope.selections)
           exec_scope.fields.each_value do |exec_field|
             @path.push(exec_field.key)
@@ -124,8 +124,10 @@ def execute_scope(exec_scope)
 
             if resolved_sources.is_a?(Promise)
               exec_field.promise = resolved_sources
+              lazy_field_keys << exec_field.key
             else
-              resolve_execution_field(exec_scope, exec_field, resolved_sources)
+              resolve_execution_field(exec_scope, exec_field, resolved_sources, lazy_field_keys)
+              lazy_field_keys.clear
             end
 
             @path.pop
@@ -141,9 +143,6 @@ def execute_scope(exec_scope)
               @path.push(exec_field.key)
               resolve_execution_field(exec_scope, exec_field, exec_field.promise.value)
               @path.pop
-
-              # could be smarter about tracking key order and checking if we got it right...
-              @unordered_keys = true
             end
           else
             # requeue the scope to wait on others that haven't built fields yet
@@ -154,7 +153,7 @@ def execute_scope(exec_scope)
         nil
       end
 
-      def resolve_execution_field(exec_scope, exec_field, resolved_sources)
+      def resolve_execution_field(exec_scope, exec_field, resolved_sources, lazy_field_keys = nil)
         parent_sources = exec_scope.sources
         parent_responses = exec_scope.responses
         field_key = exec_field.key
@@ -173,7 +172,9 @@ def resolve_execution_field(exec_scope, exec_field, resolved_sources)
           next_responses = []
           resolved_sources.each_with_index do |source, i|
             # DANGER: HOT PATH!
-            parent_responses[i][field_key] = build_composite_response(field_type, source, next_sources, next_responses)
+            response = parent_responses[i]
+            lazy_field_keys.each { |k| response[k] = nil } if lazy_field_keys && !lazy_field_keys.empty?
+            response[field_key] = build_composite_response(field_type, source, next_sources, next_responses)
           end
 
           if return_type.kind.abstract?
@@ -225,7 +226,9 @@ def resolve_execution_field(exec_scope, exec_field, resolved_sources)
           # build leaf results
           resolved_sources.each_with_index do |val, i|
             # DANGER: HOT PATH!
-            parent_responses[i][field_key] = if val.nil? || val.is_a?(StandardError)
+            response = parent_responses[i]
+            lazy_field_keys.each { |k| response[k] = nil } if lazy_field_keys && !lazy_field_keys.empty?
+            response[field_key] = if val.nil? || val.is_a?(StandardError)
               build_missing_value(field_type, val)
             elsif return_type.kind.scalar?
               coerce_scalar_value(return_type, val)

lib/graphql/cardinal/executor/response_shape.rb

@@ -26,9 +26,7 @@ def resolve_object_scope(raw_object, parent_type, selections)
             begin
               node_type = @query.get_field(parent_type, node.name).type
               named_type = node_type.unwrap
-
-              # delete and re-add to order result keys...
-              raw_value = raw_object.delete(field_name)
+              raw_value = raw_object[field_name]
 
               raw_object[field_name] = if raw_value.is_a?(ExecutionError)
                 # capture errors encountered in the response with proper path

lib/graphql/cardinal/field_resolvers.rb

@@ -18,7 +18,11 @@ def initialize(key)
       end
 
       def resolve(objects, _args, _ctx, _scope)
-        objects.map { _1[@key] }
+        objects.map do |hash|
+          hash[@key]
+        rescue StandardError => e
+          InternalError.new
+        end
       end
     end
 

test/graphql/cardinal/executor/scope_loader_test.rb → test/graphql/cardinal/executor/loaders_test.rb

@@ -2,7 +2,7 @@
 
 require "test_helper"
 
-class GraphQL::Cardinal::Executor::ScopeLoaderTest < Minitest::Test
+class GraphQL::Cardinal::Executor::LoadersTest < Minitest::Test
 
   class FancyLoader < GraphQL::Cardinal::Loader
     class << self
@@ -42,6 +42,8 @@ def resolve(objects, _args, _ctx, scope)
       first: String
       second: String
       third: String
+      syncObject: Widget
+      syncScalar: String
     }
 
     type Query {
@@ -54,6 +56,8 @@ def resolve(objects, _args, _ctx, scope)
       "first" => FirstResolver.new,
       "second" => SecondResolver.new,
       "third" => ThirdResolver.new,
+      "syncObject" => GraphQL::Cardinal::HashKeyResolver.new("syncObject"),
+      "syncScalar" => GraphQL::Cardinal::HashKeyResolver.new("syncScalar"),
     },
     "Query" => {
       "widget" => GraphQL::Cardinal::HashKeyResolver.new("widget"),
@@ -64,7 +68,7 @@ def setup
     FancyLoader.perform_keys = []
   end
 
-  def test_runs
+  def test_splits_loaders_by_group_across_fields
     document = GraphQL.parse(%|{
       widget {
         first
@@ -95,4 +99,74 @@ def test_runs
     assert_equal expected, executor.perform
     assert_equal [["Apple", "Banana"], ["Coconut"]], FancyLoader.perform_keys
   end
+
+  def test_maintains_ordered_selections_around_object_fields
+    document = GraphQL.parse(%|{
+      widget {
+        a: syncObject { first }
+        first
+        b: syncObject { first }
+        second
+      }
+    }|)
+
+    source = {
+      "widget" => {
+        "first" => "Apple",
+        "second" => "Banana",
+        "syncObject" => { "first" => "NotLazy" },
+      },
+    }
+
+    expected = {
+      "data" => {
+        "widget" => {
+          "a" => { "first" => "NotLazy-a" },
+          "first" => "Apple-a",
+          "b" => { "first" => "NotLazy-a" },
+          "second" => "Banana-a",
+        }
+      }
+    }
+
+    executor = GraphQL::Cardinal::BreadthExecutor.new(LOADER_SCHEMA, LOADER_RESOLVERS, document, source)
+    result = executor.perform
+    assert_equal expected, result
+    assert_equal result.dig("data", "widget").keys, expected.dig("data", "widget").keys
+  end
+
+  def test_maintains_ordered_selections_around_leaf_fields
+    document = GraphQL.parse(%|{
+      widget {
+        a: syncScalar
+        first
+        b: syncScalar
+        second
+      }
+    }|)
+
+    source = {
+      "widget" => {
+        "first" => "Apple",
+        "second" => "Banana",
+        "syncScalar" => "NotLazy",
+      },
+    }
+
+    expected = {
+      "data" => {
+        "widget" => {
+          "a" => "NotLazy",
+          "first" => "Apple-a",
+          "b" => "NotLazy",
+          "second" => "Banana-a",
+        }
+      }
+    }
+
+    executor = GraphQL::Cardinal::BreadthExecutor.new(LOADER_SCHEMA, LOADER_RESOLVERS, document, source)
+    result = executor.perform
+    assert_equal expected, result
+    assert_equal result.dig("data", "widget").keys, expected.dig("data", "widget").keys
+  end
 end