In Drupal 8.9.x we are seeing a tenfold increase in memory consumption when installing a project using existing config. Using git bisect it appears this problem has been introduced in #474684: Allow themes to declare dependencies on modules.

Please note that I have been testing a project that uses a composer based workflow and the listed commits are from the drupal/core subtree split.

Last known good commit

    [drush] Executing command: /home/pieter/v/joinup-dev/vendor/bin/drush --verbose --yes --root="/home/pieter/v/joinup-dev/web" --db-url="mysql://root:@127.0.0.1:3306/joinup" --site-name="Joinup" --account-name="admin" --account-pass="admin" --account-mail="admin@example.com" --existing-config site-install joinup install_configure_form.enable_update_status_module=NULL install_configure_form.enable_update_status_emails='NULL' install_settings_form.sparql.host=localhost install_settings_form.sparql.port=8890 install_settings_form.sparql.namespace=Drupal\\Driver\\Database\\joinup_sparql
 [info] Executing: command -v mysql
 [info] Executing: mysql --defaults-file=/tmp/drush_Q3FA1B --database=joinup --host=127.0.0.1 --port=3306 --silent -A < /tmp/drush_v52JDB

 // You are about to DROP all tables in your 'joinup' database. Do you want to continue?: yes.

 [info] Sites directory sites/default already exists - proceeding.
 [info] Executing: mysql --defaults-file=/tmp/drush_A5ztzC --database=joinup --host=127.0.0.1 --port=3306 --silent -A < /tmp/drush_ZS6uGF
 [info] Executing: mysql --defaults-file=/tmp/drush_D6YPWC --database=joinup --host=127.0.0.1 --port=3306 --silent -A < /tmp/drush_MDWvmC
 [info] Executing: mysql --defaults-file=/tmp/drush_KyY0aF --database=joinup --host=127.0.0.1 --port=3306 --silent -A < /tmp/drush_izwmfC
 [info] Installing from existing config at /home/pieter/v/joinup-dev/config/sync
 [notice] Starting Drupal installation. This takes a while.
 [success] Installation complete.
 [success] Memory usage: 150113928, peak memory usage: 175713912

First bad commit

    [drush] Executing command: /home/pieter/v/joinup-dev/vendor/bin/drush --verbose --yes --root="/home/pieter/v/joinup-dev/web" --db-url="mysql://root:@127.0.0.1:3306/joinup" --site-name="Joinup" --account-name="admin" --account-pass="admin" --account-mail="admin@example.com" --existing-config site-install joinup install_configure_form.enable_update_status_module=NULL install_configure_form.enable_update_status_emails='NULL' install_settings_form.sparql.host=localhost install_settings_form.sparql.port=8890 install_settings_form.sparql.namespace=Drupal\\Driver\\Database\\joinup_sparql
 [info] Executing: command -v mysql
 [info] Executing: mysql --defaults-file=/tmp/drush_FQExgb --database=joinup --host=127.0.0.1 --port=3306 --silent -A < /tmp/drush_a2dbk9

 // You are about to DROP all tables in your 'joinup' database. Do you want to continue?: yes.

 [info] Sites directory sites/default already exists - proceeding.
 [info] Executing: mysql --defaults-file=/tmp/drush_d4t6b7 --database=joinup --host=127.0.0.1 --port=3306 --silent -A < /tmp/drush_xxtsx9
 [info] Executing: mysql --defaults-file=/tmp/drush_raMCa9 --database=joinup --host=127.0.0.1 --port=3306 --silent -A < /tmp/drush_NoSTL6
 [info] Executing: mysql --defaults-file=/tmp/drush_cL66g7 --database=joinup --host=127.0.0.1 --port=3306 --silent -A < /tmp/drush_ZZdGsa
 [info] Installing from existing config at /home/pieter/v/joinup-dev/config/sync
 [notice] Starting Drupal installation. This takes a while.
 [success] Installation complete.
 [success] Memory usage: 1378101456, peak memory usage: 1403767040

The project that is affected by this problem is open source, ref: https://github.com/ec-europa/joinup-dev. Ticket tracking this problem is https://github.com/ec-europa/joinup-dev/pull/2088.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

pfrenssen created an issue. See original summary.

pfrenssen’s picture

tedbow’s picture

FileSize
3.41 KB

I pretty sure it was the change to the module installer from #474684: Allow themes to declare dependencies on modules. If I revert those changes I can do
drush si -y
locally with memory_limit = 96M using PHP 7.2.23 and sqlite

with 8.9.x I have to set
memory_limit = 300M
(I didn't check every value in between obviously but roughly)

I uploaded a patch if anyone want to test this out. Not testing this patch because it will break other tests.

tedbow’s picture

Status: Active » Needs review
FileSize
627 bytes
627 bytes

Here is try that makes be able to install again at memory_limit = 96M

In \Drupal\Core\Extension\ModuleInstaller::updateKernel()
I am not sure we need to get the extension list from the container because we never actually uninstall themes we just use the list to check if they are enabled.

longwave’s picture

I wonder if we can add a test that checks maximum memory usage during install and fails if we go over a documented limit.

catch’s picture

Wondering whether this is some kind of memory leak, or whether the theme list is just massive. Themes do tend to have much larger .info.yml files than modules.

alexpott’s picture

Status: Needs review » Needs work

@catch Hopefully not 1200mb of theme data though :)

