Performance optimizations and caching (#698)

* Combine addTypeNamespace() and addControllerNamespace() into addNamespace()

* Refactor CachedDocBlockFactory to use existing caching infrastructure

* Refactor field name

* Refactor code to use ClassFinder instead of separating namespaces

* Make caches only recompute on file changes

* Refactor EnumTypeMapper to use cached doc block factory

* Fix FileModificationClassFinderBoundCache

* Use Kcs ClassFinder cache

* One more tiny optimization for unchanged cache

* Change name of ClassFinderBoundCache

* Code style

* PHPStan

* Some fixes and renames

* Fix broken class bound cache after merge

* Fix some tests and PHPStan

* Fix all failing tests

* Fix one failing test on CI

* Fix one failing test on CI, again

* Fix --prefer-lowest tests

* Some simplifications and tests

* Simplify cached doc blocks

* Simplify cached doc blocks

* More tests for doc block factories

* Tests for the Discovery namespace

* Fix the docs build on CI and broken links

* Add a changelog entry

* Deprecate setGlobTTL() instead of removing it

* Code style

Author: oprypkhantc

.github/workflows/doc_generation.yml

@@ -24,7 +24,7 @@ jobs:
       - name: "Setup NodeJS"
         uses: actions/setup-node@v4
         with:
-          node-version: '16.x'
+          node-version: '20.x'
 
       - name: "Yarn install"
         run: yarn install

composer.json

@@ -24,7 +24,7 @@
         "symfony/cache": "^4.3 || ^5 || ^6 || ^7",
         "symfony/expression-language": "^4 || ^5 || ^6 || ^7",
         "webonyx/graphql-php": "^v15.0",
-        "kcs/class-finder": "^0.5.0"
+        "kcs/class-finder": "^0.5.1"
     },
     "require-dev": {
         "beberlei/porpaginas": "^1.2 || ^2.0",
@@ -34,7 +34,7 @@
         "myclabs/php-enum": "^1.6.6",
         "php-coveralls/php-coveralls": "^2.1",
         "phpstan/extension-installer": "^1.1",
-        "phpstan/phpstan": "^1.9",
+        "phpstan/phpstan": "^1.11",
         "phpunit/phpunit": "^10.1 || ^11.0",
         "symfony/var-dumper": "^5.4 || ^6.0 || ^7",
         "thecodingmachine/phpstan-strict-rules": "^1.0"

examples/no-framework/index.php

@@ -24,8 +24,7 @@
 ]);
 
 $factory = new SchemaFactory($cache, $container);
-$factory->addControllerNamespace('App\\Controllers')
-        ->addTypeNamespace('App');
+$factory->addNamespace('App');
 
 $schema = $factory->createSchema();
 

src/AggregateControllerQueryProvider.php

@@ -8,16 +8,12 @@
 use Psr\Container\ContainerInterface;
 use TheCodingMachine\GraphQLite\Mappers\DuplicateMappingException;
 
-use function array_filter;
-use function array_intersect_key;
-use function array_keys;
 use function array_map;
 use function array_merge;
 use function array_sum;
 use function array_values;
 use function assert;
 use function count;
-use function reset;
 use function sort;
 
 /**
@@ -94,18 +90,29 @@ private function flattenList(array $list): array
         }
 
         // We have an issue, let's detect the duplicate
-        $duplicates = array_intersect_key(...array_values($list));
-        // Let's display an error from the first one.
-        $firstDuplicate = reset($duplicates);
-        assert($firstDuplicate instanceof FieldDefinition);
+        $queriesByName = [];
+        $duplicateClasses = null;
+        $duplicateQueryName = null;
 
-        $duplicateName = $firstDuplicate->name;
+        foreach ($list as $class => $queries) {
+            foreach ($queries as $query => $field) {
+                $duplicatedClass = $queriesByName[$query] ?? null;
 
-        $classes = array_keys(array_filter($list, static function (array $fields) use ($duplicateName) {
-            return isset($fields[$duplicateName]);
-        }));
-        sort($classes);
+                if (! $duplicatedClass) {
+                    $queriesByName[$query] = $class;
 
-        throw DuplicateMappingException::createForQueryInTwoControllers($classes[0], $classes[1], $duplicateName);
+                    continue;
+                }
+
+                $duplicateClasses = [$duplicatedClass, $class];
+                $duplicateQueryName = $query;
+            }
+        }
+
+        assert($duplicateClasses !== null && $duplicateQueryName !== null);
+
+        sort($duplicateClasses);
+
+        throw DuplicateMappingException::createForQueryInTwoControllers($duplicateClasses[0], $duplicateClasses[1], $duplicateQueryName);
     }
 }

src/Cache/ClassBoundCache.php

@@ -0,0 +1,24 @@
+<?php
+
+declare(strict_types=1);
+
+namespace TheCodingMachine\GraphQLite\Cache;
+
+use ReflectionClass;
+
+interface ClassBoundCache
+{
+    /**
+     * @param callable(): TReturn $resolver
+     *
+     * @return TReturn
+     *
+     * @template TReturn
+     */
+    public function get(
+        ReflectionClass $reflectionClass,
+        callable $resolver,
+        string $key,
+        bool $withInheritance = false,
+    ): mixed;
+}

