From 5ba154a5613d4069c0b1d169c505caab96b0769d Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Tue, 26 Aug 2025 12:58:55 -0400 Subject: [PATCH 01/21] Extract dataloader changes from run-queue-3; merge lazy resolution into Dataloader --- lib/graphql/dataloader.rb | 40 +++++++- lib/graphql/dataloader/flat_dataloader.rb | 75 ++++++++++++++ lib/graphql/dataloader/null_dataloader.rb | 4 +- lib/graphql/execution/interpreter.rb | 11 +- lib/graphql/execution/interpreter/resolve.rb | 100 ------------------- lib/graphql/execution/interpreter/runtime.rb | 3 +- lib/graphql/schema.rb | 2 +- 7 files changed, 118 insertions(+), 117 deletions(-) create mode 100644 lib/graphql/dataloader/flat_dataloader.rb delete mode 100644 lib/graphql/execution/interpreter/resolve.rb diff --git a/lib/graphql/dataloader.rb b/lib/graphql/dataloader.rb index 92ddac4f6c..ad88bffb97 100644 --- a/lib/graphql/dataloader.rb +++ b/lib/graphql/dataloader.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true +require "graphql/dataloader/flat_dataloader" require "graphql/dataloader/null_dataloader" require "graphql/dataloader/request" require "graphql/dataloader/request_all" @@ -64,8 +65,14 @@ def initialize(nonblocking: self.class.default_nonblocking, fiber_limit: self.cl @nonblocking = nonblocking end @fiber_limit = fiber_limit + @steps_to_rerun_after_lazy = [] + @lazies_at_depth = nil end + attr_accessor :lazies_at_depth + + attr_reader :steps_to_rerun_after_lazy + # @return [Integer, nil] attr_reader :fiber_limit @@ -140,10 +147,10 @@ def yield(source = Fiber[:__graphql_current_dataloader_source]) end # @api private Nothing to see here - def append_job(&job) + def append_job(callable = nil, &job) # Given a block, queue it up to be worked through when `#run` is called. # (If the dataloader is already running, than a Fiber will pick this up later.) - @pending_jobs.push(job) + @pending_jobs.push(callable || job) nil end @@ -189,6 +196,8 @@ def run_isolated end def run + # TODO unify the initialization lazies_at_depth + @lazies_at_depth ||= Hash.new { |h, k| h[k] = [] } trace = Fiber[:__graphql_current_multiplex]&.current_trace jobs_fiber_limit, total_fiber_limit = calculate_fiber_limit job_fibers = [] @@ -222,6 +231,31 @@ def run end join_queues(source_fibers, next_source_fibers) end + + if @lazies_at_depth.any? + smallest_depth = nil + @lazies_at_depth.each_key do |depth_key| + smallest_depth ||= depth_key + if depth_key < smallest_depth + smallest_depth = depth_key + end + end + + if smallest_depth + lazies = @lazies_at_depth.delete(smallest_depth) + if !lazies.empty? + append_job { + lazies.each(&:value) # resolve these Lazy instances + } + job_fibers << spawn_job_fiber(trace) + end + end + elsif @steps_to_rerun_after_lazy.any? + @pending_jobs.concat(@steps_to_rerun_after_lazy) + f = spawn_job_fiber(trace) + job_fibers << f + @steps_to_rerun_after_lazy.clear + end end trace&.end_dataloader(self) @@ -331,4 +365,4 @@ def spawn_source_fiber(trace) end end -require "graphql/dataloader/async_dataloader" +require "graphql/dataloader/async_dataloader" \ No newline at end of file diff --git a/lib/graphql/dataloader/flat_dataloader.rb b/lib/graphql/dataloader/flat_dataloader.rb new file mode 100644 index 0000000000..310785c1ad --- /dev/null +++ b/lib/graphql/dataloader/flat_dataloader.rb @@ -0,0 +1,75 @@ +# frozen_string_literal: true +module GraphQL + class Dataloader + class FlatDataloader < Dataloader + def initialize(*) + # TODO unify the initialization lazies_at_depth + @lazies_at_depth ||= Hash.new { |h, k| h[k] = [] } + @steps_to_rerun_after_lazy = [] + @queue = [] + end + + def run + while @queue.any? + while (step = @queue.shift) + step.call + end + + while @lazies_at_depth&.any? + smallest_depth = nil + @lazies_at_depth.each_key do |depth_key| + smallest_depth ||= depth_key + if depth_key < smallest_depth + smallest_depth = depth_key + end + end + + if smallest_depth + lazies = @lazies_at_depth.delete(smallest_depth) + lazies.each(&:value) # resolve these Lazy instances + end + end + + if @steps_to_rerun_after_lazy.any? + @steps_to_rerun_after_lazy.each(&:call) + @steps_to_rerun_after_lazy.clear + end + end + end + + def run_isolated + prev_queue = @queue + prev_stral = @steps_to_rerun_after_lazy + prev_lad = @lazies_at_depth + @steps_to_rerun_after_lazy = [] + @queue = [] + @lazies_at_depth = @lazies_at_depth.dup&.clear + res = nil + append_job { + res = yield + } + run + res + ensure + @queue = prev_queue + @steps_to_rerun_after_lazy = prev_stral + @lazies_at_depth = prev_lad + end + + def clear_cache; end + + def yield(_source) + raise GraphQL::Error, "GraphQL::Dataloader is not running -- add `use GraphQL::Dataloader` to your schema to use Dataloader sources." + end + + def append_job(callable = nil, &block) + @queue << (callable || block) + nil + end + + def with(*) + raise GraphQL::Error, "GraphQL::Dataloader is not running -- add `use GraphQL::Dataloader` to your schema to use Dataloader sources." + end + end + end +end \ No newline at end of file diff --git a/lib/graphql/dataloader/null_dataloader.rb b/lib/graphql/dataloader/null_dataloader.rb index 7fd222d7fe..3e431c1086 100644 --- a/lib/graphql/dataloader/null_dataloader.rb +++ b/lib/graphql/dataloader/null_dataloader.rb @@ -18,8 +18,8 @@ def yield(_source) raise GraphQL::Error, "GraphQL::Dataloader is not running -- add `use GraphQL::Dataloader` to your schema to use Dataloader sources." end - def append_job - yield + def append_job(callable = nil) + callable ? callable.call : yield nil end diff --git a/lib/graphql/execution/interpreter.rb b/lib/graphql/execution/interpreter.rb index ba0b94b2aa..c2015f1aac 100644 --- a/lib/graphql/execution/interpreter.rb +++ b/lib/graphql/execution/interpreter.rb @@ -5,7 +5,6 @@ require "graphql/execution/interpreter/arguments_cache" require "graphql/execution/interpreter/execution_errors" require "graphql/execution/interpreter/runtime" -require "graphql/execution/interpreter/resolve" require "graphql/execution/interpreter/handles_raw_value" module GraphQL @@ -43,6 +42,7 @@ def run_all(schema, query_options, context: {}, max_complexity: schema.max_compl schema = multiplex.schema queries = multiplex.queries lazies_at_depth = Hash.new { |h, k| h[k] = [] } + multiplex.dataloader.lazies_at_depth = lazies_at_depth multiplex_analyzers = schema.multiplex_analyzers if multiplex.max_complexity multiplex_analyzers += [GraphQL::Analysis::MaxQueryComplexity] @@ -90,15 +90,6 @@ def run_all(schema, query_options, context: {}, max_complexity: schema.max_compl multiplex.dataloader.run - # Then, work through lazy results in a breadth-first way - multiplex.dataloader.append_job { - query = multiplex.queries.length == 1 ? multiplex.queries[0] : nil - multiplex.current_trace.execute_query_lazy(multiplex: multiplex, query: query) do - Interpreter::Resolve.resolve_each_depth(lazies_at_depth, multiplex.dataloader) - end - } - multiplex.dataloader.run - # Then, find all errors and assign the result to the query object results.each_with_index do |data_result, idx| query = queries[idx] diff --git a/lib/graphql/execution/interpreter/resolve.rb b/lib/graphql/execution/interpreter/resolve.rb deleted file mode 100644 index 570ab48e9d..0000000000 --- a/lib/graphql/execution/interpreter/resolve.rb +++ /dev/null @@ -1,100 +0,0 @@ -# frozen_string_literal: true - -module GraphQL - module Execution - class Interpreter - module Resolve - # Continue field results in `results` until there's nothing else to continue. - # @return [void] - def self.resolve_all(results, dataloader) - dataloader.append_job { resolve(results, dataloader) } - nil - end - - def self.resolve_each_depth(lazies_at_depth, dataloader) - smallest_depth = nil - lazies_at_depth.each_key do |depth_key| - smallest_depth ||= depth_key - if depth_key < smallest_depth - smallest_depth = depth_key - end - end - - if smallest_depth - lazies = lazies_at_depth.delete(smallest_depth) - if !lazies.empty? - lazies.each do |l| - dataloader.append_job { l.value } - end - # Run lazies _and_ dataloader, see if more are enqueued - dataloader.run - resolve_each_depth(lazies_at_depth, dataloader) - end - end - nil - end - - # After getting `results` back from an interpreter evaluation, - # continue it until you get a response-ready Ruby value. - # - # `results` is one level of _depth_ of a query or multiplex. - # - # Resolve all lazy values in that depth before moving on - # to the next level. - # - # It's assumed that the lazies will - # return {Lazy} instances if there's more work to be done, - # or return {Hash}/{Array} if the query should be continued. - # - # @return [void] - def self.resolve(results, dataloader) - # There might be pending jobs here that _will_ write lazies - # into the result hash. We should run them out, so we - # can be sure that all lazies will be present in the result hashes. - # A better implementation would somehow interleave (or unify) - # these approaches. - dataloader.run - next_results = [] - while !results.empty? - result_value = results.shift - if result_value.is_a?(Runtime::GraphQLResultHash) || result_value.is_a?(Hash) - results.concat(result_value.values) - next - elsif result_value.is_a?(Runtime::GraphQLResultArray) - results.concat(result_value.values) - next - elsif result_value.is_a?(Array) - results.concat(result_value) - next - elsif result_value.is_a?(Lazy) - loaded_value = result_value.value - if loaded_value.is_a?(Lazy) - # Since this field returned another lazy, - # add it to the same queue - results << loaded_value - elsif loaded_value.is_a?(Runtime::GraphQLResultHash) || loaded_value.is_a?(Runtime::GraphQLResultArray) || - loaded_value.is_a?(Hash) || loaded_value.is_a?(Array) - # Add these values in wholesale -- - # they might be modified by later work in the dataloader. - next_results << loaded_value - end - end - end - - if !next_results.empty? - # Any pending data loader jobs may populate the - # resutl arrays or result hashes accumulated in - # `next_results``. Run those **to completion** - # before continuing to resolve `next_results`. - # (Just `.append_job` doesn't work if any pending - # jobs require multiple passes.) - dataloader.run - dataloader.append_job { resolve(next_results, dataloader) } - end - - nil - end - end - end - end -end diff --git a/lib/graphql/execution/interpreter/runtime.rb b/lib/graphql/execution/interpreter/runtime.rb index a2d0f389f5..900979fe54 100644 --- a/lib/graphql/execution/interpreter/runtime.rb +++ b/lib/graphql/execution/interpreter/runtime.rb @@ -488,7 +488,8 @@ def evaluate_selection_with_resolved_keyword_args(kwarg_arguments, resolved_argu # all of its child fields before moving on to the next root mutation field. # (Subselections of this mutation will still be resolved level-by-level.) if selection_result.graphql_is_eager - Interpreter::Resolve.resolve_all([field_result], @dataloader) + # TODO what to do with this + # Interpreter::Resolve.resolve_all([field_result], @dataloader) end end diff --git a/lib/graphql/schema.rb b/lib/graphql/schema.rb index be3d20b729..1f0094f828 100644 --- a/lib/graphql/schema.rb +++ b/lib/graphql/schema.rb @@ -671,7 +671,7 @@ def union_memberships(type = nil) # @api private # @see GraphQL::Dataloader def dataloader_class - @dataloader_class || GraphQL::Dataloader::NullDataloader + @dataloader_class || GraphQL::Dataloader::FlatDataloader end attr_writer :dataloader_class From e79e04b5d598e60ed8855e8c942bc8631fa57286 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Tue, 2 Sep 2025 06:05:05 -0400 Subject: [PATCH 02/21] fix file endings --- lib/graphql/dataloader.rb | 2 +- lib/graphql/dataloader/flat_dataloader.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/graphql/dataloader.rb b/lib/graphql/dataloader.rb index ad88bffb97..8da6c39465 100644 --- a/lib/graphql/dataloader.rb +++ b/lib/graphql/dataloader.rb @@ -365,4 +365,4 @@ def spawn_source_fiber(trace) end end -require "graphql/dataloader/async_dataloader" \ No newline at end of file +require "graphql/dataloader/async_dataloader" diff --git a/lib/graphql/dataloader/flat_dataloader.rb b/lib/graphql/dataloader/flat_dataloader.rb index 310785c1ad..1e2f079ce3 100644 --- a/lib/graphql/dataloader/flat_dataloader.rb +++ b/lib/graphql/dataloader/flat_dataloader.rb @@ -72,4 +72,4 @@ def with(*) end end end -end \ No newline at end of file +end From 4d57c4d0baa537b0cfad29a37b068846a27f50f6 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Tue, 2 Sep 2025 06:12:14 -0400 Subject: [PATCH 03/21] Handle ExecutionErrors raised from leaf coercion + dataloader --- lib/graphql/execution/interpreter/runtime.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/graphql/execution/interpreter/runtime.rb b/lib/graphql/execution/interpreter/runtime.rb index 900979fe54..4bf6635973 100644 --- a/lib/graphql/execution/interpreter/runtime.rb +++ b/lib/graphql/execution/interpreter/runtime.rb @@ -668,7 +668,11 @@ def continue_field(value, owner_type, field, current_type, ast_node, next_select rescue GraphQL::ExecutionError => ex_err return continue_value(ex_err, field, is_non_null, ast_node, result_name, selection_result) rescue StandardError => err - query.handle_or_reraise(err) + begin + query.handle_or_reraise(err) + rescue GraphQL::ExecutionError => ex_err + return continue_value(ex_err, field, is_non_null, ast_node, result_name, selection_result) + end end set_result(selection_result, result_name, r, false, is_non_null) r From 43e8eb3fa0694d88cab6ab84b5c5ac8c64dbb721 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Tue, 2 Sep 2025 10:46:11 -0400 Subject: [PATCH 04/21] Fix stale references to lazies_at_depth; correctly re-enter execution in FlatDataloader; eagerly resolve mutation fields --- lib/graphql/dataloader.rb | 56 +++++++++++++------- lib/graphql/dataloader/async_dataloader.rb | 2 +- lib/graphql/dataloader/flat_dataloader.rb | 46 ++++++++++------ lib/graphql/execution/interpreter.rb | 2 +- lib/graphql/execution/interpreter/runtime.rb | 8 ++- spec/graphql/dataloader_spec.rb | 2 +- spec/graphql/execution/lazy_spec.rb | 2 +- 7 files changed, 74 insertions(+), 44 deletions(-) diff --git a/lib/graphql/dataloader.rb b/lib/graphql/dataloader.rb index 8da6c39465..c93cde1c8e 100644 --- a/lib/graphql/dataloader.rb +++ b/lib/graphql/dataloader.rb @@ -195,7 +195,8 @@ def run_isolated end end - def run + # @param trace_query_lazy [nil, Execution::Multiplex] + def run(trace_query_lazy: nil) # TODO unify the initialization lazies_at_depth @lazies_at_depth ||= Hash.new { |h, k| h[k] = [] } trace = Fiber[:__graphql_current_multiplex]&.current_trace @@ -231,24 +232,9 @@ def run end join_queues(source_fibers, next_source_fibers) end - if @lazies_at_depth.any? - smallest_depth = nil - @lazies_at_depth.each_key do |depth_key| - smallest_depth ||= depth_key - if depth_key < smallest_depth - smallest_depth = depth_key - end - end - - if smallest_depth - lazies = @lazies_at_depth.delete(smallest_depth) - if !lazies.empty? - append_job { - lazies.each(&:value) # resolve these Lazy instances - } - job_fibers << spawn_job_fiber(trace) - end + with_trace_query_lazy(trace_query_lazy) do + run_pending_lazies(job_fibers, trace) end elsif @steps_to_rerun_after_lazy.any? @pending_jobs.concat(@steps_to_rerun_after_lazy) @@ -282,6 +268,11 @@ def run_fiber(f) f.resume end + # @api private + def lazy_at_depth(depth, lazy) + @lazies_at_depth[depth] << lazy + end + def spawn_fiber fiber_vars = get_fiber_variables Fiber.new(blocking: !@nonblocking) { @@ -309,6 +300,35 @@ def merge_records(records, index_by: :id) private + def run_pending_lazies(job_fibers, trace) + smallest_depth = nil + @lazies_at_depth.each_key do |depth_key| + smallest_depth ||= depth_key + if depth_key < smallest_depth + smallest_depth = depth_key + end + end + + if smallest_depth + lazies = @lazies_at_depth.delete(smallest_depth) + if !lazies.empty? + append_job { + lazies.each(&:value) # resolve these Lazy instances + } + job_fibers << spawn_job_fiber(trace) + end + end + end + + def with_trace_query_lazy(multiplex_or_nil, &block) + if (multiplex = multiplex_or_nil) + query = multiplex.queries.length == 1 ? multiplex.queries[0] : nil + multiplex.current_trace.execute_query_lazy(query: query, multiplex: multiplex, &block) + else + yield + end + end + def calculate_fiber_limit total_fiber_limit = @fiber_limit || Float::INFINITY if total_fiber_limit < 4 diff --git a/lib/graphql/dataloader/async_dataloader.rb b/lib/graphql/dataloader/async_dataloader.rb index cdd77d4e61..a4202cb1fb 100644 --- a/lib/graphql/dataloader/async_dataloader.rb +++ b/lib/graphql/dataloader/async_dataloader.rb @@ -14,7 +14,7 @@ def yield(source = Fiber[:__graphql_current_dataloader_source]) nil end - def run + def run(trace_query_lazy: nil) trace = Fiber[:__graphql_current_multiplex]&.current_trace jobs_fiber_limit, total_fiber_limit = calculate_fiber_limit job_fibers = [] diff --git a/lib/graphql/dataloader/flat_dataloader.rb b/lib/graphql/dataloader/flat_dataloader.rb index 1e2f079ce3..3b157c2773 100644 --- a/lib/graphql/dataloader/flat_dataloader.rb +++ b/lib/graphql/dataloader/flat_dataloader.rb @@ -9,24 +9,13 @@ def initialize(*) @queue = [] end - def run + def run(trace_query_lazy: nil) while @queue.any? - while (step = @queue.shift) - step.call - end - - while @lazies_at_depth&.any? - smallest_depth = nil - @lazies_at_depth.each_key do |depth_key| - smallest_depth ||= depth_key - if depth_key < smallest_depth - smallest_depth = depth_key - end - end - - if smallest_depth - lazies = @lazies_at_depth.delete(smallest_depth) - lazies.each(&:value) # resolve these Lazy instances + run_pending_steps + with_trace_query_lazy(trace_query_lazy) do + while @lazies_at_depth&.any? + run_next_pending_lazies + run_pending_steps end end @@ -70,6 +59,29 @@ def append_job(callable = nil, &block) def with(*) raise GraphQL::Error, "GraphQL::Dataloader is not running -- add `use GraphQL::Dataloader` to your schema to use Dataloader sources." end + + private + + def run_next_pending_lazies + smallest_depth = nil + @lazies_at_depth.each_key do |depth_key| + smallest_depth ||= depth_key + if depth_key < smallest_depth + smallest_depth = depth_key + end + end + + if smallest_depth + lazies = @lazies_at_depth.delete(smallest_depth) + lazies.each(&:value) # resolve these Lazy instances + end + end + + def run_pending_steps + while (step = @queue.shift) + step.call + end + end end end end diff --git a/lib/graphql/execution/interpreter.rb b/lib/graphql/execution/interpreter.rb index c2015f1aac..21b714e4dd 100644 --- a/lib/graphql/execution/interpreter.rb +++ b/lib/graphql/execution/interpreter.rb @@ -88,7 +88,7 @@ def run_all(schema, query_options, context: {}, max_complexity: schema.max_compl } end - multiplex.dataloader.run + multiplex.dataloader.run(trace_query_lazy: multiplex) # Then, find all errors and assign the result to the query object results.each_with_index do |data_result, idx| diff --git a/lib/graphql/execution/interpreter/runtime.rb b/lib/graphql/execution/interpreter/runtime.rb index 4bf6635973..e82620d678 100644 --- a/lib/graphql/execution/interpreter/runtime.rb +++ b/lib/graphql/execution/interpreter/runtime.rb @@ -39,7 +39,6 @@ def initialize(query:, lazies_at_depth:) @query = query @current_trace = query.current_trace @dataloader = query.multiplex.dataloader - @lazies_at_depth = lazies_at_depth @schema = query.schema @context = query.context @response = nil @@ -446,7 +445,7 @@ def evaluate_selection_with_resolved_keyword_args(kwarg_arguments, resolved_argu } end - field_result = call_method_on_directives(:resolve, object, directives) do + call_method_on_directives(:resolve, object, directives) do if !directives.empty? # This might be executed in a different context; reset this info runtime_state = get_current_runtime_state @@ -488,8 +487,7 @@ def evaluate_selection_with_resolved_keyword_args(kwarg_arguments, resolved_argu # all of its child fields before moving on to the next root mutation field. # (Subselections of this mutation will still be resolved level-by-level.) if selection_result.graphql_is_eager - # TODO what to do with this - # Interpreter::Resolve.resolve_all([field_result], @dataloader) + @dataloader.run end end @@ -933,7 +931,7 @@ def after_lazy(lazy_obj, field:, owner_object:, arguments:, ast_node:, result:, current_depth += 1 result = result.graphql_parent end - @lazies_at_depth[current_depth] << lazy + @dataloader.lazy_at_depth(current_depth, lazy) lazy end else diff --git a/spec/graphql/dataloader_spec.rb b/spec/graphql/dataloader_spec.rb index d379c143ca..612126434e 100644 --- a/spec/graphql/dataloader_spec.rb +++ b/spec/graphql/dataloader_spec.rb @@ -1450,7 +1450,7 @@ class CanaryDataloader < GraphQL::Dataloader::NullDataloader end query(query_type) end.execute("{ __typename }") - assert_instance_of GraphQL::Dataloader::NullDataloader, res.context.dataloader + assert_instance_of GraphQL::Dataloader::FlatDataloader, res.context.dataloader res = FiberSchema.execute("{ __typename }") assert_instance_of GraphQL::Dataloader, res.context.dataloader refute res.context.dataloader.nonblocking? diff --git a/spec/graphql/execution/lazy_spec.rb b/spec/graphql/execution/lazy_spec.rb index 6c776a1fa4..eb295d3f5e 100644 --- a/spec/graphql/execution/lazy_spec.rb +++ b/spec/graphql/execution/lazy_spec.rb @@ -101,7 +101,7 @@ end end - it "Handles fields that return nil" do + it "Handles fields that return nil and batches lazy resultion across depths when possible" do values = [ LazyHelpers::MAGIC_NUMBER_THAT_RETURNS_NIL, LazyHelpers::MAGIC_NUMBER_WITH_LAZY_AUTHORIZED_HOOK, From e9dffaa4ad99adecef54bf3bc96698c0824e0a17 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Tue, 2 Sep 2025 10:56:02 -0400 Subject: [PATCH 05/21] Quick test fixes --- lib/graphql/dataloader.rb | 4 ++-- lib/graphql/dataloader/flat_dataloader.rb | 4 ++-- lib/graphql/dataloader/null_dataloader.rb | 2 +- spec/graphql/schema/directive_spec.rb | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/graphql/dataloader.rb b/lib/graphql/dataloader.rb index c93cde1c8e..58649f760f 100644 --- a/lib/graphql/dataloader.rb +++ b/lib/graphql/dataloader.rb @@ -232,11 +232,11 @@ def run(trace_query_lazy: nil) end join_queues(source_fibers, next_source_fibers) end - if @lazies_at_depth.any? + if !@lazies_at_depth.empty? with_trace_query_lazy(trace_query_lazy) do run_pending_lazies(job_fibers, trace) end - elsif @steps_to_rerun_after_lazy.any? + elsif !@steps_to_rerun_after_lazy.empty? @pending_jobs.concat(@steps_to_rerun_after_lazy) f = spawn_job_fiber(trace) job_fibers << f diff --git a/lib/graphql/dataloader/flat_dataloader.rb b/lib/graphql/dataloader/flat_dataloader.rb index 3b157c2773..5e9d4182dd 100644 --- a/lib/graphql/dataloader/flat_dataloader.rb +++ b/lib/graphql/dataloader/flat_dataloader.rb @@ -10,7 +10,7 @@ def initialize(*) end def run(trace_query_lazy: nil) - while @queue.any? + while !@queue.empty? run_pending_steps with_trace_query_lazy(trace_query_lazy) do while @lazies_at_depth&.any? @@ -19,7 +19,7 @@ def run(trace_query_lazy: nil) end end - if @steps_to_rerun_after_lazy.any? + if !@steps_to_rerun_after_lazy.empty? @steps_to_rerun_after_lazy.each(&:call) @steps_to_rerun_after_lazy.clear end diff --git a/lib/graphql/dataloader/null_dataloader.rb b/lib/graphql/dataloader/null_dataloader.rb index 3e431c1086..5f20b1c735 100644 --- a/lib/graphql/dataloader/null_dataloader.rb +++ b/lib/graphql/dataloader/null_dataloader.rb @@ -11,7 +11,7 @@ class NullDataloader < Dataloader # executed synchronously. def initialize(*); end - def run; end + def run(trace_query_lazy: nil); end def run_isolated; yield; end def clear_cache; end def yield(_source) diff --git a/spec/graphql/schema/directive_spec.rb b/spec/graphql/schema/directive_spec.rb index 6b1d7b1faf..3dbfcc5027 100644 --- a/spec/graphql/schema/directive_spec.rb +++ b/spec/graphql/schema/directive_spec.rb @@ -117,7 +117,7 @@ def self.resolve(obj, args, ctx) result = nil ctx.dataloader.run_isolated do result = yield - GraphQL::Execution::Interpreter::Resolve.resolve_all([result], ctx.dataloader) + ctx.dataloader.run end ctx[:count_fields] ||= Hash.new { |h, k| h[k] = [] } From bb3f2da127ef7efa07aee7fc1eecf1adc660924d Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Tue, 2 Sep 2025 11:21:04 -0400 Subject: [PATCH 06/21] Add deprecations to Execution::Resolve; better lazy/Dataloader integration; fix backtrace_spec --- lib/graphql/dataloader.rb | 62 +++++++------ lib/graphql/execution/interpreter/resolve.rb | 94 ++++++++++++++++++++ spec/graphql/backtrace_spec.rb | 2 +- 3 files changed, 129 insertions(+), 29 deletions(-) create mode 100644 lib/graphql/execution/interpreter/resolve.rb diff --git a/lib/graphql/dataloader.rb b/lib/graphql/dataloader.rb index 58649f760f..6e1dba5fe6 100644 --- a/lib/graphql/dataloader.rb +++ b/lib/graphql/dataloader.rb @@ -149,7 +149,7 @@ def yield(source = Fiber[:__graphql_current_dataloader_source]) # @api private Nothing to see here def append_job(callable = nil, &job) # Given a block, queue it up to be worked through when `#run` is called. - # (If the dataloader is already running, than a Fiber will pick this up later.) + # (If the dataloader is already running, then a Fiber will pick this up later.) @pending_jobs.push(callable || job) nil end @@ -211,30 +211,12 @@ def run(trace_query_lazy: nil) while first_pass || !job_fibers.empty? first_pass = false - while (f = (job_fibers.shift || (((next_job_fibers.size + job_fibers.size) < jobs_fiber_limit) && spawn_job_fiber(trace)))) - if f.alive? - finished = run_fiber(f) - if !finished - next_job_fibers << f - end - end - end - join_queues(job_fibers, next_job_fibers) - - while (!source_fibers.empty? || @source_cache.each_value.any? { |group_sources| group_sources.each_value.any?(&:pending?) }) - while (f = source_fibers.shift || (((job_fibers.size + source_fibers.size + next_source_fibers.size + next_job_fibers.size) < total_fiber_limit) && spawn_source_fiber(trace))) - if f.alive? - finished = run_fiber(f) - if !finished - next_source_fibers << f - end - end - end - join_queues(source_fibers, next_source_fibers) - end + run_pending_steps(trace, job_fibers, next_job_fibers, jobs_fiber_limit, source_fibers, next_source_fibers, total_fiber_limit) + if !@lazies_at_depth.empty? with_trace_query_lazy(trace_query_lazy) do - run_pending_lazies(job_fibers, trace) + run_next_pending_lazies(job_fibers, trace) + run_pending_steps(trace, job_fibers, next_job_fibers, jobs_fiber_limit, source_fibers, next_source_fibers, total_fiber_limit) end elsif !@steps_to_rerun_after_lazy.empty? @pending_jobs.concat(@steps_to_rerun_after_lazy) @@ -300,7 +282,7 @@ def merge_records(records, index_by: :id) private - def run_pending_lazies(job_fibers, trace) + def run_next_pending_lazies(job_fibers, trace) smallest_depth = nil @lazies_at_depth.each_key do |depth_key| smallest_depth ||= depth_key @@ -312,11 +294,35 @@ def run_pending_lazies(job_fibers, trace) if smallest_depth lazies = @lazies_at_depth.delete(smallest_depth) if !lazies.empty? - append_job { - lazies.each(&:value) # resolve these Lazy instances - } - job_fibers << spawn_job_fiber(trace) + lazies.each_with_index do |l, idx| + append_job { l.value } + end + job_fibers.unshift(spawn_job_fiber(trace)) + end + end + end + + def run_pending_steps(trace, job_fibers, next_job_fibers, jobs_fiber_limit, source_fibers, next_source_fibers, total_fiber_limit) + while (f = (job_fibers.shift || (((next_job_fibers.size + job_fibers.size) < jobs_fiber_limit) && spawn_job_fiber(trace)))) + if f.alive? + finished = run_fiber(f) + if !finished + next_job_fibers << f + end + end + end + join_queues(job_fibers, next_job_fibers) + + while (!source_fibers.empty? || @source_cache.each_value.any? { |group_sources| group_sources.each_value.any?(&:pending?) }) + while (f = source_fibers.shift || (((job_fibers.size + source_fibers.size + next_source_fibers.size + next_job_fibers.size) < total_fiber_limit) && spawn_source_fiber(trace))) + if f.alive? + finished = run_fiber(f) + if !finished + next_source_fibers << f + end + end end + join_queues(source_fibers, next_source_fibers) end end diff --git a/lib/graphql/execution/interpreter/resolve.rb b/lib/graphql/execution/interpreter/resolve.rb new file mode 100644 index 0000000000..102ceacb3c --- /dev/null +++ b/lib/graphql/execution/interpreter/resolve.rb @@ -0,0 +1,94 @@ +# frozen_string_literal: true + +module GraphQL + module Execution + class Interpreter + module Resolve + # Continue field results in `results` until there's nothing else to continue. + # @return [void] + # @deprecated Call `dataloader.run` instead + def self.resolve_all(results, dataloader) + warn "#{self}.#{__method__} is deprecated; Use `dataloader.run` instead.#{caller(1, 5).map { |l| "\n #{l}"}.join}" + dataloader.append_job { resolve(results, dataloader) } + nil + end + + # @deprecated Call `dataloader.run` instead + def self.resolve_each_depth(lazies_at_depth, dataloader) + warn "#{self}.#{__method__} is deprecated; Use `dataloader.run` instead.#{caller(1, 5).map { |l| "\n #{l}"}.join}" + + smallest_depth = nil + lazies_at_depth.each_key do |depth_key| + smallest_depth ||= depth_key + if depth_key < smallest_depth + smallest_depth = depth_key + end + end + + if smallest_depth + lazies = lazies_at_depth.delete(smallest_depth) + if !lazies.empty? + lazies.each do |l| + dataloader.append_job { l.value } + end + # Run lazies _and_ dataloader, see if more are enqueued + dataloader.run + resolve_each_depth(lazies_at_depth, dataloader) + end + end + nil + end + + # @deprecated Call `dataloader.run` instead + def self.resolve(results, dataloader) + warn "#{self}.#{__method__} is deprecated; Use `dataloader.run` instead.#{caller(1, 5).map { |l| "\n #{l}"}.join}" + # There might be pending jobs here that _will_ write lazies + # into the result hash. We should run them out, so we + # can be sure that all lazies will be present in the result hashes. + # A better implementation would somehow interleave (or unify) + # these approaches. + dataloader.run + next_results = [] + while !results.empty? + result_value = results.shift + if result_value.is_a?(Runtime::GraphQLResultHash) || result_value.is_a?(Hash) + results.concat(result_value.values) + next + elsif result_value.is_a?(Runtime::GraphQLResultArray) + results.concat(result_value.values) + next + elsif result_value.is_a?(Array) + results.concat(result_value) + next + elsif result_value.is_a?(Lazy) + loaded_value = result_value.value + if loaded_value.is_a?(Lazy) + # Since this field returned another lazy, + # add it to the same queue + results << loaded_value + elsif loaded_value.is_a?(Runtime::GraphQLResultHash) || loaded_value.is_a?(Runtime::GraphQLResultArray) || + loaded_value.is_a?(Hash) || loaded_value.is_a?(Array) + # Add these values in wholesale -- + # they might be modified by later work in the dataloader. + next_results << loaded_value + end + end + end + + if !next_results.empty? + # Any pending data loader jobs may populate the + # resutl arrays or result hashes accumulated in + # `next_results``. Run those **to completion** + # before continuing to resolve `next_results`. + # (Just `.append_job` doesn't work if any pending + # jobs require multiple passes.) + dataloader.run + dataloader.append_job { resolve(next_results, dataloader) } + end + + nil + end + end + end + end +end diff --git a/spec/graphql/backtrace_spec.rb b/spec/graphql/backtrace_spec.rb index ddcfc87f62..7915c54fae 100644 --- a/spec/graphql/backtrace_spec.rb +++ b/spec/graphql/backtrace_spec.rb @@ -132,7 +132,7 @@ def execute_multiplex(multiplex:) b = err.cause.backtrace assert_backtrace_includes(b, file: "backtrace_spec.rb", method: "block") assert_backtrace_includes(b, file: "field.rb", method: "resolve") - assert_backtrace_includes(b, file: "runtime.rb", method: "evaluate_selections") + assert_backtrace_includes(b, file: "runtime.rb", method: "evaluate_selection") assert_backtrace_includes(b, file: "interpreter.rb", method: "run_all") # GraphQL backtrace is present From e901a86dc80b0b41b96ae8fc5955e59e3873ce16 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Tue, 2 Sep 2025 11:32:37 -0400 Subject: [PATCH 07/21] Debug mutation test --- lib/graphql/dataloader/flat_dataloader.rb | 1 + spec/graphql/execution/interpreter_spec.rb | 24 +++++++++++++++++++--- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/lib/graphql/dataloader/flat_dataloader.rb b/lib/graphql/dataloader/flat_dataloader.rb index 5e9d4182dd..d144fcd7f0 100644 --- a/lib/graphql/dataloader/flat_dataloader.rb +++ b/lib/graphql/dataloader/flat_dataloader.rb @@ -73,6 +73,7 @@ def run_next_pending_lazies if smallest_depth lazies = @lazies_at_depth.delete(smallest_depth) + puts "-- run_next_pending_lazies #{smallest_depth} || #{lazies.size}" lazies.each(&:value) # resolve these Lazy instances end end diff --git a/spec/graphql/execution/interpreter_spec.rb b/spec/graphql/execution/interpreter_spec.rb index 798cb7dd74..eb91b73287 100644 --- a/spec/graphql/execution/interpreter_spec.rb +++ b/spec/graphql/execution/interpreter_spec.rb @@ -239,16 +239,26 @@ def nested_query(query:) class Counter < GraphQL::Schema::Object field :value, Integer, null: false + + def value + v = object.value + puts "value #{v} @ #{context.current_path}" + v + end field :lazy_value, Integer, null: false def lazy_value - Box.new { object.value } + Box.new { + puts "lazy_value #{object.value}" + object.value + } end field :increment, Counter, null: false def increment - object.value += 1 + v = object.value += 1 + puts "increment #{v}" object end end @@ -258,7 +268,8 @@ class Mutation < GraphQL::Schema::Object def increment_counter counter = context[:counter] - counter.value += 1 + v = counter.value += 1 + puts "Mutation.incrementCounter #{v}" counter end end @@ -310,6 +321,9 @@ def execute_multiplex(multiplex:) end end trace_with(EnsureThreadCleanedUp) + + # TODO also test with dataloader enabled + # use GraphQL::Dataloader end end @@ -370,7 +384,10 @@ def execute_multiplex(multiplex:) assert_nil Fiber[:__graphql_runtime_info] end + focus it "runs mutation roots atomically and sequentially" do + # It's not continuing from `increment` to value; + # Instead it's calling increment twice before it calls value at all. query_str = <<-GRAPHQL mutation { i1: incrementCounter { value lazyValue @@ -383,6 +400,7 @@ def execute_multiplex(multiplex:) GRAPHQL result = InterpreterTest::Schema.execute(query_str, context: { counter: OpenStruct.new(value: 0) }) + pp result.context.dataloader.class expected_data = { "i1" => { "value" => 1, From e9f8dc2817a4620c7aa4d1e5bc510d179d9936fa Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Tue, 2 Sep 2025 11:37:36 -0400 Subject: [PATCH 08/21] Update spec for non-depth-first behavior with FlatDataloader --- lib/graphql/dataloader/flat_dataloader.rb | 1 - spec/graphql/execution/interpreter_spec.rb | 60 ++++++++++++---------- 2 files changed, 33 insertions(+), 28 deletions(-) diff --git a/lib/graphql/dataloader/flat_dataloader.rb b/lib/graphql/dataloader/flat_dataloader.rb index d144fcd7f0..5e9d4182dd 100644 --- a/lib/graphql/dataloader/flat_dataloader.rb +++ b/lib/graphql/dataloader/flat_dataloader.rb @@ -73,7 +73,6 @@ def run_next_pending_lazies if smallest_depth lazies = @lazies_at_depth.delete(smallest_depth) - puts "-- run_next_pending_lazies #{smallest_depth} || #{lazies.size}" lazies.each(&:value) # resolve these Lazy instances end end diff --git a/spec/graphql/execution/interpreter_spec.rb b/spec/graphql/execution/interpreter_spec.rb index eb91b73287..3965d83e81 100644 --- a/spec/graphql/execution/interpreter_spec.rb +++ b/spec/graphql/execution/interpreter_spec.rb @@ -241,25 +241,36 @@ class Counter < GraphQL::Schema::Object field :value, Integer, null: false def value - v = object.value - puts "value #{v} @ #{context.current_path}" - v + counter.value end + field :lazy_value, Integer, null: false def lazy_value - Box.new { - puts "lazy_value #{object.value}" - object.value - } + Box.new { counter.value } end + field :incremented_value, Integer, hash_key: :incremented_value + field :increment, Counter, null: false def increment - v = object.value += 1 - puts "increment #{v}" - object + v = counter.value += 1 + { + counter: counter, + incremented_value: v, + } + end + + + private + + def counter + if object.is_a?(Hash) && object.key?(:counter) + object[:counter] + else + object + end end end @@ -269,8 +280,10 @@ class Mutation < GraphQL::Schema::Object def increment_counter counter = context[:counter] v = counter.value += 1 - puts "Mutation.incrementCounter #{v}" - counter + { + counter: counter, + incremented_value: v + } end end @@ -321,9 +334,6 @@ def execute_multiplex(multiplex:) end end trace_with(EnsureThreadCleanedUp) - - # TODO also test with dataloader enabled - # use GraphQL::Dataloader end end @@ -384,34 +394,30 @@ def execute_multiplex(multiplex:) assert_nil Fiber[:__graphql_runtime_info] end - focus it "runs mutation roots atomically and sequentially" do - # It's not continuing from `increment` to value; - # Instead it's calling increment twice before it calls value at all. query_str = <<-GRAPHQL mutation { i1: incrementCounter { value lazyValue - i2: increment { value lazyValue } - i3: increment { value lazyValue } + i2: increment { value incrementedValue lazyValue } + i3: increment { value incrementedValue lazyValue } } - i4: incrementCounter { value lazyValue } - i5: incrementCounter { value lazyValue } + i4: incrementCounter { value incrementedValue lazyValue } + i5: incrementCounter { value incrementedValue lazyValue } } GRAPHQL result = InterpreterTest::Schema.execute(query_str, context: { counter: OpenStruct.new(value: 0) }) - pp result.context.dataloader.class expected_data = { "i1" => { "value" => 1, # All of these get `3` as lazy value. They're resolved together, # since they aren't _root_ mutation fields. "lazyValue" => 3, - "i2" => { "value" => 2, "lazyValue" => 3 }, - "i3" => { "value" => 3, "lazyValue" => 3 }, + "i2" => { "value" => 3, "incrementedValue" => 2, "lazyValue" => 3 }, + "i3" => { "value" => 3, "incrementedValue" => 3, "lazyValue" => 3 }, }, - "i4" => { "value" => 4, "lazyValue" => 4}, - "i5" => { "value" => 5, "lazyValue" => 5}, + "i4" => { "value" => 4, "incrementedValue" => 4, "lazyValue" => 4}, + "i5" => { "value" => 5, "incrementedValue" => 5, "lazyValue" => 5}, } assert_graphql_equal expected_data, result["data"] end From 4fb63079025c924aeacb3d0595c9877c7d5fe2ee Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Tue, 2 Sep 2025 13:41:48 -0400 Subject: [PATCH 09/21] Fix context for argument errors --- lib/graphql/execution/interpreter/runtime.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/graphql/execution/interpreter/runtime.rb b/lib/graphql/execution/interpreter/runtime.rb index e82620d678..6e089be27e 100644 --- a/lib/graphql/execution/interpreter/runtime.rb +++ b/lib/graphql/execution/interpreter/runtime.rb @@ -364,6 +364,10 @@ def evaluate_selection(result_name, field_ast_nodes_or_ast_node, selections_resu else @query.arguments_cache.dataload_for(ast_node, field_defn, owner_object) do |resolved_arguments| runtime_state = get_current_runtime_state # This might be in a different fiber + runtime_state.current_field = field_defn + runtime_state.current_arguments = resolved_arguments + runtime_state.current_result_name = result_name + runtime_state.current_result = selections_result evaluate_selection_with_args(resolved_arguments, field_defn, ast_node, field_ast_nodes, owner_object, result_name, selections_result, runtime_state) end end From c2257b8bfff5ac61eb7ae4ea53ef0738e8846397 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Tue, 2 Sep 2025 13:47:31 -0400 Subject: [PATCH 10/21] Update specs for resolution order --- spec/graphql/authorization_spec.rb | 4 ++-- spec/graphql/tracing/platform_trace_spec.rb | 16 ++++++++-------- spec/graphql/tracing/platform_tracing_spec.rb | 16 ++++++++-------- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/spec/graphql/authorization_spec.rb b/spec/graphql/authorization_spec.rb index b49af2aa44..c8f389d95b 100644 --- a/spec/graphql/authorization_spec.rb +++ b/spec/graphql/authorization_spec.rb @@ -767,9 +767,9 @@ def auth_execute(*args, **kwargs) assert_equal "RelayObjectEdge", edge["__typename"] unauthorized_object_paths = [ - ["unauthorizedConnection", "edges", 0, "node"], + ["unauthorizedEdge", "node"], ["unauthorizedConnection", "nodes", 0], - ["unauthorizedEdge", "node"] + ["unauthorizedConnection", "edges", 0, "node"], ] assert_equal unauthorized_object_paths, unauthorized_res["errors"].map { |e| e["path"] } diff --git a/spec/graphql/tracing/platform_trace_spec.rb b/spec/graphql/tracing/platform_trace_spec.rb index d13a39e633..69a2b12d5e 100644 --- a/spec/graphql/tracing/platform_trace_spec.rb +++ b/spec/graphql/tracing/platform_trace_spec.rb @@ -142,23 +142,23 @@ def platform_resolve_type_key(type) "Edible.resolve_type", "Cheese.authorized", "Cheese.authorized", - "DynamicFields.authorized", - "D._", - "C.f", "Edible.resolve_type", "Cheese.authorized", "Cheese.authorized", - "DynamicFields.authorized", - "D._", - "C.f", "Edible.resolve_type", "Cheese.authorized", "Cheese.authorized", + "Edible.resolve_type", + "Milk.authorized", + "DynamicFields.authorized", + "D._", + "C.f", + "DynamicFields.authorized", + "D._", + "C.f", "DynamicFields.authorized", "D._", "C.f", - "Edible.resolve_type", - "Milk.authorized", "DynamicFields.authorized", "D._", "E.f", diff --git a/spec/graphql/tracing/platform_tracing_spec.rb b/spec/graphql/tracing/platform_tracing_spec.rb index a0d41faae3..ddf9ae11db 100644 --- a/spec/graphql/tracing/platform_tracing_spec.rb +++ b/spec/graphql/tracing/platform_tracing_spec.rb @@ -148,23 +148,23 @@ def platform_trace(platform_key, key, data) "Edible.resolve_type", "Cheese.authorized", "Cheese.authorized", - "DynamicFields.authorized", - "D._", - "C.f", "Edible.resolve_type", "Cheese.authorized", "Cheese.authorized", - "DynamicFields.authorized", - "D._", - "C.f", "Edible.resolve_type", "Cheese.authorized", "Cheese.authorized", + "Edible.resolve_type", + "Milk.authorized", + "DynamicFields.authorized", + "D._", + "C.f", + "DynamicFields.authorized", + "D._", + "C.f", "DynamicFields.authorized", "D._", "C.f", - "Edible.resolve_type", - "Milk.authorized", "DynamicFields.authorized", "D._", "E.f", From fc4067734dd19fc026f1e7ada1989a6533f4db9c Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Tue, 2 Sep 2025 14:10:12 -0400 Subject: [PATCH 11/21] Add Lazy handling to AsyncDataloader --- lib/graphql/dataloader/async_dataloader.rb | 36 ++++++++++++++++------ spec/graphql/dataloader_spec.rb | 1 + 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/lib/graphql/dataloader/async_dataloader.rb b/lib/graphql/dataloader/async_dataloader.rb index a4202cb1fb..52a856ede8 100644 --- a/lib/graphql/dataloader/async_dataloader.rb +++ b/lib/graphql/dataloader/async_dataloader.rb @@ -29,16 +29,7 @@ def run(trace_query_lazy: nil) first_pass = false fiber_vars = get_fiber_variables - while (f = (job_fibers.shift || (((job_fibers.size + next_job_fibers.size + source_tasks.size) < jobs_fiber_limit) && spawn_job_fiber(trace)))) - if f.alive? - finished = run_fiber(f) - if !finished - next_job_fibers << f - end - end - end - job_fibers.concat(next_job_fibers) - next_job_fibers.clear + run_pending_steps(job_fibers, next_job_fibers, source_tasks, jobs_fiber_limit, trace) Sync do |root_task| set_fiber_variables(fiber_vars) @@ -54,6 +45,18 @@ def run(trace_query_lazy: nil) next_source_tasks.clear end end + + if !@lazies_at_depth.empty? + with_trace_query_lazy(trace_query_lazy) do + run_next_pending_lazies(job_fibers, trace) + run_pending_steps(job_fibers, next_job_fibers, source_tasks, jobs_fiber_limit, trace) + end + elsif !@steps_to_rerun_after_lazy.empty? + @pending_jobs.concat(@steps_to_rerun_after_lazy) + f = spawn_job_fiber(trace) + job_fibers << f + @steps_to_rerun_after_lazy.clear + end end trace&.end_dataloader(self) end @@ -69,6 +72,19 @@ def run(trace_query_lazy: nil) private + def run_pending_steps(job_fibers, next_job_fibers, source_tasks, jobs_fiber_limit, trace) + while (f = (job_fibers.shift || (((job_fibers.size + next_job_fibers.size + source_tasks.size) < jobs_fiber_limit) && spawn_job_fiber(trace)))) + if f.alive? + finished = run_fiber(f) + if !finished + next_job_fibers << f + end + end + end + job_fibers.concat(next_job_fibers) + next_job_fibers.clear + end + def spawn_source_task(parent_task, condition, trace) pending_sources = nil @source_cache.each_value do |source_by_batch_params| diff --git a/spec/graphql/dataloader_spec.rb b/spec/graphql/dataloader_spec.rb index 612126434e..64afc25682 100644 --- a/spec/graphql/dataloader_spec.rb +++ b/spec/graphql/dataloader_spec.rb @@ -940,6 +940,7 @@ def self.included(child_class) query_str = "{ cookbooks { featuredRecipe { name } } }" context = { batched_calls_counter: BatchedCallsCounter.new } result = schema.execute(query_str, context: context) + assert_equal ["Cornbread", "Grits"], result["data"]["cookbooks"].map { |c| c["featuredRecipe"]["name"] } refute result.key?("errors") assert_equal 1, context[:batched_calls_counter].count end From ba2712d28dd3ad08fac1c20d4513520c51090430 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Tue, 2 Sep 2025 14:27:05 -0400 Subject: [PATCH 12/21] Fix initialization; rebuild snapshot --- lib/graphql/dataloader/async_dataloader.rb | 2 + .../graphql/dataloader/snapshots/example.json | 86 ------------------- 2 files changed, 2 insertions(+), 86 deletions(-) diff --git a/lib/graphql/dataloader/async_dataloader.rb b/lib/graphql/dataloader/async_dataloader.rb index 52a856ede8..5b1e4c6268 100644 --- a/lib/graphql/dataloader/async_dataloader.rb +++ b/lib/graphql/dataloader/async_dataloader.rb @@ -15,6 +15,8 @@ def yield(source = Fiber[:__graphql_current_dataloader_source]) end def run(trace_query_lazy: nil) + # TODO unify the initialization lazies_at_depth + @lazies_at_depth ||= Hash.new { |h, k| h[k] = [] } trace = Fiber[:__graphql_current_multiplex]&.current_trace jobs_fiber_limit, total_fiber_limit = calculate_fiber_limit job_fibers = [] diff --git a/spec/graphql/dataloader/snapshots/example.json b/spec/graphql/dataloader/snapshots/example.json index 2bdcede08b..1117679b89 100644 --- a/spec/graphql/dataloader/snapshots/example.json +++ b/spec/graphql/dataloader/snapshots/example.json @@ -2610,92 +2610,6 @@ }, "sequenceFlags": 101010101010 }, - { - "timestamp": "10101010101010", - "trustedPacketSequenceId": 101010101010, - "trackEvent": { - "type": "TYPE_COUNTER", - "trackUuid": "10101010101010", - "counterValue": "10101010101010" - }, - "sequenceFlags": 101010101010 - }, - { - "trustedPacketSequenceId": 101010101010, - "sequenceFlags": 101010101010, - "trackDescriptor": { - "uuid": "10101010101010", - "name": "Dataloader Fiber #1010", - "parentUuid": "10101010101010", - "childOrdering": "CHRONOLOGICAL" - } - }, - { - "timestamp": "10101010101010", - "trustedPacketSequenceId": 101010101010, - "trackEvent": { - "categories": [ - "Dataloader" - ], - "categoryIids": [ - "10101010101010" - ], - "type": "TYPE_INSTANT", - "trackUuid": "10101010101010", - "extraCounterValues": [ - "10101010101010", - "10101010101010" - ], - "name": "Create Execution Fiber", - "extraCounterTrackUuids": [ - "10101010101010", - "10101010101010" - ] - }, - "sequenceFlags": 101010101010 - }, - { - "trustedPacketSequenceId": 101010101010, - "sequenceFlags": 101010101010, - "trackDescriptor": { - "uuid": "10101010101010", - "name": "Exec Fiber #1010", - "parentUuid": "10101010101010", - "childOrdering": "CHRONOLOGICAL" - } - }, - { - "timestamp": "10101010101010", - "trustedPacketSequenceId": 101010101010, - "trackEvent": { - "categories": [ - "Dataloader" - ], - "categoryIids": [ - "10101010101010" - ], - "type": "TYPE_INSTANT", - "trackUuid": "10101010101010", - "extraCounterValues": [ - "10101010101010" - ], - "name": "Fiber Exit", - "extraCounterTrackUuids": [ - "10101010101010" - ] - }, - "sequenceFlags": 101010101010 - }, - { - "timestamp": "10101010101010", - "trustedPacketSequenceId": 101010101010, - "trackEvent": { - "type": "TYPE_COUNTER", - "trackUuid": "10101010101010", - "counterValue": "10101010101010" - }, - "sequenceFlags": 101010101010 - }, { "timestamp": "10101010101010", "trustedPacketSequenceId": 101010101010, From 34931293b0fa2aae55fdc0c578bb6954c9cefd4b Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Tue, 2 Sep 2025 14:29:04 -0400 Subject: [PATCH 13/21] Remove steps_to_rerun_after_lazy since it's not used yet --- lib/graphql/dataloader.rb | 8 -------- lib/graphql/dataloader/async_dataloader.rb | 5 ----- lib/graphql/dataloader/flat_dataloader.rb | 9 --------- 3 files changed, 22 deletions(-) diff --git a/lib/graphql/dataloader.rb b/lib/graphql/dataloader.rb index 6e1dba5fe6..63ad7d4f11 100644 --- a/lib/graphql/dataloader.rb +++ b/lib/graphql/dataloader.rb @@ -65,14 +65,11 @@ def initialize(nonblocking: self.class.default_nonblocking, fiber_limit: self.cl @nonblocking = nonblocking end @fiber_limit = fiber_limit - @steps_to_rerun_after_lazy = [] @lazies_at_depth = nil end attr_accessor :lazies_at_depth - attr_reader :steps_to_rerun_after_lazy - # @return [Integer, nil] attr_reader :fiber_limit @@ -218,11 +215,6 @@ def run(trace_query_lazy: nil) run_next_pending_lazies(job_fibers, trace) run_pending_steps(trace, job_fibers, next_job_fibers, jobs_fiber_limit, source_fibers, next_source_fibers, total_fiber_limit) end - elsif !@steps_to_rerun_after_lazy.empty? - @pending_jobs.concat(@steps_to_rerun_after_lazy) - f = spawn_job_fiber(trace) - job_fibers << f - @steps_to_rerun_after_lazy.clear end end diff --git a/lib/graphql/dataloader/async_dataloader.rb b/lib/graphql/dataloader/async_dataloader.rb index 5b1e4c6268..c7fde4bfb6 100644 --- a/lib/graphql/dataloader/async_dataloader.rb +++ b/lib/graphql/dataloader/async_dataloader.rb @@ -53,11 +53,6 @@ def run(trace_query_lazy: nil) run_next_pending_lazies(job_fibers, trace) run_pending_steps(job_fibers, next_job_fibers, source_tasks, jobs_fiber_limit, trace) end - elsif !@steps_to_rerun_after_lazy.empty? - @pending_jobs.concat(@steps_to_rerun_after_lazy) - f = spawn_job_fiber(trace) - job_fibers << f - @steps_to_rerun_after_lazy.clear end end trace&.end_dataloader(self) diff --git a/lib/graphql/dataloader/flat_dataloader.rb b/lib/graphql/dataloader/flat_dataloader.rb index 5e9d4182dd..2cab86fd73 100644 --- a/lib/graphql/dataloader/flat_dataloader.rb +++ b/lib/graphql/dataloader/flat_dataloader.rb @@ -5,7 +5,6 @@ class FlatDataloader < Dataloader def initialize(*) # TODO unify the initialization lazies_at_depth @lazies_at_depth ||= Hash.new { |h, k| h[k] = [] } - @steps_to_rerun_after_lazy = [] @queue = [] end @@ -18,19 +17,12 @@ def run(trace_query_lazy: nil) run_pending_steps end end - - if !@steps_to_rerun_after_lazy.empty? - @steps_to_rerun_after_lazy.each(&:call) - @steps_to_rerun_after_lazy.clear - end end end def run_isolated prev_queue = @queue - prev_stral = @steps_to_rerun_after_lazy prev_lad = @lazies_at_depth - @steps_to_rerun_after_lazy = [] @queue = [] @lazies_at_depth = @lazies_at_depth.dup&.clear res = nil @@ -41,7 +33,6 @@ def run_isolated res ensure @queue = prev_queue - @steps_to_rerun_after_lazy = prev_stral @lazies_at_depth = prev_lad end From 78bd98f7613c24d0a00fbd0e57902bd21390286d Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Tue, 2 Sep 2025 14:35:50 -0400 Subject: [PATCH 14/21] Clean up lazies_at_depth, DRY FlatDataloader --- lib/graphql/dataloader.rb | 9 +++--- lib/graphql/dataloader/async_dataloader.rb | 2 -- lib/graphql/dataloader/flat_dataloader.rb | 32 +++----------------- lib/graphql/execution/interpreter.rb | 4 +-- lib/graphql/execution/interpreter/runtime.rb | 2 +- 5 files changed, 10 insertions(+), 39 deletions(-) diff --git a/lib/graphql/dataloader.rb b/lib/graphql/dataloader.rb index 63ad7d4f11..80e0211d37 100644 --- a/lib/graphql/dataloader.rb +++ b/lib/graphql/dataloader.rb @@ -65,11 +65,9 @@ def initialize(nonblocking: self.class.default_nonblocking, fiber_limit: self.cl @nonblocking = nonblocking end @fiber_limit = fiber_limit - @lazies_at_depth = nil + @lazies_at_depth = Hash.new { |h, k| h[k] = [] } end - attr_accessor :lazies_at_depth - # @return [Integer, nil] attr_reader :fiber_limit @@ -164,6 +162,8 @@ def clear_cache def run_isolated prev_queue = @pending_jobs prev_pending_keys = {} + prev_lazies_at_depth = @lazies_at_depth + @lazies_at_depth = @lazies_at_depth.dup.clear @source_cache.each do |source_class, batched_sources| batched_sources.each do |batch_args, batched_source_instance| if batched_source_instance.pending? @@ -183,6 +183,7 @@ def run_isolated res ensure @pending_jobs = prev_queue + @lazies_at_depth = prev_lazies_at_depth prev_pending_keys.each do |source_instance, pending| pending.each do |key, value| if !source_instance.results.key?(key) @@ -194,8 +195,6 @@ def run_isolated # @param trace_query_lazy [nil, Execution::Multiplex] def run(trace_query_lazy: nil) - # TODO unify the initialization lazies_at_depth - @lazies_at_depth ||= Hash.new { |h, k| h[k] = [] } trace = Fiber[:__graphql_current_multiplex]&.current_trace jobs_fiber_limit, total_fiber_limit = calculate_fiber_limit job_fibers = [] diff --git a/lib/graphql/dataloader/async_dataloader.rb b/lib/graphql/dataloader/async_dataloader.rb index c7fde4bfb6..9781dda03b 100644 --- a/lib/graphql/dataloader/async_dataloader.rb +++ b/lib/graphql/dataloader/async_dataloader.rb @@ -15,8 +15,6 @@ def yield(source = Fiber[:__graphql_current_dataloader_source]) end def run(trace_query_lazy: nil) - # TODO unify the initialization lazies_at_depth - @lazies_at_depth ||= Hash.new { |h, k| h[k] = [] } trace = Fiber[:__graphql_current_multiplex]&.current_trace jobs_fiber_limit, total_fiber_limit = calculate_fiber_limit job_fibers = [] diff --git a/lib/graphql/dataloader/flat_dataloader.rb b/lib/graphql/dataloader/flat_dataloader.rb index 2cab86fd73..defc9f3563 100644 --- a/lib/graphql/dataloader/flat_dataloader.rb +++ b/lib/graphql/dataloader/flat_dataloader.rb @@ -2,17 +2,11 @@ module GraphQL class Dataloader class FlatDataloader < Dataloader - def initialize(*) - # TODO unify the initialization lazies_at_depth - @lazies_at_depth ||= Hash.new { |h, k| h[k] = [] } - @queue = [] - end - def run(trace_query_lazy: nil) - while !@queue.empty? + while !@pending_jobs.empty? run_pending_steps with_trace_query_lazy(trace_query_lazy) do - while @lazies_at_depth&.any? + while !@lazies_at_depth.empty? run_next_pending_lazies run_pending_steps end @@ -20,30 +14,12 @@ def run(trace_query_lazy: nil) end end - def run_isolated - prev_queue = @queue - prev_lad = @lazies_at_depth - @queue = [] - @lazies_at_depth = @lazies_at_depth.dup&.clear - res = nil - append_job { - res = yield - } - run - res - ensure - @queue = prev_queue - @lazies_at_depth = prev_lad - end - - def clear_cache; end - def yield(_source) raise GraphQL::Error, "GraphQL::Dataloader is not running -- add `use GraphQL::Dataloader` to your schema to use Dataloader sources." end def append_job(callable = nil, &block) - @queue << (callable || block) + @pending_jobs << (callable || block) nil end @@ -69,7 +45,7 @@ def run_next_pending_lazies end def run_pending_steps - while (step = @queue.shift) + while (step = @pending_jobs.shift) step.call end end diff --git a/lib/graphql/execution/interpreter.rb b/lib/graphql/execution/interpreter.rb index 21b714e4dd..b849bebd92 100644 --- a/lib/graphql/execution/interpreter.rb +++ b/lib/graphql/execution/interpreter.rb @@ -41,8 +41,6 @@ def run_all(schema, query_options, context: {}, max_complexity: schema.max_compl trace.execute_multiplex(multiplex: multiplex) do schema = multiplex.schema queries = multiplex.queries - lazies_at_depth = Hash.new { |h, k| h[k] = [] } - multiplex.dataloader.lazies_at_depth = lazies_at_depth multiplex_analyzers = schema.multiplex_analyzers if multiplex.max_complexity multiplex_analyzers += [GraphQL::Analysis::MaxQueryComplexity] @@ -73,7 +71,7 @@ def run_all(schema, query_options, context: {}, max_complexity: schema.max_compl # Although queries in a multiplex _share_ an Interpreter instance, # they also have another item of state, which is private to that query # in particular, assign it here: - runtime = Runtime.new(query: query, lazies_at_depth: lazies_at_depth) + runtime = Runtime.new(query: query) query.context.namespace(:interpreter_runtime)[:runtime] = runtime query.current_trace.execute_query(query: query) do diff --git a/lib/graphql/execution/interpreter/runtime.rb b/lib/graphql/execution/interpreter/runtime.rb index 6e089be27e..b37ab4534e 100644 --- a/lib/graphql/execution/interpreter/runtime.rb +++ b/lib/graphql/execution/interpreter/runtime.rb @@ -35,7 +35,7 @@ def current_object # @return [GraphQL::Query::Context] attr_reader :context - def initialize(query:, lazies_at_depth:) + def initialize(query:) @query = query @current_trace = query.current_trace @dataloader = query.multiplex.dataloader From d34262be739ea2c074a5d2a5f5e425ffaa88d466 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Tue, 2 Sep 2025 14:57:55 -0400 Subject: [PATCH 15/21] Update assertions --- spec/graphql/dataloader_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/graphql/dataloader_spec.rb b/spec/graphql/dataloader_spec.rb index 64afc25682..21c1d81691 100644 --- a/spec/graphql/dataloader_spec.rb +++ b/spec/graphql/dataloader_spec.rb @@ -1191,17 +1191,17 @@ def assert_last_max_fiber_count(expected_last_max_fiber_count, message = nil) res = schema.execute(query_str, context: { dataloader: fiber_counting_dataloader_class.new }) assert_nil res.context.dataloader.fiber_limit - assert_equal 12, FiberCounting.last_spawn_fiber_count + assert_equal 10, FiberCounting.last_spawn_fiber_count assert_last_max_fiber_count(9, "No limit works as expected") res = schema.execute(query_str, context: { dataloader: fiber_counting_dataloader_class.new(fiber_limit: 4) }) assert_equal 4, res.context.dataloader.fiber_limit - assert_equal 14, FiberCounting.last_spawn_fiber_count + assert_equal 12, FiberCounting.last_spawn_fiber_count assert_last_max_fiber_count(4, "Limit of 4 works as expected") res = schema.execute(query_str, context: { dataloader: fiber_counting_dataloader_class.new(fiber_limit: 6) }) assert_equal 6, res.context.dataloader.fiber_limit - assert_equal 10, FiberCounting.last_spawn_fiber_count + assert_equal 8, FiberCounting.last_spawn_fiber_count assert_last_max_fiber_count(6, "Limit of 6 works as expected") end From 5c43bcca5ead98c3aba9fef8cec3840df9e1134c Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Tue, 2 Sep 2025 15:12:49 -0400 Subject: [PATCH 16/21] Update more specs --- .../tracing/snapshots/example-rails-7-0.json | 86 ------------------- .../tracing/snapshots/example-rails-7-1.json | 86 ------------------- .../tracing/snapshots/example-rails-8-1.json | 86 ------------------- ...tive_support_notifications_tracing_spec.rb | 4 +- 4 files changed, 2 insertions(+), 260 deletions(-) diff --git a/spec/graphql/tracing/snapshots/example-rails-7-0.json b/spec/graphql/tracing/snapshots/example-rails-7-0.json index eec75d16d6..7e28d59bea 100644 --- a/spec/graphql/tracing/snapshots/example-rails-7-0.json +++ b/spec/graphql/tracing/snapshots/example-rails-7-0.json @@ -18786,92 +18786,6 @@ }, "sequenceFlags": 101010101010 }, - { - "timestamp": "10101010101010", - "trustedPacketSequenceId": 101010101010, - "trackEvent": { - "type": "TYPE_COUNTER", - "trackUuid": "10101010101010", - "counterValue": "10101010101010" - }, - "sequenceFlags": 101010101010 - }, - { - "trustedPacketSequenceId": 101010101010, - "sequenceFlags": 101010101010, - "trackDescriptor": { - "uuid": "10101010101010", - "name": "Dataloader Fiber #1010", - "parentUuid": "10101010101010", - "childOrdering": "CHRONOLOGICAL" - } - }, - { - "timestamp": "10101010101010", - "trustedPacketSequenceId": 101010101010, - "trackEvent": { - "categories": [ - "Dataloader" - ], - "categoryIids": [ - "10101010101010" - ], - "type": "TYPE_INSTANT", - "trackUuid": "10101010101010", - "extraCounterValues": [ - "10101010101010", - "10101010101010" - ], - "name": "Create Execution Fiber", - "extraCounterTrackUuids": [ - "10101010101010", - "10101010101010" - ] - }, - "sequenceFlags": 101010101010 - }, - { - "trustedPacketSequenceId": 101010101010, - "sequenceFlags": 101010101010, - "trackDescriptor": { - "uuid": "10101010101010", - "name": "Exec Fiber #1010", - "parentUuid": "10101010101010", - "childOrdering": "CHRONOLOGICAL" - } - }, - { - "timestamp": "10101010101010", - "trustedPacketSequenceId": 101010101010, - "trackEvent": { - "categories": [ - "Dataloader" - ], - "categoryIids": [ - "10101010101010" - ], - "type": "TYPE_INSTANT", - "trackUuid": "10101010101010", - "extraCounterValues": [ - "10101010101010" - ], - "name": "Fiber Exit", - "extraCounterTrackUuids": [ - "10101010101010" - ] - }, - "sequenceFlags": 101010101010 - }, - { - "timestamp": "10101010101010", - "trustedPacketSequenceId": 101010101010, - "trackEvent": { - "type": "TYPE_COUNTER", - "trackUuid": "10101010101010", - "counterValue": "10101010101010" - }, - "sequenceFlags": 101010101010 - }, { "timestamp": "10101010101010", "trustedPacketSequenceId": 101010101010, diff --git a/spec/graphql/tracing/snapshots/example-rails-7-1.json b/spec/graphql/tracing/snapshots/example-rails-7-1.json index 5eff7725ec..f54d737abc 100644 --- a/spec/graphql/tracing/snapshots/example-rails-7-1.json +++ b/spec/graphql/tracing/snapshots/example-rails-7-1.json @@ -18782,92 +18782,6 @@ }, "sequenceFlags": 101010101010 }, - { - "timestamp": "10101010101010", - "trustedPacketSequenceId": 101010101010, - "trackEvent": { - "type": "TYPE_COUNTER", - "trackUuid": "10101010101010", - "counterValue": "10101010101010" - }, - "sequenceFlags": 101010101010 - }, - { - "trustedPacketSequenceId": 101010101010, - "sequenceFlags": 101010101010, - "trackDescriptor": { - "uuid": "10101010101010", - "name": "Dataloader Fiber #1010", - "parentUuid": "10101010101010", - "childOrdering": "CHRONOLOGICAL" - } - }, - { - "timestamp": "10101010101010", - "trustedPacketSequenceId": 101010101010, - "trackEvent": { - "categories": [ - "Dataloader" - ], - "categoryIids": [ - "10101010101010" - ], - "type": "TYPE_INSTANT", - "trackUuid": "10101010101010", - "extraCounterValues": [ - "10101010101010", - "10101010101010" - ], - "name": "Create Execution Fiber", - "extraCounterTrackUuids": [ - "10101010101010", - "10101010101010" - ] - }, - "sequenceFlags": 101010101010 - }, - { - "trustedPacketSequenceId": 101010101010, - "sequenceFlags": 101010101010, - "trackDescriptor": { - "uuid": "10101010101010", - "name": "Exec Fiber #1010", - "parentUuid": "10101010101010", - "childOrdering": "CHRONOLOGICAL" - } - }, - { - "timestamp": "10101010101010", - "trustedPacketSequenceId": 101010101010, - "trackEvent": { - "categories": [ - "Dataloader" - ], - "categoryIids": [ - "10101010101010" - ], - "type": "TYPE_INSTANT", - "trackUuid": "10101010101010", - "extraCounterValues": [ - "10101010101010" - ], - "name": "Fiber Exit", - "extraCounterTrackUuids": [ - "10101010101010" - ] - }, - "sequenceFlags": 101010101010 - }, - { - "timestamp": "10101010101010", - "trustedPacketSequenceId": 101010101010, - "trackEvent": { - "type": "TYPE_COUNTER", - "trackUuid": "10101010101010", - "counterValue": "10101010101010" - }, - "sequenceFlags": 101010101010 - }, { "timestamp": "10101010101010", "trustedPacketSequenceId": 101010101010, diff --git a/spec/graphql/tracing/snapshots/example-rails-8-1.json b/spec/graphql/tracing/snapshots/example-rails-8-1.json index 5ed54a38d8..79fa536a3d 100644 --- a/spec/graphql/tracing/snapshots/example-rails-8-1.json +++ b/spec/graphql/tracing/snapshots/example-rails-8-1.json @@ -19214,92 +19214,6 @@ }, "sequenceFlags": 101010101010 }, - { - "timestamp": "10101010101010", - "trustedPacketSequenceId": 101010101010, - "trackEvent": { - "type": "TYPE_COUNTER", - "trackUuid": "10101010101010", - "counterValue": "10101010101010" - }, - "sequenceFlags": 101010101010 - }, - { - "trustedPacketSequenceId": 101010101010, - "sequenceFlags": 101010101010, - "trackDescriptor": { - "uuid": "10101010101010", - "name": "Dataloader Fiber #1010", - "parentUuid": "10101010101010", - "childOrdering": "CHRONOLOGICAL" - } - }, - { - "timestamp": "10101010101010", - "trustedPacketSequenceId": 101010101010, - "trackEvent": { - "categories": [ - "Dataloader" - ], - "categoryIids": [ - "10101010101010" - ], - "type": "TYPE_INSTANT", - "trackUuid": "10101010101010", - "extraCounterValues": [ - "10101010101010", - "10101010101010" - ], - "name": "Create Execution Fiber", - "extraCounterTrackUuids": [ - "10101010101010", - "10101010101010" - ] - }, - "sequenceFlags": 101010101010 - }, - { - "trustedPacketSequenceId": 101010101010, - "sequenceFlags": 101010101010, - "trackDescriptor": { - "uuid": "10101010101010", - "name": "Exec Fiber #1010", - "parentUuid": "10101010101010", - "childOrdering": "CHRONOLOGICAL" - } - }, - { - "timestamp": "10101010101010", - "trustedPacketSequenceId": 101010101010, - "trackEvent": { - "categories": [ - "Dataloader" - ], - "categoryIids": [ - "10101010101010" - ], - "type": "TYPE_INSTANT", - "trackUuid": "10101010101010", - "extraCounterValues": [ - "10101010101010" - ], - "name": "Fiber Exit", - "extraCounterTrackUuids": [ - "10101010101010" - ] - }, - "sequenceFlags": 101010101010 - }, - { - "timestamp": "10101010101010", - "trustedPacketSequenceId": 101010101010, - "trackEvent": { - "type": "TYPE_COUNTER", - "trackUuid": "10101010101010", - "counterValue": "10101010101010" - }, - "sequenceFlags": 101010101010 - }, { "timestamp": "10101010101010", "trustedPacketSequenceId": 101010101010, diff --git a/spec/integration/rails/graphql/tracing/active_support_notifications_tracing_spec.rb b/spec/integration/rails/graphql/tracing/active_support_notifications_tracing_spec.rb index a6b843629d..12e0721df0 100644 --- a/spec/integration/rails/graphql/tracing/active_support_notifications_tracing_spec.rb +++ b/spec/integration/rails/graphql/tracing/active_support_notifications_tracing_spec.rb @@ -43,16 +43,16 @@ "analyze_query.graphql", "analyze_multiplex.graphql", "authorized.graphql", + "execute_query.graphql", "execute_field.graphql (Query.batchedBase)", "execute_field.graphql (Query.batchedBase)", - "execute_query.graphql", "lazy_loader.graphql", "execute_field_lazy.graphql (Query.batchedBase)", "authorized.graphql", - "execute_field.graphql (Base.name)", "execute_field_lazy.graphql (Query.batchedBase)", "authorized.graphql", "execute_field.graphql (Base.name)", + "execute_field.graphql (Base.name)", "execute_field_lazy.graphql (Base.name)", "execute_field_lazy.graphql (Base.name)", "execute_query_lazy.graphql", From 2b9f736889a27da3299ba4da1725eae178deeee3 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Tue, 2 Sep 2025 15:14:34 -0400 Subject: [PATCH 17/21] Put back missing require --- lib/graphql/execution/interpreter.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/graphql/execution/interpreter.rb b/lib/graphql/execution/interpreter.rb index b849bebd92..bbc778355d 100644 --- a/lib/graphql/execution/interpreter.rb +++ b/lib/graphql/execution/interpreter.rb @@ -5,6 +5,7 @@ require "graphql/execution/interpreter/arguments_cache" require "graphql/execution/interpreter/execution_errors" require "graphql/execution/interpreter/runtime" +require "graphql/execution/interpreter/resolve" require "graphql/execution/interpreter/handles_raw_value" module GraphQL From 889bcf675c8672541cc760e277790e6f6a5b96fc Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Wed, 3 Sep 2025 15:28:43 -0400 Subject: [PATCH 18/21] Add Lazy resolution to NullDataloader and remove FlatDataloader --- lib/graphql/dataloader.rb | 1 - lib/graphql/dataloader/flat_dataloader.rb | 54 ---------------------- lib/graphql/dataloader/null_dataloader.rb | 47 +++++++++++++++---- lib/graphql/schema.rb | 2 +- spec/graphql/dataloader_spec.rb | 2 +- spec/graphql/execution/interpreter_spec.rb | 2 +- 6 files changed, 42 insertions(+), 66 deletions(-) delete mode 100644 lib/graphql/dataloader/flat_dataloader.rb diff --git a/lib/graphql/dataloader.rb b/lib/graphql/dataloader.rb index 80e0211d37..bf66c72dd8 100644 --- a/lib/graphql/dataloader.rb +++ b/lib/graphql/dataloader.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true -require "graphql/dataloader/flat_dataloader" require "graphql/dataloader/null_dataloader" require "graphql/dataloader/request" require "graphql/dataloader/request_all" diff --git a/lib/graphql/dataloader/flat_dataloader.rb b/lib/graphql/dataloader/flat_dataloader.rb deleted file mode 100644 index defc9f3563..0000000000 --- a/lib/graphql/dataloader/flat_dataloader.rb +++ /dev/null @@ -1,54 +0,0 @@ -# frozen_string_literal: true -module GraphQL - class Dataloader - class FlatDataloader < Dataloader - def run(trace_query_lazy: nil) - while !@pending_jobs.empty? - run_pending_steps - with_trace_query_lazy(trace_query_lazy) do - while !@lazies_at_depth.empty? - run_next_pending_lazies - run_pending_steps - end - end - end - end - - def yield(_source) - raise GraphQL::Error, "GraphQL::Dataloader is not running -- add `use GraphQL::Dataloader` to your schema to use Dataloader sources." - end - - def append_job(callable = nil, &block) - @pending_jobs << (callable || block) - nil - end - - def with(*) - raise GraphQL::Error, "GraphQL::Dataloader is not running -- add `use GraphQL::Dataloader` to your schema to use Dataloader sources." - end - - private - - def run_next_pending_lazies - smallest_depth = nil - @lazies_at_depth.each_key do |depth_key| - smallest_depth ||= depth_key - if depth_key < smallest_depth - smallest_depth = depth_key - end - end - - if smallest_depth - lazies = @lazies_at_depth.delete(smallest_depth) - lazies.each(&:value) # resolve these Lazy instances - end - end - - def run_pending_steps - while (step = @pending_jobs.shift) - step.call - end - end - end - end -end diff --git a/lib/graphql/dataloader/null_dataloader.rb b/lib/graphql/dataloader/null_dataloader.rb index 5f20b1c735..b49b2c53e3 100644 --- a/lib/graphql/dataloader/null_dataloader.rb +++ b/lib/graphql/dataloader/null_dataloader.rb @@ -2,17 +2,48 @@ module GraphQL class Dataloader - # The default implementation of dataloading -- all no-ops. + # GraphQL-Ruby uses this when Dataloader isn't enabled. # - # The Dataloader interface isn't public, but it enables - # simple internal code while adding the option to add Dataloader. + # It runs execution code inline and gathers lazy objects (eg. Promises) + # and resolves them during {#run}. class NullDataloader < Dataloader - # These are all no-ops because code was - # executed synchronously. + def initialize(*) + @lazies_at_depth = Hash.new { |h,k| h[k] = [] } + end + + def freeze + @lazies_at_depth.default_proc = nil + @lazies_at_depth.freeze + super + end + + def run(trace_query_lazy: nil) + with_trace_query_lazy(trace_query_lazy) do + while !@lazies_at_depth.empty? + smallest_depth = nil + @lazies_at_depth.each_key do |depth_key| + smallest_depth ||= depth_key + if depth_key < smallest_depth + smallest_depth = depth_key + end + end + + if smallest_depth + lazies = @lazies_at_depth.delete(smallest_depth) + lazies.each(&:value) # resolve these Lazy instances + end + end + end + end + + def run_isolated + prev_lazies_at_depth = @lazies_at_depth + @lazies_at_depth = @lazies_at_depth.dup.clear + yield + ensure + @lazies_at_depth = prev_lazies_at_depth + end - def initialize(*); end - def run(trace_query_lazy: nil); end - def run_isolated; yield; end def clear_cache; end def yield(_source) raise GraphQL::Error, "GraphQL::Dataloader is not running -- add `use GraphQL::Dataloader` to your schema to use Dataloader sources." diff --git a/lib/graphql/schema.rb b/lib/graphql/schema.rb index 1f0094f828..be3d20b729 100644 --- a/lib/graphql/schema.rb +++ b/lib/graphql/schema.rb @@ -671,7 +671,7 @@ def union_memberships(type = nil) # @api private # @see GraphQL::Dataloader def dataloader_class - @dataloader_class || GraphQL::Dataloader::FlatDataloader + @dataloader_class || GraphQL::Dataloader::NullDataloader end attr_writer :dataloader_class diff --git a/spec/graphql/dataloader_spec.rb b/spec/graphql/dataloader_spec.rb index 21c1d81691..a5020de012 100644 --- a/spec/graphql/dataloader_spec.rb +++ b/spec/graphql/dataloader_spec.rb @@ -1451,7 +1451,7 @@ class CanaryDataloader < GraphQL::Dataloader::NullDataloader end query(query_type) end.execute("{ __typename }") - assert_instance_of GraphQL::Dataloader::FlatDataloader, res.context.dataloader + assert_instance_of GraphQL::Dataloader::NullDataloader, res.context.dataloader res = FiberSchema.execute("{ __typename }") assert_instance_of GraphQL::Dataloader, res.context.dataloader refute res.context.dataloader.nonblocking? diff --git a/spec/graphql/execution/interpreter_spec.rb b/spec/graphql/execution/interpreter_spec.rb index 3965d83e81..87891f82da 100644 --- a/spec/graphql/execution/interpreter_spec.rb +++ b/spec/graphql/execution/interpreter_spec.rb @@ -413,7 +413,7 @@ def execute_multiplex(multiplex:) # All of these get `3` as lazy value. They're resolved together, # since they aren't _root_ mutation fields. "lazyValue" => 3, - "i2" => { "value" => 3, "incrementedValue" => 2, "lazyValue" => 3 }, + "i2" => { "value" => 2, "incrementedValue" => 2, "lazyValue" => 3 }, "i3" => { "value" => 3, "incrementedValue" => 3, "lazyValue" => 3 }, }, "i4" => { "value" => 4, "incrementedValue" => 4, "lazyValue" => 4}, From d406d3960e905f7460ad623c04321f0727512d20 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Wed, 3 Sep 2025 15:37:39 -0400 Subject: [PATCH 19/21] Use a fresh NullDataloader instance for run_isolated to support frozen instances --- lib/graphql/dataloader.rb | 2 ++ lib/graphql/dataloader/null_dataloader.rb | 13 ++++++++----- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/lib/graphql/dataloader.rb b/lib/graphql/dataloader.rb index bf66c72dd8..ca8c77725d 100644 --- a/lib/graphql/dataloader.rb +++ b/lib/graphql/dataloader.rb @@ -163,6 +163,8 @@ def run_isolated prev_pending_keys = {} prev_lazies_at_depth = @lazies_at_depth @lazies_at_depth = @lazies_at_depth.dup.clear + # Clear pending loads but keep already-cached records + # in case they are useful to the given block. @source_cache.each do |source_class, batched_sources| batched_sources.each do |batch_args, batched_source_instance| if batched_source_instance.pending? diff --git a/lib/graphql/dataloader/null_dataloader.rb b/lib/graphql/dataloader/null_dataloader.rb index b49b2c53e3..61b7aa820e 100644 --- a/lib/graphql/dataloader/null_dataloader.rb +++ b/lib/graphql/dataloader/null_dataloader.rb @@ -37,14 +37,17 @@ def run(trace_query_lazy: nil) end def run_isolated - prev_lazies_at_depth = @lazies_at_depth - @lazies_at_depth = @lazies_at_depth.dup.clear - yield - ensure - @lazies_at_depth = prev_lazies_at_depth + new_dl = self.class.new + res = nil + new_dl.append_job { + res = yield + } + new_dl.run + res end def clear_cache; end + def yield(_source) raise GraphQL::Error, "GraphQL::Dataloader is not running -- add `use GraphQL::Dataloader` to your schema to use Dataloader sources." end From 7834e2b1c934303cbe5cc110076a9087b9107b6d Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Wed, 3 Sep 2025 16:07:17 -0400 Subject: [PATCH 20/21] Revert some changes to test output --- spec/graphql/authorization_spec.rb | 4 ++-- spec/graphql/tracing/platform_trace_spec.rb | 16 ++++++++-------- spec/graphql/tracing/platform_tracing_spec.rb | 16 ++++++++-------- .../active_support_notifications_tracing_spec.rb | 4 ++-- 4 files changed, 20 insertions(+), 20 deletions(-) diff --git a/spec/graphql/authorization_spec.rb b/spec/graphql/authorization_spec.rb index c8f389d95b..b49af2aa44 100644 --- a/spec/graphql/authorization_spec.rb +++ b/spec/graphql/authorization_spec.rb @@ -767,9 +767,9 @@ def auth_execute(*args, **kwargs) assert_equal "RelayObjectEdge", edge["__typename"] unauthorized_object_paths = [ - ["unauthorizedEdge", "node"], - ["unauthorizedConnection", "nodes", 0], ["unauthorizedConnection", "edges", 0, "node"], + ["unauthorizedConnection", "nodes", 0], + ["unauthorizedEdge", "node"] ] assert_equal unauthorized_object_paths, unauthorized_res["errors"].map { |e| e["path"] } diff --git a/spec/graphql/tracing/platform_trace_spec.rb b/spec/graphql/tracing/platform_trace_spec.rb index 69a2b12d5e..d13a39e633 100644 --- a/spec/graphql/tracing/platform_trace_spec.rb +++ b/spec/graphql/tracing/platform_trace_spec.rb @@ -142,23 +142,23 @@ def platform_resolve_type_key(type) "Edible.resolve_type", "Cheese.authorized", "Cheese.authorized", - "Edible.resolve_type", - "Cheese.authorized", - "Cheese.authorized", - "Edible.resolve_type", - "Cheese.authorized", - "Cheese.authorized", - "Edible.resolve_type", - "Milk.authorized", "DynamicFields.authorized", "D._", "C.f", + "Edible.resolve_type", + "Cheese.authorized", + "Cheese.authorized", "DynamicFields.authorized", "D._", "C.f", + "Edible.resolve_type", + "Cheese.authorized", + "Cheese.authorized", "DynamicFields.authorized", "D._", "C.f", + "Edible.resolve_type", + "Milk.authorized", "DynamicFields.authorized", "D._", "E.f", diff --git a/spec/graphql/tracing/platform_tracing_spec.rb b/spec/graphql/tracing/platform_tracing_spec.rb index ddf9ae11db..a0d41faae3 100644 --- a/spec/graphql/tracing/platform_tracing_spec.rb +++ b/spec/graphql/tracing/platform_tracing_spec.rb @@ -148,23 +148,23 @@ def platform_trace(platform_key, key, data) "Edible.resolve_type", "Cheese.authorized", "Cheese.authorized", - "Edible.resolve_type", - "Cheese.authorized", - "Cheese.authorized", - "Edible.resolve_type", - "Cheese.authorized", - "Cheese.authorized", - "Edible.resolve_type", - "Milk.authorized", "DynamicFields.authorized", "D._", "C.f", + "Edible.resolve_type", + "Cheese.authorized", + "Cheese.authorized", "DynamicFields.authorized", "D._", "C.f", + "Edible.resolve_type", + "Cheese.authorized", + "Cheese.authorized", "DynamicFields.authorized", "D._", "C.f", + "Edible.resolve_type", + "Milk.authorized", "DynamicFields.authorized", "D._", "E.f", diff --git a/spec/integration/rails/graphql/tracing/active_support_notifications_tracing_spec.rb b/spec/integration/rails/graphql/tracing/active_support_notifications_tracing_spec.rb index 12e0721df0..a6b843629d 100644 --- a/spec/integration/rails/graphql/tracing/active_support_notifications_tracing_spec.rb +++ b/spec/integration/rails/graphql/tracing/active_support_notifications_tracing_spec.rb @@ -43,16 +43,16 @@ "analyze_query.graphql", "analyze_multiplex.graphql", "authorized.graphql", - "execute_query.graphql", "execute_field.graphql (Query.batchedBase)", "execute_field.graphql (Query.batchedBase)", + "execute_query.graphql", "lazy_loader.graphql", "execute_field_lazy.graphql (Query.batchedBase)", "authorized.graphql", + "execute_field.graphql (Base.name)", "execute_field_lazy.graphql (Query.batchedBase)", "authorized.graphql", "execute_field.graphql (Base.name)", - "execute_field.graphql (Base.name)", "execute_field_lazy.graphql (Base.name)", "execute_field_lazy.graphql (Base.name)", "execute_query_lazy.graphql", From add47cc3ef10c1daf748e373e62330a5dca1748c Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Wed, 3 Sep 2025 16:10:09 -0400 Subject: [PATCH 21/21] Update breadth-first spec --- spec/graphql/execution/breadth_runtime_spec.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spec/graphql/execution/breadth_runtime_spec.rb b/spec/graphql/execution/breadth_runtime_spec.rb index 9fba9561dc..a5d44bbb50 100644 --- a/spec/graphql/execution/breadth_runtime_spec.rb +++ b/spec/graphql/execution/breadth_runtime_spec.rb @@ -18,7 +18,7 @@ def initialize(query:) max_complexity: nil, ) - super(query: query, lazies_at_depth: Hash.new { |h, k| h[k] = [] }) + super(query: query) @breadth_results_by_key = {} end @@ -50,7 +50,6 @@ def evaluate_breadth_selection(objects, parent_type, node) end @dataloader.run - GraphQL::Execution::Interpreter::Resolve.resolve_each_depth(@lazies_at_depth, @dataloader) @breadth_results_by_key[result_key] end