Skip to content

Commit

Permalink
Pass is_non_null instead of tracking a list of non-null result names
Browse files Browse the repository at this point in the history
  • Loading branch information
@rmosolgo
rmosolgo committed Apr 26, 2023
1 parent 7a734f0 commit 4b9b55b
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 44 deletions.
3 changes: 2 additions & 1 deletion benchmark/run.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,12 +132,13 @@ def self.profile
def self.profile_large_introspection
schema = SILLY_LARGE_SCHEMA
Benchmark.ips do |x|
x.config(time: 10)
x.report("Run large introspection") {
schema.to_json
}
end

result = StackProf.run(mode: :wall, interval: 10) do
result = StackProf.run(mode: :wall) do
schema.to_json
end
StackProf::Report.new(result).print_text
Expand Down
74 changes: 31 additions & 43 deletions lib/graphql/execution/interpreter/runtime.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,13 @@ def initialize
end

module GraphQLResult
def initialize(result_name, parent_result)
def initialize(result_name, parent_result, is_non_null_in_parent)
@graphql_parent = parent_result
if parent_result && parent_result.graphql_dead
@graphql_dead = true
end
@graphql_result_name = result_name
@graphql_is_non_null_in_parent = is_non_null_in_parent
# Jump through some hoops to avoid creating this duplicate storage if at all possible.
@graphql_metadata = nil
end
Expand All @@ -42,22 +43,14 @@ def build_path(path_array)
end

attr_accessor :graphql_dead
attr_reader :graphql_parent, :graphql_result_name

# Although these are used by only one of the Result classes,
# it's handy to have the methods implemented on both (even though they just return `nil`)
# because it makes it easy to check if anything is assigned.
# @return [nil, Array<String>]
attr_accessor :graphql_non_null_field_names
# @return [nil, true]
attr_accessor :graphql_non_null_list_items
attr_reader :graphql_parent, :graphql_result_name, :graphql_is_non_null_in_parent

# @return [Hash] Plain-Ruby result data (`@graphql_metadata` contains Result wrapper objects)
attr_accessor :graphql_result_data
end

class GraphQLResultHash
def initialize(_result_name, _parent_result)
def initialize(_result_name, _parent_result, _is_non_null_in_parent)
super
@graphql_result_data = {}
end
Expand Down Expand Up @@ -145,7 +138,7 @@ def merge_into(into_result)
class GraphQLResultArray
include GraphQLResult

