Merge pull request #90 from freshtonic/performance/reduce-types-once

Performance tweaks and visitor semantics simplification

Author: Josh Price

lib/graphql/execution/executor.ex

@@ -23,6 +23,7 @@ defmodule GraphQL.Execution.Executor do
   """
   @spec execute(GraphQL.Schema.t, GraphQL.Document.t, list) :: result_data | {:error, %{errors: list}}
   def execute(schema, document, opts \\ []) do
+    schema          = Schema.with_type_cache(schema)
     root_value      = Keyword.get(opts, :root_value, %{})
     variable_values = Keyword.get(opts, :variable_values, %{})
     operation_name  = Keyword.get(opts, :operation_name, nil)

lib/graphql/lang/ast/composite_visitor.ex

@@ -42,43 +42,32 @@ defimpl Visitor, for: GraphQL.Lang.AST.CompositeVisitor do
   Invoke *enter* on the outer visitor first, passing the resulting accumulator to the *enter*
   call on the *inner* visitor.
 
-  If either visitor's enter method returns :break, both visitors will still be executed, but
+  If either visitor's enter method returns :skip, both visitors will still be executed, but
   then execution will cease.
   """
   def enter(composite_visitor, node, accumulator) do
-    call_in_order(
-      composite_visitor.outer_visitor,
-      composite_visitor.inner_visitor,
-      node, accumulator, &Visitor.enter/3)
+    {v1_next_action, v1_accumulator}
+      = Visitor.enter(composite_visitor.outer_visitor, node, accumulator)
+    accumulator = Map.merge(accumulator, v1_accumulator)
+
+    if v1_next_action == :skip do
+      {:skip, accumulator}
+    else
+      Visitor.enter(composite_visitor.inner_visitor, node, accumulator)
+    end
   end
 
   @doc """
   Invoke *leave* on the inner visitor first, passing the resulting accumulator to the *leave*
   call on the *outer* visitor.
 
-  If either visitor's enter method returns :break, both visitors will still be executed, but
+  If either visitor's enter method returns :skip, both visitors will still be executed, but
   then execution will cease.
   """
   def leave(composite_visitor, node, accumulator) do
-    call_in_order(
-      composite_visitor.inner_visitor,
-      composite_visitor.outer_visitor,
-      node, accumulator, &Visitor.leave/3)
-  end
-
-  # Visits two visitors in the order specified and invokes the supplied Visitor function.
-  defp call_in_order(v1, v2, node, accumulator, fun) do
-    {v1_next_action, v1_accumulator} = fun.(v1, node, accumulator)
-    accumulator = Map.merge(accumulator, v1_accumulator)
-    {v2_next_action, v2_accumulator} = fun.(v2, node, accumulator)
-
-    next_action = if v1_next_action == :break do
-      :break
-    else 
-      v2_next_action
-    end
-    
-    {next_action, v2_accumulator}
+    v1_accumulator = Visitor.leave(composite_visitor.inner_visitor, node, accumulator)
+    v2_accumulator = Visitor.leave(composite_visitor.outer_visitor, node, Map.merge(accumulator, v1_accumulator))
+    v2_accumulator
   end
 end
 

lib/graphql/lang/ast/document_info.ex

@@ -0,0 +1,27 @@
+
+defmodule GraphQL.Lang.AST.DocumentInfo do
+
+  defstruct schema: nil,
+            document: nil,
+            lookups: %{}
+
+  def new(schema, document) do
+    %GraphQL.Lang.AST.DocumentInfo{schema: schema, lookups: precompute_lookups(schema, document)}
+  end
+
+  def get_fragment_definition(document_info, name) do
+    document_info.lookups[:fragment_definitions][name]
+  end
+
+  defp precompute_lookups(_schema, document) do
+    %{
+      fragment_definitions: Enum.reduce(document.definitions, %{}, fn(definition, acc) ->
+        if definition[:kind] == :FragmentDefinition do
+          put_in(acc[definition.name.value], definition)
+        else
+          acc
+        end
+      end)
+    } 
+  end
+end

