Skip to content

Avoid repeated fragment complexity analysis - #5724

Open
ydah wants to merge 1 commit into
rmosolgo:masterfrom
ydah:avoid-repeated-fragment-complexity-analysis
Open

ydah wants to merge 1 commit into
rmosolgo:masterfrom
ydah:avoid-repeated-fragment-complexity-analysis

Conversation

@ydah

@ydah ydah commented Aug 30, 2026 •

Copy link
Copy Markdown
Contributor

QueryComplexity currently visits each fragment spread as an independently expanded tree. A fragment graph such as:

fragment F2 on Node {
  a { ...F1 }
  b { ...F1 }
}

revisits the same nested selections for every response path. Analysis time therefore grows exponentially with the fragment level, even though the query document grows linearly.

This PR caches reusable fragment complexity scopes while processing the existing Analysis::Visitor traversal. When a cached scope can be reused without changing field merging, the complexity analyzer skips that fragment's selections and attaches the cached scope instead. Merged complexity values are memoized over the resulting DAG.

Skipping is analyzer-specific: other analyzers running in the same visitor continue to receive the complete expanded traversal. Custom QueryComplexity subclasses also retain the existing path so their hooks and overridden behavior are unchanged. Cached scopes are immutable, and selections which overlap fields already present in a scope use the normal merging path.

Benchmark

Measured on Ruby 4.0.6 using binary fragment expansion. Each value is the median of five runs. The previous implementation was measured through a QueryComplexity subclass, which retains the uncached visitor path.

Levels Query size Before After Complexity Speedup
12 631 bytes 53.841 ms 0.361 ms 12,287 149x
14 733 bytes 250.142 ms 0.478 ms 49,151 523x
16 835 bytes 935.234 ms 0.469 ms 196,607 1,994x
18 937 bytes 4,239.339 ms 0.492 ms 786,431 8,617x

The level-18 query now completes complexity analysis in under one millisecond instead of taking more than four seconds.

@precomputed_selection_complexities[cache_key] = total
end

def collect_precomputed_fields(query, selections, owner_type, fields)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do I understand that this implements a new tree traversal? Could caching be implemented in the existing traversal, which is shared by other analyzers which may be in use?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that’s correct, this adds a QueryComplexity-specific traversal and skips the existing visitor for the built-in complexity analyzers.

I kept custom subclasses on the existing path because analyzer callbacks may depend on traversal order and response paths. However, that also means adding any other analyzer still causes the existing visitor to fully expand the fragment tree. For example, at level 16 I measured about 0.4ms for QueryComplexity alone and 234ms when an otherwise no-op analyzer was also present.

So I agree that this version doesn’t address the shared traversal cost cleanly enough. I’ll investigate moving the optimization into the existing traversal while preserving the current analyzer callback semantics.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’ve updated the branch so caching is now integrated into the existing Analysis::Visitor traversal. QueryComplexity can reuse a cached fragment scope and skip that fragment’s children, while any other analyzers in the same visitor still receive the full, unchanged traversal. This is important for path- or depth-sensitive custom analyzers.

I also added a regression test that runs QueryComplexity alongside another analyzer and verifies that the other analyzer still visits every expanded field. The separate precomputation and per-query visitation API have been removed.

With this version, the level-18 benchmark improved from 4,239 ms to 0.492 ms.

@ydah
ydah force-pushed the avoid-repeated-fragment-complexity-analysis branch from fad1670 to c77fcad Compare September 15, 2026 10:16
@ydah
ydah requested a review from rmosolgo September 15, 2026 10:36

This branch has not been deployed

No deployments
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.

2 participants