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

Add Configless Metadata Cache #675

Closed
wants to merge 3 commits into from

Conversation

alcaeus
Copy link
Member

@alcaeus alcaeus commented May 19, 2021

This PR deprecates the metadata_cache_driver config key in favour of automatic configuration. In debug mode, it creates an ArrayAdapter, meaning that the cache is only valid per request. This is usually desired in a development environment. When debug is disabled, it creates a PhpArrayAdapter, backed by an array cache. The cache is warmed up and stored in the file system. Since this information shouldn't be shared between multiple instances of the code, it is only logical to store this in the cache folder in an efficient format. This also mirrors a decision made in the ORM bundle to deprecate the config.

Note: This PR contains a commit from #674. I'll rebase this PR once the other one has been merged.

@alcaeus alcaeus added this to the 4.4.0 milestone May 19, 2021
@alcaeus alcaeus requested a review from malarzm May 19, 2021 11:13
@alcaeus alcaeus self-assigned this May 19, 2021
use function is_file;

/** @internal */
final class MetadataCacheWarmer extends AbstractPhpFileCacheWarmer
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll propose adding this to the doctrine-bridge in Symfony so we can drop this class. In the interest of "getting on with it", I decided to first add it here since this will only be in 5.4.

@@ -23,6 +23,8 @@
*
* In the process of generating hydrators the cache for all the metadata is primed also,
* since this information is necessary to build the hydrators in the first place.
*
* @internal
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided to make all these internal. Not that it makes much of a difference, but at least it communicates that users shouldn't interact with this directly.

alcaeus added 2 commits May 21, 2021 14:49
This deprecates configuring a metadata cache in favour of automatic configuration.
@alcaeus alcaeus force-pushed the improve-metadata-caching branch from 9a1d656 to 51b509f Compare May 21, 2021 12:49
protected function loadCacheDriver(string $cacheName, string $objectManagerName, array $cacheDriver, ContainerBuilder $container)
{
if (isset($cacheDriver['namespace'])) {
// @todo wrap
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a @todo here

case 'service':
$container->setAlias($cacheDriverServiceId, new Alias($cacheDriver['id'], false));

// @todo wrap conditionally
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and here

break;

default:
// @todo wrap
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and here :)

*
* @throws InvalidArgumentException
*/
protected function loadCacheDriver(string $cacheName, string $objectManagerName, array $cacheDriver, ContainerBuilder $container)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like something that would be nice to have in the bridge

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or is creating caches to be gone in next major?

@alcaeus
Copy link
Member Author

alcaeus commented Jan 12, 2023

Closing as I don't intend to continue working on this in the near future. If somebody wants to pick this up, please feel free to re-use the work I've already done.

@alcaeus alcaeus closed this Jan 12, 2023
@alcaeus alcaeus deleted the improve-metadata-caching branch January 12, 2023 08:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Usage of setMetadataCacheImpl raises deprecation warning
4 participants