lib/graphql/lang/ast/parallel_visitor.ex

@@ -4,7 +4,7 @@ defmodule GraphQL.Lang.AST.ParallelVisitor do
   A ParallelVisitor runs all child visitors in parallel instead of serially like the CompositeVisitor.
 
   In this context, 'in parallel' really means that for each node in the AST, each visitor will be invoked
-  for each node in the AST, but the :break/:continue return value of enter and leave is maintained per-visitor.
+  for each node in the AST, but the :skip/:continue return value of enter and leave is maintained per-visitor.
 
   This means invividual visitors can bail out of AST processing as soon as possible and not waste cycles.
 
@@ -19,31 +19,33 @@ defmodule GraphQL.Lang.AST.ParallelVisitor do
 
   defimpl Visitor do
     def enter(visitor, node, accumulator) do
-      accumulator = Enum.reduce(visitor.visitors, accumulator, fn(child_visitor, acc) ->
-        if !skipping?(accumulator, child_visitor) do
-          acc = case child_visitor |> Visitor.enter(node, accumulator) do
-            {:continue, next_accumulator} -> next_accumulator
-            {:break, next_accumulator} ->
-              put_in(next_accumulator[:skipping][child_visitor], node)
-          end
+      visitors = Enum.filter(visitor.visitors, fn(child_visitor) ->
+        !skipping?(accumulator, child_visitor)
+      end)
+      accumulator = Enum.reduce(visitors, accumulator, fn(child_visitor, accumulator) ->
+        case child_visitor |> Visitor.enter(node, accumulator) do
+          {:continue, next_accumulator} -> next_accumulator
+          {:skip, next_accumulator} ->
+            put_in(next_accumulator[:skipping][child_visitor], node)
         end
-        acc
       end)
-      {:continue, accumulator}
+      if length(visitors) > 0 do
+        {:continue, accumulator}
+      else
+        {:skip, accumulator}
+      end
     end
 
     def leave(visitor, node, accumulator) do
-      accumulator = Enum.reduce(visitor.visitors, accumulator, fn(child_visitor, acc) ->
+      Enum.reduce visitor.visitors, accumulator, fn(child_visitor, accumulator) ->
         cond do
-          !skipping?(acc, child_visitor) ->
-            {_, next_accumulator} = child_visitor |> Visitor.leave(node, acc)
-            next_accumulator
+          !skipping?(accumulator, child_visitor) ->
+            child_visitor |> Visitor.leave(node, accumulator)
           accumulator[:skipping][child_visitor] == node ->
-            put_in(acc[:skipping][child_visitor], false)
+            Map.delete(accumulator[:skipping], child_visitor)
           true -> accumulator
         end
-      end)
-      {:continue, accumulator}
+      end
     end
 
     defp skipping?(accumulator, child_visitor) do

lib/graphql/lang/ast/reducer.ex

@@ -13,33 +13,27 @@ defmodule GraphQL.Lang.AST.Reducer do
 
   def reduce(node, visitor, accumulator) do
     accumulator = InitialisingVisitor.init(visitor, accumulator)
-    {_, accumulator} = visit(node, visitor, accumulator)
+    accumulator = visit(node, visitor, accumulator)
     PostprocessingVisitor.finish(visitor, accumulator)
   end
 
   defp visit([child|rest], visitor, accumulator) do
-    {next_action, accumulator} = visit(child, visitor, accumulator)
-    case next_action do
-      :continue -> visit(rest, visitor, accumulator)
-      :break -> {:break, accumulator}
-    end
+    accumulator = visit(child, visitor, accumulator)
+    visit(rest, visitor, accumulator)
   end
 
-  defp visit([], _visitor, accumulator), do: {:continue, accumulator}
+  defp visit([], _visitor, accumulator), do: accumulator
 
-  # FIXME: we need to enforce an invariant that if a enter is called for a node, we guarantee
-  # that leave is called on a node. That means :break means "do not go deeper".
   defp visit(node, visitor, accumulator) do
     {next_action, accumulator} = Visitor.enter(visitor, node, accumulator) 
-    case next_action do
-      :continue ->
-        {next_action, accumulator} = visit_children(node, visitor, accumulator)
-        case next_action do
-          :continue -> Visitor.leave(visitor, node, accumulator)
-          :break -> {:break, accumulator}
-        end
-      :break -> {:break, accumulator}
+
+    accumulator = if next_action != :skip do
+      visit_children(node, visitor, accumulator)
+    else
+      accumulator
     end
+
+    Visitor.leave(visitor, node, accumulator)
   end
 
   defp visit_children(node = %{kind: kind}, visitor, accumulator) when is_atom(kind) do
@@ -48,12 +42,10 @@ defmodule GraphQL.Lang.AST.Reducer do
   end
 
   defp visit_each_child([child|rest], visitor, accumulator) do
-    {next_action, accumulator} = visit(child, visitor, accumulator)
-    case next_action do
-      :continue -> visit_each_child(rest, visitor, accumulator)
-      :break -> {:break, accumulator}
-    end
+    accumulator = visit(child, visitor, accumulator)
+    accumulator = visit_each_child(rest, visitor, accumulator)
+    accumulator
   end
 
-  defp visit_each_child([], _visitor, accumulator), do: {:continue, accumulator}
+  defp visit_each_child([], _visitor, accumulator), do: accumulator
 end

lib/graphql/lang/ast/type_info_visitor.ex

@@ -168,7 +168,7 @@ defmodule GraphQL.Lang.AST.TypeInfoVisitor do
         _ ->
           :ignore
       end
-      {:continue, accumulator}
+      accumulator
     end
   end
 end

lib/graphql/lang/ast/visitor.ex

@@ -31,18 +31,14 @@ defmodule GraphQL.Lang.AST do
 
     The return value should be:
 
-    {next_action, acc}
-
-    where next_action is either :break or :continue and acc is the new value of the accumulator.
-
-    :break will abort the visitor and AST traversal will cease returning the current value of the accumulator.
+    acc
     """
     def leave(visitor, node, accumulator)
   end
 
   defimpl Visitor, for: Any do
     def enter(_visitor, _node, accumulator), do: {:continue, accumulator}
-    def leave(_visitor, _node, accumulator), do: {:continue, accumulator}
+    def leave(_visitor, _node, accumulator), do: accumulator
   end
 
   defprotocol InitialisingVisitor do

lib/graphql/type/interface.ex

@@ -30,11 +30,11 @@ defmodule GraphQL.Type.Interface do
     """
     def possible_types(interface, schema) do
       # get the complete typemap from this schema
-      GraphQL.Schema.reduce_types(schema)
+      schema.type_cache
       # filter them down to a list of types that implement this interface
       |> Enum.filter(fn {_, typedef} -> GraphQL.Type.implements?(typedef, interface) end)
       # then return the type, instead of the {name, type} tuple that comes from
-      # the reduce_types call
+      # the type_cache
       |> Enum.map(fn({_,v}) -> v end)
     end
 

lib/graphql/type/introspection.ex

@@ -36,7 +36,7 @@ defmodule GraphQL.Type.Introspection do
             description: "A list of all types supported by this server.",
             type: %NonNull{ofType: %List{ofType: %NonNull{ofType: Type}}},
             resolve: fn(schema, _, _) ->
-              Map.values(GraphQL.Schema.reduce_types(schema))
+              Map.values(schema.type_cache)
             end
           },
           queryType: %{
@@ -161,7 +161,7 @@ defmodule GraphQL.Type.Introspection do
               (%GraphQL.Type.Interface{name: _name} = interface, _args, info) ->
                 AbstractType.possible_types(interface, info.schema)
               (%GraphQL.Type.Union{name: name}, _args, info) ->
-                GraphQL.Schema.reduce_types(info.schema)[name].types
+                info.schema.type_cache[name].types
               (_, _, _) -> nil
             end
           },
