Repository navigation
Conversation
Check selection.directives once in visit_selection_sets() and call should_include_node() only when a selection has directives. Find @Skip and @include in one plain loop instead of two generator expressions. Collecting a 4,000-field selection set with @include takes 22% less time; the wide-query benchmark goes from 73.9 ms to 63.1 ms. Output, errors and forbidden-directive lists are unchanged. Co-Authored-By: Aiden <aiden@weco.ai>
The shared directive check passes any selection to should_include_node, so widen the annotation to fix the mypy lint error. Co-Authored-By: Aiden <aiden@weco.ai>
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.
TL;DR
visit_selection_sets()incollect_fields.pyfaster, which groups the fields of each selection set before execution.@skip/@includecheck for selections without directives, and scan directives in one plain loop.PYTHONPATH=src python -m pytest tests/execution/test_collect_fields.py -q: 5 passed; full suite: 3,821 passed, 18 skipped.Refs #315
What & Why
Before, the loop called
should_include_node()for every selection, in three separate branches.That function built two generator expressions to find
@skipand@include, even when the selection had no directives.Now the loop checks
selection.directivesonce, before the type dispatch.It calls
should_include_node()only when directives are present.should_include_node()finds the first@skipand the first@includein one plain loop.The output stays identical.
A selection without directives was always included, so skipping the call changes no result.
The check order stays the same:
@skipfirst, then@include, then the fragment type condition.Results
collect_fieldson 4,000 plain fieldscollect_fieldson 4,000 fields, every 10th aliasedcollect_fieldson 4,000 fields with@include(if: $t)collect_fieldson 400 fields with@skip(if: true)collect_fieldson 40 inline fragments with 100 fields eachcollect_fieldson 400@deferinline fragmentscollect_fieldson 400 named fragment spreadscollect_fieldson a 10-field querytest_execute_introspection_query(repo benchmark)The
collect_fieldsrows are medians of 31 rounds, then the median of 3 alternating processes per version.The introspection row is the median of 5 alternating pytest-benchmark runs per version.
They ran on CPython 3.12.13, Linux x86-64, AMD EPYC 9R14, on one dedicated core.
Full execution benchmarks do not change, because field collection is a small share of their time.
Validation
The collect-fields tests passed (5 tests), and the full test suite passed (3,821 passed, 18 skipped).
The wide-query timing calls the public
collect_fields()andcollect_subfields()14 times on a 4,000-field selection set.Its output was compared with the unchanged code, including field order, defer usages, errors and forbidden directives.
ruff checkandruff format --checkpass on the changed file.Why this is safe and easy to maintain
The patch changes one loop and one helper in a single file and adds no dependencies.
It removes two of the three
should_include_node()calls and one of the two directive scans.Grouped field order,
FieldDetailsvalues, defer usages, raised errors and forbidden-directive lists are unchanged.tests/execution/test_collect_fields.pyand the execution tests for@skip,@include,@deferand fragments cover this code.AI assistance
Codex gpt-6.1-sol generated the candidate changes, tracked with Weco Observe.
Claude Code ran the automated checks and prepared this description.
Weco share link: https://dashboard.weco.ai/share/LnYj9kTcZUOUYikZsvxmmnif2otC4CA9