Cache the dependent/suggester counts in Redis

getDependentCount() and getSuggestCount() run a COUNT(*) over the dependent/suggester
tables on every package page view, again in the package JSON API, and again on the
dependents/suggesters pages. For a widely-required package like psr/log that is a
several-hundred-thousand-row index scan, and the package page alone accounts for 4.5M
requests per APM period.

The existing indexes already cover the queries, so there is nothing left to optimise in
the query itself - the fix is to stop running it. Cache both counts for an hour with a
random variance to spread out the refresh of the most-requested packages. A count badge
being an hour out of date is harmless, so this is TTL-only: dependent rows are written
keyed by the required package name while the writer operates on the requiring package,
so precise invalidation would mean a key deletion per required name on every update.

Keys are lowercased because the packageName columns use a case-insensitive collation and
differently-cased requests must not get separate entries.

The test env now points the cache client at its own Redis DB, and IntegrationTestCase
flushes it per test: the DB is rolled back between tests but Redis is not, so counts
keyed by package name would otherwise leak into later tests that reuse a name.

Author: Seldaek

.env.test

@@ -4,6 +4,7 @@ APP_SECRET='$ecretf0rt3st'
 DATABASE_URL="mysql://root@127.0.0.1:3306/packagist?serverVersion=8.4.7"
 MAILER_DSN=null://null
 REDIS_URL=redis://localhost/14
+REDIS_CACHE_URL=redis://localhost/15
 APP_MAILER_FROM_EMAIL=packagist@example.org
 APP_HOSTNAME=packagist.wip
 DEFAULT_URI=http://packagist.wip

phpstan-baseline.neon

@@ -54,12 +54,6 @@ parameters:
 			count: 1
 			path: src/Entity/AuditRecordRepository.php
 
-		-
-			message: '#^Method App\\Entity\\PackageRepository\:\:getSuggestCount\(\) should return int\<0, max\> but returns int\.$#'
-			identifier: return.type
-			count: 1
-			path: src/Entity/PackageRepository.php
-
 		-
 			message: '#^Query error\: Unknown column ''d\.total'' in ''order clause'' \(1054\)\.$#'
 			identifier: dba.syntaxError

src/Entity/PackageRepository.php

@@ -20,6 +20,7 @@
 use Doctrine\ORM\Query;
 use Doctrine\ORM\QueryBuilder;
 use Doctrine\Persistence\ManagerRegistry;