@@ -407,7 +407,7 @@ defmodule GraphQL.Type.Introspection do
         name: %{type: %NonNull{ofType: %String{}}}
       },
       resolve: fn(_, %{name: name}, %{schema: schema}) ->
-        GraphQL.Schema.reduce_types(schema)[name]
+        schema.type_cache[name]
       end
     }
   end

lib/graphql/type/schema.ex

@@ -2,8 +2,7 @@ defmodule GraphQL.Schema do
 
   @type t :: %GraphQL.Schema{
     query: Map,
-    mutation: Map,
-    types: [GraphQL.Type.AbstractType.t | GraphQL.Type.ObjectType.t]
+    mutation: Map
   }
 
   alias GraphQL.Type.Interface
@@ -13,7 +12,16 @@ defmodule GraphQL.Schema do
   alias GraphQL.Type.CompositeType
   alias GraphQL.Lang.AST.Nodes
 
-  defstruct query: nil, mutation: nil, types: []
+  defstruct query: nil, mutation: nil, type_cache: nil
+
+  def with_type_cache(schema = %{type_cache: nil}), do: new(schema)
+  def with_type_cache(schema), do: schema
+
+  def new(%{query: query, mutation: mutation}) do
+    %GraphQL.Schema{query: query, mutation: mutation, type_cache: do_reduce_types(query, mutation)}
+  end
+  def new(%{mutation: mutation}), do: new(%{query: nil, mutation: mutation})
+  def new(%{query: query}), do: new(%{query: query, mutation: nil})
 
   # FIXME: I think *schema* should be the first argument in this module.
   def type_from_ast(nil, _), do: nil
