Improved access speed of isPossibleType #4040

bbakerman · 2025-07-04T09:59:36Z

This builds on #4033 but it creates a cache in the ImmutableTypeRegistry so that we don't need to iterate all types on each call

@xuorig - This takes your PR and extends it more to have a reverse cache of interface -> implementing type def AND it uses your cheaper TypeInfo method in more places

This means that algo change you made in your PR is not really needed. Why? Because when we are in read only mode, we don't have to loop all types to find all implemeenting types of an interface, we did that once at build time for all interfaces. So later the type checking will be faster

I originally had a cache of interface to implementing types but it turns out its slower to eagerly build this cache than it is to run the code that used it so I brought @xuorig changes into this PR

So in benchmark terms

c7495f921d2bd10afcb4db4616817a4c67de5ddb - master back before we started @xuorig changes

Benchmark                                                         Mode  Cnt      Score      Error    Units
CreateExtendedSchemaBenchmark.benchmarkLargeSchemaCreate         thrpt    9      4.979 ±    1.058  ops/min
CreateExtendedSchemaBenchmark.benchmarkLargeSchemaCreateAvgTime   avgt    9  11205.547 ± 1306.952    ms/op


This PR

Benchmark                                                         Mode  Cnt     Score    Error    Units
CreateExtendedSchemaBenchmark.benchmarkLargeSchemaCreate         thrpt    9    46.436 ±  2.512  ops/min
CreateExtendedSchemaBenchmark.benchmarkLargeSchemaCreateAvgTime   avgt    9  1250.350 ± 26.185    ms/op


master without this PR

Benchmark                                                         Mode  Cnt     Score    Error    Units
CreateExtendedSchemaBenchmark.benchmarkLargeSchemaCreate         thrpt    9    45.801 ±  2.211  ops/min
CreateExtendedSchemaBenchmark.benchmarkLargeSchemaCreateAvgTime   avgt    9  1253.292 ± 58.405    ms/op

So overall things have gotten significantly faster in building large schemas with lots of interfaces and extends. in fact since @xuorig started his investigations and inspired changes, we are 10x faster on this benchmark

This PR does not add much more raw throughput BUT it will reduce memory used

github-actions · 2025-07-04T10:11:47Z

Test Results

323 files 323 suites 2m 53s ⏱️
4 942 tests 4 932 ✅ 10 💤 0 ❌
5 031 runs 5 021 ✅ 10 💤 0 ❌

Results for commit d0cbc63.

♻️ This comment has been updated with latest results.

bbakerman · 2025-07-04T23:34:08Z

src/main/java/graphql/schema/idl/ImmutableTypeDefinitionRegistry.java

+            }
+        }
+        return mapBuilder.build();
+    }


This now pre-computes the interface -> ImplementingTypeDefinition s at registry build time and hence later in the schema type checking stage, the repeated ask for interface information is much faster

bbakerman · 2025-07-04T23:35:43Z

src/main/java/graphql/schema/idl/TypeDefinitionRegistry.java

+     *
+     * @return true if the registry has a type by that name
+     */
+    public boolean hasType(String name) {
        return types.containsKey(name) || ScalarInfo.GRAPHQL_SPECIFICATION_SCALARS_DEFINITIONS.containsKey(name) || scalarTypes.containsKey(name) || objectTypeExtensions.containsKey(name);
    }


extra helper by string name - this will be lighter in lookup terms later on

bbakerman · 2025-07-04T23:37:16Z

