Skip to content

perf(collect_fields): skip directive checks -- 15% faster wide queries #316

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dexhunter wants to merge 2 commits into
base: main
Choose a base branch
from
Open

perf(collect_fields): skip directive checks -- 15% faster wide queries #316

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1f0aead
perf(collect_fields): skip directive checks -- 15% faster wide queries
dexhunter Nov 8, 2026
ddd3d49
fix(collect_fields): type should_include_node node as SelectionNode
dexhunter Nov 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 16 additions & 38 deletions src/graphql/execution/collect_fields.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
FragmentSpreadNode,
InlineFragmentNode,
OperationDefinitionNode,
SelectionNode,
SelectionSetNode,
)
from ..pyutils import RefSet
Expand Down Expand Up @@ -225,19 +226,17 @@ def visit_selection_sets(
on_selection_set(selection_set)

for selection in selection_set.selections:
if selection.directives and not should_include_node(
context, selection, variable_values, fragment_variable_values
):
continue
if isinstance(selection, FieldNode):
if not should_include_node(
context, selection, variable_values, fragment_variable_values
):
continue
key = get_field_entry_key(selection)
grouped_field_set[key].append(
FieldDetails(selection, defer_usage, fragment_variable_values)
)
elif isinstance(selection, InlineFragmentNode):
if not should_include_node(
context, selection, variable_values, fragment_variable_values
) or not does_fragment_condition_match(schema, selection, runtime_type):
if not does_fragment_condition_match(schema, selection, runtime_type):
continue

new_defer_usage = get_defer_usage(
Expand Down Expand Up @@ -268,12 +267,6 @@ def visit_selection_sets(
)
elif isinstance(selection, FragmentSpreadNode): # pragma: no branch
frag_name = selection.name.value

if not should_include_node(
context, selection, variable_values, fragment_variable_values
):
continue

fragment = fragments.get(frag_name)
if fragment is None or not does_fragment_condition_match(
schema, fragment.definition, runtime_type
Expand Down Expand Up @@ -370,7 +363,7 @@ def get_defer_usage(

def should_include_node(
context: CollectFieldsContext,
node: FragmentSpreadNode | FieldNode | InlineFragmentNode,
node: SelectionNode,
variable_values: VariableValues,
fragment_variable_values: FragmentVariableValues | None = None,
) -> bool:
Expand All @@ -379,18 +372,15 @@ def should_include_node(
Determines if a field should be included based on the @include and @skip
directives, where @skip has higher precedence than @include.
"""
skip_directive_node = (
next(
(
directive
for directive in node.directives
if directive.name.value == GraphQLSkipDirective.name
),
None,
)
if node.directives
else None
)
skip_directive_node: DirectiveNode | None = None
include_directive_node: DirectiveNode | None = None
for directive in node.directives or ():
name = directive.name.value
if skip_directive_node is None and name == GraphQLSkipDirective.name:
skip_directive_node = directive
elif include_directive_node is None and name == GraphQLIncludeDirective.name:
include_directive_node = directive

if skip_directive_node and context.forbid_skip_and_include:
context.forbidden_directive_instances.append(skip_directive_node)
return False
Expand All @@ -408,18 +398,6 @@ def should_include_node(
if skip and skip["if"]:
return False

include_directive_node = (
next(
(
directive
for directive in node.directives
if directive.name.value == GraphQLIncludeDirective.name
),
None,
)
if node.directives
else None
)
if include_directive_node and context.forbid_skip_and_include:
context.forbidden_directive_instances.append(include_directive_node)
return False
Expand Down
Loading