[FrameworkBundle] Introduce a cache warmer for Serializer based on PhpArrayAdapter #19507

tgalopin · 2016-08-02T09:43:22Z

Q	A
Branch?	master
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	-
License	MIT
Doc PR	-

Following the cache warmer for annotations (#18533) and for the validator (#19485), this PR introduces a cache warmer for the Serializer YAML and XML metadata configuration (mainly groups).

Based on the PhpArrayAdapter, it uses the naming conventions (Resources/config/serialization) to find the files and compile them into a single PHP file stored in the cache directory. This file uses shared memory on PHP 7.

The benefit of this PR are the same than the ones of the previous PR:

serialization metadata cache can be warmed up offline
on PHP 7, there is no need for user extension to get maximum performances (ie. if you use this PR and the other one, you probably won't need to enable APCu to have great performances)
on PHP 7 again, we are not sensitive to APCu memory fragmentation
last but not least, global performance is slightly better (I get 30us per class gain in Blackfire)

As previous work on the Serializer cache system introduced issues (see 96e418a), it would be interesting to pay careful attention to the backward compatibility during the review (ping @Ener-Getick).

GuilhemN · 2016-08-02T11:34:20Z

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

+        $values = $arrayPool->getValues();
+        $adapter->warmUp($values);
+
+        foreach ($values as $k => $v) {


I guess this is only there in case we are not on a php7 env ?

Exactly :) .

I think you should add a note specifiing that this should be removed once symfony will only support php >= 7.0

This is not planned and not done anywhere else, and I think when this will happen we will have a lot of work to adapt the whole codebase anyway. I don't think it has much added value, don't you think?

It should make the transition easier but your argument is true as well so let's not do anything it is just a fallback anyway :)

GuilhemN · 2016-08-02T11:58:57Z

This looks very promising ☺ thanks !

tgalopin · 2016-08-03T12:21:03Z

This is green and ready for intensive review :) .

dunglas · 2016-08-16T12:06:50Z

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

+            $cache = new Reference('serializer.mapping.cache.symfony');
+        }
+
+        if ($cache) {


null !== $cache?

tgalopin · 2016-08-16T18:22:01Z

I updated this PR to fix the potential BC break.

tgalopin · 2016-08-17T13:09:38Z

I fixed the compatibility with older versions of the Serializer. I disabled the cache warmer for old versions of the serializer to avoid problems in cache generation.

I think this PR is reaydy for review :) .

dunglas · 2016-08-17T13:15:11Z

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

