Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Commit415ad8c

Browse files
bug#52330 [AssetMapper] Fixing memory bug where we stored way more file content than needed (weaverryan)
This PR was merged into the 6.4 branch.Discussion----------[AssetMapper] Fixing memory bug where we stored way more file content than needed| Q | A| ------------- | ---| Branch? | 6.4| Bug fix? | yes (on Symfonycasts, without this, `asset-map:compile` takes >500mb of memory)| New feature? | no| Deprecations? | none| Issues | None| License | MITHi!This drastically improves the memory footprint when ALL assets are built, which happens in `asset-map:compile`, `debug:asset-map`, etc. Blackfire is a huge fan!https://blackfire.io/profiles/compare/4eb36732-8805-4c49-b636-d4bf8f9e2b27/graphThe MAIN optimization by far is to set `MappedAsset.content` to null if the final, compiled content matches the source file's content. This is the case for the vast-majority of mapped assets, and so the result is MUCH less content stored in memory for no reason.Thanks to `@smnandre` and his earlier PR, which allows for the `null` content on mapped assets.Cheers!Commits-------78ec570 [AssetMapper] Fixing memory bug where we stored way more file content than needed
2 parents60dec0b +78ec570 commit415ad8c

File tree

5 files changed

+42
-27
lines changed

5 files changed

+42
-27
lines changed

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,11 +104,9 @@ private function shortenPath(string $path): string
104104

105105
privatefunctioncreateManifestAndWriteFiles(SymfonyStyle$io):array
106106
{
107-
$allAssets =$this->assetMapper->allAssets();
108-
109107
$io->comment(sprintf('Compiling and writing asset files to <info>%s</info>',$this->shortenPath($this->assetsFilesystem->getDestinationPath())));
110108
$manifest = [];
111-
foreach ($allAssetsas$asset) {
109+
foreach ($this->assetMapper->allAssets()as$asset) {
112110
if (null !==$asset->content) {
113111
// The original content has been modified by the AssetMapperCompiler
114112
$this->assetsFilesystem->write($asset->publicPath,$asset->content);

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

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ class MappedAssetFactory implements MappedAssetFactoryInterface
2626

2727
privatearray$assetsCache = [];
2828
privatearray$assetsBeingCreated = [];
29-
privatearray$fileContentsCache = [];
3029

3130
publicfunction__construct(
3231
privatereadonlyPublicAssetsPathResolverInterface$assetsPathResolver,
@@ -46,14 +45,15 @@ public function createMappedAsset(string $logicalPath, string $sourcePath): ?Map
4645
$isVendor =$this->isVendor($sourcePath);
4746
$asset =newMappedAsset($logicalPath,$sourcePath,$this->assetsPathResolver->resolvePublicPath($logicalPath), isVendor:$isVendor);
4847

49-
[$digest,$isPredigested] =$this->getDigest($asset);
48+
$content =$this->compileContent($asset);
49+
[$digest,$isPredigested] =$this->getDigest($asset,$content);
5050

5151
$asset =newMappedAsset(
5252
$asset->logicalPath,
5353
$asset->sourcePath,
5454
$asset->publicPathWithoutDigest,
55-
$this->getPublicPath($asset),
56-
$this->compileContent($asset),
55+
$this->getPublicPath($asset,$content),
56+
$content,
5757
$digest,
5858
$isPredigested,
5959
$isVendor,
@@ -75,15 +75,15 @@ public function createMappedAsset(string $logicalPath, string $sourcePath): ?Map
7575
*
7676
* @return array{0: string, 1: bool}
7777
*/
78-
privatefunctiongetDigest(MappedAsset$asset):array
78+
privatefunctiongetDigest(MappedAsset$asset, ?string$content):array
7979
{
8080
// check for a pre-digested file
8181
if (preg_match(self::PREDIGESTED_REGEX,$asset->logicalPath,$matches)) {
8282
return [$matches[1],true];
8383
}
8484

8585
// Use the compiled content if any
86-
if (null !==$content =$this->compileContent($asset)) {
86+
if (null !==$content) {
8787
return [hash('xxh128',$content),false];
8888
}
8989

@@ -95,27 +95,23 @@ private function getDigest(MappedAsset $asset): array
9595

9696
privatefunctioncompileContent(MappedAsset$asset): ?string
9797
{
98-
if (\array_key_exists($asset->logicalPath,$this->fileContentsCache)) {
99-
return$this->fileContentsCache[$asset->logicalPath];
100-
}
101-
10298
if (!is_file($asset->sourcePath)) {
10399
thrownewRuntimeException(sprintf('Asset source path "%s" could not be found.',$asset->sourcePath));
104100
}
105101

106102
if (!$this->compiler->supports($asset)) {
107-
return$this->fileContentsCache[$asset->logicalPath] =null;
103+
returnnull;
108104
}
109105

110106
$content =file_get_contents($asset->sourcePath);
111-
$content =$this->compiler->compile($content,$asset);
107+
$compiled =$this->compiler->compile($content,$asset);
112108

113-
return$this->fileContentsCache[$asset->logicalPath] =$content;
109+
return$compiled !==$content ?$compiled :null;
114110
}
115111

116-
privatefunctiongetPublicPath(MappedAsset$asset): ?string
112+
privatefunctiongetPublicPath(MappedAsset$asset, ?string$content): ?string
117113
{
118-
[$digest,$isPredigested] =$this->getDigest($asset);
114+
[$digest,$isPredigested] =$this->getDigest($asset,$content);
119115

120116
if ($isPredigested) {
121117
return$this->assetsPathResolver->resolvePublicPath($asset->logicalPath);

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,9 @@ final class MappedAsset
2626
publicreadonlystring$publicExtension;
2727

2828
/**
29-
* The final content of this asset, if different from the source.
29+
* The final content of this asset if different from the sourcePath.
30+
*
31+
* If null, the content should be read from the sourcePath.
3032
*/
3133
publicreadonly ?string$content;
3234

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
useSymfony\Bundle\FrameworkBundle\Test\WebTestCase;
1515
useSymfony\Component\AssetMapper\Tests\Fixtures\AssetMapperTestAppKernel;
16+
useSymfony\Component\HttpFoundation\BinaryFileResponse;
1617

1718
class AssetMapperDevServerSubscriberFunctionalTestextends WebTestCase
1819
{
@@ -23,11 +24,12 @@ public function testGettingAssetWorks()
2324
$client->request('GET','/assets/file1-b3445cb7a86a0795a7af7f2004498aef.css');
2425
$response =$client->getResponse();
2526
$this->assertSame(200,$response->getStatusCode());
27+
$this->assertInstanceOf(BinaryFileResponse::class,$response);
2628
$this->assertSame(<<<EOF
2729
/* file1.css */
2830
body {}
2931
30-
EOF,$response->getContent());
32+
EOF,$response->getFile()->getContent());
3133
$this->assertSame('"b3445cb7a86a0795a7af7f2004498aef"',$response->headers->get('ETag'));
3234
$this->assertSame('immutable, max-age=604800, public',$response->headers->get('Cache-Control'));
3335
$this->assertTrue($response->headers->has('X-Assets-Dev'));
@@ -59,11 +61,12 @@ public function testPreDigestedAssetIsReturned()
5961
$client->request('GET','/assets/already-abcdefVWXYZ0123456789.digested.css');
6062
$response =$client->getResponse();
6163
$this->assertSame(200,$response->getStatusCode());
64+
$this->assertInstanceOf(BinaryFileResponse::class,$response);
6265
$this->assertSame(<<<EOF
6366
/* already-abcdefVWXYZ0123456789.digested.css */
6467
body {}
6568
66-
EOF,$response->getContent());
69+
EOF,$response->getFile()->getContent());
6770
}
6871

6972
protectedstaticfunctiongetKernelClass():string

‎src/Symfony/Component/AssetMapper/Tests/Factory/MappedAssetFactoryTest.php‎

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,14 +48,22 @@ public function testCreateMappedAssetRespectsPreDigestedPaths()
4848
$this->assertSame('/final-assets/already-abcdefVWXYZ0123456789.digested.css',$asset->publicPathWithoutDigest);
4949
}
5050

51-
publicfunctiontestCreateMappedAssetWithContentBasic()
51+
publicfunctiontestCreateMappedAssetWithContentThatChanged()
5252
{
53-
$assetMapper =$this->createFactory();
54-
$expected =<<<EOF
55-
/* file1.css */
56-
body {}
53+
$file1Compiler =newclass()implements AssetCompilerInterface {
54+
publicfunctionsupports(MappedAsset$asset):bool
55+
{
56+
returntrue;
57+
}
5758

58-
EOF;
59+
publicfunctioncompile(string$content,MappedAsset$asset,AssetMapperInterface$assetMapper):string
60+
{
61+
return'totally changed';
62+
}
63+
};
64+
65+
$assetMapper =$this->createFactory($file1Compiler);
66+
$expected ='totally changed';
5967

6068
$asset =$assetMapper->createMappedAsset('file1.css',__DIR__.'/../Fixtures/dir1/file1.css');
6169
$this->assertSame($expected,$asset->content);
@@ -65,6 +73,14 @@ public function testCreateMappedAssetWithContentBasic()
6573
$this->assertSame($expected,$asset->content);
6674
}
6775

76+
publicfunctiontestCreateMappedAssetWithContentThatDoesNotChange()
77+
{
78+
$assetMapper =$this->createFactory();
79+
$asset =$assetMapper->createMappedAsset('file1.css',__DIR__.'/../Fixtures/dir1/file1.css');
80+
// null content because the final content matches the file source
81+
$this->assertNull($asset->content);
82+
}
83+
6884
publicfunctiontestCreateMappedAssetWithContentErrorsOnCircularReferences()
6985
{
7086
$factory =$this->createFactory();

0 commit comments

Comments
 (0)

[8]ページ先頭

©2009-2025 Movatter.jp