Merge pull request rmosolgo#4338 from rmosolgo/analysis-perf

Eliminate dynamic dispatch from Language::Visitor and Analysis::AST::Visitor

Author: rmosolgo

benchmark/run.rb

@@ -146,18 +146,26 @@ def self.profile_large_analysis
         query_str << "  field#{f}(arg: \"a\")\n"
       end
       query_str << "  selfField { selfField { selfField { __typename } } }\n"
-      query_str << "  int0Field { ...Int0Fields }"
+      # query_str << "  int0Field { ...Int0Fields }"
       query_str << "}\n"
     end
-    query_str << "fragment Int0Fields on Interface0 { __typename }"
+    # query_str << "fragment Int0Fields on Interface0 { __typename }"
     query = GraphQL::Query.new(SILLY_LARGE_SCHEMA, query_str)
-    analyzers = [GraphQL::Analysis::AST::FieldUsage]
+    analyzers = [
+      GraphQL::Analysis::AST::FieldUsage,
+      GraphQL::Analysis::AST::QueryDepth,
+      GraphQL::Analysis::AST::QueryComplexity
+    ]
     Benchmark.ips do |x|
       x.report("Running introspection") {
         GraphQL::Analysis::AST.analyze_query(query, analyzers)
       }
     end
 
+    StackProf.run(mode: :wall, out: "last-stackprof.dump", interval: 1) do
+      GraphQL::Analysis::AST.analyze_query(query, analyzers)
+    end
+
     result = StackProf.run(mode: :wall, interval: 1) do
       GraphQL::Analysis::AST.analyze_query(query, analyzers)
     end

lib/graphql/analysis/ast/visitor.rb

@@ -65,14 +65,41 @@ def response_path
         end
 
         # Visitor Hooks
