← 输入框|github.com|刷新
Skip to content

1% possible memory and cpu improvements #3939

Open

Description

@bbakerman

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

Image

"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)

#3924
#3929
#3934
#3935

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

Image

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

  1. added
    performancework that is primarily targeted as performance improvements
    on Apr 29, 2025
  2. bbakerman commented on Apr 29, 2025

    @bbakerman
    MemberAuthor

    Findings

    • "graphql.util.IntraThreadMemoizedSupplier", 4%
      ** 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.

    • "com.google.common.collect.SingletonImmutableList", 2%
      ** why so high?? Are we wrapping something for no reason

    This is from graphql.execution.MergedField#newSingletonMergedField where it runs this code

        private MergedField(Field field, DeferredExecution deferredExecution) {
            this(ImmutableList.of(field), deferredExecution == null ? ImmutableList.of() : ImmutableList.of(deferredExecution));
        }
    

    We 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

  3. github-actions commented on Jun 29, 2025

    @github-actions
    Contributor

    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.

  4. added
    keep-openTells Stale Bot to keep PRs and issues open
    on Jul 6, 2025
  5. dfa1 commented on Jul 21, 2025

    @dfa1
    Contributor

    @bbakerman hello 😄

    The MergedField part is interesting:

    • in most cases I profiled in the past, the field had always 1 field: perhaps it helps to have a "single" field merged field concrete class?
    • this is allocated a lot by the field collector class, especially for a list with thousands of objects inside: if there are no directives that could change the resulting merged field, perhaps they could be reused?

    My 2 cents!

  6. bbakerman commented on Jul 23, 2025

    @bbakerman
    MemberAuthor

    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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    keep-openTells Stale Bot to keep PRs and issues openperformancework that is primarily targeted as performance improvements

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions