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

Undo deprecation of metadata_cache_driver configuration #1393

Closed
ruudk opened this issue Aug 29, 2021 · 5 comments
Closed

Undo deprecation of metadata_cache_driver configuration #1393

ruudk opened this issue Aug 29, 2021 · 5 comments
Milestone

Comments

@ruudk
Copy link
Contributor

ruudk commented Aug 29, 2021

After upgrading from 2.2 to 2.3 we noticed that the metadata_cache_driver option was deprecated. We removed the configuration key to get rid of the deprecation warning. We didn't think about it but after a while we noticed that our application started to slow down significantly. After investigation we noticed it was related to this metadata cache.

We are running a very large (9 years old) Symfony 5.3 application with almost 10.000 PHP files. The metadata caching allowed us to reduce our load times. We don't make changes to your entities that often as compared to other code changes. By having the metadata cache enabled we shave 100ms off our request load times.

I created a PR to bring back some of the functionality that we need but it was rejected:

Following up on @ostrolucky comment here:

We may consider undeprecating the option if there is enough interest, but since you are the only one speaking out, that's not enough. Since you have such mature project and complex workflow involving file watchers, you may as well write a compiler pass adjusting metadata cache. My experience speak against having persistent metadata cache in dev environment, it was constant struggle with changes not having any effect and having to nuke whole cache manually. Create an issue if interested and let's see if we get more feedback.

So this issue is to see if more people are using the metadata caching driver and want to keep using it.

I'd also like to know more about the reasoning and urge to deprecate the metadata caching configuration. Is there an issue/PR that explains the reasoning?

@ruudk ruudk changed the title Undo deprecation of metadata driver configuration Undo deprecation of metadata_cache_driver configuration Aug 29, 2021
@dmaicher
Copy link
Contributor

I'd also like to know more about the reasoning and urge to deprecate the metadata caching configuration. Is there an issue/PR that explains the reasoning?

There is multiple PRs about this topic. Here the original PR that introduced the new caching: #1196 (comment)

So the idea was that independent of the metadata_cache_driver configuration we would use the fastest cache available when in no-debug mode which would be the new PhpArrayAdapter cache.

Later on while providing compatibility with PSR-6 caches @alcaeus changed the approach slightly here.
So now we actually only register the fast PhpArrayAdapter if there is no other cache configured and we are in no-debug mode.

From my point of view there are not many use-cases where one would like to have a persistent cache while being in debug mode.

@ruudk your use-case could be handled with a custom compiler pass, right? Just wire the cache services yourself.

I don't have a strong opinion on this topic though. I would also be fine with reverting the deprecation and keeping the config to have full flexibility.

@ostrolucky
Copy link
Member

Ok. If someone wants to do the work for undeprecating it and create a PR, I'm ok with merging it. I think that in the end, we should try to avoid deprecating options that are not deprecated in ORM/DBAL.

@ruudk
Copy link
Contributor Author

ruudk commented Oct 5, 2021

@ostrolucky Thanks, I appreciate it. I will create the initial PR. But I might need some guidance in doing it right.

ruudk added a commit to ruudk/DoctrineBundle that referenced this issue Oct 5, 2021
This allows projects to still choose to opt-in on caching the metadata.

When the configuration key is not defined or used, the cache is created automatically.

See doctrine#1393
@ruudk
Copy link
Contributor Author

ruudk commented Oct 5, 2021

@ostrolucky Created a draft PR to undo the deprecation. Could you approve the tests so that we can see how it works?

ruudk added a commit to ruudk/DoctrineBundle that referenced this issue Oct 5, 2021
This allows projects to still choose to opt-in on caching the metadata.

When the configuration key is not defined or used, the cache is created automatically.

See doctrine#1393
ruudk added a commit to ruudk/DoctrineBundle that referenced this issue Oct 5, 2021
This allows projects to still choose to opt-in on caching the metadata.

When the configuration key is not defined or used, the cache is created automatically.

See doctrine#1393
@ostrolucky ostrolucky added this to the 2.5.0 milestone Oct 6, 2021
@ruudk
Copy link
Contributor Author

ruudk commented Oct 8, 2021

Thanks for merging #1405.

I ran 2 Blackfire profiles (with 10 samples):

A: No metadata cache driver configured, and thus the smart cache warmer is enabled in debug mode
B: Metadata cache driver configured that caches on file system.

I get a 20% performance improvement (on every request) with version B 🎉

Screenshot 2021-10-08 at 10 10 18@2x

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

No branches or pull requests

3 participants