From f8d740b65fb7267342c32cf2394690eac7c7272a Mon Sep 17 00:00:00 2001 From: Jovi De Croock Date: Sat, 17 May 2025 11:24:18 +0200 Subject: [PATCH 1/2] Optimize value coercion --- .../execution/interpreter/arguments_cache.rb | 26 ++++-- lib/graphql/schema/argument.rb | 27 +++++- lib/graphql/schema/list.rb | 10 ++- lib/graphql/schema/member/has_arguments.rb | 90 ++++++++++++------- 4 files changed, 111 insertions(+), 42 deletions(-) diff --git a/lib/graphql/execution/interpreter/arguments_cache.rb b/lib/graphql/execution/interpreter/arguments_cache.rb index 92cd9ab714..55672fb263 100644 --- a/lib/graphql/execution/interpreter/arguments_cache.rb +++ b/lib/graphql/execution/interpreter/arguments_cache.rb @@ -30,7 +30,6 @@ def fetch(ast_node, argument_owner, parent_object) @storage[argument_owner][parent_object][ast_node] = resolved_args end end - end # @yield [Interpreter::Arguments, Lazy] The finally-loaded arguments @@ -40,10 +39,25 @@ def dataload_for(ast_node, argument_owner, parent_object, &block) if (args = arg_storage[ast_node]) yield(args) else - args_hash = self.class.prepare_args_hash(@query, ast_node) - argument_owner.coerce_arguments(parent_object, args_hash, @query.context) do |resolved_args| - arg_storage[ast_node] = resolved_args - yield(resolved_args) + # Optimization: avoid arguments materialization for empty arguments + if argument_owner.any_arguments? + args_hash = self.class.prepare_args_hash(@query, ast_node) + # Fast path for empty arguments + if args_hash.empty? && !argument_owner.arguments_have_defaults? + args = GraphQL::Execution::Interpreter::Arguments::EMPTY + arg_storage[ast_node] = args + yield(args) + else + argument_owner.coerce_arguments(parent_object, args_hash, @query.context) do |resolved_args| + arg_storage[ast_node] = resolved_args + yield(resolved_args) + end + end + else + # No arguments defined on this field + args = GraphQL::Execution::Interpreter::Arguments::EMPTY + arg_storage[ast_node] = args + yield(args) end end nil @@ -97,4 +111,4 @@ def self.prepare_args_hash(query, ast_arg_or_hash_or_value) end end end -end +end \ No newline at end of file diff --git a/lib/graphql/schema/argument.rb b/lib/graphql/schema/argument.rb index 7603a557a7..3b6cee6e40 100644 --- a/lib/graphql/schema/argument.rb +++ b/lib/graphql/schema/argument.rb @@ -246,12 +246,13 @@ def prepare_value(obj, value, context: nil) end end + # Optimization: Use a faster code path for simpler argument cases # @api private def coerce_into_values(parent_object, values, context, argument_values) arg_name = graphql_name arg_key = keyword - default_used = false + # Fast path for common case - extracting value from values hash if values.key?(arg_name) value = values[arg_name] elsif values.key?(arg_key) @@ -270,6 +271,25 @@ def coerce_into_values(parent_object, values, context, argument_values) default_used = true end + # The common case is direct coercion without loading or special preparation + if statically_coercible? && !loads + coerced_value = begin + type.coerce_input(value, context) + rescue StandardError => err + context.schema.handle_or_reraise(context, err) + end + + # For simple cases we can avoid the lazy overhead completely + argument_values[arg_key] = GraphQL::Execution::Interpreter::ArgumentValue.new( + value: coerced_value, + original_value: coerced_value, + definition: self, + default_used: default_used || false, + ) + return + end + + # Handle complex cases with lazy values, loading, and preparation loaded_value = nil coerced_value = begin type.coerce_input(value, context) @@ -297,12 +317,11 @@ def coerce_into_values(parent_object, values, context, argument_values) maybe_loaded_value = loaded_value || prepared_value context.query.after_lazy(maybe_loaded_value) do |resolved_loaded_value| - # TODO code smell to access such a deeply-nested constant in a distant module argument_values[arg_key] = GraphQL::Execution::Interpreter::ArgumentValue.new( value: resolved_loaded_value, original_value: resolved_coerced_value, definition: self, - default_used: default_used, + default_used: default_used || false, ) end end @@ -433,4 +452,4 @@ def validate_deprecated_or_optional(null:, deprecation_reason:) end end end -end +end \ No newline at end of file diff --git a/lib/graphql/schema/list.rb b/lib/graphql/schema/list.rb index 1367f4a6c0..8f70d6a214 100644 --- a/lib/graphql/schema/list.rb +++ b/lib/graphql/schema/list.rb @@ -36,11 +36,17 @@ def coerce_result(value, ctx) value.map { |i| i.nil? ? nil : of_type.coerce_result(i, ctx) } end + # Optimization: avoid creating a new array if input is already an array def coerce_input(value, ctx) if value.nil? nil else - coerced = ensure_array(value).map { |item| item.nil? ? item : of_type.coerce_input(item, ctx) } + # Optimization: avoid Array() call and handle common case directly + coerced = if value.is_a?(Array) + value.map { |item| item.nil? ? item : of_type.coerce_input(item, ctx) } + else + [of_type.coerce_input(value, ctx)] + end ctx.schema.after_any_lazies(coerced, &:itself) end end @@ -83,4 +89,4 @@ def add_max_errors_reached_message(result) end end end -end +end \ No newline at end of file diff --git a/lib/graphql/schema/member/has_arguments.rb b/lib/graphql/schema/member/has_arguments.rb index 323850b997..ad19fec73b 100644 --- a/lib/graphql/schema/member/has_arguments.rb +++ b/lib/graphql/schema/member/has_arguments.rb @@ -88,6 +88,11 @@ def arguments(context = GraphQL::Query::NullContext.instance, _require_defined_a # might be nil if there are actually no arguments own_arguments_that_apply || own_arguments end + + # Check if any argument has default values defined + def arguments_have_defaults? + @arguments_have_defaults ||= all_argument_definitions.any?(&:default_value?) + end def any_arguments? !own_arguments.empty? @@ -238,39 +243,64 @@ def coerce_arguments(parent_object, values, context, &block) arg_defns = context.types.arguments(self) total_args_count = arg_defns.size + # Optimization: skip the complex logic if there are no arguments + if total_args_count == 0 + finished_args = GraphQL::Execution::Interpreter::Arguments::EMPTY + if block_given? + block.call(finished_args) + return nil + else + return finished_args + end + end + + # Optimization: for small number of arguments, use a more direct approach + if total_args_count <= 2 && !block_given? + argument_values = {} + raised_error = false + + arg_defns.each do |arg_defn| + begin + arg_defn.coerce_into_values(parent_object, values, context, argument_values) + rescue GraphQL::ExecutionError, GraphQL::UnauthorizedError => err + raised_error = true + return err + end + end + + # No lazy values for simple cases + return GraphQL::Execution::Interpreter::Arguments.new( + argument_values: argument_values, + ) + end + + # Complex case with multiple arguments or block given finished_args = nil prepare_finished_args = -> { - if total_args_count == 0 - finished_args = GraphQL::Execution::Interpreter::Arguments::EMPTY - if block_given? - block.call(finished_args) - end - else - argument_values = {} - resolved_args_count = 0 - raised_error = false - arg_defns.each do |arg_defn| - context.dataloader.append_job do - begin - arg_defn.coerce_into_values(parent_object, values, context, argument_values) - rescue GraphQL::ExecutionError, GraphQL::UnauthorizedError => err - raised_error = true - finished_args = err - if block_given? - block.call(finished_args) - end + argument_values = {} + resolved_args_count = 0 + raised_error = false + arg_defns.each do |arg_defn| + context.dataloader.append_job do + begin + arg_defn.coerce_into_values(parent_object, values, context, argument_values) + rescue GraphQL::ExecutionError, GraphQL::UnauthorizedError => err + raised_error = true + finished_args = err + if block_given? + block.call(finished_args) end + end - resolved_args_count += 1 - if resolved_args_count == total_args_count && !raised_error - finished_args = context.schema.after_any_lazies(argument_values.values) { - GraphQL::Execution::Interpreter::Arguments.new( - argument_values: argument_values, - ) - } - if block_given? - block.call(finished_args) - end + resolved_args_count += 1 + if resolved_args_count == total_args_count && !raised_error + finished_args = context.schema.after_any_lazies(argument_values.values) { + GraphQL::Execution::Interpreter::Arguments.new( + argument_values: argument_values, + ) + } + if block_given? + block.call(finished_args) end end end @@ -425,4 +455,4 @@ def own_arguments end end end -end +end \ No newline at end of file From cacb9421d827db243c4be721f8784136e8a3e9e3 Mon Sep 17 00:00:00 2001 From: Jovi De Croock Date: Sat, 17 May 2025 11:37:16 +0200 Subject: [PATCH 2/2] More things --- lib/graphql/schema/argument.rb | 31 +++++++++++++++-- lib/graphql/schema/input_object.rb | 54 ++++++++++++++++++++++++++++-- 2 files changed, 80 insertions(+), 5 deletions(-) diff --git a/lib/graphql/schema/argument.rb b/lib/graphql/schema/argument.rb index 3b6cee6e40..d01a51aac1 100644 --- a/lib/graphql/schema/argument.rb +++ b/lib/graphql/schema/argument.rb @@ -418,17 +418,44 @@ def initialize(argument) private def recursively_prepare_input_object(value, type, context) + # Quick early return for nil values + return value if value.nil? + if type.non_null? type = type.of_type end - if type.list? && !value.nil? + # Specialized handling for list types + if type.list? + # Get the inner type once to avoid repeated unwrapping inner_type = type.of_type - value.map { |v| recursively_prepare_input_object(v, inner_type, context) } + + # Fast path for empty arrays + if value.empty? + return [] + end + + # Fast path for arrays of simple scalar values without input objects + if value.is_a?(Array) && + !inner_type.list? && # Not a nested list + !inner_type.kind.input_object? && # Not an input object type + value.none? { |v| v.is_a?(GraphQL::Schema::InputObject) } # No input objects in the array + return value.dup + end + + # Standard case for lists - pre-allocate the result array + result = Array.new(value.size) + value.each_with_index do |item, idx| + result[idx] = recursively_prepare_input_object(item, inner_type, context) + end + + return result elsif value.is_a?(GraphQL::Schema::InputObject) + # Process input objects - combine validate and prepare steps value.validate_for(context) value.prepare else + # Return other values as is value end end diff --git a/lib/graphql/schema/input_object.rb b/lib/graphql/schema/input_object.rb index 0e8e8631d1..dded8ad5c8 100644 --- a/lib/graphql/schema/input_object.rb +++ b/lib/graphql/schema/input_object.rb @@ -70,10 +70,52 @@ def prepare def unwrap_value(value) case value when Array - value.map { |item| unwrap_value(item) } + # Fast path for empty arrays + return [] if value.empty? + + # Check if any item needs unwrapping (nested structures or InputObjects) + has_complex_items = false + value.each do |item| + if item.is_a?(Array) || item.is_a?(Hash) || item.is_a?(InputObject) + has_complex_items = true + break + end + end + + if has_complex_items + # Complex case: create a new array and map values + result = Array.new(value.size) + value.each_with_index do |item, idx| + result[idx] = unwrap_value(item) + end + result + else + # Simple case: array of primitive values, just duplicate + value.dup + end when Hash - value.reduce({}) do |h, (key, value)| - h.merge!(key => unwrap_value(value)) + # Fast path for empty hashes + return {} if value.empty? + + # Check if any value needs unwrapping + has_complex_values = false + value.each_value do |v| + if v.is_a?(Array) || v.is_a?(Hash) || v.is_a?(InputObject) + has_complex_values = true + break + end + end + + if has_complex_values + # Complex case: create a new hash with unwrapped values + result = {} + value.each do |key, val| + result[key] = unwrap_value(val) + end + result + else + # Simple case: hash of primitive values, just duplicate + value.dup end when InputObject value.to_h @@ -228,6 +270,12 @@ def coerce_input(value, ctx) if value.nil? return nil end + + # Fast path for empty input (if we know there are no required arguments) + if value.respond_to?(:empty?) && value.empty? && !ctx.types.arguments(self).any? { |arg_defn| arg_defn.type.non_null? && !arg_defn.default_value? } + arguments = GraphQL::Execution::Interpreter::Arguments::EMPTY + return self.new(arguments, ruby_kwargs: {}, context: ctx, defaults_used: nil) + end arguments = coerce_arguments(nil, value, ctx)