Skip to content

Avoid per-argument work when coercing literal arguments - #277

Draft
patrick91 wants to merge 2 commits into
graphql-python:mainfrom
patrick91:optimize-argument-coercion
Draft

patrick91 wants to merge 2 commits into
graphql-python:mainfrom
patrick91:optimize-argument-coercion

Conversation

@patrick91

@patrick91 patrick91 commented Sep 28, 2026 •

Copy link
Copy Markdown
Member

For context, codspeed flagged some performance regression here, so I asked Claude to investigate and found a quick win 😊

Stacked on #276, which adds the test_execute_field_arguments_sync benchmark used below. Until #276 is merged this PR also shows that commit; only the last commit (Avoid per-argument work when coercing literal arguments) belongs here.

This removes two bits of work that ran for every argument of every executed field:

  1. coerce_argument built its default-value error handler on every call. The on_arg_default_value_error closure is only needed when an argument falls back to its default value, but it was created before checking that. It's now built by a small _default_value_error_handler helper, only in the two branches that apply a default value.
  2. coerce_input_literal called replace_variables for every leaf literal. A bare variable is already handled at the top of the function, so by the time we reach a leaf type only list and object literals can still contain variables. Other literals (like 2 in value(multiplier: 2)) are already constant, so they're now passed to the scalar's coerce_input_literal directly.

Behavior is unchanged: the full test suite and the doctests of both modules pass, as do ruff check src tests, ruff format --check src tests and mypy src tests.

Locally, test_execute_field_arguments_sync from #276 gets 2–4% faster (median of 200+ rounds, alternating before/after over 4 runs), with about 3 fewer function calls per field. It's a modest win, but it's on the hot path for any query that passes arguments.

AI: I've done this with Claude 😊

@patrick91
patrick91 requested a review from Cito as a code owner September 28, 2026 23:16
@patrick91
patrick91 marked this pull request as draft September 28, 2026 23:20
@codspeed

codspeed Bot commented Sep 28, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 19 untouched benchmarks
🆕 1 new benchmark

Performance Changes

Benchmark BASE HEAD Efficiency
🆕 test_execute_field_arguments_sync N/A 120.6 ms N/A

Comparing patrick91:optimize-argument-coercion (bdab43b) with main (894141f)

Open in CodSpeed

@Cito

Cito commented Sep 29, 2026

Copy link
Copy Markdown
Member

Thank you @patrick91, will have a look as soon as I find some time. Is this ready for review (you marked it as draft)?

@patrick91

Copy link
Copy Markdown
Member Author

Thank you @patrick91, will have a look as soon as I find some time. Is this ready for review (you marked it as draft)?

I wanted to double check it when it wasn't late as a night :D and also make sure the benchmark was good 😊

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