Skip to content

Avoid exponential validation of repeated fragment spreads in QueryDepth and QueryComplexity - #1996

Closed
ChiragAgg5k wants to merge 1 commit into
webonyx:masterfrom
ChiragAgg5k:perf/fragment-memoization
Closed

ChiragAgg5k wants to merge 1 commit into
webonyx:masterfrom
ChiragAgg5k:perf/fragment-memoization

Conversation

@ChiragAgg5k

Copy link
Copy Markdown

QueryDepth and QueryComplexity re-walk a fragment's selection set at every spread. When nested fragments spread each other more than once (F0 { ...F1 ...F1 }, F1 { ...F2 ...F2 }, …), the work doubles per level, so a document of under a kilobyte can keep validation busy for seconds or longer.

Both rules now compute each fragment once per validated document and reuse the result at every spread:

  • QueryComplexity caches each fragment's complexity by name. Variables, and therefore @include/@skip and field arguments, are fixed for a validation run, and field definitions are resolved per AST node, so a fragment scores the same wherever it is spread. The cache is reset in getVisitor().
  • QueryDepth caches each fragment's depth relative to its spread point and adds the spread depth, which gives the same result as walking it in place. Cycle detection via calculatedFragments is kept; fragments found to be part of a cycle are remembered so repeated spreads into a cycle also stay linear.

Tests in QueryDepthTest and QueryComplexityTest cover a doubling-fragment document up to 50 levels (which would never finish before) with exact depth/complexity, a fragment spread at different depths, @include with repeated spreads across variable changes, and a cycle reached through repeated spreads.

Changelog

Fixed

  • Avoid exponential validation time in QueryDepth and QueryComplexity for documents that spread nested fragments repeatedly

Repeated spreads of nested fragments re-walked each fragment at every spread, making QueryDepth and QueryComplexity exponential in the nesting level.
@ChiragAgg5k

Copy link
Copy Markdown
Author

Closing; not pursuing this one for now. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant