From 4b561292e8f4c11d056c162a230d632a07d7574f Mon Sep 17 00:00:00 2001 From: Greg MacWilliam Date: Thu, 19 Jun 2025 12:39:57 -0400 Subject: [PATCH 1/3] inline ordered selections. --- lib/graphql/cardinal/executor.rb | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/lib/graphql/cardinal/executor.rb b/lib/graphql/cardinal/executor.rb index 54b5f02..8fb1575 100644 --- a/lib/graphql/cardinal/executor.rb +++ b/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? From ba050f9928dafcc21a2f06d38c6fa4f7000d3052 Mon Sep 17 00:00:00 2001 From: Greg MacWilliam Date: Thu, 19 Jun 2025 23:15:09 -0400 Subject: [PATCH 2/3] add tests. --- README.md | 14 ++-- lib/graphql/cardinal/executor.rb | 4 +- .../cardinal/executor/response_shape.rb | 4 +- .../{scope_loader_test.rb => loaders_test.rb} | 78 ++++++++++++++++++- 4 files changed, 87 insertions(+), 13 deletions(-) rename test/graphql/cardinal/executor/{scope_loader_test.rb => loaders_test.rb} (51%) diff --git a/README.md b/README.md index d598917..9b8579b 100644 --- a/README.md +++ b/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. diff --git a/lib/graphql/cardinal/executor.rb b/lib/graphql/cardinal/executor.rb index 8fb1575..37b399d 100644 --- a/lib/graphql/cardinal/executor.rb +++ b/lib/graphql/cardinal/executor.rb @@ -226,7 +226,9 @@ def resolve_execution_field(exec_scope, exec_field, resolved_sources, lazy_field # 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) diff --git a/lib/graphql/cardinal/executor/response_shape.rb b/lib/graphql/cardinal/executor/response_shape.rb index d331299..2e1adff 100644 --- a/lib/graphql/cardinal/executor/response_shape.rb +++ b/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 diff --git a/test/graphql/cardinal/executor/scope_loader_test.rb b/test/graphql/cardinal/executor/loaders_test.rb similarity index 51% rename from test/graphql/cardinal/executor/scope_loader_test.rb rename to test/graphql/cardinal/executor/loaders_test.rb index 1b38bad..7dd0546 100644 --- a/test/graphql/cardinal/executor/scope_loader_test.rb +++ b/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 From 4cc6fa25470490db2b6a2f6ce7daf6ece5ced07a Mon Sep 17 00:00:00 2001 From: Greg MacWilliam Date: Thu, 19 Jun 2025 23:22:40 -0400 Subject: [PATCH 3/3] error handling --- lib/graphql/cardinal/field_resolvers.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/graphql/cardinal/field_resolvers.rb b/lib/graphql/cardinal/field_resolvers.rb index 4538398..e2a9cb0 100644 --- a/lib/graphql/cardinal/field_resolvers.rb +++ b/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