Add a api_platform.metadata_cache parameter #1725

dunglas · 2018-02-22T17:07:44Z

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	n/a
License	MIT
Doc PR	todo

TLDR:

Add the following in config/packages/test/framework.yaml to dramatically improve the speed of Behat tests:

parameters:
   api_platform.metadata_cache: true

Follows #1724 (comment)

TODO: fix tests

It's significant on my computer:

Before

Behat: 2m18.08s (2.16Gb)
PHPUnit: Time: 21.17 seconds, Memory: 104.25MB

After

Behat: 0m42.23s (498.70Mb)
Time: 25.72 seconds, Memory: 102.25MB

Can you try with your code base @jdeniau?

soyuka · 2018-02-22T17:18:26Z

tests/Fixtures/app/config/config_test.yml


 services:
+


could you remove this line :P

soyuka · 2018-02-22T17:19:33Z

src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php

@@ -457,7 +457,11 @@ private function registerBundlesConfiguration(array $bundles, array $config, Xml
    private function registerCacheConfiguration(ContainerBuilder $container)
    {
        // Don't use system cache pool in dev
-        if (!$container->getParameter('kernel.debug')) {
+        if ($container->hasParameter('api_platform.metadata_cache')) {
+           if (!$container->getParameter('api_platform.metadata_cache'))  {


can't we merge these?

jdeniau · 2018-02-22T22:22:04Z

@dunglas I'm running test at this very moment, but I think it will do the trick for us as that was the return; in registerCacheConfiguration that was very long.

There are two things that I see that is a little "blocking":

👍 for api-platform DX, 👎 for the others (ok "=" for the others, but there are no emoji for that 😃)

The fact that the user need to set the api_platform.metadata_cache parameter to true.
Of course, it's set for api-platorm, so 👍 for it, but even if documented, there may be a lot of user not setting it.
I'm wondering why not auto-activate it if kernel env is "test" ? ~~wich does lead to the second code snippet in #1724 (comment) and to the following point~~

cache invalidation

~~If I remember correctly, the second code snippet in FeatureContext is done because Symfony's cache is written only when the cache is cleared, not between two different behat runs~~
I think that my issue was because I deactivated kernel.debug, but with the metadata_cache parameter, it seems to work file ! 🥇

After reading your code and making some test, I'm now wondering why you register all the caches as ArrayAdapter in debug mode instead of letting the cache.pool do it's job ? (I do not know this part of Symfony very well though)

jdeniau · 2018-02-22T22:53:55Z

The tests finished to run on our server ~~and I'm a little disappointed by the results~~
For the record I ran the tests on our CI server, which is very busy at night with database replication and stuff like that. I will re-run the test, tomorrow or monday to give you "real day-to-day" data

(I obfuscated the tests name for confidentiality reason)

Without optimization:

- 69 shuffled test classes into the queue.
- Will be consumed by 8 parallel Processes.
1	1/69	✔	1 s 250 ms	features/v.feature
7	2/69	✔	4 s 288 ms	features/w.feature
1	3/69	✔	2 min 27 s 196 ms	features/c.feature
7	4/69	✔	3 min 37 s 344 ms	features/o.feature
8	5/69	✔	4 min 18 s 4 ms	features/t.feature
6	6/69	✔	8 min 23 s 357 ms	features/p.feature

And then jenkins crashed 😉

With the `metadata_cache` set to true

(I ran only the 6 tests)

- 6 shuffled test classes into the queue.
- Will be consumed by 8 parallel Processes.
3	1/6	✔	1 s 232 ms	features/v.feature
2	2/6	✔	2 s 920 ms	features/w.feature
1	3/6	✔	1 min 9 s 706 ms	features/c.feature
5	4/6	✔	1 min 46 s 215 ms	features/o.feature
4	5/6	✔	2 min 2 s 674 ms	features/t.feature
6	6/6	✔	3 min 34 s 540 ms	features/p.feature

The win is about 50%, which is not bad

With kernel.debug set to false

- 6 shuffled test classes into the queue.
- Will be consumed by 8 parallel Processes.
6	1/6	✔	1 s 66 ms	features/v.feature
3	2/6	✔	2 s 783 ms	features/w.feature
1	3/6	✔	1 min 14 s 787 ms	features/c.feature
5	4/6	✔	1 min 52 s 721 ms	features/o.feature
4	5/6	✔	2 min 10 s 269 ms	features/t.feature
2	6/6	✔	3 min 47 s 400 ms	features/p.feature

Nearly the same time that with metadata_cache set to true

For the record, the "p.feature" file usually take 40 seconds to run, that's why I was disappointed ;)

I ran the o.feature test only on ouy development server and it took ~ 15 sec / 500 Mb with kernel.debug false or metadata_cache true and ~ 85 sec / 1.5 Gb without optimizations so big 👍 no matter what on the config point

soyuka · 2018-02-23T09:37:23Z

src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php

-        if (!$container->getParameter('kernel.debug')) {
-            return;
+        if ($container->hasParameter('api_platform.metadata_cache') ? $container->getParameter('api_platform.metadata_cache') : !$container->getParameter('kernel.debug')) {
+               return;


indent fail on the return?

meyerbaptiste · 2018-03-06T15:32:59Z

tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php

@@ -44,6 +44,7 @@
 use Prophecy\Argument;
 use Prophecy\Exception\Doubler\MethodNotFoundException;
 use Symfony\Bundle\SecurityBundle\SecurityBundle;
+use Symfony\Component\Cache\Adapter\ArrayAdapter;


teohhanhui · 2018-03-15T14:39:01Z

Not sure this is the right way to do it, or that the naming is obvious enough...

Backport #1725 into 2.2 branch.

bendavies · 2019-02-16T16:07:02Z

just a hint that this extremely useful parameter does not seem to be documented, and this comes up on slack at least a few times a week.

soyuka reviewed Feb 22, 2018

View reviewed changes

dunglas force-pushed the add-api_platform.metadata_cache branch from 16cfb89 to 8d3c291 Compare February 22, 2018 21:07

soyuka reviewed Feb 23, 2018

View reviewed changes

dunglas force-pushed the add-api_platform.metadata_cache branch from 7b2585f to 54ef999 Compare February 23, 2018 11:06

dunglas force-pushed the add-api_platform.metadata_cache branch from 54ef999 to b4c4023 Compare March 5, 2018 08:57

meyerbaptiste approved these changes Mar 6, 2018

View reviewed changes

Add a api_platform.metadata_cache parameter

52a3a32

dunglas force-pushed the add-api_platform.metadata_cache branch from b4c4023 to 52a3a32 Compare March 15, 2018 12:58

dunglas merged commit 97616e2 into api-platform:master Mar 15, 2018

dunglas deleted the add-api_platform.metadata_cache branch March 15, 2018 13:43

This was referenced Apr 6, 2018

Milestone for the next release? #1818

Closed

Backport #1725 into 2.2 branch. #1827

Merged

dunglas added a commit that referenced this pull request Apr 9, 2018

Merge pull request #1827 from Zarthus/backport-2.2-issue-1725

905ac01

Backport #1725 into 2.2 branch.

Simperfit mentioned this pull request May 2, 2018

Slow API requests in dev env when upgrading from 2.1.3 to ^2.1.4 #1842

Closed

teohhanhui mentioned this pull request May 29, 2018

Reflexion Docblock is very costly for a very small gain #1981

Closed

soyuka mentioned this pull request Sep 17, 2018

Extremely slow DIC rebuild with api-platform enabled api-platform/api-platform#838

Closed

soyuka mentioned this pull request Feb 19, 2019

Document metadata cache parameter api-platform/docs#733

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Add a api_platform.metadata_cache parameter #1725

Add a api_platform.metadata_cache parameter #1725

Uh oh!

dunglas commented Feb 22, 2018 •

edited

Loading

Uh oh!

soyuka Feb 22, 2018

Uh oh!

soyuka Feb 22, 2018

Uh oh!

jdeniau commented Feb 22, 2018

Uh oh!

jdeniau commented Feb 22, 2018

Uh oh!

soyuka Feb 23, 2018

Uh oh!

meyerbaptiste Mar 6, 2018

Uh oh!

teohhanhui commented Mar 15, 2018

Uh oh!

bendavies commented Feb 16, 2019

Uh oh!

Uh oh!

Uh oh!

Add a api_platform.metadata_cache parameter #1725

Add a api_platform.metadata_cache parameter #1725

Uh oh!

Conversation

dunglas commented Feb 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

soyuka Feb 22, 2018

Choose a reason for hiding this comment

Uh oh!

soyuka Feb 22, 2018

Choose a reason for hiding this comment

Uh oh!

jdeniau commented Feb 22, 2018

👍 for api-platform DX, 👎 for the others (ok "=" for the others, but there are no emoji for that 😃)

cache invalidation

Uh oh!

jdeniau commented Feb 22, 2018

Without optimization:

With the metadata_cache set to true

With kernel.debug set to false

Uh oh!

soyuka Feb 23, 2018

Choose a reason for hiding this comment

Uh oh!

meyerbaptiste Mar 6, 2018

Choose a reason for hiding this comment

Uh oh!

teohhanhui commented Mar 15, 2018

Uh oh!

bendavies commented Feb 16, 2019

Uh oh!

Uh oh!

dunglas commented Feb 22, 2018 •

edited

Loading

With the `metadata_cache` set to true