bug #50394 [AssetMapper] Avoid loading potentially ALL assets in dev …

…server (weaverryan)

This PR was squashed before being merged into the 6.3 branch.

Discussion
----------

[AssetMapper] Avoid loading potentially ALL assets in dev server

| Q             | A
| ------------- | ---
| Branch?       | 6.3
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | None
| License       | MIT
| Doc PR        | Still TODO

Hi!

One other rough edge found when using on a real project. The dev server needs to quickly go from "public path" -> "logical path", so it can then look up the `MappedAsset`. Previously, for every served asset, it would loop over ALL assets until it found a match. We have a `CachedMappedAssetFactory`, which makes this happen quickly, but it still loads many assets into memory, including their contents. I was seeing a 13mb jump in the memory on those requests in a modest-sized app. If someone was serving a lot of images, it could get huge. No reason to do that work multiple times.

Btw, none of this happens on production - caching is here just for dev experience.

Thanks!

Commits
-------

bb97591 [AssetMapper] Avoid loading potentially ALL assets in dev server

src/Symfony/Bundle/FrameworkBundle/Resources/config/asset_mapper.php

@@ -83,6 +83,7 @@
                 service('asset_mapper'),
                 abstract_arg('asset public prefix'),
                 abstract_arg('extensions map'),
+                service('cache.asset_mapper'),
             ])
             ->tag('kernel.event_subscriber', ['event' => RequestEvent::class])
 

src/Symfony/Bundle/FrameworkBundle/Resources/config/cache.php

@@ -66,6 +66,11 @@
             ->private()
             ->tag('cache.pool')
 
+        ->set('cache.asset_mapper')
+            ->parent('cache.system')
+            ->private()
+            ->tag('cache.pool')
+
         ->set('cache.messenger.restart_workers_signal')
             ->parent('cache.app')
             ->private()

src/Symfony/Component/AssetMapper/AssetMapper.php

@@ -44,21 +44,15 @@ public function getAsset(string $logicalPath): ?MappedAsset
         return $this->mappedAssetFactory->createMappedAsset($logicalPath, $filePath);
     }
 
-    /**
-     * @return MappedAsset[]
-     */
-    public function allAssets(): array
+    public function allAssets(): iterable
     {
-        $assets = [];
         foreach ($this->mapperRepository->all() as $logicalPath => $filePath) {
             $asset = $this->getAsset($logicalPath);
             if (null === $asset) {
                 throw new \LogicException(sprintf('Asset "%s" could not be found.', $logicalPath));
             }
-            $assets[] = $asset;
+            yield $asset;
         }
-
-        return $assets;
     }
 
     public function getAssetFromSourcePath(string $sourcePath): ?MappedAsset

src/Symfony/Component/AssetMapper/AssetMapperDevServerSubscriber.php

@@ -11,6 +11,7 @@
 
 namespace Symfony\Component\AssetMapper;
 
+use Psr\Cache\CacheItemPoolInterface;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpKernel\Event\RequestEvent;
@@ -104,6 +105,7 @@ public function __construct(
         private readonly AssetMapperInterface $assetMapper,
         string $publicPrefix = '/assets/',
         array $extensionsMap = [],
+        private readonly ?CacheItemPoolInterface $cacheMapCache = null,
     ) {
         $this->publicPrefix = rtrim($publicPrefix, '/').'/';
         $this->extensionsMap = array_merge(self::EXTENSIONS_MAP, $extensionsMap);
@@ -120,13 +122,7 @@ public function onKernelRequest(RequestEvent $event): void
             return;
         }
 
-        $asset = null;
-        foreach ($this->assetMapper->allAssets() as $assetCandidate) {
-            if ($pathInfo === $assetCandidate->getPublicPath()) {
-                $asset = $assetCandidate;
-                break;
-            }
-        }
+        $asset = $this->findAssetFromCache($pathInfo);
 
         if (!$asset) {
             throw new NotFoundHttpException(sprintf('Asset with public path "%s" not found.', $pathInfo));
@@ -160,4 +156,37 @@ private function getMediaType(string $path): ?string
 
         return $this->extensionsMap[$extension] ?? null;
     }
