Repository navigation
perf(router): build subgraph error paths by push and pop - #1537
Merged
Merged
Conversation
kamilkisiela
added this pull request to stack #1534
September 10, 2026 11:59
|
🐋 This PR was built and pushed to the following Docker images: Image Names: Platforms: Image Tags: |
kamilkisiela
marked this pull request as ready for review
September 10, 2026 13:55
dotansimha
approved these changes
Sep 10, 2026
kamilkisiela
force-pushed
the
kamil-qp-cache-errpath
branch
from
September 11, 2026 09:12
84bc6a8 to
bf3f7f6
Compare
`traverse_and_callback_mut` walks the response for every entity a subgraph
returned, and when that response carries errors it builds a GraphQL error path
alongside. It built that path by cloning: `concat` took `&self`, cloned the
whole accumulated `Vec<GraphQLErrorPathSegment>`, and pushed one segment - once
per descent step, per visited node. Each clone is a fresh `Vec` allocation plus
a fresh `String` for every field name already on the path.
Carry one path instead and push and pop it as the walk descends. Only the
callback needs an owned path, so only the callback clones. The public
`traverse_and_callback_mut` signature is unchanged; it seeds the path and
delegates to an inner `walk_mut` that threads `&mut Option<GraphQLErrorPath>`.
`GraphQLErrorPath::{concat, concat_index, concat_str}` had no other callers and
are replaced by `push_index`, `push_field` and `pop`.
This runs only when a subgraph response has errors - `initial_error_path` is
`None` otherwise, and both forms then do nothing. A model of the two walks over
the same tree, counting allocations, puts the saving at 32-34% for the shallow
two-level paths that dominate and 52-60% for deeper ones; the leaf clone is
identical in both, so what goes away is the per-step interior allocation. Those
figures are from a transcription of the two loops, not from the router.
Sharing one mutable path across siblings is the risk the clone-per-step form did
not have: a missing `pop` leaks steps from one entity into the next, and a type
condition is the delicate case because it descends without adding a step.
`error_paths_survive_type_conditions_and_sibling_branches` pins that. It passes
on `main` as well - it describes behaviour this commit preserves, not a bug it
fixes.
kamilkisiela
force-pushed
the
kamil-qp-cache-errpath
branch
from
September 11, 2026 10:55
bf3f7f6 to
6683318
Compare
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
2 participants
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.
Error paths in subgraph responses use now fewer allocations.
When a subgraph returns errors, the router builds a GraphQL error path while walking the response.
Previously, each step cloned the whole path built so far and then added one more segment. That meant repeated allocations as the path got deeper. The router now reuses one path while walking the response. It pushes a segment when going deeper and pops it when going back.
The path is only cloned when an error actually needs to keep it.