Conversation
bb3fc51 to
ca09137
Compare
ca09137 to
5b161ce
Compare
|
Hey, thanks so much for tracking down these improvements and sharing your results. This looks like a great win. I thought I had already avoided PrepareObjectStep whenever I could but I must have missed a spot! I have a couple other big issues to review at the moment but I'll take a close look at this one soon. |
rmosolgo
left a comment
There was a problem hiding this comment.
Hey, sorry it took me a while to make time to review this. It seems like these three commits are unrelated so I reviewed each one independently.
-
Argument node indexing
This looks fine with me. Out of curiosity, do you have many fields in your app with 32 or more arguments? I don't think I've ever seen a field with that many arguments, but maybe it would happen for a mutation that receives a lot of inputs.
-
Skip PrepareObjectStep
I definitely want to avoid these when appropriate. Did you find them allocated in your app when you could avoid them?
Also, could you share the benchmark you used to measure the reduced memory usage? I'd like to see exactly what conditions were triggering these allocations.
-
LoadArgumentsStep
This makes sense to me. If you have a long array of IDs that will load arguments, they can be treated altogether. Did you have an input like this in your app that motivated this optimization?
| if any_lazy_results? | ||
| @field_results_are_eager = false | ||
| @runner.dataloader.lazy_at_depth(path.size, self) | ||
| elsif @pending_steps.nil? || @pending_steps.empty? |
There was a problem hiding this comment.
There is an implicit else branch here which doesn't assign @field_results_are_eager to anything. How is @field_results_are_eager handled when the code follows that else branch?
This PR reduces three input-size-dependent costs in
GraphQL::Execution::Nextwithout adding or changing public APIs:PrepareObjectStepallocations when no preparation is requiredLoadArgumentStepallocations for list arguments usingloads:Each optimization is kept in a separate commit and includes coverage for the compatibility-sensitive paths it changes.
Changes
ba4c954 : Index large execution argument nodes
Large argument collections previously performed a linear AST scan for every argument definition, making argument lookup quadratic as the number of arguments increased.
This commit builds a name index when there are at least 32 argument nodes. Smaller collections keep the existing linear lookup to avoid paying for a Hash allocation on common queries.
The index uses
||=so invalid documents executed withvalidate: falsepreserve the existing behavior of selecting the first duplicate argument.ae5dc8d : Skip unnecessary object preparation steps
Concrete, eager composite values previously allocated one
PrepareObjectStepper returned object, even when no authorization, type resolution, scoping, post-processing, or finalization was required.This commit adds a conservative direct-result path for eager concrete objects. The existing preparation path remains in use for:
The direct path preserves authorization trace events expected by schemas configured with lazy resolution.
bb3fc51 : Batch list argument loading steps
A list argument using
loads:previously allocated oneLoadArgumentStepfor every list item.This commit replaces those per-item step objects with one
LoadArgumentsStepstate machine. Public loading and authorization hooks are still called once per item and retain their existing order.The batch step preserves:
object_loadedtrace orderGraphQL::Current.fieldduring trace hooksrescue_fromhandling for loading and trace hook failuresScalar
loads:arguments continue using the existingLoadArgumentStep.Benchmarks
Environment
ruby 4.0.0 (2025-12-25 revision 553f1675f3) +PRISMThe benchmark uses pre-parsed documents with
validate: falseto isolate execution work. Each timing is the median of five samples after warmup. Before timing, the benchmark verifies that the legacy andExecution::Nextresults match.Allocation totals were collected with
MemoryProfiler.Argument node indexing
The index intentionally trades a small allocation increase for eliminating the quadratic lookup cost. A zero-argument query changed from 59.41 µs to 59.90 µs (+0.8%), within measurement noise.
Skipping object preparation
This workload returns 1,000 eager concrete objects with 25 scalar fields each.
Authorization and abstract-type fallback workloads continue using
PrepareObjectStep. Repeated measurements of those fallback paths were within 2% of the baseline and had unchanged allocation counts.Batched list argument loads
For 1,000 loaded values:
A follow-up alternating-process run after the error-handling hardening showed the same allocation counts. Runtime varied by less than approximately 1% compared with the pre-hardening implementation.