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

[FrameworkBundle] CacheWarmers are never executed in kernel.build_dir #50357

Closed
Okhoshi opened this issue May 17, 2023 · 1 comment · Fixed by #50391
Closed

[FrameworkBundle] CacheWarmers are never executed in kernel.build_dir #50357

Okhoshi opened this issue May 17, 2023 · 1 comment · Fixed by #50391

Comments

@Okhoshi
Copy link
Contributor

Okhoshi commented May 17, 2023

Description

While working on #50253, I realised that CacheWarmers are never executed in the kernel.build_dir (except in the case of the bug that I fixed in #50253).

For now, in a brand new Symfony project, only the compiled container is dumped in the build_dir. Looking at the documentation for kernel.build_dir (Build directory), it is described as

This directory can be used to separate read-only cache (i.e. the compiled container) from read-write cache (i.e. cache pools).

However, there's plenty of other ressources that could be considered as read-only IMHO: the router cache, Doctrine proxy classes, Symfony Config classes, translations, and probably many more.

Additionally, non-optional CacheWarmers are run during container init, in the cache_dir, which is erased if you run cache:clear command. Since the non-optional CacheWarmers are not executed in cache:clear command, because they run during container init, all the artefacts they produce are actually lost if you run cache:clear without an already existing container, which is happening on every composer install (some Doctrine CacheWarmers are concerned by this at least).

Besides, the mechanism of CacheWarmer seems a bit weird to me in some implementations of the interface. More specifically, the Symfony\Bundle\FrameworkBundle\Routing\Router that implements the WarmableInterface.

For reference, here's its implementation of the warmup function

    /**
     * @return string[] A list of classes to preload on PHP 7.4+
     */
    public function warmUp(string $cacheDir): array
    {
        $currentDir = $this->getOption('cache_dir');

        // force cache generation
        $this->setOption('cache_dir', $cacheDir);
        $this->getMatcher();
        $this->getGenerator();

        $this->setOption('cache_dir', $currentDir);

        return [
            $this->getOption('generator_class'),
            $this->getOption('matcher_class'),
        ];
    }

It seems to me that it's really error prone to have such back and forth with the cache_dir option. If someone changes the cache_dir setting of the Router, it won't be properly warmed up anymore, without any notice to the user.

So, to sum up a little bit, I can see three different issues, that may or may not be solve together.

  • build_dir is under utilised and many read-only warm ups could make use of it
  • Non-optional warmer artefacts are lost on initial cache:clear
  • WarmableInterface does not protect from misconfiguration.

I'm willing to contribute and propose a change to solve at least the first issue but would like to get some opinions on the best way to solve it beforehand :)

Example

s new --dir symfony_build_dir_problem
cd ./symfony_build_dir_problem
echo '<?php

namespace App;

use Symfony\Bundle\FrameworkBundle\Kernel\MicroKernelTrait;
use Symfony\Component\HttpKernel\Kernel as BaseKernel;

class Kernel extends BaseKernel
{
    use MicroKernelTrait;

    public function getBuildDir(): string
    {
        return $this->getProjectDir()."/var/build/".$this->environment;
    }
}' > ./src/Kernel.php
composer install -q

Then see the content of var/build/dev and var/cache/dev.

nicolas-grekas added a commit that referenced this issue Oct 17, 2023
…gument to `WarmableInterface::warmup` to warm read-only artefacts in `build_dir` (Okhoshi)

This PR was squashed before being merged into the 6.4 branch.

Discussion
----------

[FrameworkBundle][HttpKernel] Introduce `$buildDir` argument to `WarmableInterface::warmup` to warm read-only artefacts in `build_dir`

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | Fix #50357
| License       | MIT
| Doc PR        | symfony/symfony-docs#... <!-- required for new features -->

See #50357 for the details and the reproduction steps. In this PR, I also moved `ConfigBuilderCacheWarmer` to use the `buildDir` argument.

Commits
-------

8b604ff [FrameworkBundle][HttpKernel] Introduce `$buildDir` argument to `WarmableInterface::warmup` to warm read-only artefacts in `build_dir`
@dkarlovi
Copy link
Contributor

dkarlovi commented May 13, 2024

Looking at my app with build dir enabled on latest 7.0.x (and ignoring the PRs which are in progress), I can see several artifacts which seem to still be created in cache_dir even though they should be created in build_dir:

  1. var/cache/prod/doctrine/orm/default_metadata.php, the actual proxies are moved correctly with latest recipe, not sure what writes this file here
  2. var/cache/prod/live_components_twig_templates.map, this one is created in both build_dir and cache_dir for some reason (?), both files have identical content
  3. var/cache/prod/translations/, am I missing some config here?
  4. var/cache/prod/webpack_encore.cache.php - fix: correctly wire the build-time file into kernel.build_dir webpack-encore-bundle#232

Twig, Router, Validator, Serializer are AFAIK in progress or already merged so should be OK.

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