Skip to content

Commit

Permalink
Merge pull request #4664 from rmosolgo/avoid-hash-merge
Browse files Browse the repository at this point in the history
Avoid Hash#merge on large subclass schemas
  • Loading branch information
rmosolgo authored Oct 16, 2023
2 parents 9eee764 + 3ef57c9 commit d5968d9
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 11 deletions.
5 changes: 5 additions & 0 deletions Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,11 @@ namespace :bench do
GraphQLBenchmark.profile_large_introspection
end

task :profile_small_query_on_large_schema do
prepare_benchmark
GraphQLBenchmark.profile_small_query_on_large_schema
end

desc "Run analysis on a big query"
task :profile_large_analysis do
prepare_benchmark
Expand Down
24 changes: 24 additions & 0 deletions benchmark/run.rb
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,30 @@ def self.profile_boot

SILLY_LARGE_SCHEMA = build_large_schema

def self.profile_small_query_on_large_schema
schema = Class.new(SILLY_LARGE_SCHEMA)
Benchmark.ips do |x|
x.report("Run small query") {
schema.execute("{ __typename }")
}
end

result = StackProf.run(mode: :wall, interval: 1) do
schema.execute("{ __typename }")
end
StackProf::Report.new(result).print_text

StackProf.run(mode: :wall, out: "tmp/small_query.dump", interval: 1) do
schema.execute("{ __typename }")
end

report = MemoryProfiler.report do
schema.execute("{ __typename }")
end
puts "\n\n"
report.pretty_print
end

def self.profile_large_introspection
schema = SILLY_LARGE_SCHEMA
Benchmark.ips do |x|
Expand Down
31 changes: 26 additions & 5 deletions lib/graphql/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -536,18 +536,17 @@ def dataloader_class
attr_writer :dataloader_class

def references_to(to_type = nil, from: nil)
@own_references_to ||= Hash.new { |h, k| h[k] = [] }
@own_references_to ||= {}
if to_type
if !to_type.is_a?(String)
to_type = to_type.graphql_name
end

if from
@own_references_to[to_type] << from
refs = @own_references_to[to_type] ||= []
refs << from
else
own_refs = @own_references_to[to_type]
inherited_refs = find_inherited_value(:references_to, EMPTY_HASH)[to_type] || EMPTY_ARRAY
own_refs + inherited_refs
get_references_to(to_type) || EMPTY_ARRAY
end
else
# `@own_references_to` can be quite large for big schemas,
Expand Down Expand Up @@ -922,6 +921,7 @@ def resolve_type(type, obj, ctx)
def inherited(child_class)
if self == GraphQL::Schema
child_class.directives(default_directives.values)
child_class.extend(SubclassGetReferencesTo)
end
# Make sure the child class has these built out, so that
# subclasses can be modified by later calls to `trace_with`
Expand Down Expand Up @@ -1397,6 +1397,27 @@ def own_query_analyzers
def own_multiplex_analyzers
@own_multiplex_analyzers ||= []
end

# This is overridden in subclasses to check the inheritance chain
def get_references_to(type_name)
@own_references_to[type_name]
end
end

module SubclassGetReferencesTo
def get_references_to(type_name)
own_refs = @own_references_to[type_name]
inherited_refs = superclass.references_to(type_name)
if inherited_refs&.any?
if own_refs&.any?
own_refs + inherited_refs
else
inherited_refs
end
else
own_refs
end
end
end

# Install these here so that subclasses will also install it.
Expand Down
7 changes: 2 additions & 5 deletions lib/graphql/schema/warden.rb
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ def initialize(context:, schema:)
@types = @visible_types = @reachable_types = @visible_parent_fields =
@visible_possible_types = @visible_fields = @visible_arguments = @visible_enum_arrays =
@visible_enum_values = @visible_interfaces = @type_visibility = @type_memberships =
@visible_and_reachable_type = @unions = @unfiltered_interfaces = @references_to =
@visible_and_reachable_type = @unions = @unfiltered_interfaces =
@reachable_type_set =
nil
end
Expand Down Expand Up @@ -356,14 +356,11 @@ def root_type?(type_defn)
end

def referenced?(type_defn)
@references_to ||= @schema.references_to
graphql_name = type_defn.unwrap.graphql_name
members = @references_to[graphql_name] || NO_REFERENCES
members = @schema.references_to(graphql_name)
members.any? { |m| visible?(m) }
end

NO_REFERENCES = [].freeze

def orphan_type?(type_defn)
@schema.orphan_types.include?(type_defn)
end
Expand Down
5 changes: 4 additions & 1 deletion spec/graphql/schema_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,6 @@ class CustomSubscriptions < GraphQL::Subscriptions::ActionCableSubscriptions
METHODS_TO_CACHE = {
types: 1,
union_memberships: 1,
references_to: 1,
possible_types: 5, # The number of types with fields accessed in the query
}

Expand Down Expand Up @@ -456,4 +455,8 @@ class QueryRequiredSchema < GraphQL::Schema
refute_includes full_res.to_s, "oldSource"
end
end

it "starts with no references_to" do
assert_equal({}, GraphQL::Schema.references_to)
end
end

0 comments on commit d5968d9

Please sign in to comment.