Repository navigation
Conversation
completeValueForObject collected the sub fields of the current field for every object in the result, so a list of N objects repeated the same selection set walk, @skip/@include evaluation and fragment expansion N times. The result only depends on the merged field and the object type, since variables and fragments are fixed for an execution, so it is now memoized per execution in ExecutionContext. The cache is bypassed when incremental support is enabled (deferred executions are created per collection) or when a custom ConditionalNodeDecision is present (it may not be deterministic). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Meemaw
marked this pull request as ready for review
October 18, 2026 17:45
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
ExecutionStrategy.completeValueForObjectrunsFieldCollector.collectFieldsfor every object in the result. For a list of N objects this repeats the same work N times: walking the selection set, evaluating@skip/@include, expanding fragments and building a freshLinkedHashMap+MergedFields +MergedSelectionSet.The result only depends on the merged field, the resolved object type, the variables and the fragments, and the last two are fixed for an execution. Async-profiler on
ExecutionBenchmarkshowed field collection at ~16% of CPU and ~20% of allocated bytes.Change
The collected sub fields are memoized per execution in
ExecutionContext, keyed byMergedFieldand then by the resolvedGraphQLObjectType. Every object of the same type under the same field reuses the same immutableMergedSelectionSet. Sub fields are shared too, so cache hits cascade down the tree.MergedField.equalscompares the class and the ASTFieldnodes (Fielduses identity), so two merged fields are only equal when they come from exactly the same set of AST fields. Interface/union results are keyed per concrete object type.@defer) is enabled, becauseDeferredExecutioninstances are created per collection, orConditionalNodeDecisionis in theGraphQLContext, because the callback is not guaranteed to be deterministic.ExecutionContext.ExecutionContextplus one@Internalgetter,getCollectedSubFields(). Happy to move it elsewhere if you prefer.Benchmarks
Apple Silicon, JDK 21. Base vs this branch jars were run interleaved (alternating, 3 rounds × 2 forks each = 6 forks per side) to cancel out machine drift. Δ shows the 95% confidence interval.
ExecutionBenchmark.benchmarkThroughputSimpleQueryBenchmark.benchMarkSimpleQueriesThroughputIntrospectionBenchmark(large-schema-2)An earlier run on a quieter machine of the same change (before a cosmetic
computeIfAbsentrefactor) gave tighter intervals: ExecutionBenchmark +9.3% ± 0.8%, SimpleQuery +15.0% ± 1.1%, Introspection +18.4% ± 8.1%. An A/A run (base vs base) on that machine showed no significant difference, with a noise floor of about ±1.8% onExecutionBenchmark.¹ Introspection bytes/op vary between forks even for the same jar (96.9M vs 114.5M observed for base), so treat that row as indicative only.
Tests
New
CollectedSubFieldsReuseTestcovers:ConditionalNodeDecisionis still asked for every object, so the cache is bypassedTo check the tests guard the fallbacks, I temporarily removed the bypass condition: the
ConditionalNodeDecisiontest then returned wrong data and the incremental test failed. The full test suite passes, apart from the 14 locale-dependentStringValueParsingUnicodeTestfailures that also occur onmasterwith a non-English JVM locale (fixed separately in #4500).🤖 Generated with Claude Code