rustberry modernize-v3: apollo-rs 1.31 + PyO3 0.28 + perf [DRAFT REVIEW] - #5
Draft
erikwrede wants to merge 8 commits into
Draft
rustberry modernize-v3: apollo-rs 1.31 + PyO3 0.28 + perf [DRAFT REVIEW]#5erikwrede wants to merge 8 commits into
erikwrede wants to merge 8 commits into
Conversation
Port the source tree from pyo3 0.21 / apollo-compiler 1.0-era APIs to the
current 0.28 / 1.31 surface so the crate compiles again.
Highlights:
- Replace `&PyAny`/`PyObject`/`PyModule::import` with `Bound<'py, PyAny>`,
`Py<PyAny>`, and the `Bound`-returning `Python::import` from pyo3 0.28.
- Switch `Python::with_gil` to the 0.28 `Python::attach` API and use
`py.detach(...)` (formerly `allow_threads`) for GIL-free Rust work.
- Drop legacy `import_bound`/`PyDict::new_bound`/`to_object` helpers in
favour of the new APIs.
- Update apollo-compiler usage: `document.operations.iter()` instead of
the removed `all_operations`, `response::ResponseDataPathSegment`
instead of `execution::ResponseDataPathElement`, and `Diagnostic::to_json`
for converting validation errors.
- Rebuild the `#[pymodule]` registration to take a `&Bound<'_, PyModule>`.
- Enable the `py-clone` pyo3 feature so the derive-based `#[pyo3(get)]`
getters can compile against `Py<...>` and `Vec<Py<...>>` fields. Every
getter site holds the GIL so the runtime requirement is satisfied.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Round out the public API:
- `Selection::FragmentSpread` and `Selection::InlineFragment` arms in
`MirrorConversionContext` now produce `FragmentSpreadNode` and
`InlineFragmentNode` mirror nodes (added to `reduced_core_mirror`
alongside `FragmentDefinitionNode`). They report the right `kind`
string and pretend to be the matching `graphql-core` class via the
`__class__` getter so `isinstance` checks still work.
- `QueryCompiler.validate` and `add_validate` now return a
`list[GraphQLError]` mirroring `graphql-core`'s diagnostic shape
instead of an opaque bool. Empty list = valid.
- Bad schemas / bad queries now raise `ValueError` with the formatted
apollo-compiler diagnostics instead of panicking.
- `GraphQLError.extensions` returns an empty dict (was a `panic!`).
- All mirror nodes are `#[pyclass(frozen)]` and skip the deprecated
auto-`FromPyObject` derive.
- Cache class lookups in module-level `OnceLock`s instead of importing
`graphql.language.ast` on every `__class__` getter call.
- Replace deprecated `downcast_into` with `cast_into`.
- Refresh `_rustberry.pyi` stubs to match the new return types and
re-export `Document`/`GraphQLError`/`SourceLocation` from
`rustberry`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Switch from the legacy `[tool.poetry]` block to PEP 621 `[project]` metadata. Pin the Python floor to 3.11 (matching the abi3-py311 wheel selection in `Cargo.toml`), depend on `graphql-core>=3.2` for the mirror-node `__class__` lookups, and route maturin through the new `python-source`/`module-name` knobs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous formatting via `Display` triggered apollo-compiler's ariadne renderer, which printed "Unable to fetch source" lines to stderr whenever the schema source wasn't attached to the document's source map. Format errors via `Diagnostic::to_json().message` instead so the surface stays quiet on the failure path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous mirror getters re-encoded UTF-8 strings on every access: `NameNode.value` cloned a Rust `String` into a fresh `PyString`, and every `kind` getter allocated a new Python string for a literal that never changes. graphql-core's executor reads these attributes thousands of times per request, so the per-call ~28 ns FFI cost showed up as ~200 us of pure overhead on a 2.4 ms `execute` call. Cache the Python representation once at construction: * Textual leaves (`NameNode.value`, `IntValueNode.value`, `FloatValueNode.value`, `StringValueNode.value`, `EnumValueNode.value`) now hold `Py<PyString>`. The auto-derived `#[pyo3(get)]` returns a borrowed view per access; no allocation, no UTF-8 decode. * `kind` is served from a per-pyclass `OnceLock<Py<PyString>>` seeded with `PyString::intern`, so all instances share one interned Python string. Identity-based equality at the Python level then short-circuits `node.kind == "field"` checks. `mirror_converter.rs` interns names and enum/int/float values at construction and uses a plain (non-interned) `PyString` for arbitrary string literals so we don't pollute the interpreter's intern table with user data. Microbenchmark (`benchmarks/bench_pyclass_overhead.py`) before/after: rustberry mirror NameNode median 27.9 ns -> 17.1 ns graphql-core NameNode median 10.0 ns (unchanged) Smoke-tested with `execute_sync` on a query that uses fields, arguments, and named fragments; results match graphql-core exactly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…phql-core helpers
Strawberry's runtime error formatter and graphql-core's coercion helpers
read AST node.loc and GraphQLError.{original_error, nodes, source, positions}.
Mirror nodes and our error type didn't expose those, so any path that
formatted an error against a rustberry-mirror document raised
AttributeError instead of returning the actual error.
Mirror nodes get a constant loc=None getter (apollo-compiler doesn't carry
per-AST-node source positions). GraphQLError gets original_error, nodes,
source, positions all = None — apollo-compiler diagnostics don't preserve
those either, but downstream helpers probe them.
Profiling rustberry mirror nodes against graphql-core's executor showed the per-access attribute cost was no longer a bottleneck (4.6 ns, parity with pure-Python __slots__) BUT collection-field access was 5-7x slower: rustberry posts.directives: 87 ns vs graphql-core 17 ns rustberry posts.arguments: 98 ns vs graphql-core 17 ns rustberry bool(directives): 92 ns vs graphql-core 23 ns Reason: `#[pyo3(get)]` on a `Vec<#[pyclass]>` field reconstructs a fresh PyList on EVERY attribute access. graphql-core's executor reads field.directives, field.arguments, field.selection_set.selections on every resolver call, and runs `if node.directives:` truthiness checks constantly during validation/coercion. The fix: store these fields as Py<PyTuple> built once at conversion time. Each later access returns the same tuple via a refcount bump. After this change: rustberry posts.directives: 16.2 ns (parity) rustberry posts.arguments: 16.7 ns (parity) rustberry bool(directives): 23.8 ns (parity) End-to-end relay_feed: 11.19 ms -> 10.86 ms (~3% improvement on a heavy-resolver query where execute dominates).
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
1 participant
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.
Private review surface — fork draft
8 commits modernizing rustberry. Stacked on top of master.
What's in the stack
b13a8615b0b6fb14cb14af3cfcbab3a1d15881066bdf9799368d3e1eUntracked locally (not in this PR)
benchmarks/directory with the benchmark suite (bench_real_world.py, bench_pyclass_overhead.py, bench_rustberry_vs_core.py, etc.) and captured.txtresults — let me know if you want these checked in tootest_parser/test_real_world_smoke.py— smoke tests against GitHub & SpaceX schemasStatus
maturin build --releaseproducesrustberry-0.0.15-cp311-abi3-macosx_11_0_arm64.whlcleanlystrawberry-rustberry(separate repo) which has 14 passing tests