[FrameworkBundle] cache ClassMetadataFactory in debug #35109

bastnic · Dec 26, 2019

Q	A
Branch?	master
Bug fix?	no (performance)
New feature?	performance...
Deprecations?	no
Tickets	#35096
License	MIT
Doc PR

Cf #35096, validator and serialization yaml/xml files are parsed at each requests. But they are already tracked by a DirectoryResource in

symfony/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php

Lines 1246 to 1261 in bd9dc7c

    
           private function registerMappingFilesFromConfig(ContainerBuilder $container, array $config, callable $fileRecorder) 
        
           { 
        
               foreach ($config['mapping']['paths'] as $path) { 
        
                   if (is_dir($path)) { 
        
                       $this->registerMappingFilesFromDir($path, $fileRecorder); 
        
                       $container->addResource(new DirectoryResource($path, '/^$/')); 
        
                   } elseif ($container->fileExists($path, false)) { 
        
                       if (!preg_match('/\.(xml|ya?ml)$/', $path, $matches)) { 
        
                           throw new \RuntimeException(sprintf('Unsupported mapping type in "%s", supported types are XML & Yaml.', $path)); 
        
                       } 
        
                       $fileRecorder($matches[1], $path); 
        
                   } else { 
        
                       throw new \RuntimeException(sprintf('Could not open file or directory "%s".', $path)); 
        
                   } 
        
               } 
        
           }

So it only needs a real cache purge on validator / serializer sytem cache, and I use the CachePoolClearerCacheWarmer from APIP that already does similar things.

Handling that sort of cache in Symfony fixes a lot of others performance issue.

dunglas · Dec 26, 2019

The test failures look related.

We already track modification in serialization/validator config directory so we just need to clear the cache at warmup. Idea taken from apip: https://github.com/api-platform/core/blob/master/src/Bridge/Symfony/Bundle/CacheWarmer/CachePoolClearerCacheWarmer.php

bastnic · Dec 27, 2019

The test failures look related.

Fixed.

fabpot · Jan 7, 2020

Thank you @bastnic.

…astnic) This PR was merged into the 5.1-dev branch. Discussion ---------- [FrameworkBundle] cache ClassMetadataFactory in debug | Q | A | ------------- | --- | Branch? | master | Bug fix? | no (performance) | New feature? | performance... | Deprecations? | no | Tickets | #35096 | License | MIT | Doc PR | Cf #35096, validator and serialization yaml/xml files are parsed at each requests. But they are already tracked by a `DirectoryResource` in https://github.com/symfony/symfony/blob/bd9dc7c573f537c5a751fc3543774cab9ad4770b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php#L1246-L1261 So it only needs a real cache purge on validator / serializer sytem cache, and I use the [CachePoolClearerCacheWarmer](https://github.com/api-platform/core/blob/master/src/Bridge/Symfony/Bundle/CacheWarmer/CachePoolClearerCacheWarmer.php) from APIP that already does similar things. Handling that sort of cache in Symfony fixes a lot of others performance issue. ![image](https://user-images.githubusercontent.com/84887/71452294-e6869480-2783-11ea-9aa8-431db3edfd8d.png) Commits ------- 5332498 [FrameworkBundle] cache ClassMetadataFactory in debug

teohhanhui · Jan 7, 2020

src/Symfony/Bundle/FrameworkBundle/CacheWarmer/CachePoolClearerCacheWarmer.php

+ *
+ * Do not use in production!
+ *
+ * @author Kévin Dunglas <dunglas@gmail.com>


Pretty sure I was the author 😂

can you send a PR? :)

…amxavier) This PR was merged into the 5.1 branch. Discussion ---------- Fix author for class CachePoolClearerCacheWarmer | Q | A | ------------- | --- | Branch? | 5.1 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | Fix #35109 (comment) | License | MIT | Doc PR | symfony/symfony-docs#... Commits ------- b3d0812 Fix author for class CachePoolClearerCacheWarmer

…amxavier) This PR was merged into the 5.1 branch. Discussion ---------- Fix author for class CachePoolClearerCacheWarmer | Q | A | ------------- | --- | Branch? | 5.1 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | Fix symfony/symfony#35109 (comment) | License | MIT | Doc PR | symfony/symfony-docs#... Commits ------- b3d0812976 Fix author for class CachePoolClearerCacheWarmer

