Skip to content

Comments

Reduce allocations in Async$Many and ResultPath on execution hot path#4252

Merged
andimarek merged 6 commits intomasterfrom
async-many-materialized-list-optimization
Feb 19, 2026
Merged

Reduce allocations in Async$Many and ResultPath on execution hot path#4252
andimarek merged 6 commits intomasterfrom
async-many-materialized-list-optimization

Conversation

@andimarek
Copy link
Member

@andimarek andimarek commented Feb 19, 2026 •
edited
Loading

Summary

Three execution hot-path optimizations identified via async-profiler:

  1. Async$Many.materialisedList(): Replace ArrayList copy with zero-copy Arrays.asList() wrapper — eliminates one allocation + array copy per selection set on the synchronous execution path
  2. ResultPath.toStringValue: Make lazy (computed on first toString() call) — the string representation is never read during normal execution, only for error reporting
  3. GraphQLCodeRegistry DataFetcher lookup: Add public getDataFetcher(String, String, GraphQLFieldDefinition) overload using a nested Map<String, Map<String, DataFetcherFactory<?>>> for fast lookup by type/field name strings, avoiding creation of a throwaway FieldCoordinates object on every field fetch (~54 KB/op reduction in allocations)

Also adds ExecutionBenchmark JMH benchmark and async-profiler support to build.gradle.

Benchmark results

Existing benchmarks (master vs optimized)

Benchmark Metric Master Optimized Change
SimpleQueryBenchmark throughput 1929 ops/s 2379 ops/s +23%
SimpleQueryBenchmark avg time 0.538 ms/op 0.421 ms/op -22%
TwitterBenchmark throughput 167 ops/s 232 ops/s +39%
TwitterBenchmark avg time 6.0 ms/op 4.3 ms/op -28%

ExecutionBenchmark (new — only on optimized branch)

Variant Throughput Avg time
Baseline 3414 ops/s 0.294 ms/op
DataLoader 2009 ops/s 0.512 ms/op

Test plan

  • Full test suite passes (./gradlew test)
  • JMH benchmarks run successfully
  • Arrays.asList returns a fixed-size list — verified no consumer mutates the returned list
  • Lazy ResultPath.toString() verified safe: same pattern as hashCode() (racy single-check idiom on immutable data)
  • CodeRegistry fast-path produces identical results to existing getDataFetcher — same lookup logic, different key structure

🤖 Generated with Claude Code

@andimarek @claude
Avoid redundant array copy in Async$Many for synchronous execution path
acc6c10 
In the all-synchronous execution path (no CompletableFutures), Async$Many
allocated an Object[] to collect field values, then copied them into a new
ArrayList in materialisedList(). Replace the copy with Arrays.asList() which
wraps the existing array at zero cost.

Benchmarked with a new ExecutionBenchmark (balanced tree: ~530 fields, ~2000
result scalars, depth 5) showing ~5% throughput improvement on the synchronous
path. Also adds async-profiler support to build.gradle for JMH profiling.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
@github-actions
Copy link
Contributor

github-actions bot commented Feb 19, 2026 •
edited
Loading

Test Results

  335 files  ±0    335 suites  ±0   5m 3s ⏱️ -1s
5 376 tests ±0  5 367 ✅ ±0  9 💤 ±0  0 ❌ ±0 
5 465 runs  ±0  5 456 ✅ ±0  9 💤 ±0  0 ❌ ±0 

Results for commit c52498b. ± Comparison against base commit f8f9892.

This pull request removes 196 and adds 172 tests. Note that renamed tests count towards both. 
	?

	, expected: combo-\"\\\b\f\n\r\t, #4]
                __schema { types { fields { args { type { name fields { name }}}}}}
                __schema { types { fields { type { name fields { name }}}}}
                __schema { types { inputFields { type { inputFields { name }}}}}
                __schema { types { interfaces { fields { type { interfaces { name } } } } } }
                __schema { types { name} }
                __type(name : "t") { name }
                a1: __schema { types { name} }
                a1: __type(name : "t") { name }
…

