From 0dbce0f70d7dcecccbf44dedab65e713ef59ace8 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Fri, 7 Dec 2018 08:48:19 -0500 Subject: [PATCH 1/2] Update some random hotspots --- lib/graphql/execution/interpreter/runtime.rb | 55 +++++++++++++++----- lib/graphql/schema/field.rb | 12 ++++- lib/graphql/schema/member/has_fields.rb | 12 +++-- 3 files changed, 61 insertions(+), 18 deletions(-) diff --git a/lib/graphql/execution/interpreter/runtime.rb b/lib/graphql/execution/interpreter/runtime.rb index 7586155d1f..e8750fc0e5 100644 --- a/lib/graphql/execution/interpreter/runtime.rb +++ b/lib/graphql/execution/interpreter/runtime.rb @@ -19,9 +19,14 @@ def initialize(query:, response:) @query = query @schema = query.schema @context = query.context + @interpreter_context = @context.namespace(:interpreter) @response = response @dead_paths = {} @types_at_paths = {} + # A cache of { Class => { String => Schema::Field } } + # Which assumes that MyObject.get_field("myField") will return the same field + # during the lifetime of a query + @fields_cache = Hash.new { |h, k| h[k] = {} } end def final_value @@ -55,8 +60,15 @@ def gather_selections(owner_type, selections, selections_by_name) when GraphQL::Language::Nodes::Field if passes_skip_and_include?(node) response_key = node.alias || node.name - s = selections_by_name[response_key] ||= [] - s << node + selections = selections_by_name[response_key] + if selections + s = Array(selections) + s << node + selections_by_name[response_key] = s + else + # No selection was foudn for this field yet + selections_by_name[response_key] = node + end end when GraphQL::Language::Nodes::InlineFragment if passes_skip_and_include?(node) @@ -91,10 +103,16 @@ 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 = {} gather_selections(owner_type, selections, selections_by_name) - selections_by_name.each do |result_name, field_ast_nodes| - ast_node = field_ast_nodes.first + selections_by_name.each do |result_name, field_ast_nodes_or_ast_node| + if field_ast_nodes_or_ast_node.is_a?(Array) + field_ast_nodes = field_ast_nodes_or_ast_node + ast_node = field_ast_nodes.first + else + field_ast_nodes = nil + ast_node = field_ast_nodes_or_ast_node + end field_name = ast_node.name - field_defn = owner_type.get_field(field_name) + field_defn = @fields_cache[owner_type][field_name] ||= 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)) @@ -137,6 +155,9 @@ def evaluate_selections(path, owner_object, owner_type, selections, root_operati when :path kwarg_arguments[:path] = next_path when :lookahead + if !field_ast_nodes + field_ast_nodes = [ast_node] + end kwarg_arguments[:lookahead] = Execution::Lookahead.new( query: query, ast_nodes: field_ast_nodes, @@ -147,11 +168,17 @@ def evaluate_selections(path, owner_object, owner_type, selections, root_operati end end - next_selections = field_ast_nodes.inject([]) { |memo, f| memo.concat(f.selections) } + # Optimize for the case that field is selected only once + if field_ast_nodes.nil? || field_ast_nodes.size == 1 + next_selections = ast_node.selections + else + next_selections = [] + field_ast_nodes.each { |f| next_selections.concat(f.selections) } + end app_result = query.trace("execute_field", {field: field_defn, path: next_path}) do - context.namespace(:interpreter)[:current_path] = next_path - context.namespace(:interpreter)[:current_field] = field_defn + @interpreter_context[:current_path] = next_path + @interpreter_context[:current_field] = field_defn field_defn.resolve(object, kwarg_arguments, context) end @@ -251,12 +278,13 @@ def continue_field(path, value, field, type, ast_node, next_selections, is_non_n write_in_response(path, response_list) inner_type = type.of_type idx = 0 - value.map do |inner_value| + value.each do |inner_value| next_path = path.dup next_path << idx next_path.freeze idx += 1 set_type_at_path(next_path, inner_type) + # This will update `response_list` with the lazy after_lazy(inner_value, path: next_path, field: field) do |inner_inner_value| # reset `is_non_null` here and below, because the inner type will have its own nullability constraint continue_value = continue_value(next_path, inner_inner_value, field, false, ast_node) @@ -265,6 +293,7 @@ def continue_field(path, value, field, type, ast_node, next_selections, is_non_n end end end + response_list when "NON_NULL" inner_type = type.of_type # For fields like `__schema: __Schema!` @@ -305,12 +334,12 @@ def resolve_if_late_bound_type(type) # @param eager [Boolean] Set to `true` for mutation root fields only # @return [GraphQL::Execution::Lazy, Object] If loading `object` will be deferred, it's a wrapper over it. def after_lazy(obj, field:, path:, eager: false) - context.namespace(:interpreter)[:current_path] = path - context.namespace(:interpreter)[:current_field] = field + @interpreter_context[:current_path] = path + @interpreter_context[:current_field] = field if schema.lazy?(obj) lazy = GraphQL::Execution::Lazy.new(path: path, field: field) do - context.namespace(:interpreter)[:current_path] = path - context.namespace(:interpreter)[:current_field] = field + @interpreter_context[:current_path] = path + @interpreter_context[:current_field] = field # Wrap the execution of _this_ method with tracing, # but don't wrap the continuation below inner_obj = query.trace("execute_field_lazy", {field: field, path: path}) do diff --git a/lib/graphql/schema/field.rb b/lib/graphql/schema/field.rb index 5ab7ff05cf..95983df3c1 100644 --- a/lib/graphql/schema/field.rb +++ b/lib/graphql/schema/field.rb @@ -400,7 +400,17 @@ def authorized?(object, context) true end - self_auth && arguments.each_value.all? { |a| a.authorized?(object, context) } + if self_auth + # Faster than `.any?` + arguments.each_value do |arg| + if !arg.authorized?(object, context) + return false + end + end + true + else + false + end end # Implement {GraphQL::Field}'s resolve API. diff --git a/lib/graphql/schema/member/has_fields.rb b/lib/graphql/schema/member/has_fields.rb index c67ddb78b9..c2d8631697 100644 --- a/lib/graphql/schema/member/has_fields.rb +++ b/lib/graphql/schema/member/has_fields.rb @@ -67,12 +67,16 @@ def 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 + if (f = own_fields[field_name]) + f + else + for ancestor in ancestors + if ancestor.respond_to?(:own_fields) && f = ancestor.own_fields[field_name] + return f + end end + nil end - nil end # Register this field with the class, overriding a previous one if needed. From e70ce3e6949f4c6ff39250a0828ae2b4f20e89ab Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Fri, 7 Dec 2018 09:36:02 -0500 Subject: [PATCH 2/2] Tighten up some more --- lib/graphql/execution/interpreter/runtime.rb | 33 +++++++++++++------ lib/graphql/schema/member/base_dsl_methods.rb | 6 ++-- 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/lib/graphql/execution/interpreter/runtime.rb b/lib/graphql/execution/interpreter/runtime.rb index e8750fc0e5..7b670c56d7 100644 --- a/lib/graphql/execution/interpreter/runtime.rb +++ b/lib/graphql/execution/interpreter/runtime.rb @@ -61,26 +61,34 @@ def gather_selections(owner_type, selections, selections_by_name) if passes_skip_and_include?(node) response_key = node.alias || node.name selections = selections_by_name[response_key] + # if there was already a selection of this field, + # use an array to hold all selections, + # otherise, use the single node to represent the selection if selections + # This field was already selected at least once, + # add this node to the list of selections s = Array(selections) s << node selections_by_name[response_key] = s else - # No selection was foudn for this field yet + # No selection was found for this field yet selections_by_name[response_key] = node end end when GraphQL::Language::Nodes::InlineFragment if passes_skip_and_include?(node) - include_fragmment = if node.type + if node.type type_defn = schema.types[node.type.name] type_defn = type_defn.metadata[:type_class] - possible_types = query.warden.possible_types(type_defn).map { |t| t.metadata[:type_class] } - possible_types.include?(owner_type) + # Faster than .map{}.include?() + query.warden.possible_types(type_defn).each do |t| + if t.metadata[:type_class] == owner_type + gather_selections(owner_type, node.selections, selections_by_name) + break + end + end else - true - end - if include_fragmment + # it's an untyped fragment, definitely continue gather_selections(owner_type, node.selections, selections_by_name) end end @@ -89,9 +97,11 @@ def gather_selections(owner_type, selections, selections_by_name) fragment_def = query.fragments[node.name] type_defn = schema.types[fragment_def.type.name] type_defn = type_defn.metadata[:type_class] - possible_types = schema.possible_types(type_defn).map { |t| t.metadata[:type_class] } - if possible_types.include?(owner_type) - gather_selections(owner_type, fragment_def.selections, selections_by_name) + schema.possible_types(type_defn).each do |t| + if t.metadata[:type_class] == owner_type + gather_selections(owner_type, fragment_def.selections, selections_by_name) + break + end end end else @@ -104,6 +114,9 @@ def evaluate_selections(path, owner_object, owner_type, selections, root_operati selections_by_name = {} gather_selections(owner_type, selections, selections_by_name) selections_by_name.each do |result_name, field_ast_nodes_or_ast_node| + # As a performance optimization, the hash key will be a `Node` if + # there's only one selection of the field. But if there are multiple + # selections of the field, it will be an Array of nodes if field_ast_nodes_or_ast_node.is_a?(Array) field_ast_nodes = field_ast_nodes_or_ast_node ast_node = field_ast_nodes.first diff --git a/lib/graphql/schema/member/base_dsl_methods.rb b/lib/graphql/schema/member/base_dsl_methods.rb index 29e91f781d..498b56a089 100644 --- a/lib/graphql/schema/member/base_dsl_methods.rb +++ b/lib/graphql/schema/member/base_dsl_methods.rb @@ -83,9 +83,11 @@ def overridden_graphql_name # The default name is the Ruby constant name, # without any namespaces and with any `-Type` suffix removed def default_graphql_name - raise NotImplementedError, 'Anonymous class should declare a `graphql_name`' if name.nil? + @default_graphql_name ||= begin + raise NotImplementedError, 'Anonymous class should declare a `graphql_name`' if name.nil? - name.split("::").last.sub(/Type\Z/, "") + name.split("::").last.sub(/Type\Z/, "") + end end def visible?(context)