8000 bug #50394 [AssetMapper] Avoid loading potentially ALL assets in dev … · symfony/symfony@cc65825 · GitHub
[go: up one dir, main page]
Skip to content

Commit cc65825

Browse files
nicolas-grekasnicolas-grekas
committed
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
2 parents 03d8302 + bb97591 commit cc65825

File tree

8 files changed

+51
-19
lines changed

8 files changed

+51
-19
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@
8383
service('asset_mapper'),
8484
abstract_arg('asset public prefix'),
8585
abstract_arg('extensions map'),
86+
service('cache.asset_mapper'),
8687
])
8788
->tag('kernel.event_subscriber', ['event' => RequestEvent::class])
8889

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,11 @@
6666
->private()
6767
->tag('cache.pool')
6868

69+
->set('cache.asset_mapper')
70+
->parent('cache.system')
71+
->private()
72+
->tag('cache.pool')
73+
6974
->set('cache.messenger.restart_workers_signal')
7075
->parent('cache.app')
7176
->private()

‎src/Symfony/Component/AssetMapper/AssetMapper.php

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,21 +44,15 @@ public function getAsset(string $logicalPath): ?MappedAsset
4444
return $this->mappedAssetFactory->createMappedAsset($logicalPath, $filePath);
4545
}
4646

47-
/**
48-
* @return MappedAsset[]
49-
*/
50-
public function allAssets(): array
47+
public function allAssets(): iterable
5148
{
52-
$assets = [];
5349
foreach ($this->mapperRepository->all() as $logicalPath => $filePath) {
5450
$asset = $this->getAsset($logicalPath);
5551
if (null === $asset) {
5652
throw new \LogicException(sprintf('Asset "%s" could not be found.', $logicalPath));
5753
}
58-
$assets[] = $asset;
54+
yield $asset;
5955
}
60-
61-
return $assets;
6256
}
6357

6458
public function getAssetFromSourcePath(string $sourcePath): ?MappedAsset

‎src/Symfony/Component/AssetMapper/AssetMapperDevServerSubscriber.php

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Component\AssetMapper;
1313

14+
use Psr\Cache\CacheItemPoolInterface;
1415
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
1516
use Symfony\Component\HttpFoundation\Response;
1617
use Symfony\Component\HttpKernel\Event\RequestEvent;
@@ -104,6 +105,7 @@ public function __construct(
104105
private readonly AssetMapperInterface $assetMapper,
105106
string $publicPrefix = '/assets/',
106107
array $extensionsMap = [],
108+
private readonly ?CacheItemPoolInterface $cacheMapCache = null,
107109
) {
108110
$this->publicPrefix = rtrim($publicPrefix, '/').'/';
109111
$this->extensionsMap = array_merge(self::EXTENSIONS_MAP, $extensionsMap);
@@ -120,13 +122,7 @@ public function onKernelRequest(RequestEvent $event): void
120122
return;
121123
}
122124

123-
$asset = null;
124-
foreach ($this< 1E80 /span>->assetMapper->allAssets() as $assetCandidate) {
125-
if ($pathInfo === $assetCandidate->getPublicPath()) {
126-
$asset = $assetCandidate;
127-
break;
128-
}
129-
}
125+
$asset = $this->findAssetFromCache($pathInfo);
130126

131127
if (!$asset) {
132128
throw new NotFoundHttpException(sprintf('Asset with public path "%s" not found.', $pathInfo));
@@ -160,4 +156,37 @@ private function getMediaType(string $path): ?string
160156

161157
return $this->extensionsMap[$extension] ?? null;
162158
}
159+
160+
private function findAssetFromCache(string $pathInfo): ?MappedAsset
161+
{
162+
$cachedAsset = null;
163+
if (null !== $this->cacheMapCache) {
164+
$cachedAsset = $this->cacheMapCache->getItem(hash('xxh128', $pathInfo));
165+
$asset = $cachedAsset->isHit() ? $this->assetMapper->getAsset($cachedAsset->get()) : null;
166+
167+
if (null !== $asset && $asset->getPublicPath() === $pathInfo) {
168+
return $asset;
169+
}
170+
}
171+
172+
// we did not find a match
173+
$asset = null;
174+
foreach ($this->assetMapper->allAssets() as $assetCandidate) {
175+
if ($pathInfo === $assetCandidate->getPublicPath()) {
176+
$asset = $assetCandidate;
177+
break;
178+
}
179+
}
180+
181+
if (null === $asset) {
182+
return null;
183+
}
184+
185+
if (null !== $cachedAsset) {
186+
$cachedAsset->set($asset->getLogicalPath());
187+
$this->cacheMapCache->save($cachedAsset);
188+
}
189+
190+
return $asset;
191+
}
163192
}

‎src/Symfony/Component/AssetMapper/AssetMapperInterface.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,9 @@ public function getAsset(string $logicalPath): ?MappedAsset;
2828
/**
2929
* Returns all mapped assets.
3030
*
31-
* @return MappedAsset[]
31+
* @return iterable<MappedAsset>
3232
*/
33-
public function allAssets(): array;
33+
public function allAssets(): iterable;
3434

3535
/**
3636
* Fetches the asset given its source path (i.e. filesystem path).

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ private function createManifestAndWriteFiles(SymfonyStyle $io, string $publicDir
118118
{
119119
$allAssets = $this->assetMapper->allAssets();
120120

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

136137
return $manifest;
137138
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,8 @@ public function testAllAssets()
6969
});
7070

7171
$assets = $assetMapper->allAssets();
72+
$this->assertIsIterable($assets);
73+
$assets = iterator_to_array($assets);
7274
$this->assertCount(8, $assets);
7375
$this->assertInstanceOf(MappedAsset::class, $assets[0]);
7476
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ public function testAssetsAreCompiled()
5252
$res = $tester->execute([]);
5353
$this->assertSame(0, $res);
5454
// match Compiling \d+ assets
55-
$this->assertMatchesRegularExpression('/Compiling \d+ assets/', $tester->getDisplay());
55+
$this->assertMatchesRegularExpression('/Compiled \d+ assets/', $tester->getDisplay());
5656

5757
$this->assertFileExists($targetBuildDir.'/subdir/file5-f4fdc37375c7f5f2629c5659a0579967.js');
5858
$this->assertSame(<<<EOF

0 commit comments

Comments
 (0)
0