A more memory efficient directives container by bbakerman · Pull Request #4027 · graphql-java/graphql-java

bbakerman · 2025-06-28T06:50:57Z

When we wrote the graphql.language.DirectivesContainer we put default methods on it to save code in each language element that implemented it

But because interfaces cant have state, then it we doing a per "group by" etc... when each call was made.

This introduces a DirectivesHolder helper class (like we do in the runtime via graphql.DirectivesUtil.DirectivesHolder) that holds the immutable state and helps each implementing class implement the DirectivesContainer methods

So while there is more code in each, it more efficient than the default interface methods

bbakerman · 2025-06-28T06:53:43Z

src/main/java/graphql/language/DirectivesContainer.java

     * @return a map of all directives by directive name
     */
-    default Map<String, List<Directive>> getDirectivesByName() {
-        return ImmutableMap.copyOf(allDirectivesByName(getDirectives()));


This was terribly memory inefficient

bbakerman · 2025-06-28T06:53:58Z

src/main/java/graphql/language/DirectivesContainer.java

     * @return the directives or empty list if there is not one with that name
     */
-    default List<Directive> getDirectives(String directiveName) {
-        return getDirectivesByName().getOrDefault(directiveName, emptyList());


This was terribly memory inefficient

bbakerman · 2025-06-28T06:54:07Z

src/main/java/graphql/language/DirectivesContainer.java

     * @return true if the AST node contains one or more directives by the specified name
     */
-    default boolean hasDirective(String directiveName) {
-        return !getDirectives(directiveName).isEmpty();


This was terribly memory inefficient

bbakerman · 2025-06-28T06:54:42Z

src/main/java/graphql/language/DirectivesContainer.java

@@ -34,20 +34,16 @@ public interface DirectivesContainer<T extends DirectivesContainer> extends Node
     *


We no longer have default methods - but rather a memory efficient DirectivesHolder helper

bbakerman · 2025-06-28T06:55:12Z

src/main/java/graphql/language/NodeUtil.java

        NodeChildrenContainer newChildren = namedChildren.transform(builder -> builder.removeChild(childLocationToRemove.getName(), childLocationToRemove.getIndex()));
        return node.withNewChildren(newChildren);
    }
+


Our new helper thats immutable and more memory efficient

github-actions · 2025-06-28T07:02:40Z

Test Results

318 files - 632 318 suites - 632 2m 47s ⏱️ - 5m 38s
4 884 tests - 312 4 874 ✅ - 313 10 💤 + 1 0 ❌ ±0
4 973 runs - 9 940 4 963 ✅ - 9 921 10 💤 - 19 0 ❌ ±0

Results for commit dcc6a93. ± Comparison against base commit aef8032.

This pull request removes 480 and adds 146 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 property value [fetcher: <graphql.schema.PropertyDataFetcher@7c781c42 propertyName=property function=null>, #0]
graphql.ScalarsBooleanTest ‑ parseValue throws exception for invalid input <java.lang.Object@4c4ee4bb>
graphql.ScalarsBooleanTest ‑ serialize throws exception for invalid input <java.lang.Object@5a86d950>
graphql.ScalarsIDTest ‑ parseValue allows any object via String.valueOf <java.lang.Object@36cb7f37>
graphql.ScalarsIDTest ‑ serialize allows any object via String.valueOf <java.lang.Object@2de99f4c>
graphql.ScalarsIntTest ‑ parseValue throws exception for invalid input <java.lang.Object@5f6220>
graphql.ScalarsIntTest ‑ serialize throws exception for invalid input <java.lang.Object@30a923bc>
graphql.analysis.QueryTraverserTest ‑ builder doesn't allow null arguments [document: Document{definitions=[OperationDefinition{name='null', operation=QUERY, variableDefinitions=[], directives=graphql.language.NodeUtil$DirectivesHolder@3db946e9, selectionSet=SelectionSet{selections=[Field{name='foo', alias='null', arguments=[], directives=graphql.language.NodeUtil$DirectivesHolder@6abc09e9, selectionSet=null}]}}]}, operationName: foo, root: Field{name='null', alias='null', arguments=[], directives=graphql.language.NodeUtil$DirectivesHolder@73445b5b, selectionSet=null}, rootParentType: null, fragmentsByName: [:], #3]
graphql.analysis.QueryTraverserTest ‑ builder doesn't allow null arguments [document: Document{definitions=[OperationDefinition{name='null', operation=QUERY, variableDefinitions=[], directives=graphql.language.NodeUtil$DirectivesHolder@5aa68b8d, selectionSet=SelectionSet{selections=[Field{name='foo', alias='null', arguments=[], directives=graphql.language.NodeUtil$DirectivesHolder@7fbb901f, selectionSet=null}]}}]}, operationName: foo, root: Field{name='null', alias='null', arguments=[], directives=graphql.language.NodeUtil$DirectivesHolder@27e7613f, selectionSet=null}, rootParentType: null, fragmentsByName: null, #1]
graphql.analysis.QueryTraverserTest ‑ builder doesn't allow null arguments [document: Document{definitions=[OperationDefinition{name='null', operation=QUERY, variableDefinitions=[], directives=graphql.language.NodeUtil$DirectivesHolder@65c1b630, selectionSet=SelectionSet{selections=[Field{name='foo', alias='null', arguments=[], directives=graphql.language.NodeUtil$DirectivesHolder@4af90e56, selectionSet=null}]}}]}, operationName: null, root: Field{name='null', alias='null', arguments=[], directives=graphql.language.NodeUtil$DirectivesHolder@28c0703c, selectionSet=null}, rootParentType: null, fragmentsByName: null, #0]
…

This pull request skips 1 test.

graphql.schema.fetching.LambdaFetchingSupportTest ‑ different class loaders induce certain behaviours

xuorig

👌 perfect

A more memory efficient directives container

dcc6a93

bbakerman added this to the 25.x breaking changes milestone Jun 28, 2025

bbakerman mentioned this pull request Jun 28, 2025

avoid allocating in hasDirective #4026

Closed

bbakerman commented Jun 28, 2025

View reviewed changes

xuorig approved these changes Jun 28, 2025

View reviewed changes

bbakerman merged commit 309fa9d into master Jul 4, 2025
2 checks passed

invliD mentioned this pull request Dec 9, 2025

DirectivesHolder does not implement toString #4190

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

A more memory efficient directives container#4027

A more memory efficient directives container#4027
bbakerman merged 1 commit intomasterfrom
immutable-language-directive-containers

bbakerman commented Jun 28, 2025

Uh oh!

bbakerman Jun 28, 2025

Uh oh!

bbakerman Jun 28, 2025

Uh oh!

bbakerman Jun 28, 2025

Uh oh!

bbakerman Jun 28, 2025

Uh oh!

bbakerman Jun 28, 2025

Uh oh!

github-actions bot commented Jun 28, 2025

Uh oh!

xuorig left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -34,20 +34,16 @@ public interface DirectivesContainer<T extends DirectivesContainer> extends Node
		*

Conversation

bbakerman commented Jun 28, 2025

Uh oh!

bbakerman Jun 28, 2025

Choose a reason for hiding this comment

Uh oh!

bbakerman Jun 28, 2025

Choose a reason for hiding this comment

Uh oh!

bbakerman Jun 28, 2025

Choose a reason for hiding this comment

Uh oh!

bbakerman Jun 28, 2025

Choose a reason for hiding this comment

Uh oh!

bbakerman Jun 28, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jun 28, 2025

Test Results

Uh oh!

xuorig left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants