Problem/Motivation

I have a project with a lot of configurations (3 037, mostly because of multiple languages). The site installation from existing configuration (drush site:install --existing-config) takes about 10 minutes. I did profile the installation and this is the TOP 10 slow calls:

TOP 10 calls before

As you can see, Drupal\Core\Config\FileStorage->getAllCollectionNamesHelper() is the slowest operation during installation.

Also, take a look at php::SplFileInfo->isDir(). It called 13'937'541 times, and almost all the calls (13'919'940) comes from Drupal\Core\Config\FileStorage->getAllCollectionNamesHelper(), but php::SplFileInfo->isDir() is slow by itself:

SplFileInfo before

This is clearly because Drupal\Core\Config\FileStorage->getAllCollectionNamesHelper() doesn't cache its result in any way. It's always calculated in runtime. It seems to me like a bottleneck.

Steps to reproduce

Install the website from existing configs and profile it. The more configs you have, the more problems you will see.

Proposed resolution

Provide at least a static caching for the results of this method.

Remaining tasks

  1. Provide a patch with a static caching and see how tests are going.
  2. Decide should it be cached and how.
  3. Implement it.
CommentFileSizeAuthor
#20 ddev-site-install.png415.63 KBwebflo
#11 callstack-3389567.png429.46 KBwebflo
#2 screenshot_top10_after.png124.58 KBniklan
#2 drupal-3389567-2.patch1.34 KBniklan
screenshot_splfileinfo_before.png57.35 KBniklan
screenshot_top10_before.png121.48 KBniklan

Issue fork drupal-3389567

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

Niklan created an issue. See original summary.

niklan’s picture

niklan
Primary language Russian
Location Russia, Perm
commented
Status: Active » Needs review
StatusFileSize
new drupal-3389567-2.patch1.34 KB
new screenshot_top10_after.png124.58 KB

This is a very dirty patch to see how tests will react on it.

This is TOP 10 calls during installation after fix applied.

TOP 10 calls after

You can see that both, Drupal\Core\Config\FileStorage->getAllCollectionNamesHelper() and php::SplFileInfo->isDir() disappears from the list.

  • Drupal\Core\Config\FileStorage->getAllCollectionNamesHelper() calls went from 36'392 → 4'551, total self cost (in ms) 37'790 → 96
  • php::SplFileInfo->isDir() calls went from 13'937'541 → 23'710, total self cost (in ms) 36'307→ 72

I measured installation time by time drush site:install --existing-config and results are:

  • Before: real 8m 38.06s
  • After: real 7m 9.09s

It's roughly ~15% increase in site installation speed.

Status: Needs review » Needs work

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

niklan’s picture

niklan
Primary language Russian
Location Russia, Perm
commented

It looks like some code will break. Can we decorate that service specifically for installation process? Because I don't think this caching is actually needed for regular runtime process, but it's a very significant improvement for installation. I don't see any cases where configurations are changed on disc during installation, hence, this caching should not do any harm.

andypost’s picture

andypost
he/him
Primary language Russian
commented

Is it happening during installation only?

niklan’s picture

niklan
Primary language Russian
Location Russia, Perm
commented

This profiling results for drush site:install --existing-config with a specific website configuration. It is not an issue on default profiles (because they are simple and small) or a smaller website (which have fewer configs). But a multilingual website, with something like 10 languages, a lot of content types, fields, etc., configs grows in size very fast, and this is where the issue starts. The profiler is clearly shows, that the more configurations you have, the more calls for Drupal\Core\Config\FileStorage->getAllCollectionNamesHelper() which is bottlenecked by a php::SplFileInfo->isDir() - the unique calls of which is hundreds times fewer than the actual one. In my example, it means for one unique file, it calls this check 587 times.

This issue reproduced everywhere, on different developers PCs and CI's.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

I feel we need to fund out why we're call getAllCollectionNames() and work from there.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Related issues: +#3421483: Use APCu file cache for config schema

static cache is workaround, as file cache should be applied in other place

andypost’s picture

andypost
he/him
Primary language Russian
commented

bottlenecked by a php::SplFileInfo->isDir() - the unique calls of which is hundreds times fewer than the actual one. In my example, it means for one unique file, it calls this check 587 times.

it means filecache miss

webflo’s picture

webflo commented

\Drupal\Core\Config\FileStorage::getAllCollectionNamesHelper gets called by \Drupal\Core\Config\FileStorage::getAllCollectionNames

webflo’s picture

webflo commented
StatusFileSize
new callstack-3389567.png429.46 KB

I looked into CachedStorage, but CachedStorage does not cache getAllCollectionNames. It calls the inner storage immediately.

It might make sense to optimize \Drupal\Core\Config\FileStorage::getAllCollectionNames, it would benefit a few other use cases too.

The calls during installation are coming from ConfigImporter and StorageComparer.

webflo’s picture

webflo commented
Status: Needs work » Needs review

The performance issue is in FileStorage->getAllCollectionNames. But I think it's better to fix the problem at a higher level, because StorageComparer recalculates the getAllCollectionNames very often. This is unnecessary and can be determined at the beginning of the import, not after each sync step.

webflo opened merge request !13236

webflo’s picture

webflo commented
Issue tags: +Performance
smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: +needs profiling

Drupal\Tests\language\Kernel\OverriddenConfigImportTest seems to be consistently failing and believe related to the change here.

Can we get after profiling for this change?

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

webflo’s picture

webflo commented
Status: Needs work » Needs review

OverriddenConfigImportTest passes.

webflo’s picture

webflo commented
Issue tags: +DevDaysAthens2026
borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented

I think this looks great, but like @smustgrave asked in #15, seeing numbers on the reduced amount of calls would be great.

webflo’s picture

webflo commented
StatusFileSize
new ddev-site-install.png415.63 KB

I profiled it with xhprof, ddev, and standard profile (drush si --existing-config -y). Filtered the list down to getAllCollectionNames. There are 330 fewer function class than before.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -needs profiling

Believe based on #20 this is a net improvement for sure.

catch’s picture

catch
he/him
Primary language English
commented

I'm not entirely sure about this one.

In StorageComparer there are two calls to ::getAllCollectionNames().

1. createChangelist - this builds the change list, which is cached.

2. hasChanges this gets all of the collection names, then checks them against the changelist.

What I'm wondering is - could ::hasChanges() directly check the changelist -e.g. do any of the collections in it have any of the operations, and if so, would that reduce the number of calls? If that worked, it would mean no extra static caching. If it doesn't work, it would be good to know where the other calls to ::getAllCollectionNames() are coming from.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs review
smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Per #22