src/Cache/ClassBoundCacheContract.php

@@ -0,0 +1,86 @@
+<?php
+
+declare(strict_types=1);
+
+namespace TheCodingMachine\GraphQLite\Cache;
+
+use ReflectionClass;
+
+use function array_unique;
+use function Safe\filemtime;
+
+class FilesSnapshot
+{
+    /** @param array<string, int> $dependencies */
+    private function __construct(
+        private readonly array $dependencies,
+    )
+    {
+    }
+
+    /** @param list<string> $files */
+    public static function for(array $files): self
+    {
+        $dependencies = [];
+
+        foreach (array_unique($files) as $file) {
+            $dependencies[$file] = filemtime($file);
+        }
+
+        return new self($dependencies);
+    }
+
+    public static function forClass(ReflectionClass $class, bool $withInheritance = false): self
+    {
+        return self::for(
+            self::dependencies($class, $withInheritance),
+        );
+    }
+
+    public static function alwaysUnchanged(): self
+    {
+        return new self([]);
+    }
+
+    /** @return list<string> */
+    private static function dependencies(ReflectionClass $class, bool $withInheritance = false): array
+    {
+        $filename = $class->getFileName();
+
+        // Internal classes are treated as always the same, e.g. you'll have to drop the cache between PHP versions.
+        if ($filename === false) {
+            return [];
+        }
+
+        $files = [$filename];
+
+        if (! $withInheritance) {
+            return $files;
+        }
+
+        if ($class->getParentClass() !== false) {
+            $files = [...$files, ...self::dependencies($class->getParentClass(), $withInheritance)];
+        }
+
+        foreach ($class->getTraits() as $trait) {
+            $files = [...$files, ...self::dependencies($trait, $withInheritance)];
+        }
+
+        foreach ($class->getInterfaces() as $interface) {
+            $files = [...$files, ...self::dependencies($interface, $withInheritance)];
+        }
+
+        return $files;
+    }
+
+    public function changed(): bool
+    {
+        foreach ($this->dependencies as $filename => $modificationTime) {
+            if ($modificationTime !== filemtime($filename)) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+}

src/Cache/ClassBoundCacheContractFactory.php

@@ -0,0 +1,41 @@
+<?php
+
+declare(strict_types=1);
+
+namespace TheCodingMachine\GraphQLite\Cache;
+
+use Psr\SimpleCache\CacheInterface;
+use ReflectionClass;
+
+use function str_replace;
+
+class SnapshotClassBoundCache implements ClassBoundCache
+{
+    /** @param callable(ReflectionClass, bool $withInheritance): FilesSnapshot $filesSnapshotFactory */
+    public function __construct(
+        private readonly CacheInterface $cache,
+        private readonly mixed $filesSnapshotFactory,
+    ) {
+    }
+
+    public function get(ReflectionClass $reflectionClass, callable $resolver, string $key = '', bool $withInheritance = false): mixed
+    {
+        $cacheKey = $reflectionClass->getName() . '__' . $key;
+        $cacheKey = str_replace(['\\', '{', '}', '(', ')', '/', '@', ':'], '_', $cacheKey);
+
+        $item = $this->cache->get($cacheKey);
+
+        if ($item !== null && ! $item['snapshot']->changed()) {
+            return $item['data'];
+        }
+
+        $item = [
+            'data' => $resolver(),
+            'snapshot' => ($this->filesSnapshotFactory)($reflectionClass, $withInheritance),
+        ];
+
+        $this->cache->set($cacheKey, $item);
+
+        return $item['data'];
+    }
+}

src/Cache/ClassBoundCacheContractFactoryInterface.php

@@ -0,0 +1,41 @@
+<?php
+
+declare(strict_types=1);
+
+namespace TheCodingMachine\GraphQLite\Discovery\Cache;
+
+use ReflectionClass;
+use TheCodingMachine\GraphQLite\Discovery\ClassFinder;
+
+/**
+ * Cache that computes a final value based on class that exist in the application found with
+ * the {@see ClassFinder}, and one that allows invalidating only parts of the cache when those
+ * classes change, instead of having to invalidate the whole cache on every change.
+ */
+interface ClassFinderComputedCache
+{
+    /**
+     * Compute the value of the cache. The $finder and $key are self-explanatory; the $map and $reduce need
+     * a bit of an explanation: $map is called with each reflection found by $finder, and expects any value to be returned.
+     * It will then be stored in a Map<string (filename), TEntry (return from $map)>. Once all classes are iterated,
+     * $reduce will then be called with that map, and it's final result is returned.
+     *
+     * Now the point of this is now whenever file A changes, we can automatically remove entries generated for it
+     * and simply call $map only for classes from file A, leaving all other entries untouched and not having to
+     * waste resources on the rest of them. We then only need to call the cheap $reduce and have the final result :)
+     *
+     * @param callable(ReflectionClass<object>): TEntry $map
+     * @param callable(array<string, TEntry>): TReturn $reduce
+     *
+     * @return TReturn
+     *
+     * @template TEntry of mixed
+     * @template TReturn of mixed
+     */
+    public function compute(
+        ClassFinder $classFinder,
+        string $key,
+        callable $map,
+        callable $reduce,
+    ): mixed;
+}