Stop creating NonNullableFieldValidator every for every object or list field by bbakerman · Pull Request #3929 · graphql-java/graphql-java

bbakerman · 2025-04-24T02:46:35Z

Today we create a NonNullableFieldValidator for every list item and every object we encounter. We do this because the constructor took a ExecutionStepInfo which IF there was a problem was used say what field was in error

BUT we dont need this. It is passed into the validate method via the ExecutionStrategyParameters anyway

So we can create 1 at execution time and use it through out

This save nemory allocation for cases where you have lots of object fields and / or lost of list fields.

This is NOT a breaking change - NonNullableFieldValidator is a internal class and this only changes the constructor shape

…t field

bbakerman · 2025-04-24T02:47:22Z

src/main/java/graphql/execution/Execution.java

        ResultPath path = ResultPath.rootPath();
        ExecutionStepInfo executionStepInfo = newExecutionStepInfo().type(operationRootType).path(path).build();
-        NonNullableFieldValidator nonNullableFieldValidator = new NonNullableFieldValidator(executionContext, executionStepInfo);
+        NonNullableFieldValidator nonNullableFieldValidator = new NonNullableFieldValidator(executionContext);


No need for the executionStepInfo here. Its passed in during validate

So this instance gets used for the entirety of the operation

bbakerman · 2025-04-24T02:47:34Z

src/main/java/graphql/execution/ExecutionStrategy.java

                instrumentationParams, executionContext.getInstrumentationState()
        ));

-        NonNullableFieldValidator nonNullableFieldValidator = new NonNullableFieldValidator(executionContext, executionStepInfo);


no need to recreate it

bbakerman · 2025-04-24T02:48:03Z

src/main/java/graphql/execution/ExecutionStrategy.java

-        NonNullableFieldValidator nonNullableFieldValidator = new NonNullableFieldValidator(executionContext, executionStepInfo);
-
        ExecutionStrategyParameters newParameters = parameters.transform(builder ->
                builder.executionStepInfo(executionStepInfo)


see - executionStepInfo is in the ExecutionStrategyParameters always

bbakerman · 2025-04-24T02:48:18Z

src/main/java/graphql/execution/ExecutionStrategy.java

            ExecutionStepInfo stepInfoForListElement = executionStepInfoFactory.newExecutionStepInfoForListElement(executionStepInfo, indexedPath);

-            NonNullableFieldValidator nonNullableFieldValidator = new NonNullableFieldValidator(executionContext, stepInfoForListElement);
-


No need - same reason

bbakerman · 2025-04-24T02:48:50Z

src/main/java/graphql/execution/ExecutionStrategyParameters.java

        this.fields = assertNotNull(fields, () -> "fields is null");
        this.source = source;
-        this.nonNullableFieldValidator = nonNullableFieldValidator;
+        this.nonNullableFieldValidator = assertNotNull(nonNullableFieldValidator, () -> "requires a NonNullValidator");;


Just to be more specific - this helped find tests that "half create" objects for testing reasons

bbakerman · 2025-04-24T02:49:44Z

src/main/java/graphql/execution/SubscriptionExecutionStrategy.java

-        return parameters.transform(builder -> builder.field(firstField).path(fieldPath));
+        NonNullableFieldValidator nonNullableFieldValidator = new NonNullableFieldValidator(executionContext);
+        return parameters.transform(builder ->
+                builder.field(firstField).path(fieldPath).nonNullFieldValidator(nonNullableFieldValidator));


Subscriptions is a special case - because it starts a new ExecutionContext instance - So we do re-create it here - but only once - not per published message

bbakerman · 2025-04-24T02:50:19Z

src/test/groovy/graphql/execution/AsyncExecutionStrategyTest.groovy

                .newParameters()
                .executionStepInfo(typeInfo)
                .fields(mergedSelectionSet(['hello': mergedField([Field.newField('hello').build()]), 'hello2': mergedField([Field.newField('hello2').build()])]))
+                .nonNullFieldValidator(new NonNullableFieldValidator(executionContext))


The rest of the changes are test fixups where they half create support objects with just enough brains to work

github-actions · 2025-04-24T02:50:27Z

Test Results

314 files ±0 314 suites ±0 52s ⏱️ -2s
3 610 tests ±0 3 605 ✅ ±0 5 💤 ±0 0 ❌ ±0
3 699 runs ±0 3 694 ✅ ±0 5 💤 ±0 0 ❌ ±0

Results for commit 80f4a3b. ± Comparison against base commit 397c050.

This pull request removes 174 and adds 152 tests. Note that renamed tests count towards both.

	?
                __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 }
                a2 :  __type(name : "t1") { name }
…

