Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Metadata cache uses ArrayAdapter instead of APCu #4975

Closed
usu opened this issue Sep 18, 2022 · 28 comments
Closed

Metadata cache uses ArrayAdapter instead of APCu #4975

usu opened this issue Sep 18, 2022 · 28 comments
Labels

Comments

@usu
Copy link
Contributor

usu commented Sep 18, 2022

API Platform version(s) affected: 3.0.0

Description
3.0 uses ArrayAdapter as Metadata Cache. In 2.7 the default was to utilize APCu when it was installed.

Was this changed by purpose?

--> If yes, I couldn't find this documented. The docu still says:

If the Symfony Cache component is available (the default in the API Platform distribution), it automatically enables support for the best cache adapter available.

--> If no, I think the following commit introduced the issue:
c76720e

Or do I need to change any config on my side during 3.0 upgrade?

How to reproduce
Upgrade to 3.0
Check symfony profiler for cache hit rate, which is significantly lower than before, as no Metadata is cached between separate requests.

@emmanuelballery
Copy link
Contributor

Hi!

registerCacheConfiguration in the ApiPlatformExtension is indeed breaking the main caching abilities of our apps.

private function registerCacheConfiguration(ContainerBuilder $container): void
{
if (!$container->hasParameter('kernel.debug') || !$container->getParameter('kernel.debug')) {
$container->removeDefinition('api_platform.cache_warmer.cache_pool_clearer');
}
$container->register('api_platform.cache.metadata.property', ArrayAdapter::class);
$container->register('api_platform.cache.metadata.resource', ArrayAdapter::class);
$container->register('api_platform.cache.metadata.resource_collection', ArrayAdapter::class);
$container->register('api_platform.cache.route_name_resolver', ArrayAdapter::class);
$container->register('api_platform.cache.identifiers_extractor', ArrayAdapter::class);
$container->register('api_platform.elasticsearch.cache.metadata.document', ArrayAdapter::class);
}

In prod we lost the filesystem cache. ArrayAdapter is not enough. Removing those lines from ApiPlatformExtension is fixing this caching mecanism because config/metadata/resource.xml is referencing the cache.system (adapter.filesystem in prod).

<service id="api_platform.cache.metadata.resource_collection" parent="cache.system" public="false">
<tag name="cache.pool" />
</service>

For big apps, dev is really slow, as each required metadata is created once for each request, adding seconds to the clock. Opening Swagger UI is not possible (well xdebug stops us at 30 secondes) and exporting the full openapi schema is... difficult.

@alanpoulain I'm really sorry to bother you. Since you worked on this, would you be king enough to take a look when you'll have the time and maybe guide us to help fix this.

Thank you!

@acirulis
Copy link
Contributor

acirulis commented Sep 21, 2022

I have successfully migrated first project from 2.6 to 3.0 (Nice first impressions 👍) - however I immediately spotted significant drop in performance. Request, which took under 500ms now takes >1500ms on (ENV=dev, DEBUG=true) and also takes 400ms vs 200ms on (ENV=prod, DEBUG=false).
I am using Redis Cache adapter for Symfony, no changes in configuration between 2.6 and 3.0.

It seems that changes mentioned by @emmanuelballery - registerCacheConfiguration in the ApiPlatformExtension helps to bring back performance to normal.

@webda2l
Copy link
Contributor

webda2l commented Sep 21, 2022

I have successfully migrated first project from 2.6 to 3.0 (Nice first impressions +1) - however I immediately spotted significant drop in performance. Request, which took under 500ms now takes >1500ms on (ENV=dev, DEBUG=true) and also takes 400ms vs 200ms on (ENV=prod, DEBUG=false). I am using Redis Cache adapter for Symfony, no changes in configuration between 2.6 and 3.0.

It seems that changes mentioned by @emmanuelballery - registerCacheConfiguration in the ApiPlatformExtension helps to bring back performance to normal.

In dev, https://symfony.com/blog/new-in-symfony-6-1-serializer-profiling come at cost but there is definitely a performance issue related to the cache with the 3.0.

@acirulis
Copy link
Contributor

Oh nice, @webda2l - since I started project with 6.0, after upgrade to 6.1 Serializer Profiling was disabled by default. After your mention I enabled it & it will be great tool for debugging. Performance impact is minimal.

@emmanuelballery
Copy link
Contributor

Meanwhile, you can avoid this performance issue by overriding api platform cache in your services.yaml:

services:
    api_platform.cache.metadata.property:
        parent: cache.system
        tags: [ cache.pool ]
    api_platform.cache.metadata.resource:
        parent: cache.system
        tags: [ cache.pool ]
    api_platform.cache.metadata.resource_collection:
        parent: cache.system
        tags: [ cache.pool ]
    api_platform.cache.route_name_resolver:
        parent: cache.system
        tags: [ cache.pool ]
    api_platform.cache.identifiers_extractor:
        parent: cache.system
        tags: [ cache.pool ]
    api_platform.elasticsearch.cache.metadata.document:
        parent: cache.system
        tags: [ cache.pool ]

