Configuration being imported by the ConfigImporter sometimes has stale original data

Somewhat surprisingly this seems to speed up installing standard from config by a second!

WITH MR

time vendor/bin/drush si  --existing-config -y 
 You are about to:
 * DROP all tables in your 'drupal8alt' database.

 // Do you want to continue?: yes.

 [notice] Starting Drupal installation. This takes a while.
 [notice] Performed install task: install_select_language
 [notice] Performed install task: install_select_profile
 [notice] Performed install task: install_load_profile
 [notice] Performed install task: install_verify_requirements
 [notice] Performed install task: install_verify_database_ready
 [notice] Performed install task: install_base_system
 [notice] Performed install task: install_bootstrap_full
 [notice] Performed install task: install_config_import_batch
 [notice] Performed install task: install_config_download_translations
 [notice] Performed install task: install_config_revert_install_changes
 [notice] Performed install task: install_configure_form
 [notice] Performed install task: install_finished
 [success] Installation complete.  User name: admin  User password: 5NA5Xeo5DU

________________________________________________________
Executed in    6.51 secs    fish           external
   usr time    3.71 secs    0.09 millis    3.71 secs
   sys time    0.76 secs    5.33 millis    0.75 secs

HEAD

time vendor/bin/drush si  --existing-config -y
 You are about to:
 * DROP all tables in your 'drupal8alt' database.

 // Do you want to continue?: yes.

 [notice] Starting Drupal installation. This takes a while.
 [notice] Performed install task: install_select_language
 [notice] Performed install task: install_select_profile
 [notice] Performed install task: install_load_profile
 [notice] Performed install task: install_verify_requirements
 [notice] Performed install task: install_verify_database_ready
 [notice] Performed install task: install_base_system
 [notice] Performed install task: install_bootstrap_full
 [notice] Performed install task: install_config_import_batch
 [notice] Performed install task: install_config_download_translations
 [notice] Performed install task: install_config_revert_install_changes
 [notice] Performed install task: install_configure_form
 [notice] Performed install task: install_finished
 [success] Installation complete.  User name: admin  User password: PVtQqXFK5r

________________________________________________________
Executed in    8.49 secs    fish           external
   usr time    4.96 secs    0.09 millis    4.96 secs
   sys time    0.88 secs    4.78 millis    0.88 secs

Here's more evidence of why we're saving so much time...

Sorry - I tried to paste html it... but it doesn't work so this is an image... comparing installing standard install with an without the MR applied.

As you can see there is a significant difference in the number of function calls. I've not quite managed to explain why this is so but I think it is because the storage comparer expects to be able to do stuff to the memory backend but it can't when there's yet another memory backend hidden from it.

So the more I think about it the more I wonder about problems caused by wrapping the target storage even when not at install time. I think any secondary config writes are going to cause problems because the memory cache storage that the storage comparer has added is going to be unaware.

Also I don't like making the underlying active storage public during the install.

We introduced this in #2411689: Use a MemoryBackend in StorageComparer so that configuration import validators don't have to reread data from disk or the db I think the use-case is still valid. So I think what needs to happen is that we disable the memory cache on the target storage in storage comparer once we enter the write phase of a configuration import.

We also need to care about what happens in install_config_revert_install_changes() too.

@borisson_ I working on understanding the problem here. I think we need to address the underlying problem - it's actually not just about the ConfigImporter during the installer (although it is most definitely worse there).

Here's what's happening as far as I can tell. The config importer is using the storages injected into the storage comparer for reading a writing. When the config importer installs a module, the module installer will rebuild the container and write to core.extension using the newly created config.storage service. At this point the storage in the storage comparer and the container are out of sync. Then once all the modules are installed we go into the importing config module. Normally this only handles configuration entities but simple config config is dealt with by making the module installer use the storage comparer's source storage. The core.extension configuration is special however, because it does not belong to any module. For some reason the target storage's version of the core.extension configuration is completely out-of-date so when we re-save it, it looks like we're installing all the modules again - so any event listener that reacts to a module install via the config save is doing a lot of unnecessary work. An example of this is \Drupal\Core\Update\UpdateRegistry::onConfigSave().

I've posted a new MR that results in the same speed-ups for installing from config but will also speed up importing configuration during regular runtime.

HEAD

$ time vendor/bin/drush si  --existing-config -y
________________________________________________________
Executed in    8.79 secs    fish           external
   usr time    5.02 secs    0.08 millis    5.02 secs
   sys time    0.88 secs    2.91 millis    0.87 secs

WITH MR

$ time vendor/bin/drush si  --existing-config -y
________________________________________________________
Executed in    6.76 secs    fish           external
   usr time    3.73 secs   84.00 micros    3.73 secs
   sys time    0.75 secs  734.00 micros    0.75 secs

Now to write a test...

I've tweaked the issue summary and title to be about what is actually being fixed here. We still have double memory caches in the installer but I think we should fix that after we fix this as this is more important.

Created #3407802: Using the ConfigImporter during installation results in a double memory cache to address the original issue.

Fixed the test. As all the changes were to test code setting back to RTBC.

FYI: We plan to remove the interface - see #3410037: Deprecate StorageComparerInterface

Here's a patch for 10.1 and probably 10.2...