graphql.AssertTest ‑ assertFalse with different number of error args but false does not throw assertions [toRun: <graphql.AssertTest$__spock_feature_0_21prov0_closure23@75de29c0 delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1, #0]
graphql.AssertTest ‑ assertFalse with different number of error args but false does not throw assertions [toRun: <graphql.AssertTest$__spock_feature_0_21prov0_closure24@fc807c1 delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1 arg2, #1]
graphql.AssertTest ‑ assertFalse with different number of error args but false does not throw assertions [toRun: <graphql.AssertTest$__spock_feature_0_21prov0_closure25@296e281a delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1 arg2 arg3, #2]
graphql.AssertTest ‑ assertFalse with different number of error args throws assertions [toRun: <graphql.AssertTest$__spock_feature_0_20prov0_closure20@6dcc40f5 delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1, #0]
graphql.AssertTest ‑ assertFalse with different number of error args throws assertions [toRun: <graphql.AssertTest$__spock_feature_0_20prov0_closure21@2b680207 delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1 arg2, #1]
graphql.AssertTest ‑ assertFalse with different number of error args throws assertions [toRun: <graphql.AssertTest$__spock_feature_0_20prov0_closure22@70887727 delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1 arg2 arg3, #2]
graphql.AssertTest ‑ assertNotNull with different number of  error args throws assertions [toRun: <graphql.AssertTest$__spock_feature_0_5prov0_closure3@730f9695 delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1, #0]
graphql.AssertTest ‑ assertNotNull with different number of  error args throws assertions [toRun: <graphql.AssertTest$__spock_feature_0_5prov0_closure4@146dcfe6 delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1 arg2, #1]
graphql.AssertTest ‑ assertNotNull with different number of  error args throws assertions [toRun: <graphql.AssertTest$__spock_feature_0_5prov0_closure5@1b1f5012 delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1 arg2 arg3, #2]
graphql.AssertTest ‑ assertNotNull with different number of error args with non null does not throw assertions [toRun: <graphql.AssertTest$__spock_feature_0_6prov0_closure6@51c959a4 delegate=inaccessible owner=inaccessible thisObject=inaccessible resolveStrategy=inaccessible directive=inaccessible parameterTypes=inaccessible maximumNumberOfParameters=inaccessible bcw=inaccessible thisType=inaccessible>, expectedMessage: error arg1, #0]
…

♻️ This comment has been updated with latest results.

@andimarek @claude
Make ResultPath.toStringValue lazy and avoid intermediate string allo…
45d13fc 
…cations

The toString representation of ResultPath was eagerly computed in the
constructor via initString(), but is never read during normal query
execution — only used for error reporting. Make it lazy (computed on
first toString() call) to eliminate all string work from the hot path.

Also inline segmentToString() into initString() to avoid intermediate
String allocations when the value is eventually computed, letting Java's
StringConcatFactory handle it as a single multi-arg concat.

Benchmarked ~30-78% throughput improvement vs master across all execution
benchmarks.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
@andimarek andimarek changed the title Avoid redundant array copy in Async$Many materialisedList Reduce allocations in Async$Many and ResultPath on execution hot path Feb 19, 2026
andimarek and others added 4 commits February 19, 2026 20:43
@andimarek @claude
Add fast-path DataFetcher lookup avoiding FieldCoordinates allocation
659fe34 
Every field fetch created a throwaway FieldCoordinates object just for a
HashMap lookup. Add an internal nested Map<String, Map<String, ...>>
(typeName → fieldName → factory) built at CodeRegistry construction time,
and an internal getDataFetcher(String, String, GraphQLFieldDefinition)
method that does the lookup by strings directly. Use this in
ExecutionStrategy.fetchField to skip FieldCoordinates creation entirely.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
@andimarek
Merge remote-tracking branch 'origin/master' into async-many-material…
9a54c18 
…ized-list-optimization
@andimarek @claude
Make fast-path getDataFetcher public and document performance benefit
4011983 
Remove @internal from the String-based getDataFetcher overload and add
proper javadoc documenting the ~54 KB/op allocation savings and 5-9%
throughput improvement over the FieldCoordinates-based lookup.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
@andimarek @claude
Simplify ResultPath.initString() — inlining is unnecessary with lazy …
c52498b 
…init

Since toStringValue is lazily computed (once per path), the manual
inlining of segmentToString() provides no measurable performance
benefit. Simplify back to the clean delegation.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
@andimarek andimarek merged commit 9ae83ab into master Feb 19, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

26.x is a breaking change release (ot...

Development

Successfully merging this pull request may close these issues.

1 participant

@andimarek