Optimization - Skip processing non-existing fields in FieldsWillMerge

[francisbeaudoin]

Context

It was observed that if a query includes thousands of non-existing fields, it takes a lot of processing time within the FieldsWillMerge#find_conflicts_within.

What

The iteration loop represents n(n-1^2)/2, where n is the number of duplicated fields being queried, and the underlying call to #find_conflict is using considerable time. For example, a query with 2000 times the same non-existing field i.e.query { a a ... 2000 times } takes ~6000 ms to process and skipping these fields reduces the response time to ~500ms.

Optimization

As far as I can tell, skipping non-existing fields from the FieldsWillMerge rule doesn't alter the business logic.

The only caveat of this optimization, is that if a service using this library was relying on the Schema#validate_timeout to safeguard from a potentially malicious query, it may no longer reach the configured timeout but rather instead return multiple undefinedField errors, which can be mitigated by the Schema#validate_max_errors.

[francisbeaudoin]

As an alternative, we may be able to further reduce the processing time by not adding the fields entirely here by conditionality adding on unless definition.nil? but I'm not clear on all of the implications hence if it makes sense.

graphql-ruby/lib/graphql/static_validation/rules/fields_will_merge.rb

Line 345 in 8c0e258

fields << Field.new(node, definition, owner_type, parents)

[rmosolgo]

Thanks for this improvement!