Avoid extra allocation/clone in SubgraphExecutionRequest #229

ardatan · 2025-07-01T14:36:29Z

Avoid cloning variables and operation string(aka query) for subgraph requests

gemini-code-assist

Summary of Changes

Hello @ardatan, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request primarily focuses on optimizing the performance of GraphQL request processing, especially for calls made to subgraphs. It achieves this by refactoring internal data structures to minimize memory allocations and data cloning, ensuring that query strings, operation names, and variables are passed by reference where possible. Additionally, it includes general improvements to the query projection logic to further reduce allocations and refines the gateway's handling of incoming requests.

Highlights

Reduced Allocations for Subgraph Requests: I've introduced a new SubgraphExecutionRequest struct that utilizes borrowed data (&str, &Value) for query strings, operation names, and variables. This change significantly reduces memory allocations and data cloning when preparing and executing requests against subgraphs, leading to improved performance.
Improved Subgraph Executor Interface: The SubgraphExecutor trait and its various implementations (e.g., for HTTP and async-graphql executors) have been updated to accept the new, more efficient SubgraphExecutionRequest type, ensuring that all subgraph communication benefits from the reduced allocation strategy.
Gateway Request Type Refactoring: The ExecutionRequest struct, which represents incoming GraphQL requests at the gateway level, has been redefined and isolated within bin/gateway/src/pipeline/graphql_request_params.rs. This clarifies the separation between the gateway's external request handling and the internal subgraph execution logic.
Optimized Query Projection: I've refactored several functions within the query projection logic (lib/query-plan-executor/src/projection.rs) to operate on immutable references and eliminate unnecessary string allocations. These general improvements contribute to overall performance gains beyond just subgraph requests.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

github-actions · 2025-07-01T14:37:41Z

Federation Audit Results

188 tests ±0 177 ✅ ±0 3s ⏱️ ±0s
41 suites ±0 0 💤 ±0
41 files ±0 11 ❌ ±0

For more details on these failures, see this check.

Results for commit 5c02b42. ± Comparison against base commit 7b6ac07.

♻️ This comment has been updated with latest results.

github-actions · 2025-07-01T14:37:42Z

	Tests	Passed ☑️	Failed ❌️	Time ⏱
federation-audit \| abstract-types.xml	18 ran	13 ✅	5 ❌	509ms
federation-audit \| child-type-mismatch.xml	4 ran	1 ✅	3 ❌	135ms
federation-audit \| circular-reference-interface.xml	2 ran	1 ✅	1 ❌	48ms
federation-audit \| complex-entity-call.xml	1 ran	1 ✅		84ms
federation-audit \| corrupted-supergraph-node-id.xml	10 ran	8 ✅	2 ❌	142ms
federation-audit \| enum-intersection.xml	5 ran	5 ✅		72ms
federation-audit \| fed1-external-extends-resolvable.xml	1 ran	1 ✅		30ms
federation-audit \| fed1-external-extends.xml	4 ran	4 ✅		65ms
federation-audit \| fed1-external-extension.xml	4 ran	4 ✅		65ms
federation-audit \| fed2-external-extends.xml	4 ran	4 ✅		68ms
federation-audit \| fed2-external-extension.xml	4 ran	4 ✅		66ms
federation-audit \| include-skip.xml	4 ran	4 ✅		89ms
federation-audit \| input-object-intersection.xml	3 ran	3 ✅		37ms
federation-audit \| interface-object-with-requires.xml	7 ran	7 ✅		118ms
federation-audit \| keys-mashup.xml	1 ran	1 ✅		31ms
federation-audit \| mutations.xml	4 ran	4 ✅		79ms
federation-audit \| mysterious-external.xml	2 ran	2 ✅		53ms
federation-audit \| nested-provides.xml	2 ran	2 ✅		34ms
federation-audit \| node.xml	1 ran	1 ✅		26ms
federation-audit \| non-resolvable-interface-object.xml	7 ran	7 ✅		77ms
federation-audit \| null-keys.xml	1 ran	1 ✅		31ms
federation-audit \| override-type-interface.xml	4 ran	4 ✅		72ms
federation-audit \| override-with-requires.xml	4 ran	4 ✅		90ms
federation-audit \| parent-entity-call-complex.xml	1 ran	1 ✅		35ms
federation-audit \| parent-entity-call.xml	1 ran	1 ✅		29ms
federation-audit \| provides-on-interface.xml	2 ran	2 ✅		45ms
federation-audit \| provides-on-union.xml	2 ran	2 ✅		43ms
federation-audit \| requires-interface.xml	5 ran	5 ✅		77ms
federation-audit \| requires-requires.xml	5 ran	5 ✅		107ms
federation-audit \| requires-with-argument.xml	5 ran	5 ✅		104ms
federation-audit \| requires-with-fragments.xml	6 ran	6 ✅		97ms
federation-audit \| shared-root.xml	2 ran	2 ✅		44ms
federation-audit \| simple-entity-call.xml	1 ran	1 ✅		24ms
federation-audit \| simple-inaccessible.xml	4 ran	4 ✅		54ms
federation-audit \| simple-interface-object.xml	13 ran	13 ✅		184ms
federation-audit \| simple-override.xml	2 ran	2 ✅		36ms
federation-audit \| simple-requires-provides.xml	12 ran	12 ✅		204ms
federation-audit \| typename.xml	6 ran	6 ✅		74ms
federation-audit \| unavailable-override.xml	2 ran	2 ✅		33ms
federation-audit \| union-interface-distributed.xml	10 ran	10 ✅		113ms
federation-audit \| union-intersection.xml	12 ran	12 ✅		162ms

