diff --git a/Rakefile b/Rakefile index d50ce5d97c..782d1b0777 100644 --- a/Rakefile +++ b/Rakefile @@ -92,6 +92,12 @@ namespace :bench do prepare_benchmark GraphQLBenchmark.profile end + + desc "Run benchmarks on a very large result" + task :profile_large_result do + prepare_benchmark + GraphQLBenchmark.profile_large_result + end end namespace :test do diff --git a/benchmark/run.rb b/benchmark/run.rb index 24968a6a85..3afc758d03 100644 --- a/benchmark/run.rb +++ b/benchmark/run.rb @@ -51,4 +51,92 @@ def self.profile printer.print(STDOUT, {}) end + + # Adapted from https://github.com/rmosolgo/graphql-ruby/issues/861 + def self.profile_large_result + schema = ProfileLargeResult::Schema + document = ProfileLargeResult::ALL_FIELDS + Benchmark.ips do |x| + x.report("Querying for #{ProfileLargeResult::DATA.size} objects") { + schema.execute(document: document) + } + end + + result = RubyProf.profile do + schema.execute(document: document) + end + printer = RubyProf::FlatPrinter.new(result) + # printer = RubyProf::GraphHtmlPrinter.new(result) + # printer = RubyProf::FlatPrinterWithLineNumbers.new(result) + printer.print(STDOUT, {}) + + report = MemoryProfiler.report do + schema.execute(document: document) + end + + report.pretty_print + end + + module ProfileLargeResult + DATA = 1000.times.map { + { + id: SecureRandom.uuid, + int1: SecureRandom.random_number(100000), + int2: SecureRandom.random_number(100000), + string1: SecureRandom.base64, + string2: SecureRandom.base64, + boolean1: SecureRandom.random_number(1) == 0, + boolean2: SecureRandom.random_number(1) == 0, + int_array: 10.times.map { SecureRandom.random_number(100000) }, + string_array: 10.times.map { SecureRandom.base64 }, + boolean_array: 10.times.map { SecureRandom.random_number(1) == 0 }, + } + } + + + class FooType < GraphQL::Schema::Object + field :id, ID, null: false + field :int1, Integer, null: false + field :int2, Integer, null: false + field :string1, String, null: false + field :string2, String, null: false + field :boolean1, Boolean, null: false + field :boolean2, Boolean, null: false + field :string_array, [String], null: false + field :int_array, [Integer], null: false + field :boolean_array, [Boolean], null: false + end + + class QueryType < GraphQL::Schema::Object + description "Query root of the system" + field :foos, [FooType], null: false, description: "Return a list of Foo objects" + def foos + DATA + end + end + + class Schema < GraphQL::Schema + query QueryType + if TESTING_INTERPRETER + use GraphQL::Execution::Interpreter + end + end + + ALL_FIELDS = GraphQL.parse <<-GRAPHQL + { + foos { + id + int1 + int2 + string1 + string2 + boolean1 + boolean2 + stringArray + intArray + booleanArray + } + } + GRAPHQL + end end diff --git a/lib/graphql/execution/interpreter/hash_response.rb b/lib/graphql/execution/interpreter/hash_response.rb index 68214a468b..487338c308 100644 --- a/lib/graphql/execution/interpreter/hash_response.rb +++ b/lib/graphql/execution/interpreter/hash_response.rb @@ -20,29 +20,24 @@ def inspect # Add `value` at `path`. # @return [void] - def write(path, value, propagating_nil: false) - write_target = @result - if write_target - if path.none? - @result = value - else - path.each_with_index do |path_part, idx| - next_part = path[idx + 1] - if next_part.nil? - if write_target[path_part].nil? || (propagating_nil) - write_target[path_part] = value - else - raise "Invariant: Duplicate write to #{path} (previous: #{write_target[path_part].inspect}, new: #{value.inspect})" - end - else - # Don't have to worry about dead paths here - # because it's tracked by the runtime, - # and values for dead paths are not sent to this method. - write_target = write_target.fetch(path_part, :__unset) - end - end + def write(path, value) + if path.size == 0 # faster than #none? + @result = value + elsif (write_target = @result) + i = 0 + prefinal_steps = path.size - 1 + # Use `while` to avoid a closure + while i < prefinal_steps + path_part = path[i] + i += 1 + write_target = write_target[path_part] end + path_part = path[i] + write_target[path_part] = value + else + # The response is completely nulled out end + nil end end diff --git a/lib/graphql/execution/interpreter/runtime.rb b/lib/graphql/execution/interpreter/runtime.rb index 2fd5aedc3b..e8dc38523f 100644 --- a/lib/graphql/execution/interpreter/runtime.rb +++ b/lib/graphql/execution/interpreter/runtime.rb @@ -90,12 +90,11 @@ def gather_selections(owner_type, selections, selections_by_name) def evaluate_selections(path, owner_object, owner_type, selections, root_operation_type: nil) selections_by_name = {} - owner_type = resolve_if_late_bound_type(owner_type) gather_selections(owner_type, selections, selections_by_name) selections_by_name.each do |result_name, fields| ast_node = fields.first field_name = ast_node.name - field_defn = owner_type.fields[field_name] + field_defn = owner_type.get_field(field_name) is_introspection = false if field_defn.nil? field_defn = if owner_type == schema.query.metadata[:type_class] && (entry_point_field = schema.introspection_system.entry_point(name: field_name)) @@ -111,7 +110,10 @@ def evaluate_selections(path, owner_object, owner_type, selections, root_operati return_type = resolve_if_late_bound_type(field_defn.type) - next_path = [*path, result_name].freeze + next_path = path.dup + next_path << result_name + next_path.freeze + # This seems janky, but we need to know # the field's return type at this path in order # to propagate `null` @@ -146,36 +148,36 @@ def evaluate_selections(path, owner_object, owner_type, selections, root_operati end after_lazy(app_result, field: field_defn, path: next_path, eager: root_operation_type == "mutation") do |inner_result| - should_continue, continue_value = continue_value(next_path, inner_result, field_defn, return_type, ast_node) - if should_continue + continue_value = continue_value(next_path, inner_result, field_defn, return_type, ast_node) + if HALT != continue_value continue_field(next_path, continue_value, field_defn, return_type, ast_node, next_selections) end end end end + HALT = Object.new def continue_value(path, value, field, as_type, ast_node) - if value.nil? || value.is_a?(GraphQL::ExecutionError) - if value.nil? - if as_type.non_null? - err = GraphQL::InvalidNullError.new(field.owner, field, value) - write_in_response(path, err, propagating_nil: true) - else - write_in_response(path, nil) - end + if value.nil? + if as_type.non_null? + err = GraphQL::InvalidNullError.new(field.owner, field, value) + write_invalid_null_in_response(path, err) else - value.path ||= path - value.ast_node ||= ast_node - write_in_response(path, value, propagating_nil: as_type.non_null?) + write_in_response(path, nil) end - false - elsif value.is_a?(Array) && value.all? { |v| v.is_a?(GraphQL::ExecutionError) } + HALT + elsif value.is_a?(GraphQL::ExecutionError) + value.path ||= path + value.ast_node ||= ast_node + write_execution_errors_in_response(path, [value]) + HALT + elsif value.is_a?(Array) && value.any? && value.all? { |v| v.is_a?(GraphQL::ExecutionError) } value.each do |v| v.path ||= path v.ast_node ||= ast_node end - write_in_response(path, value, propagating_nil: as_type.non_null?) - false + write_execution_errors_in_response(path, value) + HALT elsif value.is_a?(GraphQL::UnauthorizedError) # this hook might raise & crash, or it might return # a replacement value @@ -187,20 +189,18 @@ def continue_value(path, value, field, as_type, ast_node) continue_value(path, next_value, field, as_type, ast_node) elsif GraphQL::Execution::Execute::SKIP == value - false + HALT else - return true, value + value end end def continue_field(path, value, field, type, ast_node, next_selections) - type = resolve_if_late_bound_type(type) - - case type.kind - when TypeKinds::SCALAR, TypeKinds::ENUM + case type.kind.name + when "SCALAR", "ENUM" r = type.coerce_result(value, context) write_in_response(path, r) - when TypeKinds::UNION, TypeKinds::INTERFACE + when "UNION", "INTERFACE" resolved_type = query.resolve_type(type, value) possible_types = query.possible_types(type) @@ -208,39 +208,45 @@ def continue_field(path, value, field, type, ast_node, next_selections) parent_type = field.owner type_error = GraphQL::UnresolvedTypeError.new(value, field, parent_type, resolved_type, possible_types) schema.type_error(type_error, context) - write_in_response(path, nil, propagating_nil: field.type.non_null?) + write_in_response(path, nil) else resolved_type = resolved_type.metadata[:type_class] continue_field(path, value, field, resolved_type, ast_node, next_selections) end - when TypeKinds::OBJECT + when "OBJECT" object_proxy = begin type.authorized_new(value, context) rescue GraphQL::ExecutionError => err err end after_lazy(object_proxy, path: path, field: field) do |inner_object| - should_continue, continue_value = continue_value(path, inner_object, field, type, ast_node) - if should_continue + continue_value = continue_value(path, inner_object, field, type, ast_node) + if HALT != continue_value write_in_response(path, {}) evaluate_selections(path, continue_value, type, next_selections) end end - when TypeKinds::LIST + when "LIST" write_in_response(path, []) inner_type = type.of_type - value.each_with_index.each do |inner_value, idx| - next_path = [*path, idx].freeze + idx = 0 + value.each do |inner_value| + next_path = path.dup + next_path << idx + next_path.freeze set_type_at_path(next_path, inner_type) after_lazy(inner_value, path: next_path, field: field) do |inner_inner_value| - should_continue, continue_value = continue_value(next_path, inner_inner_value, field, inner_type, ast_node) - if should_continue + continue_value = continue_value(next_path, inner_inner_value, field, inner_type, ast_node) + if HALT != continue_value continue_field(next_path, continue_value, field, inner_type, ast_node, next_selections) end end + idx += 1 end - when TypeKinds::NON_NULL + when "NON_NULL" inner_type = type.of_type + # For fields like `__schema: __Schema!` + inner_type = resolve_if_late_bound_type(inner_type) # Don't `set_type_at_path` because we want the static type, # we're going to use that to determine whether a `nil` should be propagated or not. continue_field(path, value, field, inner_type, ast_node, next_selections) @@ -305,8 +311,9 @@ def after_lazy(obj, field:, path:, eager: false) def arguments(graphql_object, arg_owner, ast_node) kwarg_arguments = {} + arg_defns = arg_owner.arguments ast_node.arguments.each do |arg| - arg_defn = arg_owner.arguments[arg.name] + arg_defn = arg_defns[arg.name] # Need to distinguish between client-provided `nil` # and nothing-at-all is_present, value = arg_to_value(graphql_object, arg_defn.type, arg.value) @@ -319,7 +326,7 @@ def arguments(graphql_object, arg_owner, ast_node) kwarg_arguments[arg_defn.keyword] = value end end - arg_owner.arguments.each do |name, arg_defn| + arg_defns.each do |name, arg_defn| if arg_defn.default_value? && !kwarg_arguments.key?(arg_defn.keyword) kwarg_arguments[arg_defn.keyword] = arg_defn.default_value end @@ -386,27 +393,35 @@ def flatten_ast_value(v) end end - def write_in_response(path, value, propagating_nil: false) + def write_invalid_null_in_response(path, invalid_null_error) + if !dead_path?(path) + schema.type_error(invalid_null_error, context) + write_in_response(path, nil) + add_dead_path(path) + end + end + + def write_execution_errors_in_response(path, errors) + if !dead_path?(path) + errors.each do |v| + context.errors << v + end + write_in_response(path, nil) + add_dead_path(path) + end + end + + def write_in_response(path, value) if dead_path?(path) return else - if value.is_a?(GraphQL::ExecutionError) || (value.is_a?(Array) && value.any? && value.all? { |v| v.is_a?(GraphQL::ExecutionError)}) - Array(value).each do |v| - context.errors << v - end - write_in_response(path, nil, propagating_nil: propagating_nil) - add_dead_path(path) - elsif value.is_a?(GraphQL::InvalidNullError) - schema.type_error(value, context) - write_in_response(path, nil, propagating_nil: true) - add_dead_path(path) - elsif value.nil? && path.any? && type_at(path).non_null? + if value.nil? && path.any? && type_at(path).non_null? # This nil is invalid, try writing it at the previous spot propagate_path = path[0..-2] - write_in_response(propagate_path, value, propagating_nil: true) + write_in_response(propagate_path, value) add_dead_path(propagate_path) else - @response.write(path, value, propagating_nil: propagating_nil) + @response.write(path, value) end end end diff --git a/lib/graphql/schema/member/has_arguments.rb b/lib/graphql/schema/member/has_arguments.rb index a3722ac9d3..66728d30a8 100644 --- a/lib/graphql/schema/member/has_arguments.rb +++ b/lib/graphql/schema/member/has_arguments.rb @@ -29,9 +29,13 @@ def add_argument(arg_defn) # @return [Hash GraphQL::Schema::Argument] Arguments defined on this thing, keyed by name. Includes inherited definitions def arguments - inherited_arguments = ((self.is_a?(Class) && superclass.respond_to?(:arguments)) ? superclass.arguments : {}) + inherited_arguments = ((self.is_a?(Class) && superclass.respond_to?(:arguments)) ? superclass.arguments : nil) # Local definitions override inherited ones - inherited_arguments.merge(own_arguments) + if inherited_arguments + inherited_arguments.merge(own_arguments) + else + own_arguments + end end # @param new_arg_class [Class] A class to use for building argument definitions diff --git a/lib/graphql/schema/member/has_fields.rb b/lib/graphql/schema/member/has_fields.rb index 2ebdd67e0f..c67ddb78b9 100644 --- a/lib/graphql/schema/member/has_fields.rb +++ b/lib/graphql/schema/member/has_fields.rb @@ -66,6 +66,15 @@ def fields all_fields end + def get_field(field_name) + for ancestor in ancestors + if ancestor.respond_to?(:own_fields) && f = ancestor.own_fields[field_name] + return f + end + end + nil + end + # Register this field with the class, overriding a previous one if needed. # Also, add a parent method for resolving this field. # @param field_defn [GraphQL::Schema::Field] @@ -123,7 +132,7 @@ def add_super_method(field_key, method_name) end default_resolve_module.module_eval <<-RUBY, __FILE__, __LINE__ + 1 def #{method_name}(**args) - field_inst = self.class.fields[#{field_key}] || raise(%|Failed to find field #{field_key} for \#{self.class} among \#{self.class.fields.keys}|) + field_inst = self.class.get_field(#{field_key}) || raise(%|Failed to find field #{field_key} for \#{self.class} among \#{self.class.fields.keys}|) field_inst.resolve_field_method(self, args, context) end RUBY