@@ -24,32 +32,32 @@ defmodule GraphQL.Schema do
     %GraphQL.Type.List{ofType: type_from_ast(input_type_ast.type, schema)}
   end
   def type_from_ast(%{kind: :NamedType} = input_type_ast, schema) do
-    reduce_types(schema) |> Map.get(input_type_ast.name.value, :not_found)
+    schema.type_cache |> Map.get(input_type_ast.name.value, :not_found)
   end
 
-  def reduce_types(type) do
+  defp do_reduce_types(query, mutation) do
     %{}
-    |> reduce_types(type.query)
-    |> reduce_types(type.mutation)
+    |> reduce_types(query)
+    |> reduce_types(mutation)
     |> reduce_types(Introspection.Schema.type)
   end
 
-  def reduce_types(typemap, %{ofType: list_type}) do
+  defp reduce_types(typemap, %{ofType: list_type}) do
     reduce_types(typemap, list_type)
   end
 
-  def reduce_types(typemap, %Interface{} = type) do
+  defp reduce_types(typemap, %Interface{} = type) do
     Map.put(typemap, type.name, type)
   end
 
-  def reduce_types(typemap, %Union{} = type) do
+  defp reduce_types(typemap, %Union{} = type) do
     typemap = Map.put(typemap, type.name, type)
     Enum.reduce(type.types, typemap, fn(fieldtype,map) ->
        reduce_types(map, fieldtype)
     end)
   end
 
-  def reduce_types(typemap, %ObjectType{} = type) do
+  defp reduce_types(typemap, %ObjectType{} = type) do
     if Map.has_key?(typemap, type.name) do
       typemap
     else
@@ -65,10 +73,10 @@ defmodule GraphQL.Schema do
     end
   end
 
-  def reduce_types(typemap, %{name: name} = type), do: Map.put(typemap, name, type)
-  def reduce_types(typemap, nil), do: typemap
+  defp reduce_types(typemap, %{name: name} = type), do: Map.put(typemap, name, type)
+  defp reduce_types(typemap, nil), do: typemap
 
-  def reduce_types(typemap, type_module) when is_atom(type_module) do
+  defp reduce_types(typemap, type_module) when is_atom(type_module) do
     reduce_types(typemap, apply(type_module, :type, []))
   end