+     */
+    public function warmUp($cacheDir)
+    {
+        if (!class_exists('Symfony\Component\Serializer\Mapping\Factory\CacheClassMetadataFactory')


You can use the ::class constant for all those checks.

dunglas · 2016-08-17T14:28:22Z

src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/FrameworkExtensionTest.php

@@ -542,8 +543,18 @@ public function testObjectNormalizerRegistered()

    public function testSerializerCacheActivated()
    {
+        if (!class_exists('Symfony\Component\Serializer\Mapping\Factory\CacheClassMetadataFactory')


Can use ::class too (same in all tests).

Missed this one, thanks :)

I updated this

dunglas · 2016-08-17T14:28:48Z

👍, good work!

fabpot · 2016-08-17T17:44:05Z

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

+    {
+        if (!class_exists(CacheClassMetadataFactory::class)
+            || !method_exists(XmlFileLoader::class, 'getMappedClasses')
+            || !method_exists(YamlFileLoader::class, 'getMappedClasses')) {


should be on one line

…pArrayAdapter

tgalopin · 2016-08-18T08:41:53Z

Updated

nicolas-grekas · 2016-09-06T09:15:29Z

ping @symfony/deciders

fabpot · 2016-09-14T20:50:57Z

Thank you @tgalopin.

…zer based on PhpArrayAdapter (tgalopin) This PR was merged into the 3.2-dev branch. Discussion ---------- [FrameworkBundle] Introduce a cache warmer for Serializer based on PhpArrayAdapter | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Following the cache warmer for annotations (#18533) and for the validator (#19485), this PR introduces a cache warmer for the Serializer YAML and XML metadata configuration (mainly groups). Based on the PhpArrayAdapter, it uses the naming conventions (Resources/config/serialization) to find the files and compile them into a single PHP file stored in the cache directory. This file uses shared memory on PHP 7. The benefit of this PR are the same than the ones of the previous PR: - serialization metadata cache can be warmed up offline - on PHP 7, there is no need for user extension to get maximum performances (ie. if you use this PR and the other one, you probably won't need to enable APCu to have great performances) - on PHP 7 again, we are not sensitive to APCu memory fragmentation last but not least, global performance is slightly better (I get 30us per class gain in Blackfire) As previous work on the Serializer cache system introduced issues (see 96e418a), it would be interesting to pay careful attention to the backward compatibility during the review (ping @Ener-Getick). Commits ------- 810f469 [FrameworkBundle] Introduce a cache warmer for Serializer based on PhpArrayAdapter

carsonbot added Status: Needs Review FrameworkBundle Bug Feature BC Break Deprecation labels Aug 2, 2016

tgalopin changed the title ~~[FrameworkBundle] Introduce a cache warmer for Serializer based on PhpArrayAdapter~~ [WIP][FrameworkBundle] Introduce a cache warmer for Serializer based on PhpArrayAdapter Aug 2, 2016

tgalopin force-pushed the php-array-serializer branch from 9f42021 to 8f29578 Compare August 2, 2016 09:50

nicolas-grekas removed BC Break Bug Deprecation labels Aug 2, 2016

nicolas-grekas changed the title ~~[WIP][FrameworkBundle] Introduce a cache warmer for Serializer based on PhpArrayAdapter~~ [FrameworkBundle] Introduce a cache warmer for Serializer based on PhpArrayAdapter Aug 2, 2016

tgalopin force-pushed the php-array-serializer branch 3 times, most recently from f336e86 to 578d101 Compare August 2, 2016 10:54

GuilhemN reviewed Aug 2, 2016
View reviewed changes

tgalopin force-pushed the php-array-serializer branch 7 times, most recently from 140ef34 to 3dd2989 Compare August 3, 2016 09:24

tgalopin force-pushed the php-array-serializer branch from 3dd2989 to cb0f550 Compare August 3, 2016 13:06

dunglas reviewed Aug 16, 2016
View reviewed changes

tgalopin force-pushed the php-array-serializer branch 4 times, most recently from 1a318f0 to e9bd2d8 Compare August 16, 2016 17:10

tgalopin force-pushed the php-array-serializer branch 4 times, most recently from 1a6a85d to 38ab487 Compare August 17, 2016 12:54

dunglas reviewed Aug 17, 2016
View reviewed changes

tgalopin force-pushed the php-array-serializer branch from 38ab487 to 37fbb30 Compare August 17, 2016 13:25

dunglas reviewed Aug 17, 2016
View reviewed changes

tgalopin force-pushed the php-array-serializer branch from 37fbb30 to a83ff96 Compare August 17, 2016 14:34

fabpot reviewed Aug 17, 2016
View reviewed changes

[FrameworkBundle] Introduce a cache warmer for Serializer based on Ph…

810f469

…pArrayAdapter

tgalopin force-pushed the php-array-serializer branch from a83ff96 to 810f469 Compare August 17, 2016 18:09

fabpot merged commit 810f469 into symfony:master Sep 14, 2016

tgalopin deleted the php-array-serializer branch September 15, 2016 13:06

fabpot mentioned this pull request Oct 27, 2016

Release v3.2.0-BETA1 #20317

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FrameworkBundle] Introduce a cache warmer for Serializer based on PhpArrayAdapter #19507

[FrameworkBundle] Introduce a cache warmer for Serializer based on PhpArrayAdapter #19507

tgalopin commented Aug 2, 2016 •

edited

Loading

GuilhemN Aug 2, 2016

tgalopin Aug 2, 2016

GuilhemN Aug 2, 2016

tgalopin Aug 2, 2016

GuilhemN Aug 2, 2016

GuilhemN commented Aug 2, 2016

tgalopin commented Aug 3, 2016

dunglas Aug 16, 2016

tgalopin commented Aug 16, 2016

tgalopin commented Aug 17, 2016 •

edited

Loading

dunglas Aug 17, 2016

dunglas Aug 17, 2016

tgalopin Aug 17, 2016

tgalopin Aug 17, 2016

dunglas commented Aug 17, 2016

fabpot Aug 17, 2016

tgalopin commented Aug 18, 2016

nicolas-grekas commented Sep 6, 2016

fabpot commented Sep 14, 2016

[FrameworkBundle] Introduce a cache warmer for Serializer based on PhpArrayAdapter #19507

[FrameworkBundle] Introduce a cache warmer for Serializer based on PhpArrayAdapter #19507

Conversation

tgalopin commented Aug 2, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GuilhemN commented Aug 2, 2016

tgalopin commented Aug 3, 2016

Choose a reason for hiding this comment

tgalopin commented Aug 16, 2016

tgalopin commented Aug 17, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dunglas commented Aug 17, 2016

Choose a reason for hiding this comment

tgalopin commented Aug 18, 2016

nicolas-grekas commented Sep 6, 2016

fabpot commented Sep 14, 2016

tgalopin commented Aug 2, 2016 •

edited

Loading

tgalopin commented Aug 17, 2016 •

edited

Loading