graphql.DataFetcherTest ‑ get Boolean property value [fetcher: <graphql.schema.PropertyDataFetcher@7f1ef916 propertyName=booleanField function=null>, #0]
graphql.DataFetcherTest ‑ get Boolean property value [fetcher: <graphql.schema.SingletonPropertyDataFetcher@5c441290>, #1]
graphql.DataFetcherTest ‑ get Boolean property value with get [fetcher: <graphql.schema.PropertyDataFetcher@17dd671f propertyName=booleanFieldWithGet function=null>, #0]
graphql.DataFetcherTest ‑ get Boolean property value with get [fetcher: <graphql.schema.SingletonPropertyDataFetcher@5c441290>, #1]
graphql.DataFetcherTest ‑ get property value [fetcher: <graphql.schema.PropertyDataFetcher@47c40b56 propertyName=property function=null>, #0]
graphql.DataFetcherTest ‑ get property value [fetcher: <graphql.schema.SingletonPropertyDataFetcher@5c441290>, #1]
graphql.DataFetcherTest ‑ get public field value as property [fetcher: <graphql.schema.PropertyDataFetcher@9b9a327 propertyName=publicField function=null>, #0]
graphql.DataFetcherTest ‑ get public field value as property [fetcher: <graphql.schema.SingletonPropertyDataFetcher@5c441290>, #1]
graphql.ScalarsBooleanTest ‑ parseValue throws exception for invalid input <java.lang.Object@3f17bc1e>
graphql.ScalarsBooleanTest ‑ serialize throws exception for invalid input <java.lang.Object@1f9b614d>
…

♻️ This comment has been updated with latest results.

…ullableFieldValidator-all-the-time # Conflicts: # src/main/java/graphql/execution/ExecutionStrategy.java # src/main/java/graphql/execution/SubscriptionExecutionStrategy.java

Stop creating NonNullableFieldValidator every for every object or lis…

d472058

…t field

bbakerman added this to the 23.1 non-breaking changes and bugfixes milestone Apr 24, 2025

bbakerman commented Apr 24, 2025

View reviewed changes

bbakerman added the performance work that is primarily targeted as performance improvements label Apr 24, 2025

bbakerman requested review from andimarek and dondonz April 27, 2025 06:31

bbakerman mentioned this pull request Apr 29, 2025

1% possible memory and cpu improvements #3939

Open

dondonz modified the milestones: 25.x breaking changes, 24.0 May 13, 2025

dondonz approved these changes May 14, 2025

View reviewed changes

bbakerman added 2 commits May 15, 2025 15:04

Merge remote-tracking branch 'origin/master' into dont-alllocate-NonN…

3685647

…ullableFieldValidator-all-the-time # Conflicts: # src/main/java/graphql/execution/ExecutionStrategy.java # src/main/java/graphql/execution/SubscriptionExecutionStrategy.java

Merged in master

80f4a3b

dondonz merged commit abb2cdd into master May 15, 2025
2 checks passed

dondonz mentioned this pull request May 15, 2025

Cherry pick PR 3929 reduce nonnullablefieldvalidator allocations #3974

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Stop creating NonNullableFieldValidator every for every object or list field#3929

Stop creating NonNullableFieldValidator every for every object or list field#3929
dondonz merged 3 commits intomasterfrom
dont-alllocate-NonNullableFieldValidator-all-the-time

bbakerman commented Apr 24, 2025

Uh oh!

bbakerman Apr 24, 2025

Uh oh!

bbakerman Apr 24, 2025

Uh oh!

bbakerman Apr 24, 2025

Uh oh!

bbakerman Apr 24, 2025

Uh oh!

bbakerman Apr 24, 2025

Uh oh!

bbakerman Apr 24, 2025

Uh oh!

bbakerman Apr 24, 2025

Uh oh!

github-actions bot commented Apr 24, 2025 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		ExecutionStepInfo stepInfoForListElement = executionStepInfoFactory.newExecutionStepInfoForListElement(executionStepInfo, indexedPath);

		NonNullableFieldValidator nonNullableFieldValidator = new NonNullableFieldValidator(executionContext, stepInfoForListElement);

Conversation

bbakerman commented Apr 24, 2025

Uh oh!

bbakerman Apr 24, 2025

Choose a reason for hiding this comment

Uh oh!

bbakerman Apr 24, 2025

Choose a reason for hiding this comment

Uh oh!

bbakerman Apr 24, 2025

Choose a reason for hiding this comment

Uh oh!

bbakerman Apr 24, 2025

Choose a reason for hiding this comment

Uh oh!

bbakerman Apr 24, 2025

Choose a reason for hiding this comment

Uh oh!

bbakerman Apr 24, 2025

Choose a reason for hiding this comment

Uh oh!

bbakerman Apr 24, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions bot commented Apr 24, 2025 •

edited

Loading