[PropertyAccess] Add PSR-6 cache

[dunglas]

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

Follow #16294

[Koc]

I'm afraid that it wouldn't work for ACCESS_TYPE_METHOD case, because constant has value 0

[dunglas]

Good catch it should be fixed now.

[Koc]

IMHO it would be fine provide more information, like ->info('Service name used for cache')->example('property_accessor.cache.apc')

[Tobion]

the example should not be included IMO as the examplified service doesn't exist. but info is indeed a good idea

[Tobion]

IMO when a doctrine cache is injected, the internal array cache should not be used.

If I already inject an in-memory cache, I probably don't want to have it cached twice in memory. Also doctrine already has an ArrayCache. For multi-caching, there is ChainCache.
The internal cache would thus only be useful when I don't have the doctrine cache library.

[Koc]

@Tobion class property could save 2 methods call: Cache::fetch and apc_fetch (and maybe Cache::doFetch and maybe other)

[dunglas]

@Tobion if we go that way, we can just remove totally the local cache array and rely only on Doctrine Cache to enable this performance optimization.
It will require the installation of an extra library (that can be preconfigured in the standard edition) but simplifies the code base and maintenance.

[Tobion]

Yes relying fully on doctrine cache is also possible and the most flexible and the most explicit.

The problem with an internal array cache like now is that it is magic from the outside. And it doesn't really work well together with another cache layer like doctrine.

[Koc]

Guys, please measure performance before and after removing internal array cache, maybe using blackfire profile. There is enough funnction calls to retrieve values from cache: fetch, doFetch, getNamespacedId, getNamespaceVersion etc. https://github.com/doctrine/cache/blob/master/lib/Doctrine/Common/Cache/CacheProvider.php

[dunglas]

Here is a benchmark (tldr use the internal cache, not Doctrine ArrayCache).

The comparison: https://blackfire.io/profiles/compare/3a1b8a29-125e-4f19-b5ba-ff0785c082fe/graph

It's more than 30% faster and consume less memory to use the internal cache than relying on the DoctrineChain. I remove this commit.

With Doctrine Cache only (internal array removed and chained ArrayCache and ApcCache):

<?php

require 'vendor/autoload.php';
require 'Foo.php';

use Doctrine\Common\Cache\ArrayCache;
use Doctrine\Common\Cache\ApcCache;
use Doctrine\Common\Cache\ChainCache;
use Symfony\Component\PropertyAccess\PropertyAccess;

$cache = new ChainCache([new ArrayCache(), new ApcCache()]);
$accessor = new PropertyAccessor(false, false, $cache);

echo 'Property Access Benchmark'.PHP_EOL;

$start = microtime(true);

for ($i = 0; $i < 10000; ++$i) {
    $foo = new Foo();
    $accessor->setValue($foo, 'bar', 'Lorem');
    $accessor->setValue($foo, 'baz', 'Ipsum');
    $accessor->getValue($foo, 'bar');
    $accessor->getValue($foo, 'baz');
}

echo 'Time: '.(microtime(true) - $start).PHP_EOL;

With internal array and Doctrine ApcCache:

<?php

require 'vendor/autoload.php';
require 'Foo.php';

use Doctrine\Common\Cache\ApcCache;
use Symfony\Component\PropertyAccess\PropertyAccess;

$cache = new ApcCache();
$accessor = new PropertyAccessor(false, false, $cache);

echo 'Property Access Benchmark'.PHP_EOL;

$start = microtime(true);

for ($i = 0; $i < 10000; ++$i) {
    $foo = new Foo();
    $accessor->setValue($foo, 'bar', 'Lorem');
    $accessor->setValue($foo, 'baz', 'Ipsum');
    $accessor->getValue($foo, 'bar');
    $accessor->getValue($foo, 'baz');
}

echo 'Time: '.(microtime(true) - $start).PHP_EOL;

[dunglas]

I've just added support for PropertyPath caching (it was the last bottleneck) and it's impressive:

35% of performance improvement just for this specific optim: https://blackfire.io/profiles/compare/4e5545b5-7537-4502-851b-baf65ad2f2e4/graph
And if we compare to Symfony 2.7.6 (before I start to optimize PropertyAccess), the Component is now 82% faster (after the first run, as the cache must be warmed): https://blackfire.io/profiles/compare/c4142593-9195-4426-806c-9a055670fada/graph

@stof you pointed out on the previous PR that PropertyPath is used outside of this component. I've looked to add factories to share the cache between components but it introduces a lof of complexity: bunch of new classes and services, complex dependency graph because of Doctrine cache... Everything done here is private, so I think we can start like that and refactor it to share the cache between components if someone need/want to work on it.

[Tobion]

How is invalidation of the cache handled? Does the user have to clear his cache manually each time an object in the property path is changed as this can change the access strategy?

[dunglas]

I'm not sure I understand your point. It caches the structure of classes and properties so the only time cache must be flushed it's when the code is modified (and it's almost always the time).

[Koc]

User should use same invalidation method like for apc class loader or validator metadata cache: clear cache after deploy.

@dunglas nice results 👍

[Tobion]

So the recommended way is to only use caching on the property accessor in production mode I assume? Would be worth to mention that in the phpdoc I think.

[dunglas]

@Tobion yes it's only for production but it's already what we suggest for other similar caches. Using the standard edition, you just need to install APC and to uncomment these lines https://github.com/symfony/symfony-standard/blob/master/app/config/config_prod.yml#L4-L14 and you get good performance for free. It's also why I think we need to keep APC services (it's user friendly).

[dunglas]

Travis failures look unrelated.

[dunglas]

There is a "drawback": it's not possible to disable the ApcAdapter if apcu is installed and enabled. Is it acceptable?
But also a benefit: event using the standalone component, the class will be as fast as possible without having to create services manually.

[mvrhov]

@dunglas: I'm not comfortable with apc being default. The cache cannot be cleared from the console this means that after each deploy the php-fpm/apache should be restarted, ...

[Koc]

@dunglas can you try temporary disable psr6-caching inside getPropertyPath method and leave only class property cache? Also you can create instance of the property accessor outside foreach - for cases when we read/write many data inside one request

[nicolas-grekas]

👍, please squash my commit :-)
Could be interesting to see the perf with PhpFilesAdapter (in another PR)

[dunglas]

@symfony/deciders can I merge this PR please (one +1 missing)?

[nicolas-grekas]

sprintf('The Symfony Cache component must be installed to use %s().', __METHOD__)

[fabpot]

👍

[dunglas]

Comments from @nicolas-grekas @fabpot fixed. Can be merged.

[nicolas-grekas]

Thank you @dunglas.