Problem/Motivation

We rebuild a container after every single module install. Perhaps we can do better.

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#12 interdiff-3185978-12.txt1.64 KBdawehner
#12 3185978-12.patch23.42 KBdawehner
#10 interdiff-3185978.txt903 bytesdawehner
#10 3185978-10.patch23.37 KBdawehner
#8 3185978-8.patch23 KBalexpott
#8 5-8-interdiff.txt5.41 KBalexpott
#5 3185978-5.patch18.12 KBalexpott
#5 2-5-interdiff.txt4.27 KBalexpott
#2 3185978-2.patch18.09 KBalexpott
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alexpott created an issue. See original summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Active » Needs review
FileSize
3185978-2.patch18.09 KB

This is very experimental but it halves the time to do a standard install from the command line. Some stuff will be broken as ConfigInstaller::checkConfigurationToInstall() is not firing but that should only affect a few tests. Let's see...

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented

On a standard non interactive install (ie. via drush, quickstart, or DrupalCI) this patch results in 39 less container rebuilds

Status: Needs review » Needs work

The last submitted patch, 2: 3185978-2.patch, failed testing. View results

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Needs work » Needs review
FileSize
2-5-interdiff.txt4.27 KB
3185978-5.patch18.12 KB

This fixes InstallUninstallTest... a green version of that is a good sign.

Status: Needs review » Needs work

The last submitted patch, 5: 3185978-5.patch, failed testing. View results

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented

DrupalCi run 49 mins... compared to 57 mins on a recent run. I did expect test installation to speed up too because this affects the number container rebuilds done during \Drupal\Core\Test\FunctionalTestSetupTrait::installModulesFromClassProperty() too.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Needs work » Needs review
FileSize
5-8-interdiff.txt5.41 KB
3185978-8.patch23 KB

A little bit of progress to fix the config installer issue.

Status: Needs review » Needs work

The last submitted patch, 8: 3185978-8.patch, failed testing. View results

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented
Status: Needs work » Needs review
FileSize
3185978-10.patch23.37 KB
interdiff-3185978.txt903 bytes

I investigated two of the failures right now:

1. testInstallProfileConfigOverwrite Long story short, ... when determining, whether the module to install has conflicting configuration, we create a storage object for checking different collections ("", language.en etc.). This operation throws away the $folders information in InstallerStorage
2. ResolvedLibraryDefinitionsFilesMatchTest As part of installing configuration we trigger a full rebuild of all available block derivatives. In order for this to be successful, the entity schemas needs to be installed already. This isn't the case with the current change, as now all configuration is installed before all entity schemas are installed.

@alexpott Is it fundamentally needed that all configuration is now installed before the DB/entity schemas are installed?

Status: Needs review » Needs work

The last submitted patch, 10: 3185978-10.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented
Status: Needs work » Needs review
FileSize
3185978-12.patch23.42 KB
interdiff-3185978-12.txt1.64 KB

This diff changes the way how entity schema is installed.

This now installs the entity schema for each module before it installs the configuration.

PS: There is a bigger underlying problem: It looks like the namespaces injected into the kernel during the installer contains already all modules, not just the ones which are already installed.
One approach which could be done is to reset the container completely during each installed module.

Status: Needs review » Needs work

The last submitted patch, 12: 3185978-12.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented

PS: There is a bigger underlying problem: It looks like the namespaces injected into the kernel during the installer contains already all modules, not just the ones which are already installed.

By all the modules - you mean all the the module that will be installed - right? Yeah this was an intention - it needs all the namespaces in order to be built just once. We need the autoloading to work for that container compile. And yep this is a big change with likely side effects. I'm not sure we can land this as the default installer during D9 but maybe we can land it as an experimental installer then becomes the default in D10. I'm not sure. Sometimes I wished we didn't futz with the composer autoloader at all. So that composer require drupal/module would make it's code autoloadable it exactly the same way doing composer require for any non-drupal code works.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented

I totally follow your general idea ... as in, the autoloading should not be coupled to the process of toggling modules on or off, aka. the autoloading should be 100% purely based on composer for example.

From my perspective the main issue might be in the plugin system. Next thing I'll try is to dynamically change the plugin namespaces somehow.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
Related issues: +#2160091: drupal_rebuild() rebuilds container twice, since drupal_flush_all_caches() also rebuilds it

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.