Given we only need the result of themeExtensionList->getList() in uninstall how about we do \Drupal::service('extension.list.theme')->getList() at the beginning of \Drupal\Core\Extension\ModuleInstaller::uninstall() like we do for $module_data = \Drupal::service('extension.list.module')->getList();. Injecting dependent services here is fraught and ideally we'd change this thing to not have any services injected. #2380293: Properly inject services into ModuleInstaller has taken forever for a reason :)

I should have remembered that issue and we should have avoided injecting things here... oops.

alexpott’s picture

Status: Needs work » Needs review
FileSize
3.05 KB
3.05 KB

Implementing #7

pfrenssen’s picture

I can confirm that removing this line resolves the problem in our project, with the experimental patch from #4 we are back at reasonable memory usage:

    [drush] Executing command: /home/pieter/v/joinup-dev/vendor/bin/drush --verbose --yes --root="/home/pieter/v/joinup-dev/web" --db-url="mysql://root:@127.0.0.1:3306/joinup" --site-name="Joinup" --account-name="admin" --account-pass="admin" --account-mail="admin@example.com" --existing-config site-install joinup install_configure_form.enable_update_status_module=NULL install_configure_form.enable_update_status_emails='NULL' install_settings_form.sparql.host=localhost install_settings_form.sparql.port=8890 install_settings_form.sparql.namespace=Drupal\\Driver\\Database\\joinup_sparql
 [info] Executing: command -v mysql
 [info] Executing: mysql --defaults-file=/tmp/drush_rsW28H --database=joinup --host=127.0.0.1 --port=3306 --silent -A < /tmp/drush_nxVFmI

 // You are about to DROP all tables in your 'joinup' database. Do you want to continue?: yes.

 [info] Sites directory sites/default already exists - proceeding.
 [info] Executing: mysql --defaults-file=/tmp/drush_fX1UOL --database=joinup --host=127.0.0.1 --port=3306 --silent -A < /tmp/drush_s5J3ZI
 [info] Executing: mysql --defaults-file=/tmp/drush_scTlXI --database=joinup --host=127.0.0.1 --port=3306 --silent -A < /tmp/drush_DbDB4J
 [info] Executing: mysql --defaults-file=/tmp/drush_RpqxHJ --database=joinup --host=127.0.0.1 --port=3306 --silent -A < /tmp/drush_I2YOaM
 [info] Installing from existing config at /home/pieter/v/joinup-dev/config/sync
 [notice] Starting Drupal installation. This takes a while.
 [success] Installation complete.
 [success] Memory usage: 149592416, peak memory usage: 175182728
  • Memory usage: 149592416 (142.7MB)
  • Peak memory usage: 175182728 (167.1MB)

Nice sleuthing!

pfrenssen’s picture

Result from the patch from #8:

 [success] Memory usage: 149590568, peak memory usage: 175180864
  • Memory usage: 142.7MB
  • Peak memory usage: 167.1MB
lauriii’s picture

I did some profiling on this and can confirm increase of memory consumption after #474684: Allow themes to declare dependencies on modules. Based on some research it seems like the problem is with assigning $this->themeExtensionList with the new instance of the theme extension list, not in the process of getting extension.list.theme service.

+++ b/core/lib/Drupal/Core/Extension/ModuleInstaller.php
@@ -386,7 +372,7 @@ public function uninstall(array $module_list, $uninstall_dependents = TRUE) {
-      $theme_list = $this->themeExtensionList->getList();
+      $theme_list = \Drupal::service('extension.list.theme')->getList();

While we're waiting for @pfrenssen to confirm that this patch addresses the problem in their environment, we might want to consider adding a comment explaining why we are loading the theme extension list this way.

alexpott’s picture

Installing standard from config on 8.9.x With the patch applied

 [debug] Calling install_drupal(Composer\Autoload\ClassLoader, array) [0.25 sec, 10.39 MB]
 [success] Installation complete.  User name: admin  User password: EEmwrGWiKL [4.71 sec, 48.15 MB]

Without the patch

 [debug] Calling install_drupal(Composer\Autoload\ClassLoader, array) [3.2 sec, 10.39 MB]
 [success] Installation complete.  User name: admin  User password: VRUJ4wfk8k [7.75 sec, 83.5 MB]
lauriii’s picture

Status: Needs review » Reviewed & tested by the community

I got very similar results to #12 when installing Drupal with standard install profile:

Wall Time  2min 19s
I/O Wait      4.12s
CPU Time   2min 15s
Memory         85MB
Network         n/a     n/a     n/a
SQL           498ms 27854rq

It seems like I crossposted with @pfrenssen on #11 because #10 already confirms that their concerns have been addressed.

longwave’s picture

> I should have remembered that issue and we should have avoided injecting things here... oops.

While we are here we could add a comment as to why this service isn't injected for now, to avoid a future refactor making the same mistake?

alexpott’s picture

@longwave check out all the other \Drupal::service() calls in this class. The same would happen if we injected \Drupal::service('extension.list.module')

alexpott’s picture

catch’s picture

Fixing credit.

  • catch committed df1d689 on 9.1.x
    Issue #3126003 by tedbow, alexpott, pfrenssen, lauriii: Increased memory...

  • catch committed 789cd65 on 9.0.x
    Issue #3126003 by tedbow, alexpott, pfrenssen, lauriii: Increased memory...

  • catch committed efa1e2a on 8.9.x
    Issue #3126003 by tedbow, alexpott, pfrenssen, lauriii: Increased memory...
catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 9.1.x, 9.0.x, 8.9.x, thanks!

alexpott’s picture

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.