+        [
+          :operation_definition, :fragment_definition,
+          :inline_fragment, :field, :directive, :argument, :fragment_spread
+        ].each do |node_type|
+          module_eval <<-RUBY, __FILE__, __LINE__
+          def call_on_enter_#{node_type}(node, parent)
+            @analyzers.each do |a|
+              begin
+                a.on_enter_#{node_type}(node, parent, self)
+              rescue AnalysisError => err
+                @rescued_errors << err
+              end
+            end
+          end
+
+          def call_on_leave_#{node_type}(node, parent)
+            @analyzers.each do |a|
+              begin
+                a.on_leave_#{node_type}(node, parent, self)
+              rescue AnalysisError => err
+                @rescued_errors << err
+              end
+            end
+          end
+
+          RUBY
+        end
 
         def on_operation_definition(node, parent)
           object_type = @schema.root_type_for_operation(node.operation_type)
           @object_types.push(object_type)
           @path.push("#{node.operation_type}#{node.name ? " #{node.name}" : ""}")
-          call_analyzers(:on_enter_operation_definition, node, parent)
+          call_on_enter_operation_definition(node, parent)
           super
-          call_analyzers(:on_leave_operation_definition, node, parent)
+          call_on_leave_operation_definition(node, parent)
           @object_types.pop
           @path.pop
         end
@@ -81,19 +108,19 @@ def on_fragment_definition(node, parent)
           on_fragment_with_type(node) do
             @path.push("fragment #{node.name}")
             @in_fragment_def = false
-            call_analyzers(:on_enter_fragment_definition, node, parent)
+            call_on_enter_fragment_definition(node, parent)
             super
             @in_fragment_def = false
-            call_analyzers(:on_leave_fragment_definition, node, parent)
+            call_on_leave_fragment_definition(node, parent)
           end
         end
 
         def on_inline_fragment(node, parent)
           on_fragment_with_type(node) do
             @path.push("...#{node.type ? " on #{node.type.name}" : ""}")
-            call_analyzers(:on_enter_inline_fragment, node, parent)
+            call_on_enter_inline_fragment(node, parent)
             super
-            call_analyzers(:on_leave_inline_fragment, node, parent)
+            call_on_leave_inline_fragment(node, parent)
           end
         end
 
@@ -114,12 +141,10 @@ def on_field(node, parent)
           @skipping = @skip_stack.last || skip?(node)
           @skip_stack << @skipping
 
-          call_analyzers(:on_enter_field, node, parent)
+          call_on_enter_field(node, parent)
           super
-
           @skipping = @skip_stack.pop
-
-          call_analyzers(:on_leave_field, node, parent)
+          call_on_leave_field(node, parent)
           @response_path.pop
           @field_definitions.pop
           @object_types.pop
@@ -129,9 +154,9 @@ def on_field(node, parent)
         def on_directive(node, parent)
           directive_defn = @schema.directives[node.name]
           @directive_definitions.push(directive_defn)
-          call_analyzers(:on_enter_directive, node, parent)
+          call_on_enter_directive(node, parent)
           super
-          call_analyzers(:on_leave_directive, node, parent)
+          call_on_leave_directive(node, parent)
           @directive_definitions.pop
         end
 
@@ -153,29 +178,23 @@ def on_argument(node, parent)
 
           @argument_definitions.push(argument_defn)
           @path.push(node.name)
-          call_analyzers(:on_enter_argument, node, parent)
+          call_on_enter_argument(node, parent)
           super
-          call_analyzers(:on_leave_argument, node, parent)
+          call_on_leave_argument(node, parent)
           @argument_definitions.pop
           @path.pop
         end
 
         def on_fragment_spread(node, parent)
           @path.push("... #{node.name}")
-          call_analyzers(:on_enter_fragment_spread, node, parent)
+          call_on_enter_fragment_spread(node, parent)
           enter_fragment_spread_inline(node)
           super
           leave_fragment_spread_inline(node)
-          call_analyzers(:on_leave_fragment_spread, node, parent)
+          call_on_leave_fragment_spread(node, parent)
           @path.pop
         end
 
-        def on_abstract_node(node, parent)
-          call_analyzers(:on_enter_abstract_node, node, parent)
-          super
-          call_analyzers(:on_leave_abstract_node, node, parent)
-        end
-
         # @return [GraphQL::BaseType] The current object type
         def type_definition
           @object_types.last
@@ -226,9 +245,7 @@ def enter_fragment_spread_inline(fragment_spread)
 
           object_types << object_type
 
-          fragment_def.selections.each do |selection|
-            visit_node(selection, fragment_def)
-          end
+          on_fragment_definition_children(fragment_def)
         end
 
         # Visit a fragment spread inline instead of visiting the definition
@@ -242,16 +259,6 @@ def skip?(ast_node)
           dir.any? && !GraphQL::Execution::DirectiveChecks.include?(dir, query)
         end
 
-        def call_analyzers(method, node, parent)
-          @analyzers.each do |analyzer|
-            begin
-              analyzer.public_send(method, node, parent, self)
-            rescue AnalysisError => err
-              @rescued_errors << err
-            end
-          end
-        end
-
         def on_fragment_with_type(node)
           object_type = if node.type
             @query.warden.get_type(node.type.name)

lib/graphql/language/nodes.rb

@@ -148,11 +148,19 @@ def visit_method
 
               class << self
                 attr_accessor :children_method_name
+
+                def visit_method
+                  :on_#{name_underscored}
+                end
               end
               self.children_method_name = :#{name_underscored}s
             RUBY
           end
 
+          def children_of_type
+            @children_methods
+          end
+
           private
 
           # Name accessors which return lists of nodes,
@@ -300,7 +308,7 @@ class Argument < AbstractNode
         #   @return [String] the key for this argument
 
         # @!attribute value
-        #   @return [String, Float, Integer, Boolean, Array, InputObject] The value passed for this key
+        #   @return [String, Float, Integer, Boolean, Array, InputObject, VariableIdentifier] The value passed for this key
 
         def children
           @children ||= Array(value).flatten.select { |v| v.is_a?(AbstractNode) }
@@ -325,35 +333,6 @@ class DirectiveDefinition < AbstractNode
         )
       end
 
-      # This is the AST root for normal queries
-      #
-      # @example Deriving a document by parsing a string
-      #   document = GraphQL.parse(query_string)
-      #
-      # @example Creating a string from a document
-      #   document.to_query_string
-      #   # { ... }
-      #
-      # @example Creating a custom string from a document
-      #  class VariableScrubber < GraphQL::Language::Printer
-      #    def print_argument(arg)
-      #      "#{arg.name}: <HIDDEN>"
-      #    end
-      #  end
-      #
-      #  document.to_query_string(printer: VariableScrubber.new)
-      #
-      class Document < AbstractNode
-        scalar_methods false
-        children_methods(definitions: nil)
-        # @!attribute definitions
-        #   @return [Array<OperationDefinition, FragmentDefinition>] top-level GraphQL units: operations or fragments
-
-        def slice_definition(name)
-          GraphQL::Language::DefinitionSlice.slice(self, name)
-        end
-      end
-
       # An enum value. The string is available as {#name}.
       class Enum < NameOnlyNode
       end
@@ -526,6 +505,35 @@ class OperationDefinition < AbstractNode
         self.children_method_name = :definitions
       end
 
+      # This is the AST root for normal queries
+      #
+      # @example Deriving a document by parsing a string
+      #   document = GraphQL.parse(query_string)
+      #
+      # @example Creating a string from a document
+      #   document.to_query_string
+      #   # { ... }
+      #
+      # @example Creating a custom string from a document
+      #  class VariableScrubber < GraphQL::Language::Printer
+      #    def print_argument(arg)
+      #      "#{arg.name}: <HIDDEN>"
+      #    end
+      #  end
+      #
+      #  document.to_query_string(printer: VariableScrubber.new)
+      #
+      class Document < AbstractNode
+        scalar_methods false
+        children_methods(definitions: nil)
+        # @!attribute definitions
+        #   @return [Array<OperationDefinition, FragmentDefinition>] top-level GraphQL units: operations or fragments
+
+        def slice_definition(name)
+          GraphQL::Language::DefinitionSlice.slice(self, name)
+        end
+      end
+
       # A type name, used for variable definitions
       class TypeName < NameOnlyNode
       end