Use a better container cache key

Created on 26 February 2025, about 1 month ago

Problem/Motivation

At the moment our container cache key is constructed from
$parts = ['service_container', $this->environment, \Drupal::VERSION, Settings::get('deployment_identifier'), PHP_OS, serialize(Settings::get('container_yamls'))];

This is great for core because it means that any updates that introduce new services just work. But it is painful for contrib because it has to always add empty updates to indicate that a container rebuild is necessary.

Steps to reproduce

Update any contrib module.

Proposed resolution

I think we can do better and use the composer hash in the container key. This will mean more container rebuilds but things will be more reliable.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

πŸ“Œ Task
Status

Active

Version

11.0 πŸ”₯

Component

base system

Created by

πŸ‡¬πŸ‡§United Kingdom alexpott πŸ‡ͺπŸ‡ΊπŸŒ

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @alexpott
  • πŸ‡¬πŸ‡§United Kingdom alexpott πŸ‡ͺπŸ‡ΊπŸŒ
  • πŸ‡¬πŸ‡§United Kingdom alexpott πŸ‡ͺπŸ‡ΊπŸŒ

    I think we have some options to improve this. The simplest is to hash the composer installed versions raw data. This means that the container will be rebuilt on any change to composer. We could achieve the same thing if we used the lock hash but that would mean we would need to read composer.lock whereas using InstalledVersions::getAllRawData() means that PHP can opcache everything. Another option would be have a composer plugin that could write the lock hash to a PHP file for us to read - in a similar way that composer writes the installed versions.

    There are some side effects of this approach. One is any change to your composer installed versions will trigger a container rebuild even if a the module, theme or library is not Drupal installed yet. Another is because we take the root project reference into account if your root composer is checked in to git and this repo contains your custom code then any change to custom code will cause a container rebuild (if you do a composer install afterwards). For me this is a good thing.

  • πŸ‡¬πŸ‡§United Kingdom longwave UK

    Can we discover the class name of the autoloader, as that appears to contain the lockfile hash? This is presumably to work around similar problems with opcode caching.

        "content-hash": "7f3c203e5297306f9277da08961993f2",
    

    then in vendor/composer/autoload_real.php:

    class ComposerAutoloaderInit7f3c203e5297306f9277da08961993f2
    
  • Merge request !11300Use a better container cache key β†’ (Open) created by alexpott
  • πŸ‡¬πŸ‡§United Kingdom alexpott πŸ‡ͺπŸ‡ΊπŸŒ
  • πŸ‡¬πŸ‡§United Kingdom longwave UK

    The class is only used for initialization so there is nothing holding on to it after the autoloader is set up but you can find the name

    > array_filter(get_declared_classes(), fn($class) => str_starts_with($class, 'ComposerAutoloaderInit'));
    = [
        257 => "ComposerAutoloaderInit7f3c203e5297306f9277da08961993f2",
      ]
    

    This is probably faster than serialize and hash?

  • πŸ‡¬πŸ‡§United Kingdom alexpott πŸ‡ͺπŸ‡ΊπŸŒ

    @longwave this class does not always use the hash... locally on my core checkout where I work...

    array_filter(get_declared_classes(), fn($class) => str_starts_with($class, 'ComposerAutoloaderInit'));
    = [
        241 => "ComposerAutoloaderInitDrupal9",
      ]
    

    I have nfi why InitDrupal9 :D

  • πŸ‡¬πŸ‡§United Kingdom alexpott πŸ‡ͺπŸ‡ΊπŸŒ

    FWIW generating the hash takes 0.00072503089904785 Seconds

    And that suffix is not reliable... it's generated like this:

                $suffix = $config->get('autoloader-suffix');
    
                // carry over existing autoload.php's suffix if possible and none is configured
                if (null === $suffix && Filesystem::isReadable($vendorPath.'/autoload.php')) {
                    $content = (string) file_get_contents($vendorPath.'/autoload.php');
                    if (Preg::isMatch('{ComposerAutoloaderInit([^:\s]+)::}', $content, $match)) {
                        $suffix = $match[1];
                    }
                }
    
                if (null === $suffix) {
                    $suffix = $locker !== null && $locker->isLocked() ? $locker->getLockData()['content-hash'] : bin2hex(random_bytes(16));
                }
    

    See AutoloaderGenerator.php - I guess mine is a carry-over?!!?! No idea tbh.

  • πŸ‡¬πŸ‡§United Kingdom alexpott πŸ‡ͺπŸ‡ΊπŸŒ

    The reason I decided to have a look at this is because of πŸ› Updating to 8.x-2.0-beta20 from 8.x-2.0-beta19 causes WSD Active and a feeling that it is unfair that core doesn't have to have empty updates for container rebuilds while contrib does. And that difference makes things harder to explain and remember.

  • πŸ‡¬πŸ‡§United Kingdom alexpott πŸ‡ͺπŸ‡ΊπŸŒ

    Re #9 so removing vendor and regenerating gets me a class using the composer hash. But I think we've proved that that method is unreliable. I think if we want the most performant option we're going to need to have a composer plugin write this info for us into a php file.

  • πŸ‡¬πŸ‡§United Kingdom longwave UK

    Drupal9 is a remnant from #3254149: Remove config.autoloader-suffix from composer.json β†’ - but yeah if it can be overridden and isn't guaranteed to change then it's not reliable here I guess.

  • πŸ‡¬πŸ‡§United Kingdom alexpott πŸ‡ͺπŸ‡ΊπŸŒ

    Turns out writing the hash out is not tricky at all.

  • πŸ‡¬πŸ‡§United Kingdom alexpott πŸ‡ͺπŸ‡ΊπŸŒ

    I just realised we don't want the hash from the composer.lock file - that's the content hash ie. composer.json hash. What we need need is a hash of the lock file contents - ie. something that changes when a version changes. So I've updated the code to do that.

  • πŸ‡¬πŸ‡§United Kingdom alexpott πŸ‡ͺπŸ‡ΊπŸŒ
  • πŸ‡¬πŸ‡§United Kingdom longwave UK

    @joachim reminded me that this solution could also be used to solve πŸ› Wrong DRUPAL_ROOT with non-standard code structure Needs review

  • The Needs Review Queue Bot β†’ tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

    This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

    Consult the Drupal Contributor Guide β†’ to find step-by-step guides for working with issues.

  • πŸ‡¬πŸ‡§United Kingdom alexpott πŸ‡ͺπŸ‡ΊπŸŒ

    It would be great to have some reviews on this...

  • πŸ‡¬πŸ‡§United Kingdom joachim

    > public static function getCode(PackageInterface $root_package, InstalledRepositoryInterface $repository): string {

    We'll need to add a parameter to that when πŸ› Wrong DRUPAL_ROOT with non-standard code structure Needs review joins in to add its own data to the DrupalInstalled class, probably the ManageOptions object. But that's fine to happen in that other issue, as the class with this method is marked as @internal.

    > scaffold does not run if you install Drupal from core git repo

    How do we resolve this?

  • πŸ‡¬πŸ‡§United Kingdom alexpott πŸ‡ͺπŸ‡ΊπŸŒ

    > scaffold does not run if you install Drupal from core git repo

    Well this issue resolves this for this case because we're calling the preAutoload dump from core code now so it's way better. I don't think scaffolding has to run but there being a common preAutoload dump is necessary and a good thing. This issue introduces that and will make πŸ› Wrong DRUPAL_ROOT with non-standard code structure Needs review easier as you'll be able to rely on the presence of the constant.

Production build 0.71.5 2024