github-actions · 2025-07-01T14:37:56Z

GraphQL over HTTP Audit Results

61 tests ±0 61 ✅ ±0 0s ⏱️ ±0s
1 suites ±0 0 💤 ±0
1 files ±0 0 ❌ ±0

Results for commit 5c02b42. ± Comparison against base commit 7b6ac07.

♻️ This comment has been updated with latest results.

gemini-code-assist

Code Review

This pull request introduces the SubgraphExecutionRequest to avoid extra allocation/clone in subgraph requests. The changes are well-propagated through the executor implementations. There are a few suggestions for better maintainability and readability.

lib/query-plan-executor/src/executors/http.rs

lib/query-plan-executor/src/projection.rs

github-actions · 2025-07-01T14:43:28Z

✅ `k6-benchmark` results

     ✓ response code was 200
     ✓ no graphql errors
     ✓ valid response structure

     █ setup

     checks.........................: 100.00% ✓ 8115      ✗ 0   
     data_received..................: 239 MB  7.9 MB/s
     data_sent......................: 3.2 MB  106 kB/s
     http_req_blocked...............: avg=73.86µs  min=1.64µs   med=4.52µs   max=6.19ms   p(90)=6.79µs   p(95)=8.32µs  
     http_req_connecting............: avg=67.98µs  min=0s       med=0s       max=6.13ms   p(90)=0s       p(95)=0s      
     http_req_duration..............: avg=108.06ms min=4.47ms   med=97.45ms  max=440.44ms p(90)=165.24ms p(95)=194.23ms
       { expected_response:true }...: avg=108.06ms min=4.47ms   med=97.45ms  max=440.44ms p(90)=165.24ms p(95)=194.23ms
     http_req_failed................: 0.00%   ✓ 0         ✗ 2725
     http_req_receiving.............: avg=1.5ms    min=36.01µs  med=68.95µs  max=112.05ms p(90)=651.61µs p(95)=2.13ms  
     http_req_sending...............: avg=397.86µs min=9.22µs   med=21.73µs  max=130.36ms p(90)=99.17µs  p(95)=1.13ms  
     http_req_tls_handshaking.......: avg=0s       min=0s       med=0s       max=0s       p(90)=0s       p(95)=0s      
     http_req_waiting...............: avg=106.15ms min=4.39ms   med=95.85ms  max=429.39ms p(90)=160.55ms p(95)=186.19ms
     http_reqs......................: 2725    90.108164/s
     iteration_duration.............: avg=556.04ms min=101.12ms med=542.39ms max=1s       p(90)=664.63ms p(95)=723.41ms
     iterations.....................: 2705    89.446819/s
     vus............................: 50      min=50      max=50
     vus_max........................: 50      min=50      max=50

dotansimha · 2025-07-02T07:46:55Z

lib/query-plan-executor/src/executors/http.rs

+        // We may want to remove it, but let's see.
+        let mut body = String::with_capacity(4096);
+        body.push_str("{\"query\":");
+        write_and_escape_string(&mut body, execution_request.query);


dotansimha · 2025-07-02T07:47:19Z

lib/query-plan-executor/src/executors/http.rs

-        let mut body =
-            "{\"query\":".to_string() + &serde_json::to_string(&execution_request.query).unwrap();
+        // We may want to remove it, but let's see.
+        let mut body = String::with_capacity(4096);


4096 is generally enough? how did we end up with this value?

it will never be correct

dotansimha

lgtm. @kamilkisiela please review as well.

Avoid cloning variables and operation string(aka `query`) for subgraph requests

Also includes #226 & #229 & #245 --------- Co-authored-by: Kamil Kisiela <kamil.kisiela@gmail.com>

ardatan requested a review from kamilkisiela July 1, 2025 14:36

gemini-code-assist bot reviewed Jul 1, 2025

View reviewed changes

ardatan force-pushed the avoid_extra_allocation branch from e808033 to ca72f46 Compare July 1, 2025 14:38

gemini-code-assist bot reviewed Jul 1, 2025

View reviewed changes

