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

[RFC][Framework][Validator] Decouple validator cache from kernel.debug parameter #30290

Open
fbourigault opened this Issue Feb 18, 2019 · 4 comments

Comments

Projects
None yet
4 participants
@fbourigault
Copy link
Contributor

fbourigault commented Feb 18, 2019

Currently, validator caching is only enabled when kernel.debug is false.

if (!$container->getParameter('kernel.debug')) {
$validatorBuilder->addMethodCall('setMetadataCache', [new Reference('validator.mapping.cache.symfony')]);
}

Having the validation cache enabled while running tests would save a lot of time re-parsing validation metadata.

In my application, I added this in Kernel::process():

if ('test' === $container->getParameter('kernel.environment')) {
    $validatorBuilder = $container->getDefinition('validator.builder');
    $validatorBuilder->addMethodCall('setMetadataCache', [new Reference('validator.mapping.cache.symfony')]);
}

And the performance improvement is quite large: https://blackfire.io/profiles/compare/969f7c8d-596f-43fb-8d0a-df443b353ac3/graph

So, what about decoupling the validation metadata cache from the kernel.debug parameter to allow it being enabled in test environment?

@javiereguiluz

This comment has been minimized.

Copy link
Member

javiereguiluz commented Feb 18, 2019

@fbourigault the performance gain (-43% CPU time) is mind-blowing! Thanks for taking the time to prepare this proposal.

@xabbuh

This comment has been minimized.

Copy link
Member

xabbuh commented Feb 18, 2019

We could possibly observe similar differences for other parts of Symfony where we make use of caching (the container, Twig etc.). So I am not sure that doing a change only for constraints is the right solution or if we should rather document this and suggest to run tests with the debug flag disabled instead.

@stof

This comment has been minimized.

Copy link
Member

stof commented Feb 18, 2019

Well, to be honest, the biggest improvement I ever made for my testsuite is to disable the debug mode for all functional tests. This forced me to force clearing the cache in the PHPUnit bootstrap script to preserve DX (not requiring to clear it manually to avoid using cache from an old test run), but that's all (and here is the code: https://gist.github.com/stof/fb45ca5940159b2741081e1cb2dae3b0).
Checking cache for freshness automatically (which is the core meaning of the kernel debug mode) to avoid having to clear it manually is totally useless during a test run where we would boot the kernel hundreds of times without changing the code.

@fbourigault

This comment has been minimized.

Copy link
Contributor Author

fbourigault commented Feb 19, 2019

Thank you for the tip @stof. I switched to this ultimate optimisation for an extra 15% gain.

I wrote the following bootstrap script for Behat:

<?php
# features/bootstrap/bootstrap.php

use Symfony\Component\Config\Resource\SelfCheckingResourceChecker;
use Symfony\Component\Config\ResourceCheckerConfigCache;
use Symfony\Component\Filesystem\Filesystem;

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

$cacheDir = __DIR__ . '/../../var/cache/test';
$configCache = new ResourceCheckerConfigCache("{$cacheDir}/appAppKernelTestContainer.php", [new SelfCheckingResourceChecker()]);
if (!$configCache->isFresh()) {
    $filesystem = new Filesystem();
    $filesystem->remove($cacheDir);
}

I know this is an hacky solution, but it avoid having some cache-not-fresh issues when running Behat tests at development phase.

Did I miss something that could make my cache outdated and not refreshing?

Is there something we can do to ship this to a larger audience?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.