Improve performance of subtype checks (#860)

Author: stayallive

benchmarks/StarWarsBench.php

@@ -91,6 +91,29 @@ public function benchQueryWithFragment(): void
         );
     }
 
+    public function benchQueryWithInterfaceFragment(): void
+    {
+        $q = '
+        query UseInterfaceFragment {
+          luke: human(id: "1000") {
+            ...CharacterFragment
+          }
+          leia: human(id: "1003") {
+            ...CharacterFragment
+          }
+        }
+
+        fragment CharacterFragment on Character {
+          name
+        }
+        ';
+
+        GraphQL::executeQuery(
+            StarWarsSchema::build(),
+            $q
+        );
+    }
+
     public function benchStarWarsIntrospectionQuery(): void
     {
         GraphQL::executeQuery(

benchmarks/shim.php

@@ -25,4 +25,5 @@
 $b->benchHeroQuery();
 $b->benchNestedQuery();
 $b->benchQueryWithFragment();
+$b->benchQueryWithInterfaceFragment();
 $b->benchStarWarsIntrospectionQuery();

src/Type/Schema.php

@@ -20,9 +20,11 @@
 use GraphQL\Utils\InterfaceImplementations;
 use GraphQL\Utils\TypeInfo;
 use GraphQL\Utils\Utils;
+use InvalidArgumentException;
 use Traversable;
 
 use function array_map;
+use function get_class;
 use function implode;
 use function is_array;
 use function is_callable;
@@ -60,13 +62,6 @@ class Schema
      */
     private $resolvedTypes = [];
 
-    /**
-     * Lazily initialized.
-     *
-     * @var array<string, array<string, ObjectType|UnionType>>
-     */
-    private $subTypeMap;
-
     /**
      * Lazily initialised.
      *
@@ -524,26 +519,15 @@ public function isPossibleType(AbstractType $abstractType, ObjectType $possibleT
      */
     public function isSubType(AbstractType $abstractType, ImplementingType $maybeSubType): bool
     {
-        if (! isset($this->subTypeMap[$abstractType->name])) {
-            $this->subTypeMap[$abstractType->name] = [];
-
-            if ($abstractType instanceof UnionType) {
-                foreach ($abstractType->getTypes() as $type) {
-                    $this->subTypeMap[$abstractType->name][$type->name] = true;
-                }
-            } else {
-                $implementations = $this->getImplementations($abstractType);
-                foreach ($implementations->objects() as $type) {
-                    $this->subTypeMap[$abstractType->name][$type->name] = true;
-                }
+        if ($abstractType instanceof InterfaceType) {
+            return $maybeSubType->implementsInterface($abstractType);
+        }
 
-                foreach ($implementations->interfaces() as $type) {
-                    $this->subTypeMap[$abstractType->name][$type->name] = true;
-                }
-            }
+        if ($abstractType instanceof UnionType) {
+            return $abstractType->isPossibleType($maybeSubType);
         }
 
-        return isset($this->subTypeMap[$abstractType->name][$maybeSubType->name]);
+        throw new InvalidArgumentException(sprintf('$abstractType must be of type UnionType|InterfaceType got: %s.', get_class($abstractType)));
     }
 
     /**

tests/Type/LazyTypeLoaderTest.php

@@ -296,11 +296,6 @@ public function testWorksWithTypeLoader(): void
                 'Node',
                 'Content',
                 'PostStoryMutationInput',
-                'Query.fields',
-                'Content.fields',
-                'Node.fields',
-                'Mutation.fields',
-                'BlogStory.fields',
             ],
             $this->calls
         );

tests/Type/SchemaTest.php

@@ -5,12 +5,15 @@
 namespace GraphQL\Tests\Type;
 
 use GraphQL\Error\InvariantViolation;
+use GraphQL\Type\Definition\AbstractType;
 use GraphQL\Type\Definition\Directive;
 use GraphQL\Type\Definition\InputObjectType;
 use GraphQL\Type\Definition\InterfaceType;
 use GraphQL\Type\Definition\ObjectType;
+use GraphQL\Type\Definition\ResolveInfo;
 use GraphQL\Type\Definition\Type;
 use GraphQL\Type\Schema;
+use InvalidArgumentException;
 use PHPUnit\Framework\TestCase;
 
 class SchemaTest extends TestCase
@@ -130,4 +133,26 @@ public function testIncludesInputTypesOnlyUsedInDirectives(): void
         self::assertArrayHasKey('DirInput', $typeMap);
         self::assertArrayHasKey('WrappedDirInput', $typeMap);
     }
+
+    // Sub Type
+
+    /**
+     * @see it('validates argument to isSubType to be of the correct type')
+     */
+    public function testThrowsInvalidArgumentExceptionWhenInvalidTypeIsPassedToIsSubType(): void
+    {
+        $this->expectException(InvalidArgumentException::class);
+
+        $anonymousAbstractType = new class implements AbstractType {
+            public function resolveType($objectValue, $context, ResolveInfo $info)
+            {
+                return null;
+            }
+        };
+
+        $this->schema->isSubType(
+            $anonymousAbstractType,
+            new InterfaceType(['name' => 'Interface'])
+        );
+    }
 }

tests/Type/TypeLoaderTest.php

@@ -244,11 +244,6 @@ public function testWorksWithTypeLoader(): void
                 'Node',
                 'Content',
                 'PostStoryMutationInput',
-                'Query.fields',
-                'Content.fields',
-                'Node.fields',
-                'Mutation.fields',
-                'BlogStory.fields',
             ],
             $this->calls
         );