lib/query-plan-executor/src/executors/http.rs Outdated Show resolved Hide resolved

lib/query-plan-executor/src/projection.rs Show resolved Hide resolved

ardatan force-pushed the avoid_extra_allocation branch from ca72f46 to 5c02b42 Compare July 1, 2025 14:40

ardatan force-pushed the avoid_extra_allocation branch 2 times, most recently from 5308c5e to 80a25ea Compare July 1, 2025 14:48

dotansimha reviewed Jul 2, 2025

View reviewed changes

dotansimha approved these changes Jul 2, 2025

View reviewed changes

kamilkisiela approved these changes Jul 2, 2025

View reviewed changes

ardatan force-pushed the parallelism branch from 6e3d3b7 to 126733b Compare July 2, 2025 13:34

ardatan force-pushed the avoid_extra_allocation branch from 80a25ea to a5d0ef0 Compare July 2, 2025 13:37

Base automatically changed from parallelism to fix-simple-interface-object July 2, 2025 14:11

Avoid extra allocation/clone in SubgraphExecutionRequest

7d58d87

ardatan force-pushed the avoid_extra_allocation branch from a5d0ef0 to 7d58d87 Compare July 2, 2025 14:13

ardatan merged commit 588f47a into fix-simple-interface-object Jul 2, 2025
10 checks passed

ardatan deleted the avoid_extra_allocation branch July 2, 2025 14:14

ardatan mentioned this pull request Jul 2, 2025

Fix simple-interface-object audit / fix possible type check #224

Merged

ardatan added a commit that referenced this pull request Jul 2, 2025

Avoid extra allocation/clone in SubgraphExecutionRequest (#229)

0ea5d94

Avoid cloning variables and operation string(aka `query`) for subgraph requests

ardatan added a commit that referenced this pull request Jul 3, 2025

Avoid extra allocation/clone in SubgraphExecutionRequest (#229)

9f2ef80

Avoid cloning variables and operation string(aka `query`) for subgraph requests

ardatan added a commit that referenced this pull request Jul 8, 2025

Avoid extra allocation/clone in SubgraphExecutionRequest (#229)

3c27031

Avoid cloning variables and operation string(aka `query`) for subgraph requests

ardatan added a commit that referenced this pull request Jul 8, 2025

Fix simple-interface-object audit / fix possible type check (#224)

bab2823

Also includes #226 & #229 & #245 --------- Co-authored-by: Kamil Kisiela <kamil.kisiela@gmail.com>

ardatan added a commit that referenced this pull request Jul 8, 2025

Fix simple-interface-object audit / fix possible type check (#224)

240c90b

Also includes #226 & #229 & #245 --------- Co-authored-by: Kamil Kisiela <kamil.kisiela@gmail.com>

ardatan added a commit that referenced this pull request Jul 9, 2025

Fix simple-interface-object audit / fix possible type check (#224)

03b551d

Also includes #226 & #229 & #245 --------- Co-authored-by: Kamil Kisiela <kamil.kisiela@gmail.com>

ardatan added a commit that referenced this pull request Jul 16, 2025

Fix simple-interface-object audit / fix possible type check (#224)

d3c1c57

Also includes #226 & #229 & #245 --------- Co-authored-by: Kamil Kisiela <kamil.kisiela@gmail.com>

ardatan added a commit that referenced this pull request Jul 17, 2025

Fix simple-interface-object audit / fix possible type check (#224)

6d14d38

Also includes #226 & #229 & #245 --------- Co-authored-by: Kamil Kisiela <kamil.kisiela@gmail.com>

ardatan added a commit that referenced this pull request Jul 18, 2025

Fix simple-interface-object audit / fix possible type check (#224)

3e89eba

Also includes #226 & #229 & #245 --------- Co-authored-by: Kamil Kisiela <kamil.kisiela@gmail.com>

ardatan added a commit that referenced this pull request Jul 21, 2025

Fix simple-interface-object audit / fix possible type check (#224)

9dec09d

Also includes #226 & #229 & #245 --------- Co-authored-by: Kamil Kisiela <kamil.kisiela@gmail.com>

Avoid extra allocation/clone in SubgraphExecutionRequest #229

Avoid extra allocation/clone in SubgraphExecutionRequest #229

Uh oh!

Conversation

ardatan commented Jul 1, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Footnotes

Uh oh!

github-actions bot commented Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Federation Audit Results

Uh oh!

github-actions bot commented Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

GraphQL over HTTP Audit Results

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ k6-benchmark results

Uh oh!

dotansimha Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

dotansimha Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

kamilkisiela Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

kamilkisiela Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

dotansimha left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions bot commented Jul 1, 2025 •

edited

Loading

github-actions bot commented Jul 1, 2025 •

edited

Loading

github-actions bot commented Jul 1, 2025 •

edited

Loading

github-actions bot commented Jul 1, 2025 •

edited

Loading

✅ `k6-benchmark` results