… disabled (bastnic) This PR was merged into the 5.4 branch. Discussion ---------- [FrameworkBundle] enable metadata cache when annotation is disabled | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #...  | License | MIT | Doc PR | symfony/symfony-docs#...  When using Annotations, annotations are cached at `AnnotationLoader` level. Which is cleared when entities are changed. So the dev experience is optimal. ``` [ClassMetadataFactory.php](vendor/symfony/serializer/Mapping/Factory/ClassMetadataFactory.php") on line 51: [Symfony\Component\Serializer\Mapping\Loader\LoaderChain](vendor/symfony/serializer/Mapping/Loader/LoaderChain.php&line=28#line28) {#543 ▼ -loaders: array:1 [▼ 0 => [Symfony\Component\Serializer\Mapping\Loader\AnnotationLoader](vendor/symfony/serializer/Mapping/Loader/AnnotationLoader.php&line=33#line33) {#544 ▼ -reader: [Doctrine\Common\Annotations\PsrCachedReader](vendor/doctrine/annotations/lib/Doctrine/Common/Annotations/PsrCachedReader.php&line=22#line22) {#262 ▼ -delegate: [Doctrine\Common\Annotations\AnnotationReader](vendor/doctrine/annotations/lib/Doctrine/Common/Annotations/AnnotationReader.php&line=20#line20) {#263 ▶} -cache: [Symfony\Component\Cache\Adapter\PhpArrayAdapter](vendor/symfony/cache/Adapter/PhpArrayAdapter.php&line=32#line32) {#277 ▶} -debug: true -loadedAnnotations: array:14 [▶] -loadedFilemtimes: array:4 [▶] } } ] } ``` When using yaml files, there is no cache at the loader level so I added in the past the same cache as for the prod env, as the metadata are effectively cleared when using only yaml config files. #35109 The regression introduced by my patch is for people that do not use mapping files but use annotations. #41961 But now, we are in the opposite situation: no cache for people using mapping files but not annotations. On a current project it means loading 83 yaml files for each dev requests. It's not good at all. A simple local fix is to add that in a dev services files. ```yaml serializer.mapping.cache_class_metadata_factory: class: 'Symfony\Component\Serializer\Mapping\Factory\CacheClassMetadataFactory' decorates: 'serializer.mapping.class_metadata_factory' arguments: ['`@serializer`.mapping.cache_class_metadata_factory.inner', '`@serializer`.mapping.cache.symfony'] ``` ![image](https://user-images.githubusercontent.com/84887/224833940-b8e7ad72-e7f6-44e7-a924-0b20f2f0cdae.png) A solution in Symfony could be: 1/ only yaml/xml mapping files (`enable_annotations: false`) : cache like prod => that what I did in this PR, as it fixes the current perf regressions on my different projects. There is no cache on yaml/xml file as soon as annotation is enabled (which is the default) 2/ add a cache at reader level for yaml/xml loader 3/ add a cache cleaner at metadata level when annotation are enabled Commits ------- 1773dff [FrameworkBundle] enable metadata cache when annotation is disabled

… disabled (bastnic) This PR was merged into the 5.4 branch. Discussion ---------- [FrameworkBundle] enable metadata cache when annotation is disabled | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #...  | License | MIT | Doc PR | symfony/symfony-docs#...  When using Annotations, annotations are cached at `AnnotationLoader` level. Which is cleared when entities are changed. So the dev experience is optimal. ``` [ClassMetadataFactory.php](vendor/symfony/serializer/Mapping/Factory/ClassMetadataFactory.php") on line 51: [Symfony\Component\Serializer\Mapping\Loader\LoaderChain](vendor/symfony/serializer/Mapping/Loader/LoaderChain.php&line=28#line28) {#543 ▼ -loaders: array:1 [▼ 0 => [Symfony\Component\Serializer\Mapping\Loader\AnnotationLoader](vendor/symfony/serializer/Mapping/Loader/AnnotationLoader.php&line=33#line33) {#544 ▼ -reader: [Doctrine\Common\Annotations\PsrCachedReader](vendor/doctrine/annotations/lib/Doctrine/Common/Annotations/PsrCachedReader.php&line=22#line22) {#262 ▼ -delegate: [Doctrine\Common\Annotations\AnnotationReader](vendor/doctrine/annotations/lib/Doctrine/Common/Annotations/AnnotationReader.php&line=20#line20) {#263 ▶} -cache: [Symfony\Component\Cache\Adapter\PhpArrayAdapter](vendor/symfony/cache/Adapter/PhpArrayAdapter.php&line=32#line32) {#277 ▶} -debug: true -loadedAnnotations: array:14 [▶] -loadedFilemtimes: array:4 [▶] } } ] } ``` When using yaml files, there is no cache at the loader level so I added in the past the same cache as for the prod env, as the metadata are effectively cleared when using only yaml config files. symfony/symfony#35109 The regression introduced by my patch is for people that do not use mapping files but use annotations. symfony/symfony#41961 But now, we are in the opposite situation: no cache for people using mapping files but not annotations. On a current project it means loading 83 yaml files for each dev requests. It's not good at all. A simple local fix is to add that in a dev services files. ```yaml serializer.mapping.cache_class_metadata_factory: class: 'Symfony\Component\Serializer\Mapping\Factory\CacheClassMetadataFactory' decorates: 'serializer.mapping.class_metadata_factory' arguments: ['`@serializer`.mapping.cache_class_metadata_factory.inner', '`@serializer`.mapping.cache.symfony'] ``` ![image](https://user-images.githubusercontent.com/84887/224833940-b8e7ad72-e7f6-44e7-a924-0b20f2f0cdae.png) A solution in Symfony could be: 1/ only yaml/xml mapping files (`enable_annotations: false`) : cache like prod => that what I did in this PR, as it fixes the current perf regressions on my different projects. There is no cache on yaml/xml file as soon as annotation is enabled (which is the default) 2/ add a cache at reader level for yaml/xml loader 3/ add a cache cleaner at metadata level when annotation are enabled Commits ------- 1773dff5ce [FrameworkBundle] enable metadata cache when annotation is disabled

