fix performance degradation on handling conflict fields

[mrtc0]

When a crafted query is received, it significantly exhausts computing resources like the CPU, which negatively impacts response time.
For example, adding the following benchmark takes about 7 ~ 8 seconds in my environment (M1 Mac / 32GB).

from graphql import build_schema, parse, validate from graphql.utilities import get_introspection_query from ..fixtures import big_schema_sdl # noqa: F401 def test_validate_conflict_fields_query(benchmark, big_schema_sdl): # noqa: F811 schema = build_schema(big_schema_sdl, assume_valid=True) fields = "__typename " * 1000 query = parse('query {{ {fields} }}'.format(fields=fields)) result = benchmark(lambda: validate(schema, query)) assert result == []

$ time poetry run pytest tests/benchmarks/test_validate_gql_conflict_fields.py Executed in 8.23 secs fish external usr time 7.60 secs 0.06 millis 7.60 secs sys time 0.22 secs 1.01 millis 0.21 secs

This is a similar problem in graphql-js and has been fixed.
This PR has been ported to make the same changes.

This change will greatly improve performance:

$ time poetry run pytest tests/benchmarks/test_validate_gql_conflict_fields.py Executed in 1.93 secs fish external usr time 1.47 secs 0.07 millis 1.47 secs sys time 0.18 secs 1.22 millis 0.18 secs

[Cito]

Thanks. Will look into this as quickly as possible.

[mrtc0]

@Cito Hi, Any update would be appreciated.

[Cito]

It's on my list for this weekend. Sorry, I can manage this project only in my spare time.

[Cito]

Thanks a lot for submitting this PR, very well done!

The same_arguments function can be simplified a bit by returning early if one of the values is false, instead of building the list and then checking all results.

The test for OverlappingFieldsCanBeMergedRule from graphql.js should be added as well, this will probably also solve the problem that line 611 is not covered.

It would be also good to make the benchmark test more similar in name and content to the GraphQL.js one - I always try to replicate things as closely as possible, since this makes maintenance much easier.

Let me know if you would like to make these changes, otherwise I will do it immediately after merging this PR.

[mrtc0]

@Cito Thank you for the review. I've made changes to the code based on your advice.

[Cito]

Thanks a lot, looks good. Will fix the formatting issues after merging.

[Cito]

A release with this fix will be available soon.

[datur]

Hey @Cito 👋 do you know when you might be putting up a release that includes this fix? If there's anything I can do to help it along let me know

[Cito]

@datur It's already contained in v3.3.0a5. It's still labelled as an alpha release because the corresponding GraphQL.js version 17 is also still labelled as alpha.