graphql-ruby
graphql-ruby copied to clipboard
Optimization - Skip processing non-existing fields in FieldsWillMerge
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.
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.
https://github.com/rmosolgo/graphql-ruby/blob/8c0e25848e3615c639e2702daf4ab5e7543585b9/lib/graphql/static_validation/rules/fields_will_merge.rb#L345
Thanks for this improvement!