def initialize(_result_name, _parent_result)
def initialize(_result_name, _parent_result, _is_non_null_in_parent)
super
@graphql_result_data = []
end
Expand Down Expand Up @@ -209,7 +202,7 @@ def initialize(query:, lazies_at_depth:)
@schema = query.schema
@context = query.context
@multiplex_context = query.multiplex.context
@response = GraphQLResultHash.new(nil, nil)
@response = GraphQLResultHash.new(nil, nil, false)
# Identify runtime directives by checking which of this schema's directives have overridden `def self.resolve`
@runtime_directive_names = []
noop_resolve_owner = GraphQL::Schema::Directive.singleton_class
Expand Down Expand Up @@ -276,7 +269,7 @@ def run_eager
# directly evaluated and the results can be written right into the main response hash.
tap_or_each(gathered_selections) do |selections, is_selection_array|
if is_selection_array
selection_response = GraphQLResultHash.new(nil, nil)
selection_response = GraphQLResultHash.new(nil, nil, false)
final_response = @response
else
selection_response = @response
Expand Down Expand Up @@ -442,9 +435,6 @@ def evaluate_selection(result_name, field_ast_nodes_or_ast_node, owner_object, o
# the field's return type at this path in order
# to propagate `null`
return_type_non_null = return_type.non_null?
if return_type_non_null
(selections_result.graphql_non_null_field_names ||= []).push(result_name)
end
# Set this before calling `run_with_directives`, so that the directive can have the latest path
st = get_current_runtime_state
st.current_field = field_defn
Expand Down Expand Up @@ -589,13 +579,9 @@ def dead_result?(selection_result)
selection_result.graphql_dead # || ((parent = selection_result.graphql_parent) && parent.graphql_dead)
end

def set_result(selection_result, result_name, value, is_child_result)
def set_result(selection_result, result_name, value, is_child_result, is_non_null)
if !dead_result?(selection_result)
if value.nil? &&
( # there are two conditions under which `nil` is not allowed in the response:
(selection_result.graphql_non_null_list_items) || # this value would be written into a list that doesn't allow nils
((nn = selection_result.graphql_non_null_field_names) && nn.include?(result_name)) # this value would be written into a field that doesn't allow nils
)
if value.nil? && is_non_null
# This is an invalid nil that should be propagated
# One caller of this method passes a block,
# namely when application code returns a `nil` to GraphQL and it doesn't belong there.
Expand All @@ -605,11 +591,12 @@ def set_result(selection_result, result_name, value, is_child_result)
# TODO the code is trying to tell me something.
yield if block_given?
parent = selection_result.graphql_parent
name_in_parent = selection_result.graphql_result_name
if parent.nil? # This is a top-level result hash
@response = nil
else
set_result(parent, name_in_parent, nil, false)
name_in_parent = selection_result.graphql_result_name
is_non_null_in_parent = selection_result.graphql_is_non_null_in_parent
set_result(parent, name_in_parent, nil, false, is_non_null_in_parent)
set_graphql_dead(selection_result)
end
elsif is_child_result
Expand Down Expand Up @@ -651,13 +638,13 @@ def continue_value(value, parent_type, field, is_non_null, ast_node, result_name
case value
when nil
if is_non_null
set_result(selection_result, result_name, nil, false) do
set_result(selection_result, result_name, nil, false, is_non_null) do
# This block is called if `result_name` is not dead. (Maybe a previous invalid nil caused it be marked dead.)
err = parent_type::InvalidNullError.new(parent_type, field, value)
schema.type_error(err, context)
end
else
set_result(selection_result, result_name, nil, false)
set_result(selection_result, result_name, nil, false, is_non_null)
end
HALT
when GraphQL::Error
Expand All @@ -670,7 +657,7 @@ def continue_value(value, parent_type, field, is_non_null, ast_node, result_name
value.ast_node ||= ast_node
context.errors << value
if selection_result
set_result(selection_result, result_name, nil, false)
set_result(selection_result, result_name, nil, false, is_non_null)
end
end
HALT
Expand Down Expand Up @@ -724,9 +711,9 @@ def continue_value(value, parent_type, field, is_non_null, ast_node, result_name
if selection_result
if list_type_at_all
result_without_errors = value.map { |v| v.is_a?(GraphQL::ExecutionError) ? nil : v }
set_result(selection_result, result_name, result_without_errors, false)
set_result(selection_result, result_name, result_without_errors, false, is_non_null)
else
set_result(selection_result, result_name, nil, false)
set_result(selection_result, result_name, nil, false, is_non_null)
end
end
end
Expand All @@ -736,7 +723,7 @@ def continue_value(value, parent_type, field, is_non_null, ast_node, result_name
end
when GraphQL::Execution::Interpreter::RawValue
# Write raw value directly to the response without resolving nested objects
set_result(selection_result, result_name, value.resolve, false)
set_result(selection_result, result_name, value.resolve, false, is_non_null)
HALT
else
value
Expand Down Expand Up @@ -764,7 +751,7 @@ def continue_field(value, owner_type, field, current_type, ast_node, next_select
rescue StandardError => err
schema.handle_or_reraise(context, err)
end
set_result(selection_result, result_name, r, false)
set_result(selection_result, result_name, r, false, is_non_null)
r
when "UNION", "INTERFACE"
resolved_type_or_lazy = resolve_type(current_type, value)
Expand All @@ -782,7 +769,7 @@ def continue_field(value, owner_type, field, current_type, ast_node, next_select
err_class = current_type::UnresolvedTypeError
type_error = err_class.new(resolved_value, field, parent_type, resolved_type, possible_types)
schema.type_error(type_error, context)
set_result(selection_result, result_name, nil, false)
set_result(selection_result, result_name, nil, false, is_non_null)
nil
else
continue_field(resolved_value, owner_type, field, resolved_type, ast_node, next_selections, is_non_null, owner_object, arguments, result_name, selection_result)
Expand All @@ -797,8 +784,8 @@ def continue_field(value, owner_type, field, current_type, ast_node, next_select
after_lazy(object_proxy, ast_node: ast_node, field: field, owner_object: owner_object, arguments: arguments, trace: false, result_name: result_name, result: selection_result) do |inner_object|
continue_value = continue_value(inner_object, owner_type, field, is_non_null, ast_node, result_name, selection_result)
if HALT != continue_value
response_hash = GraphQLResultHash.new(result_name, selection_result)
set_result(selection_result, result_name, response_hash, true)
response_hash = GraphQLResultHash.new(result_name, selection_result, is_non_null)
set_result(selection_result, result_name, response_hash, true, is_non_null)
gathered_selections = gather_selections(continue_value, current_type, next_selections)
# There are two possibilities for `gathered_selections`:
# 1. All selections of this object should be evaluated together (there are no runtime directives modifying execution).
Expand All @@ -810,7 +797,7 @@ def continue_field(value, owner_type, field, current_type, ast_node, next_select
# (Technically, it's possible that one of those entries _doesn't_ require isolation.)
tap_or_each(gathered_selections) do |selections, is_selection_array|
if is_selection_array
this_result = GraphQLResultHash.new(result_name, selection_result)
this_result = GraphQLResultHash.new(result_name, selection_result, is_non_null)
final_result = response_hash
else
this_result = response_hash
Expand Down Expand Up @@ -843,12 +830,12 @@ def continue_field(value, owner_type, field, current_type, ast_node, next_select
# This is true for objects, unions, and interfaces
use_dataloader_job = !inner_type.unwrap.kind.input?
inner_type_non_null = inner_type.non_null?
response_list = GraphQLResultArray.new(result_name, selection_result)
response_list.graphql_non_null_list_items = inner_type_non_null
set_result(selection_result, result_name, response_list, true)
idx = 0
response_list = GraphQLResultArray.new(result_name, selection_result, is_non_null)
set_result(selection_result, result_name, response_list, true, is_non_null)
idx = nil
list_value = begin
value.each do |inner_value|
idx ||= 0
this_idx = idx
idx += 1
if use_dataloader_job
Expand Down Expand Up @@ -879,8 +866,9 @@ def continue_field(value, owner_type, field, current_type, ast_node, next_select
ex_err
end
end

continue_value(list_value, owner_type, field, inner_type.non_null?, ast_node, result_name, selection_result)
# Detect whether this error came while calling `.each` (before `idx` is set) or while running list *items* (after `idx` is set)
error_is_non_null = idx.nil? ? is_non_null : inner_type.non_null?
continue_value(list_value, owner_type, field, error_is_non_null, ast_node, result_name, selection_result)
else
raise "Invariant: Unhandled type kind #{current_type.kind} (#{current_type})"
end
Expand Down Expand Up @@ -1007,7 +995,7 @@ def after_lazy(lazy_obj, field:, owner_object:, arguments:, ast_node:, result:,
if eager
lazy.value
else
set_result(result, result_name, lazy, false)
set_result(result, result_name, lazy, false, false) # is_non_null is irrelevant here
current_depth = 0
while result
current_depth += 1
Expand Down

0 comments on commit 4b9b55b

Please sign in to comment.