@stale
Copy link

stale bot commented Nov 20, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Nov 20, 2022
@MrSpider
Copy link

The service api_platform.metadata.property.identifier_metadata_factory which is suggested to be removed in 3.0 by a TODO comment (TODO: in 3.0 the serializer property metadata factory doesn't need the resource metadata anymore so these will be removed) has no cache associated with it.

After adding a simple array cache for this service the performance significantly improves when the metadata caches are rebuilt.

# Speeds up api-platform cache build substantially
# May have to be removed soon when api-platform removes these services
# see vendor/api-platform/core/src/Symfony/Bundle/Resources/config/metadata/property.xml:41
api_platform.cache.metadata.identifier:
    parent: cache.adapter.array
    tags: [ cache.pool ]
api_platform.metadata.property.identifier_metadata_factory.cached:
    class: ApiPlatform\Metadata\Property\Factory\CachedPropertyMetadataFactory
    decorates: api_platform.metadata.property.identifier_metadata_factory
    decoration_priority: 99
    arguments:
        - '@api_platform.cache.metadata.identifier'
        - '@api_platform.metadata.property.identifier_metadata_factory.cached.inner'

@soyuka
Copy link
Member

soyuka commented Nov 28, 2022

Is there something I can do to help with this?

@acirulis
Copy link
Contributor

Ok, I think we got to the bottom of this - back in 2016 there was PR introducing ArrayAdapter for dev environment ( #653 ) because Symfony 2.x required to manually clear cache after every configuration change. Current version of Symfony does it automatically and it would be ok to remove Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php:registerCacheConfiguration() method altogether and rely on framework cache config.

@soyuka
Copy link
Member

soyuka commented Nov 28, 2022

Nice @acirulis do you have some time to open a patch?

Shouldn't we also fix our services to inline with:

services:
    api_platform.cache.metadata.property:
        parent: cache.system
        tags: [ cache.pool ]
    api_platform.cache.metadata.resource:
        parent: cache.system
        tags: [ cache.pool ]
    api_platform.cache.metadata.resource_collection:
        parent: cache.system
        tags: [ cache.pool ]
    api_platform.cache.route_name_resolver:
        parent: cache.system
        tags: [ cache.pool ]
    api_platform.cache.identifiers_extractor:
        parent: cache.system
        tags: [ cache.pool ]
    api_platform.elasticsearch.cache.metadata.document:
        parent: cache.system
        tags: [ cache.pool ]

@webda2l
Copy link
Contributor

webda2l commented Nov 28, 2022

Ok, I think we got to the bottom of this - back in 2016 there was PR introducing ArrayAdapter for dev environment ( #653 ) because Symfony 2.x required to manually clear cache after every configuration change. Current version of Symfony does it automatically and it would be ok to remove Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php:registerCacheConfiguration() method altogether and rely on framework cache config.

👍 I disabled this line two month ago on a fork without issue on prod or dev. OLYBe@8e95a09

@k0d3r1s
Copy link

k0d3r1s commented Nov 28, 2022

@soyuka that inline service configuration is not needed anymore if you remove this code in php code. this re-configuration just acts as fix for what happens in php code

@acirulis
Copy link
Contributor

ok @soyuka, will do a PR.

@soyuka
Copy link
Member

soyuka commented Nov 30, 2022

thanks to everyone investing that! It'll be available in the next patch release.

@soyuka
Copy link
Member

soyuka commented Dec 9, 2022

https://github.com/api-platform/core/releases/tag/v3.0.7 is out with the patch thanks everyone

@MariusJam
Copy link
Contributor

Hi,

Even after upgrade to 3.0.7, there is no major change for us. Cache regeneration is still very long on our dev environment. A single change in an ApiResource leads to a cache regeneration taking more than one minute (our app being quite heavy). This makes development quite painful...

On 2.6, cache regeneration was already quite long (~30s) but this is more than the double now.

First cache generation seems much faster though, it looks like it is really with regeneration that there is an issue.

Not sure how I can help solve this ?

@k0d3r1s
Copy link

k0d3r1s commented Dec 14, 2022

@MariusJam do you have cache in your project configured to use pools, or maybe at least redis/apc? I find the default symfony cache config not really friendly with api-platform.

@MariusJam
Copy link
Contributor

Hello @k0d3r1s,

We have ACPU installed and have some custom use cases for it. However, we didn't change the default API-P configuration for :

  • api_platform.cache.metadata.property
  • api_platform.cache.metadata.resource
  • api_platform.cache.metadata.resource_collection

AFAIK they use the defaut symfony cache.system pool and I'm a bit hesitant to change the adapter used by cache.system as the Symfony documentation states that :

While it is possible to reconfigure the system cache, it's recommended to keep the default configuration applied to it by Symfony.

=> https://symfony.com/doc/current/cache.html
=> symfony/symfony-docs#12774

@gnito-org
Copy link

What is your output of the command:

./bin/console debug:container api_platform.cache.metadata.resource_collection

@simpletoimplement
Copy link

just tried 3.0.7 version after upgrade from 3.0.5 and encountered same slowness.
but after running bin/console cache:pool:prune to prune all old cache, everything started to work much faster

@acirulis
Copy link
Contributor

@MariusJam out of curiosity, when you are saying "our app being quite heavy" - how many endpoints/operations/properties are you talking about?

@MariusJam
Copy link
Contributor

MariusJam commented Dec 15, 2022

Hi all,

Thanks for your inputs, I answer below:

What is your output of the command:

./bin/console debug:container api_platform.cache.metadata.resource_collection

Information for Service "api_platform.cache.metadata.resource_collection"
=========================================================================

 An adapter that collects data about all cache calls.

 ---------------- -------------------------------------------------------------------------------------------------------------------------------------------------------- 
  Option           Value                                                                                                                                                   
 ---------------- -------------------------------------------------------------------------------------------------------------------------------------------------------- 
  Service ID       api_platform.cache.metadata.resource_collection                                                                                                         
  Class            Symfony\Component\Cache\Adapter\TraceableAdapter                                                                                                        
  Tags             cache.pool                                                                                                                                              
                   kernel.reset (method: reset)                                                                                                                            
  Public           no                                                                                                                                                      
  Synthetic        no                                                                                                                                                      
  Lazy             no                                                                                                                                                      
  Shared           yes                                                                                                                                                     
  Abstract         no                                                                                                                                                      
  Autowired        no                                                                                                                                                      
  Autoconfigured   no                                                                                                                                                      
  Usages           services_resetter, cache.system_clearer, cache.global_clearer, data_collector.cache, api_platform.metadata.resource.metadata_collection_factory.cached  
 ---------------- -------------------------------------------------------------------------------------------------------------------------------------------------------- 


 ! [NOTE] The "api_platform.cache.metadata.resource_collection" service or alias has been removed or inlined when the   
 !        container was compiled.

just tried 3.0.7 version after upgrade from 3.0.5 and encountered same slowness. but after running bin/console cache:pool:prune to prune all old cache, everything started to work much faster

Yes, first regeneration after cache:pool:prune is a bit faster here (~35s) but next regenerations are as slow as before (~55s)

@MariusJam out of curiosity, when you are saying "our app being quite heavy" - how many endpoints/operations/properties are you talking about?

Mmm ~70 ApiResources and ~200 operations. Properties: way too much to be counted.

@gnito-org
Copy link

My app has 90 API resources and more than 200 operations.

I have the explicit cache config active.

So, when I do:

  1. rm -rf var/cache/*
  2. Refresh https://127.0.0.1:8000/api

It takes roughly 13 seconds to refresh.

@MrSpider
Copy link

The situation I described in #4975 (comment) is still present in 3.0.7. I profiled a whole cache build to find out that the the service api_platform.metadata.property.identifier_metadata_factory (defined in Symfony/Bundle/Resources/config/metadata/property.xml) is directly used in the LinkFactory without the usage of any cache and gets therefore called a lot for each resource doing the same work over and over again.

Our cache build times improved from 50 seconds to about 8 seconds after adding the cache decorator I described earlier.

@simpletoimplement
Copy link

simpletoimplement commented Dec 15, 2022

i would guess that there are multiple issues here surrounding cache.
3.0.7. vastly improved performance for me. @MrSpider your decorator doesn't do much for me, for example. looks like it depends on use cases here.
how about making a fix for your case as PR? api-platform should have code fix for these problems and not by adding some random cache pool configs or decorators. it is great that workarounds exist but they are kinda bound to specific github issues and not documented anywhere, so probably there are a lot of people who struggle with these issues and don't visit github, or don't go through open issues

@MariusJam
Copy link
Contributor

MariusJam commented Dec 16, 2022

@gnito-org I've tried the explicit config as well but that doesn't change anything in our case. If I'm correct this is actually normal as this explicit config is the default config in 3.0.7.

@MrSpider Thanks !! That helps a lot in our case. I've used cache.system rather than cache.adapter.array and I'm getting even better performances. Cache regeneration takes now ~18s here, this is a major improvement.

@MrSpider
Copy link

@MariusJam Be careful when using anything other than a array cache. AFAIK Using a cache other than array without telling the service api_platform.cache_warmer.cache_pool_clearer about that new cache could lead to cases where some file changes in dev environment are not registered without a manual cache clear.

@soyuka
Copy link
Member

soyuka commented Dec 17, 2022

don't hesitate to open a new pr or an issue if we can improve something!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

10 participants