+
+    private function findAssetFromCache(string $pathInfo): ?MappedAsset
+    {
+        $cachedAsset = null;
+        if (null !== $this->cacheMapCache) {
+            $cachedAsset = $this->cacheMapCache->getItem(hash('xxh128', $pathInfo));
+            $asset = $cachedAsset->isHit() ? $this->assetMapper->getAsset($cachedAsset->get()) : null;
+
+            if (null !== $asset && $asset->getPublicPath() === $pathInfo) {
+                return $asset;
+            }
+        }
+
+        // we did not find a match
+        $asset = null;
+        foreach ($this->assetMapper->allAssets() as $assetCandidate) {
+            if ($pathInfo === $assetCandidate->getPublicPath()) {
+                $asset = $assetCandidate;
+                break;
+            }
+        }
+
+        if (null === $asset) {
+            return null;
+        }
+
+        if (null !== $cachedAsset) {
+            $cachedAsset->set($asset->getLogicalPath());
+            $this->cacheMapCache->save($cachedAsset);
+        }
+
+        return $asset;
+    }
 }

src/Symfony/Component/AssetMapper/AssetMapperInterface.php

@@ -28,9 +28,9 @@ public function getAsset(string $logicalPath): ?MappedAsset;
     /**
      * Returns all mapped assets.
      *
-     * @return MappedAsset[]
+     * @return iterable<MappedAsset>
      */
-    public function allAssets(): array;
+    public function allAssets(): iterable;
 
     /**
      * Fetches the asset given its source path (i.e. filesystem path).

src/Symfony/Component/AssetMapper/Command/AssetMapperCompileCommand.php

@@ -118,7 +118,7 @@ private function createManifestAndWriteFiles(SymfonyStyle $io, string $publicDir
     {
         $allAssets = $this->assetMapper->allAssets();
 
-        $io->comment(sprintf('Compiling <info>%d</info> assets to <info>%s%s</info>', \count($allAssets), $publicDir, $this->publicAssetsPathResolver->resolvePublicPath('')));
+        $io->comment(sprintf('Compiling assets to <info>%s%s</info>', $publicDir, $this->publicAssetsPathResolver->resolvePublicPath('')));
         $manifest = [];
         foreach ($allAssets as $asset) {
             // $asset->getPublicPath() will start with a "/"
@@ -132,6 +132,7 @@ private function createManifestAndWriteFiles(SymfonyStyle $io, string $publicDir
             $manifest[$asset->getLogicalPath()] = $asset->getPublicPath();
         }
         ksort($manifest);
+        $io->comment(sprintf('Compiled <info>%d</info> assets', \count($manifest)));
 
         return $manifest;
     }

src/Symfony/Component/AssetMapper/Tests/AssetMapperTest.php

@@ -69,6 +69,8 @@ public function testAllAssets()
             });
 
         $assets = $assetMapper->allAssets();
+        $this->assertIsIterable($assets);
+        $assets = iterator_to_array($assets);
         $this->assertCount(8, $assets);
         $this->assertInstanceOf(MappedAsset::class, $assets[0]);
     }

src/Symfony/Component/AssetMapper/Tests/Command/AssetsMapperCompileCommandTest.php

@@ -52,7 +52,7 @@ public function testAssetsAreCompiled()
         $res = $tester->execute([]);
         $this->assertSame(0, $res);
         // match Compiling \d+ assets
-        $this->assertMatchesRegularExpression('/Compiling \d+ assets/', $tester->getDisplay());
+        $this->assertMatchesRegularExpression('/Compiled \d+ assets/', $tester->getDisplay());
 
         $this->assertFileExists($targetBuildDir.'/subdir/file5-f4fdc37375c7f5f2629c5659a0579967.js');
         $this->assertSame(<<<EOF