+use Predis\Client;
 
 /**
  * @author Jordi Boggiano <j.boggiano@seld.be>
@@ -32,8 +33,10 @@ class PackageRepository extends ServiceEntityRepository
     // @phpstan-ignore classConstant.unused
     private const LISTING_WITH_AUTO_UPDATE_WARNINGS_FIELDS = 'id, name, description, type, gitHubStars, frozen, language, abandoned, replacementPackage, autoUpdated, repository';
 
-    public function __construct(ManagerRegistry $registry)
-    {
+    public function __construct(
+        ManagerRegistry $registry,
+        private Client $redisCache,
+    ) {
         parent::__construct($registry, Package::class);
     }
 
@@ -627,14 +630,16 @@ public function getReadmeContentsByPackageIds(array $ids): array
      */
     public function getDependentCount(string $name, ?int $type = null): int
     {
-        $sql = 'SELECT COUNT(*) count FROM dependent WHERE packageName = :name';
-        $args = ['name' => $name];
-        if (null !== $type) {
-            $sql .= ' AND type = :type';
-            $args['type'] = $type;
-        }
+        return $this->getCachedCount('dep-count:'.strtolower($name).':'.($type ?? 'all'), function () use ($name, $type): int {
+            $sql = 'SELECT COUNT(*) count FROM dependent WHERE packageName = :name';
+            $args = ['name' => $name];
+            if (null !== $type) {
+                $sql .= ' AND type = :type';
+                $args['type'] = $type;
+            }
 
-        return max(0, (int) $this->getEntityManager()->getConnection()->fetchOne($sql, $args));
+            return (int) $this->getEntityManager()->getConnection()->fetchOne($sql, $args);
+        });
     }
 
     /**
@@ -709,10 +714,37 @@ public function getDefaultBranchRequireFor(array $requirers, string $requiree):
      */
     public function getSuggestCount(string $name): int
     {
-        $sql = 'SELECT COUNT(*) count FROM suggester WHERE packageName = :name';
-        $args = ['name' => $name];
+        return $this->getCachedCount('sug-count:'.strtolower($name), function () use ($name): int {
+            $sql = 'SELECT COUNT(*) count FROM suggester WHERE packageName = :name';
+
+            return (int) $this->getEntityManager()->getConnection()->fetchOne($sql, ['name' => $name]);
+        });
+    }
+
+    /**
+     * Both counts are rendered as tab labels on every package page view, where the COUNT(*) is a
+     * large index scan for widely-required packages like psr/log. Being an hour out of date on a
+     * badge is harmless, so this is TTL-only with no explicit invalidation.
+     *
+     * Keys are lowercased because the packageName columns use a case-insensitive collation, so
+     * differently-cased requests must not get separate entries.
+     *
+     * @param callable(): int $compute
+     *
+     * @return int<0, max>
+     */
+    private function getCachedCount(string $cacheKey, callable $compute): int
+    {
+        $cached = $this->redisCache->get($cacheKey);
+        if ($cached !== null) {
+            return max(0, (int) $cached);
+        }
+
+        $count = max(0, $compute());
+        // random variance spreads out the refresh of the most-requested packages
+        $this->redisCache->setex($cacheKey, 3600 + random_int(0, 600), (string) $count);
 
-        return (int) $this->getEntityManager()->getConnection()->fetchOne($sql, $args);
+        return $count;
     }
 
     /**

tests/Entity/PackageRepositoryTest.php

@@ -12,10 +12,13 @@
 
 namespace App\Tests\Entity;
 
+use App\Entity\Dependent;
 use App\Entity\Package;
 use App\Entity\PackageFreezeReason;
 use App\Entity\PackageRepository;
+use App\Entity\Suggester;
 use App\Tests\IntegrationTestCase;
+use Predis\Client;
 
 class PackageRepositoryTest extends IntegrationTestCase
 {
@@ -197,4 +200,63 @@ public function testGetFilteredQueryBuilderExcludesSuppressedByDefault(): void
         self::assertContains('vendor/spam', $withFrozen);
         self::assertContains('vendor/malware', $withFrozen);
     }
+
+    public function testGetDependentCountIsCachedPerType(): void
+    {
+        $requirer = self::createPackage('test/requirer', 'https://example.org/requirer');
+        $devRequirer = self::createPackage('test/dev-requirer', 'https://example.org/dev-requirer');
+        $this->store($requirer, $devRequirer);
+        $this->store(
+            new Dependent($requirer, 'test/required', Dependent::TYPE_REQUIRE),
+            new Dependent($devRequirer, 'test/required', Dependent::TYPE_REQUIRE_DEV),
+        );
+
+        self::assertSame(2, $this->packageRepository->getDependentCount('test/required'));
+        self::assertSame(1, $this->packageRepository->getDependentCount('test/required', Dependent::TYPE_REQUIRE));
+        self::assertSame(1, $this->packageRepository->getDependentCount('test/required', Dependent::TYPE_REQUIRE_DEV));
+
+        // each variant gets its own key, so the type filter cannot be served from the unfiltered count
+        self::assertSame('2', $this->redisCache()->get('dep-count:test/required:all'));
+        self::assertSame('1', $this->redisCache()->get('dep-count:test/required:'.Dependent::TYPE_REQUIRE));
+        self::assertSame('1', $this->redisCache()->get('dep-count:test/required:'.Dependent::TYPE_REQUIRE_DEV));
+    }
+
+    public function testGetDependentCountReadsTheCacheAndIgnoresNameCasing(): void
+    {
+        $this->redisCache()->set('dep-count:test/required:all', '42');
+
+        self::assertSame(42, $this->packageRepository->getDependentCount('test/required'));
+        // packageName uses a case-insensitive collation, so casing must not produce a second entry
+        self::assertSame(42, $this->packageRepository->getDependentCount('Test/Required'));
+    }
+
+    public function testGetSuggestCountIsCached(): void
+    {
+        $suggester = self::createPackage('test/suggester', 'https://example.org/suggester');
+        $this->store($suggester);
+        $this->store(new Suggester($suggester, 'test/suggested'));
+
+        self::assertSame(1, $this->packageRepository->getSuggestCount('test/suggested'));
+        self::assertSame('1', $this->redisCache()->get('sug-count:test/suggested'));
+
+        $this->redisCache()->set('sug-count:test/suggested', '7');
+        self::assertSame(7, $this->packageRepository->getSuggestCount('test/suggested'));
+    }
+
+    public function testCountsCacheZeroSoUnknownPackagesDoNotRequeryEveryPageView(): void
+    {
+        self::assertSame(0, $this->packageRepository->getDependentCount('test/nothing-requires-this'));
+        self::assertSame(0, $this->packageRepository->getSuggestCount('test/nothing-requires-this'));
+
+        self::assertSame('0', $this->redisCache()->get('dep-count:test/nothing-requires-this:all'));
+        self::assertSame('0', $this->redisCache()->get('sug-count:test/nothing-requires-this'));
+    }
+
+    private function redisCache(): Client
+    {
+        $client = static::getContainer()->get('snc_redis.cache');
+        self::assertInstanceOf(Client::class, $client);
+
+        return $client;
+    }
 }

tests/IntegrationTestCase.php

@@ -31,6 +31,11 @@ protected function setUp(): void
         $this->client = self::createClient();
         $this->client->disableReboot(); // prevent reboot to keep the transaction
 
+        // The DB is rolled back per test but Redis is not, so cached values keyed by package name
+        // would leak into later tests that reuse a name. The cache client has its own DB in the
+        // test env (REDIS_CACHE_URL) so this cannot clear state the default client owns.
+        static::getContainer()->get('snc_redis.cache')->flushdb();
+
         static::getService(Connection::class)->beginTransaction();
 
         parent::setUp();