Merge pull request rmosolgo#4456 from rmosolgo/better-argument-caching

Cache resolved arguments better

Author: rmosolgo

lib/graphql/execution/interpreter/arguments_cache.rb

@@ -7,49 +7,50 @@ class ArgumentsCache
         def initialize(query)
           @query = query
           @dataloader = query.context.dataloader
-          @storage = Hash.new do |h, ast_node|
-            h[ast_node] = Hash.new do |h2, arg_owner|
-              h2[arg_owner] = Hash.new do |h3, parent_object|
-                dataload_for(ast_node, arg_owner, parent_object) do |kwarg_arguments|
-                  h3[parent_object] = @query.after_lazy(kwarg_arguments) do |resolved_args|
-                    h3[parent_object] = resolved_args
-                  end
-                end
-
-                if !h3.key?(parent_object)
-                  # TODO should i bother putting anything here?
-                  h3[parent_object] = NO_ARGUMENTS
-                else
-                  h3[parent_object]
-                end
+          @storage = Hash.new do |h, argument_owner|
+            args_by_parent = if argument_owner.arguments_statically_coercible?
+              shared_values_cache = {}
+              Hash.new do |h2, ignored_parent_object|
+                h2[ignored_parent_object] = shared_values_cache
+              end
+            else
+              Hash.new do |h2, parent_object|
+                args_by_node = {}
+                args_by_node.compare_by_identity
+                h2[parent_object] = args_by_node
               end
             end
+            args_by_parent.compare_by_identity
+            h[argument_owner] = args_by_parent
           end
+          @storage.compare_by_identity
         end
 
         def fetch(ast_node, argument_owner, parent_object)
-          # If any jobs were enqueued, run them now,
-          # since this might have been called outside of execution.
-          # (The jobs are responsible for updating `result` in-place.)
-          if !@storage.key?(ast_node) || !@storage[ast_node].key?(argument_owner)
-            @dataloader.run_isolated do
-              @storage[ast_node][argument_owner][parent_object]
+          # This runs eagerly if no block is given
+          @storage[argument_owner][parent_object][ast_node] ||= begin
+            args_hash = self.class.prepare_args_hash(@query, ast_node)
+            kwarg_arguments = argument_owner.coerce_arguments(parent_object, args_hash, @query.context)
+            @query.after_lazy(kwarg_arguments) do |resolved_args|
+              @storage[argument_owner][parent_object][ast_node] = resolved_args
             end
           end
-          # Ack, the _hash_ is updated, but the key is eventually
-          # overridden with an immutable arguments instance.
-          # The first call queues up the job,
-          # then this call fetches the result.
-          # TODO this should be better, find a solution
-          # that works with merging the runtime.rb code
-          @storage[ast_node][argument_owner][parent_object]
+
         end
 
         # @yield [Interpreter::Arguments, Lazy<Interpreter::Arguments>] The finally-loaded arguments
         def dataload_for(ast_node, argument_owner, parent_object, &block)
           # First, normalize all AST or Ruby values to a plain Ruby hash
-          args_hash = self.class.prepare_args_hash(@query, ast_node)
-          argument_owner.coerce_arguments(parent_object, args_hash, @query.context, &block)
+          arg_storage = @storage[argument_owner][parent_object]
+          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)
+            end
+          end
           nil
         end
 

lib/graphql/schema/argument.rb

@@ -198,8 +198,8 @@ def type
 
       def statically_coercible?
         return @statically_coercible if defined?(@statically_coercible)
-
-        @statically_coercible = !@prepare.is_a?(String) && !@prepare.is_a?(Symbol)
+        requires_parent_object = @prepare.is_a?(String) || @prepare.is_a?(Symbol) || @own_validators
+        @statically_coercible = !requires_parent_object
       end
 
       # Apply the {prepare} configuration to `value`, using methods from `obj`.

lib/graphql/schema/field.rb

@@ -235,7 +235,7 @@ def initialize(type: nil, name: nil, owner: nil, null: nil, description: NOT_CON
         @name = -(camelize ? Member::BuildType.camelize(name_s) : name_s)
 
         @description = description
-        @type = @owner_type = @own_validators = @own_directives = @own_arguments = nil # these will be prepared later if necessary
+        @type = @owner_type = @own_validators = @own_directives = @own_arguments = @arguments_statically_coercible = nil # these will be prepared later if necessary
 
         self.deprecation_reason = deprecation_reason
 

lib/graphql/schema/member/has_arguments.rb

@@ -320,9 +320,11 @@ def validate_directive_argument(arg_defn, value)
         end
 
         def arguments_statically_coercible?
-          return @arguments_statically_coercible if defined?(@arguments_statically_coercible)
-
-          @arguments_statically_coercible = all_argument_definitions.all?(&:statically_coercible?)
+          if defined?(@arguments_statically_coercible) && !@arguments_statically_coercible.nil?
+            @arguments_statically_coercible
+          else
+            @arguments_statically_coercible = all_argument_definitions.all?(&:statically_coercible?)
+          end
         end
 
         module ArgumentClassAccessor