src/main/java/graphql/schema/idl/TypeDefinitionRegistry.java

    public Optional<TypeDefinition> getType(Type type) {
-        String typeName = TypeInfo.typeInfo(type).getName();
-        return getType(typeName);
+        return getType(typeName(type));


TypeInfo.typeInfo(type).getName() allocates a TypeInfo object in order to just get a single attribute of it - the name of the type (and not its null wrapping etc...)

So this is more light weight and we avoid an extra allocation

This is then repeated in all the getType lookup methods

bbakerman · 2025-07-04T23:39:28Z

src/main/java/graphql/schema/idl/TypeDefinitionRegistry.java

    public Optional<TypeDefinition> getType(String typeName) {
+        return Optional.ofNullable(getTypeOrNull(typeName));


The older Optional returning methods are left in place but the new getTypeOrNull are more efficient - not need for a Optional allocation in order to know if something is present or not

So all the other inner code that used Optional<T> getType now used T getTypeOrNull variants

bbakerman · 2025-07-04T23:39:57Z

src/main/java/graphql/schema/idl/TypeDefinitionRegistry.java

        }
-        return Optional.empty();
+        return null;


This is cheaper - no Optional allocated

bbakerman · 2025-07-04T23:40:30Z

src/main/java/graphql/schema/idl/TypeDefinitionRegistry.java

-            return definition instanceof UnionTypeDefinition || definition instanceof InterfaceTypeDefinition;
+        TypeDefinition typeDefinition = getTypeOrNull(type);
+        if (typeDefinition != null) {
+            return typeDefinition instanceof UnionTypeDefinition || typeDefinition instanceof InterfaceTypeDefinition;


See now we use the cheaper getTypeOrNull methods internally

bbakerman · 2025-07-04T23:40:38Z

src/main/java/graphql/schema/idl/TypeDefinitionRegistry.java

-            return definition instanceof ObjectTypeDefinition || definition instanceof InterfaceTypeDefinition;
+        TypeDefinition typeDefinition = getTypeOrNull(type);
+        if (typeDefinition != null) {
+            return typeDefinition instanceof ObjectTypeDefinition || typeDefinition instanceof InterfaceTypeDefinition;


bbakerman · 2025-07-04T23:40:48Z

src/main/java/graphql/schema/idl/TypeDefinitionRegistry.java

@@ -569,7 +665,7 @@ public boolean isObjectTypeOrInterface(Type type) {
     * @return true if its an object type
     */
    public boolean isObjectType(Type type) {
-        return getType(type, ObjectTypeDefinition.class).isPresent();
+        return getTypeOrNull(type, ObjectTypeDefinition.class) != null;


bbakerman · 2025-07-04T23:44:32Z

src/main/java/graphql/schema/idl/TypeDefinitionRegistry.java

+                getAllImplementationsOf(targetInterface),
+                typeDefinition -> typeDefinition instanceof ObjectTypeDefinition,
+                typeDefinition -> (ObjectTypeDefinition) typeDefinition
+        );


unwound the .stream() and replaced it with a cheaper immutable builder version. This is known to be faster

bbakerman · 2025-07-04T23:44:56Z

src/main/java/graphql/schema/idl/TypeDefinitionRegistry.java

-        TypeDefinition targetObjectTypeDef = getType(possibleType).get();
-        TypeDefinition abstractTypeDef = getType(abstractType).get();
+        TypeDefinition targetObjectTypeDef = Objects.requireNonNull(getTypeOrNull(possibleType));
+        TypeDefinition abstractTypeDef = Objects.requireNonNull(getTypeOrNull(abstractType));


required for nullaway now that I marked a few things

bbakerman · 2025-07-04T23:45:10Z

src/main/java/graphql/schema/idl/TypeDefinitionRegistry.java

-                if (checkType.isPresent()) {
-                    if (checkType.get().getName().equals(targetObjectTypeDef.getName())) {
+                ObjectTypeDefinition checkType = getTypeOrNull(memberType, ObjectTypeDefinition.class);
+                if (checkType != null) {


bbakerman · 2025-07-04T23:46:40Z

src/main/java/graphql/schema/idl/TypeInfo.java

+            }
+        }
+        return (TypeName) type;
+    }


This is take from @xuorig s code in his PR.

This is cheaper than allocating a TypeInfo object just to get its name

bbakerman · 2025-07-04T23:47:30Z

src/main/java/graphql/schema/idl/TypeInfo.java

+     */
+    public static String typeName(Type<?> type) {
+        return getTypeName(type).getName();
+    }


a version that is just the string of the TypeName - easier to use in map lookups

bbakerman · 2025-07-04T23:48:39Z

src/test/groovy/graphql/schema/idl/ImmutableTypeDefinitionRegistryTest.groovy

+        allImplementationsOf.collect({ it.getName() }).every { it.startsWith("DT") }
+
+        implementationsOf.size() == 5
+        implementationsOf.collect({ it.getName() }).every { it.startsWith("DT") }


This is just to make sure we did what we thought we would do and I can debug and confirm the pre-built map is as expected and its going to be cheaper when a full schema build is done and repeated calls are made on it to check interfaces and types

However I killed the cache code but I figured this code is still valid

bbakerman · 2025-07-04T23:49:00Z

src/test/groovy/graphql/schema/idl/SchemaTypeCheckerTest.groovy

@@ -32,7 +32,7 @@ import static java.lang.String.format
 class SchemaTypeCheckerTest extends Specification {

    static TypeDefinitionRegistry parseSDL(String spec) {
-        new SchemaParser().parse(spec)
+        new SchemaParser().parse(spec).readOnly()


This is more real - in the production code the registry is immutable read only mode

bbakerman · 2025-07-04T23:49:22Z

src/test/groovy/graphql/schema/idl/TypeDefinitionRegistryTest.groovy

+        where:
+        typeOfReg   | _
+        "mutable"   | _
+        "immutable" | _


test both variations work as expected

bbakerman · 2025-07-04T23:49:33Z

src/test/groovy/graphql/schema/idl/TypeDefinitionRegistryTest.groovy

+        where:
+        typeOfReg   | _
+        "mutable"   | _
+        "immutable" | _


test both variations work as expected

bbakerman · 2025-07-04T23:49:41Z

src/test/groovy/graphql/schema/idl/TypeDefinitionRegistryTest.groovy

+        where:
+        typeOfReg   | _
+        "mutable"   | _
+        "immutable" | _


test both variations work as expected

…provements

xuorig · 2025-07-05T14:19:57Z

.I would have thought the schema checks hit every interface but perhaps not. I need to look into this more

Yeah not quite, at least not for this particular issue. isPossibleType is mostly called from isSubType which is used to check that each field on interface / matching concrete object type are compatible types. So depending on the benchmark, could vary lot. Need lots of abstract type fields on interfaces to see an effect.

bbakerman · 2025-07-06T03:58:00Z

bin/jmh.sh

+
+./gradlew clean jmhJar
+
+java -jar "$JAR" "$@"


A little helper to make it easier to run the JMH tests on the command line

bbakerman · 2025-07-06T03:58:44Z

src/jmh/java/benchmark/CreateExtendedSchemaBenchmark.java

+        f1 : String
+    }
+     */
+    private static String mkSDL() {


a benchmark with lots of extends type X and interfaces

bbakerman · 2025-07-06T03:59:51Z

src/main/java/graphql/schema/idl/SchemaTypeChecker.java

-            if (!typeRegistry.hasType(unwrapped)) {
+            String name = TypeInfo.typeName(t);
+            if (!typeRegistry.hasType(name)) {
+                TypeName unwrapped = TypeInfo.typeInfo(t).getTypeName();


faster look up - we only create a TypeInfo if its in error

bbakerman · 2025-07-06T04:00:01Z

src/main/java/graphql/schema/idl/SchemaTypeChecker.java

-            if (!typeRegistry.hasType(unwrapped)) {
+            String name = TypeInfo.typeName(ivType);
+            if (!typeRegistry.hasType(name)) {
+                TypeName unwrapped = TypeInfo.typeInfo(ivType).getTypeName();


faster look up - we only create a TypeInfo if its in error

bbakerman · 2025-07-06T04:01:18Z

src/main/java/graphql/schema/idl/TypeDefinitionRegistry.java

+                            if (interfaceTypeDef.getName().equals(targetInterface.getName())) {
+                                return true;
+                            }
+                        }


moved away from .stream() to the faster immutable code

bbakerman · 2025-07-06T04:02:09Z

src/main/java/graphql/schema/idl/TypeDefinitionRegistry.java

+                    }
+                }
+            }
+            return false;


This is @xuorig s code from his PR in this place

bbakerman · 2025-07-06T04:03:18Z

src/test/groovy/graphql/schema/idl/TypeInfoTest.groovy

+        "[named!]"    | "named"
+        "[named!]!"   | "named"
+        "[[named!]!]" | "named"
+    }


from @xuorig PR

dondonz · 2025-07-06T21:00:07Z

Wow the benchmark improvement is super impressive. Thanks so much @xuorig !

andimarek · 2025-07-07T06:59:35Z

src/main/java/graphql/schema/idl/TypeDefinitionRegistry.java

+     *
+     * @return an optional {@link TypeDefinition} or empty if it's not found
+     */
+    public <T extends TypeDefinition> Optional<T> getType(String typeName, Class<T> ofType) {


Just return null instead using Optional? Saves also an object allocation

Ah .. I see you have it actually below ... how about just killing this method then?

how about just killing this method then?

I will deprecate them but I think they are still used in our code

They are all now deprecated

bbakerman · 2025-07-07T23:06:49Z

src/main/java/graphql/schema/idl/ImplementingTypesChecker.java

@@ -29,7 +29,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.Optional;
+import java.util.Objects;


Getting rid of the Optional methods and using the getOrNull ones

bbakerman · 2025-07-07T23:08:15Z

src/main/java/graphql/schema/idl/TypeDefinitionRegistry.java

    public Optional<TypeDefinition> getType(Type type) {
-        String typeName = TypeInfo.typeInfo(type).getName();
-        return getType(typeName);
+        return Optional.ofNullable(getTypeOrNull(type));
    }


Now deprecated - getTypeOrNull preferred

Improved access speed of isPossibleType

eb7f62e

bbakerman added this to the 25.x breaking changes milestone Jul 4, 2025

bbakerman added 2 commits July 5, 2025 09:19

Improved access speed of isPossibleType - added javadoc

88057b0

Improved access speed of isPossibleType - added javadoc and code tweaks

04cbbd6

bbakerman commented Jul 4, 2025

View reviewed changes

Improved access speed of isPossibleType - less code same cost

eaf6e40

bbakerman commented Jul 4, 2025

View reviewed changes

bbakerman mentioned this pull request Jul 4, 2025

Iterate over implementations in isPossibleType #4033

Closed

Merge remote-tracking branch 'origin/master' into is-possible-type-im…

e08fc0a

…provements

bbakerman added 2 commits July 6, 2025 13:06

Moved back to Marc's implementation instead of precaching

3e82b74

Added benchmark and bin helper

21ac5c9

bbakerman commented Jul 6, 2025

View reviewed changes

andimarek reviewed Jul 7, 2025

View reviewed changes

deprecated the Optional<T> getType methods and replaced all their usages

d0cbc63

bbakerman commented Jul 7, 2025

View reviewed changes

bbakerman requested a review from andimarek July 7, 2025 23:10

dondonz merged commit ecaebb1 into master Jul 10, 2025
2 checks passed

dondonz deleted the is-possible-type-improvements branch July 10, 2025 00:42

		public Optional<TypeDefinition> getType(String typeName) {
		return Optional.ofNullable(getTypeOrNull(typeName));

Improved access speed of isPossibleType #4040

Improved access speed of isPossibleType #4040

Uh oh!

Conversation

bbakerman commented Jul 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

Test Results

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bbakerman Jul 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bbakerman Jul 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xuorig commented Jul 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dondonz commented Jul 6, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bbakerman commented Jul 4, 2025 •

edited

Loading

github-actions bot commented Jul 4, 2025 •

edited

Loading

bbakerman Jul 4, 2025 •

edited

Loading

bbakerman Jul 4, 2025 •

edited

Loading

xuorig commented Jul 5, 2025 •

edited

Loading