Skip to content

1% possible memory and cpu improvements #3939

Open
@bbakerman

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% ??

Metadata

Metadata

Assignees

No one assigned

    Labels

    performancework 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