carsonbot added Status: Needs Review FrameworkBundle labels Dec 26, 2019

bastnic force-pushed the feature/35096-dev-cache-ClassMetadataFactory branch from 07b42d9 to 2eec11b Compare December 26, 2019 01:02

nicolas-grekas added this to the next milestone Dec 26, 2019

dunglas approved these changes Dec 26, 2019

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Dec 26, 2019

bastnic force-pushed the feature/35096-dev-cache-ClassMetadataFactory branch from 2eec11b to 536338a Compare December 27, 2019 00:15

bastnic force-pushed the feature/35096-dev-cache-ClassMetadataFactory branch from 536338a to 5332498 Compare December 27, 2019 00:45

fabpot approved these changes Jan 7, 2020

View reviewed changes

fabpot merged commit 5332498 into symfony:master Jan 7, 2020

bastnic deleted the feature/35096-dev-cache-ClassMetadataFactory branch January 7, 2020 19:48

nicolas-grekas mentioned this pull request Jan 7, 2020

Add a cache warmer to clear cache pools in dev api-platform/core#2650

Merged

teohhanhui reviewed Jan 7, 2020

View reviewed changes

nicolas-grekas modified the milestones: next, 5.1 May 4, 2020

guilliamxavier mentioned this pull request Jul 7, 2020

Fix author for class CachePoolClearerCacheWarmer #37517

Merged

bastnic mentioned this pull request Mar 13, 2023

[FrameworkBundle] enable metadata cache when annotation is disabled #49679

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

[FrameworkBundle] cache ClassMetadataFactory in debug #35109

[FrameworkBundle] cache ClassMetadataFactory in debug #35109

Uh oh!

bastnic commented Dec 26, 2019 •

edited

Loading

Uh oh!

dunglas commented Dec 26, 2019

Uh oh!

bastnic commented Dec 27, 2019

Uh oh!

fabpot commented Jan 7, 2020

Uh oh!

teohhanhui Jan 7, 2020

Uh oh!

nicolas-grekas Jan 7, 2020

Uh oh!

Uh oh!

	private function registerMappingFilesFromConfig(ContainerBuilder $container, array $config, callable $fileRecorder)
	{
	foreach ($config['mapping']['paths'] as $path) {
	if (is_dir($path)) {
	$this->registerMappingFilesFromDir($path, $fileRecorder);
	$container->addResource(new DirectoryResource($path, '/^$/'));
	} elseif ($container->fileExists($path, false)) {
	if (!preg_match('/\.(xml\|ya?ml)$/', $path, $matches)) {
	throw new \RuntimeException(sprintf('Unsupported mapping type in "%s", supported types are XML & Yaml.', $path));
	}
	$fileRecorder($matches[1], $path);
	} else {
	throw new \RuntimeException(sprintf('Could not open file or directory "%s".', $path));
	}
	}
	}

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

Uh oh!

[FrameworkBundle] cache ClassMetadataFactory in debug #35109

[FrameworkBundle] cache ClassMetadataFactory in debug #35109

Uh oh!

Conversation

bastnic commented Dec 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dunglas commented Dec 26, 2019

Uh oh!

bastnic commented Dec 27, 2019

Uh oh!

fabpot commented Jan 7, 2020

Uh oh!

teohhanhui Jan 7, 2020

Choose a reason for hiding this comment

Uh oh!

nicolas-grekas Jan 7, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bastnic commented Dec 26, 2019 •

edited

Loading