Repository navigation
1% possible memory and cpu improvements #3939
Description
graphql-java is a mature and optimised library. So its increasingly difficult to find performance improvements that really stand out.
This leads us to the 1% club where we need to do a bunch of work to get a small 1% gain
This issue tries to capture investigations of these 1%'ers and hence be a list of possible micro improvements
Profiler investigations - memory
Running ComplexQueryPeformance as load and using JProfiler
The biggest objects are as follows
"graphql.execution.ExecutionStrategyParameters", 4% of memory
"graphql.execution.ExecutionStrategyParameters$Builder", 4% of memory use
"graphql.execution.ExecutionStepInfo" 2%
"graphql.execution.ExecutionStepInfo$Builder", 2%
"graphql.execution.NonNullableFieldValidator", 2%
"graphql.execution.FetchedValue", 2%
I have PRS that try to reduce all these - eg either remove the builder usage for "transforms" (ExecutionStrategyParameters and ExecutionStepInfo)
or avoid re-allocating per field (NonNullableFieldValidator and FetchedValue)
The builder ones reduce by memory per type by half right - because we no longer create a builder to actually create a ExecutionStepInfo say
interesting future things to look into
-
"graphql.util.IntraThreadMemoizedSupplier", 4%
** why so heavy - do we strictly need it -
"graphql.schema.FieldCoordinates" 2%
** can we avoid this some how - flyweights? -
"graphql.schema.PropertyFetchingImpl$CacheKey", 2%
** can we avoid this some how - flyweights? -
"com.google.common.collect.SingletonImmutableList", 2%
** why so high?? Are we wrapping something for no reason -
"graphql.schema.DataFetcherFactoryEnvironment", 2%
** "graphql.schema.DataFetcherFactoryEnvironment$Builder" 2%
** this can go I think
asserts cost cpu and memory
eg :
this.executionStepInfo = assertNotNull(executionStepInfo, () -> "executionStepInfo is null");
These asserts are great for code reasons - but they cost something to run
Can we use the jspecify annotations here and remove these because static analysis is giving us the safety ?
cpu hot spots to look into
-
1.6% - 1,256 ms - 1,372,469 hot spot inv. graphql.schema.GraphQLCodeRegistry.getDataFetcherImpl
-
1.5% - 1,190 ms - 7,119,736 hot spot inv. graphql.schema.GraphQLTypeUtil.unwrapNonNull
-
1.5% - 1,209 ms - 7,088,722 hot spot inv. graphql.execution.ExecutionStepInfo.getUnwrappedNonNullType
-
1.3% - 1,010 ms - 12,160,902 hot spot inv. graphql.Assert.assertNotNull(java.lang.Object, java.util.function.Supplier)
-
1.0% - 786 ms - 1,485,657 hot spot inv. graphql.execution.ExecutionStrategy.unboxPossibleDataFetcherResult
This is an interesting one - its called on DataFetch BUT its also called in completeValueForList
meaning can a DF returns a DataFetcherResult that itself is a list of DataFetcherResult of values. Is this even remotely sensible?? Can we kill it in lists the name of 1% ??


Activity
bbakerman commentedon Apr 29, 2025
Findings
** why so heavy - do we strictly need it
This is actually saving us work and memory. This wraps the DataFetcherEnvironment creation (and heavy ish object to create) when we end up calling LightDataFetchers - eg PropertyDataFetchers
So while its allocating memory - its saving memory and cpu work.
** why so high?? Are we wrapping something for no reason
This is from
graphql.execution.MergedField#newSingletonMergedFieldwhere it runs this codeWe have lots of MergedFields and hence we have lots of SingletonImmutableList created
"graphql.schema.FieldCoordinates" 2%
** can we avoid this some how - flyweights?
"graphql.schema.PropertyFetchingImpl$CacheKey", 2%
** can we avoid this some how - flyweights?
We could to flyweights and it saves memory but its slower to run
See #3940 for more details
github-actions commentedon Jun 29, 2025
Hello, this issue has been inactive for 60 days, so we're marking it as stale. If you would like to continue this discussion, please comment within the next 30 days or we'll close the issue.
dfa1 commentedon Jul 21, 2025
@bbakerman hello 😄
The MergedField part is interesting:
My 2 cents!
bbakerman commentedon Jul 23, 2025
See #4056 for a better MergedField impl
This takes the idea presented above and creates a MergedField that is a single field value and then a MultiMergedField specialisation that holds multiple fields should that be needed
This should save a list object per MergedField object when there is not multiple fields present