[FrameworkBundle] cache ClassMetadataFactory in debug #35109

bastnic · 2019-12-26T01:02:04Z

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, $
8000
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 · 2019-12-26T11:12:38Z

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 · 2019-12-27T18:39:22Z

The test failures look related.

Fixed.

fabpot · 2020-01-07T19:35:37Z

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 · 2020-01-07T22:36:26Z

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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, $ 8000 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));
	}
	}
	}

Uh oh!

[FrameworkBundle] cache ClassMetadataFactory in debug #35109

[FrameworkBundle] cache ClassMetadataFactory in debug #35109

Uh oh!

Conversation

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!