Preload configuration objects

Can we leverage the existing CacheArray for this?

Also, I'd think that the config names to preload depend on the page/path/route? (Not sure what to do about sub-requests in the long run, but that problem will apply to every route-based CacheArray anyway then.)

Not CacheArray, but maybe #1858616: Extract generic CacheCollector implementation and interface from CacheArray :). CacheArray is slow as it needs to go through ArrayAccess, which was necessary in 7.x but we can define real API's now in 8.x.

1) The language/attitude in this issue generally discourages others from commenting.

2) The fact that a setIsNew() method is required on Config only clarifies even further that we've lost a sane separation between ConfigFactory and Config. Config was the sole and only responsible service that handled the loading and state of config objects previously. ConfigFactory suddenly took over ~50% of that, without a clear definition of which service is responsible for what. This consequence was raised as critical concern for the change proposal that added caching to ConfigFactory, but was brushed away. This patch here tries to advance on that caching, so it's only logical and natural that it is running into the exact architectural problems that were known upfront.

3) The caching layer in the config system has to be transparent to all other systems and services. If any external code needs to manually reset or flush the cache of the config factory, then there is a bug in config's caching mechanism itself. There's pre-existing, similar code that manually accounts for renames in the config factory, but that only exists because the issue that introduced the config factory caching did not want to account for the already existing #1739900: Add a rename operation to config storage controllers. Again, known issue, also brushed away. There's a follow-up issue somewhere to fix that. In any case, manual calls into the config factory's cache reset methods are wrong. Renames are the only exception currently.

This patch unintentionally hits some big nails on the broken separation of concerns between Config and ConfigFactory. Ignoring that problem space further and working around it by slapping new, public API methods and behavior onto the system doesn't feel right to me and is guaranteed to make a later, proper refactoring considerably harder.

The architectural problem stems from the fact that this change adds a preload facility to the system, which attempts to inject preloaded data into the system.

One potential way to work around the architectural problems is to cache the actual Config object instead of its data within. In essence, allow a Config object instance to be serialized (in one environment) and deserialized (in another environment).

http://php.net/manual/en/class.serializable.php might help us to achieve that. (I've seen it being actively used in Symfony only recently, which looks really neat.) That way, we can skip the workarounds for isNew entirely and serialize the required properties instead.

Yeah, the general idea of preloading is fine I think. We had it for variables already, and due to the bundling of multiple values into larger config objects, it was pretty clear from the beginning that we'll have to (re-)introduce preloading.

That said, I'm not sure whether I didn't expect more performance gains from this change. AFAICS, the patch cuts away ~10-20 minutes from the total testsuite time... although that's comparing the time of two different testbots, so it doesn't really mean anything, and alas, it's also fairly easy to make HEAD faster these days, since HEAD really can't get any slower.

In other words, I'm a little concerned about the net effect of this change. This optimization was (at least in my mind) planned to be one of the last that would happen for the D8 release - considerably improving the overall system performance after fixing all other performance regressions - or in hypothetical numbers: sitting in front of a beta release with a total testsuite time of 25 minutes and cutting it down to 20 with this final optimization. Focus on "final", because after this, there's not much left that can be tweaked. :-/

#18: I'm intrigued by your third paragraph. Why would we only want to do this optimization very late in the development cycle? Because we'd be less inclined to fix other performance problems, since they'd be less impactful, relatively speaking? But then you say there's not much that can be tweaked after this… which I don't really understand.

There's no value to leaving performance optimizations until the last minute.

Currently the incredibly poor performance of config() is making it very difficult to detect other performance regressions introduced into core. A fixed 5ms slow down might be 1% of the request instead of 5% given a slower system overall.

This means that patches that introduce lots of config() calls alongside other performance regressions are being let in with unknown status on just how bad they are, because it's hard to separate where exactly the regression comes from if it's from multiple places.

Caching per-route sounds worth looking at but not necessarily in this issue.

Where the caching is going to be most likely to be polluted is config entities - i.e. we don't want to load every view on every page, or every field config or instance (although those aren't configuration entities yet anyway but when they are). Also admin pages, since setting/viewing a configuration object doesn't necessarily mean it's needed on runtime.

Tagging.

Just a reroll, but would love to see this moving forward. Will try and do a proper review soon.

Highly interested in this.

Field API currently maintains its own cache of field and instance definitions (FieldInfo class). These are about to become ConfigEntities, and thus FieldInfo will cache ConfigEntities. Ideally, we'd want to get rid of that extra layer and use the CMI cache instead of caching our own ConfigEntities on top of it.

The thing is, though, that FieldInfo currently goes a long way to implement smart cache strategy - that was #1040790: _field_info_collate_fields() memory usage.
In short : field and instance definitions are cached in separate entries by bundle, and loaded on-demand only for the entity types and bundles that are involved in the current page request.

From a (quick) glance, it seems the current patch would do : "the moment a field definition has been loaded once, it will be preloaded on each subsequent request" ?
Would put us back to "all field and instances definitions for all entity types and bundles are loaded in memory unconditionnally" pretty quick, no ?

We discussed that issue in irc a few weeks ago in respect to both Views and Field API. The outcome of that was to have two pre-load caches:

a. Preloads everything from CMI until there's a route name.

b. Preloads everything between the route name and the end of the request.

This way, (a) is loaded every request, (b) caches based on the route name and only holds items needed in there (and not already in (a)).

The idea being that the same views / entity types / bundles ought to be found for the same routes more or less, while there's also a finite number of route names (except for stuff under admin/ which makes up > 70% on most sites) so should still get a good hit rate (as opposed to caching per-path or similar).

Shouldn't this be at least major?

The plan in #36 sounds appealing - cache config by route. Or more precisely, "cache the list of config items names to multiple-pre-load at the beginning of the request, by route", if I get this right ?

If something like this gets in, I guess it would deprecate the smart-but-complex "by bundle" caching strategy in FieldInfo, since two caching strategies on top of each other become hell to predict and debug.

I wonder whether "by route" is going to be granular enough, though. For the same route, the set of config items needed might vary a lot depending on various considerations ? Permissions of the current user comes to mind first, but I guess there are others...
So for example we'd end up loading much more config data than actually needed to serve an anonymous page request ?

What about maybe just aiming for the ability to do multiple-load on config items, without trying to be too smart for now ?

I mean just being able to do something like the following would speed Field API quite a bit :

$config_names = array('mymodule.foo', 'mymodule.bar');
$config_items = config_load_multiple($config_names); // <- uses $cache->getMultiple()
$item = $config_items['mymodule.foo']->get();

Related: #1971158: Follow-up: Add loadMultiple() and listAll() caching to (cached) config storage, #6, among others :)

[edit: never mind, this comment is irrelevant]

Config listeners are below cache point, right ?
So multiple load would still be a win on warm caches (multiple cache reads) even if they're a wash on cold caches ?

If we push down multiload to this level then we can also rely on multiload in locale speeding up the loading of those overrides somewhat. I'm not sure how faster that is going to be.

I'd probably go for pushing the multiple load down to the listeners as well. I never really liked the listeners approach very much and this was the main reason why - having to have a complete layer on top of everything doing stuff twice but we're stuck with that now.

re @beejeebus #48: right, sorry, I thought listeners kicked in before data was cached. My bad, scratch #47.

Tagging.

Tagging.

That's fine with me. I wasn't comfortable with config events going in at all and there's only a couple of contrib modules that are likely to use them anyway.

#1966538: Translation is not updated for configuration, when the config changes may be some stuff related, although it does not make drastic changes to the locale event listener, last time I checked.

If we're going to convert ConfigFactory to ConfigSomething, can we get a ConfigSomethingInterface? AFAIR ConfigFactory has been hardcoding Config(). While I have no immediate plans for something better, I would like that our premises align with our reality.

Generalizing to multiple++

I've never been entire comfortable with the context solution and anything we can do to improve performance here will be a big bonus.

Lets explore the suggestions in #55

Un-postponed!

1. +++ b/core/lib/Drupal/Core/Config/ConfigFactory.php
   @@ -184,6 +194,11 @@ public function get($name) {
   +    if (!$this->preloadEventFired) {
   +      $this->preloadEventFired = TRUE;
   +      $names = array_unique($names + $this->getPreloadNames());
   +    }
   

   This code is a bit hart to understand. If it is not fired yet, set it to fired. Let's just switch the two lines.

2. +++ b/core/lib/Drupal/Core/Config/ConfigPreloadEvent.php
   @@ -0,0 +1,65 @@
   +   * @return self
   ...
   +   * @return self
   

   We use "@return $this" now.

3. +++ b/core/lib/Drupal/Core/Config/ConfigPreloadSubscriber.php
   @@ -0,0 +1,40 @@
   +    $settings_config_names = Settings::getSingleton()->get('config_preload_names', array());
   

   It would be cool if we could just inject the settings service instead.

4. +++ b/core/lib/Drupal/Core/Config/ConfigPreloadSubscriber.php
   @@ -0,0 +1,40 @@
   +  /**
   +   * Returns the names of any configuration objects in Settings.
   +   *
   +   * @param \Drupal\Core\Config\ConfigPreloadEvent $event
   +   *   Configuration preload event to respond to.
   +   */
   

   The documentation is a bit inaccurate.

Fixes issues in #71

If we have to reroll again.

+++ b/core/lib/Drupal/Core/Config/ConfigFactory.php
@@ -262,6 +277,18 @@ protected function loadModuleOverrides(array $names) {
+   * Get a list of configuration object names to preload.

Gets

Thanks Jibran. I've fixed that and added a unit test for ConfigPreloadSubscriber::onConfigPreloadNamesGetSettingsNames().

It is always good to see a unit test!

+++ b/core/lib/Drupal/Core/Config/ConfigFactory.php
@@ -184,6 +194,11 @@ public function loadMultiple(array $names) {
+    if (!$this->preloadEventFired) {
+      $names = array_unique($names + $this->getPreloadNames());

Does this mean that the first call to loadMultiple() will receive more results than the ones it asked for ?
Looks like it might have surprising effects if the code foreaches on the results, you're not really supposed to receive results for keys you never asked ?
Couldn't we sliently add the items to preload to the list but then ditch the entries before returning actual results ?

#77 looks like a needs review.

Another idea maybe a bit too late and maybe a bit more hardcore would have been to dump configuration as generated PHP files containing arrays, such as the container is being dumped. This way it could also serve as a security layer where files could not be changed by a hacker that does not have access to writing to the filesystem, and be protected by paranoid admin with restrive file permissions, and would also go to OPCode cache ensuring both 0 file stat and 0 SQL or cache backend queries. This would vanish any need for preloading anything, would be super simple, altered configuration could be dumped too (considering that alteration is not supposed to be variable) and give a very good performance boost altogether (and maybe tend to a 0-query bootstrap).

Cross-referencing #2172561: Config overrides may spill over to undesired places, I started looking into this and got distracted by performance issues with the config override stuff in general, posted my findings over there for now as that seems to become a general follow-up issue for that...

This mostly looks good, but I noticed some additional issues:

1. +++ b/core/lib/Drupal/Core/Config/ConfigFactory.php
   @@ -74,6 +76,13 @@ class ConfigFactory implements EventSubscriberInterface {
   +  protected $preloadEventFired;
   
   @@ -184,6 +194,11 @@ public function get($name) {
      public function loadMultiple(array $names) {
   ...
   +    if (!$this->preloadEventFired) {
   +      $names = array_unique($names + $this->getPreloadNames());
   +      $this->preloadEventFired = TRUE;
   +    }
   
   @@ -262,6 +277,18 @@ protected function loadModuleOverrides(array $names) {
   +  protected function getPreloadNames() {
   
   +++ b/core/lib/Drupal/Core/Config/ConfigPreloadEvent.php
   @@ -0,0 +1,64 @@
   +class ConfigPreloadEvent extends Event {
   

   Hm. I'm concerned about the name "preload".

   Everywhere else in Drupal, "preload" denotes a callback/hook facility for a specific "load" operation.

   → "preload" is even more confusing here, because we are, in fact, in a loadMultiple() method.

   What we're really doing here is to "prime" the config factory cache. It's a cache priming facility.

   Could we rename these classes and variables accordingly to avoid confusion? E.g.:

   $preloadEventFired → $isCachePrimed + isCachePrimed() method
   getPreloadNames() → primeCache()
   ConfigPreloadEvent → ConfigCachePrimeEvent

2. +++ b/core/lib/Drupal/Core/Config/ConfigPreloadSubscriber.php
   @@ -0,0 +1,54 @@
   +    $events['config.preload.names'][] = array('onConfigPreloadNamesGetSettingsNames', 48);
   

   48? Why 48? :)

   Shouldn't a base/default event subscriber like this just simply use a priority of 0?

   It's the first and only subscriber for this event, no?

3. +++ b/sites/default/default.settings.php
   @@ -278,6 +278,14 @@
   +#$settings['config_preload_names'] = array();
   

   All other commented out default settings in settings.php are using a space after the #.

+++ b/core/lib/Drupal/Core/Config/ConfigPreloadEvent.php
@@ -0,0 +1,64 @@
+  public function addNames(array $names) {
+    $this->names += $names;

Shouldn't this be an array_merge()?

I've reviewed the code from an understandability POV — I don't know the config system at all. Mostly nitpicks though.

1. +++ b/core/lib/Drupal/Core/Config/ConfigPreloadSubscriber.php
   @@ -0,0 +1,40 @@
   +  }
   +}
   

   Should have an empty line in between.

2. +++ b/core/lib/Drupal/Core/Config/ConfigFactory.php
   @@ -154,6 +164,11 @@ public function get($name) {
   +    if (!$this->preloadEventFired) {
   +      $this->preloadEventFired = TRUE;
   +      $names = array_unique($names + $this->getPreloadNames());
   +    }
   

   Why would you set this to TRUE without actually firing it?

   The preload event is fired in getPreloadNames(). This should be set to TRUE after that method is called. Or maybe it should even be set it inside that method?

3. +++ b/core/lib/Drupal/Core/Config/ConfigFactory.php
   @@ -232,6 +247,18 @@ protected function loadModuleOverrides(array $names) {
   +    $this->eventDispatcher->dispatch('config.preload.names', $config_preload_event);
   +    return $config_preload_event->getNames();
   

   While reading the patch, I thought this event was triggered *after* preloading had occurred, to indicate to other code that config preloading was done.

   But apparently it's quite the opposite: this event is used to *collect* a list of names of config objects to be preloaded!

   I think this could be documented more clearly in this class, and in ConfigPreloadEvent.

4. +++ b/core/lib/Drupal/Core/Config/ConfigPreloadEvent.php
   @@ -0,0 +1,65 @@
   +
   
   +++ b/core/lib/Drupal/Core/Config/ConfigPreloadSubscriber.php
   @@ -0,0 +1,40 @@
   +
   

   Trailing newlines.

5. +++ b/core/lib/Drupal/Core/Config/ConfigPreloadSubscriber.php
   @@ -0,0 +1,40 @@
   +   * Implements EventSubscriberInterface::getSubscribedEvents().
   

   Shouldn't this be {@inheritdoc}?

6. +++ b/core/lib/Drupal/Core/Config/ConfigPreloadSubscriber.php
   @@ -0,0 +1,40 @@
   + * Core configuration preload subscriber.
   

   s/preload subscriber/preload event subscriber/?

7. +++ b/core/lib/Drupal/Core/Config/ConfigPreloadSubscriber.php
   @@ -0,0 +1,40 @@
   +   * Returns the names of any configuration objects in Settings.
   

   What is "Settings"?

   If it's settings.php, then why is it written with a capital letter?

   Maybe an @see to relevant code would be helpful here?

Interdiff?

Thank you — The naming looks much better now! :)

1. +++ b/core/lib/Drupal/Core/Config/ConfigCachePrimeEvent.php
   @@ -0,0 +1,67 @@
   +   * Configuration names.
   +   *
   +   * @var array
   +   */
   +  protected $names = array();
   

   Is this a list of names to prime, or a list of names that have been primed? Can we clarify the phpDoc?

2. +++ b/core/lib/Drupal/Core/Config/ConfigCachePrimeEvent.php
   @@ -0,0 +1,67 @@
   +   * Gets configuration names.
   +   *
   +   * @return array
   +   *   The list of configuration names that can be overridden.
   +   */
   +  public function getNames() {
   

   1. s/Gets/Returns/ ?

   2. The @return description appears to be copied from elsewhere?

3. +++ b/core/lib/Drupal/Core/Config/ConfigCachePrimeEvent.php
   @@ -0,0 +1,67 @@
   +   * @return self
   ...
   +    return $this;
   ...
   +   * @return self
   ...
   +    return $this;
   

   @return $this

4. +++ b/core/lib/Drupal/Core/Config/ConfigCachePrimeEvent.php
   @@ -0,0 +1,67 @@
   +  public function addNames(array $names) {
   +    $this->names += $names;
   +    $this->names = array_unique($this->names);
   

   If $names is an indexed array, then we need to use array_merge() here, because += will not add indexes that exist already.

   array_merge() should also make the array_unique() obsolete.

5. +++ b/core/lib/Drupal/Core/Config/ConfigCachePrimeSubscriber.php
   @@ -0,0 +1,40 @@
   +class ConfigCachePrimeSubscriber implements EventSubscriberInterface {
   

   Do we really need a separate subscriber object here?

   What's wrong with making ConfigFactory subscribe directly?

6. +++ b/core/lib/Drupal/Core/Config/ConfigCachePrimeSubscriber.php
   @@ -0,0 +1,40 @@
   +    $settings_config_names = Settings::getSingleton()->get('config_cache_prime_names', array());
   

   Fine with this name for now, but we need to review + fix the overall situation of setting names at some point:

   #2160705: settings.php setting names are inconsistent (and undocumented)

7. +++ b/core/lib/Drupal/Core/Config/ConfigFactory.php
   @@ -71,6 +73,13 @@ class ConfigFactory implements ConfigFactoryInterface, EventSubscriberInterface
   +   * Whether or not the cache has been primed yet.
   

   s/yet/already/ ?

8. +++ b/core/lib/Drupal/Core/Config/ConfigFactory.php
   @@ -232,6 +246,24 @@ protected function loadModuleOverrides(array $names) {
   +    // Yes, this is stupid, but this is Drupal.
   

   The comment reads fine and is informative, if you'd remove this line. :P

9. +++ b/sites/default/default.settings.php
   @@ -278,6 +278,14 @@
   + * Configuration cache prime.
   + *
   + * The core configuration system will prime it's cache with any configuration
   + * objects named in this array.
   

   In summary: "Configuration object names to prime in cache."

   In description:
   - Let's remove "core"
   - s/it's/its/
   - s/any/the/

Last patch re-rolled. Now the patch applies successfully.

Last patch re-rolled. Now the patch applies successfully.

We need some tests of the functionality.

From my limited understanding, I think this will require some integration tests. Not sure whether this is a new test class or there is an existing one which we can add to?

In the meantime, I've rerolled and cleaned up some imports and removed an unused global $conf.

The list of cache gets that I posted in #1194136-52: Re-organise core cache bins could be interesting for this. Note that this is the list after applying the patch there, which renames a lot of bins, but doesn't touch cache_config.

+++ b/core/lib/Drupal/Core/Config/ConfigCachePrimeSubscriber.php
@@ -0,0 +1,40 @@
+    $events['config.prime.cache'][] = array('onConfigCachePrimeGetSettingsNames');

+++ b/core/lib/Drupal/Core/Config/ConfigFactory.php
@@ -228,6 +239,23 @@ protected function loadModuleOverrides(array $names) {
+    $this->eventDispatcher->dispatch('config.cache.prime', $config_cache_prime_event);

Let's play a game of "Spot the difference" :)

Yeah, this definitely needs tests :p

Use the ConfigEvents class to make life easier too :)

After fixing that, it does work, but there's a problem right now..

The are a few things that are happening during bootstrap that have a huge influence whether and what you want to preload. The most important is the page cache.

If you look at the list from the referenced issue, this is what happens in early bootstrap:

cache_config: system.module*
cache_config: system.performance
=== > CACHE HIT/MISS.

cache_bootstrap: module_implements
cache_config: system.file
cache_config: system.filter

* in DrupalKernel::initializeContainer(), should apparently *not* happen unless module list changed, but that logics seems broken. different issue, we still have the problem even there would only be system.performance

Obviously, if you're going to have a page cache hit, you don't want to load all the stuff that comes below. But the preload kicks in on the first config file being loaded, which right now is system.module. Which is too early to know that.

One idea I have is to special case system.module and system.performance, only trigger preload() after that.

Another possibly crazy idea is to invert the preload pattern, trigger something somewhere that simply calls $cache_factory->loadMultiple() instead of the implicit magic that we have right now. That would also make it equally easy to have an on request event listener that is capable of loading route specific config. Risk is to end up with various pre-loaders that all load a few config files.

We have page_cache_without_database, that should skip all config gets on a cache hit when set. At least provides a workaround for sites that know about it.

The preloaded config is similar to the variables table in 7.x in that it's all or nothing when that's disabled though yeah.

If we were going to special case that system.performance, then it should only happen if the page is eligible to be a cache hit - i.e. no session cookie. Looks like an easy change in _drupal_bootstrap_page_cache() we just don't do that currently. Although still needs a way to trigger preloading or not.

Then we'd only load the config separately for requests that have a chance of a cache hit, and not for authenticated users / users with a session where we know there's no chance of a hit, and they don't want the extra i/o.

Also cross-linking #2177461: Refactor page caching into a service. I haven't reviewed that patch yet but it's obviously relevant to this.

The example I was thinking of was ThemeHandlerTest. The CachedStorageTest gives examples of mocking config storage.

OK. Here's a crack at a unit test for loading the primed config names in loadMultiple. It actually picked up a bug in merging the name arrays. How about that??

Also added a constant for the event to ConfigEvents.

Looks good, a few questions and some general nitpicks

1. +++ b/core/lib/Drupal/Core/Config/ConfigCachePrimeEvent.php
   @@ -0,0 +1,66 @@
   +use Drupal\Core\Language\Language;
   
   +++ b/core/lib/Drupal/Core/Config/ConfigCachePrimeSubscriber.php
   @@ -0,0 +1,40 @@
   +use Symfony\Component\HttpKernel\KernelEvents;
   

   Appears unused?

2. +++ b/core/lib/Drupal/Core/Config/ConfigEvents.php
   @@ -57,4 +57,11 @@
   +   * @see \Drupal\Core\Config\ConfigCachePrimeSubscriber::getSubscribedEvents().
   

   Should we reference subscribers here? Seems like tail wagging the dog.

3. +++ b/core/lib/Drupal/Core/Config/ConfigFactory.php
   @@ -228,6 +239,23 @@ protected function loadModuleOverrides(array $names) {
   +    $this->isCachePrimed = TRUE;
   

   could we have an 'isPriming' property for this use case instead? Just to distinguish between the two states.

4. +++ b/core/tests/Drupal/Tests/Core/Config/ConfigFactoryTest.php
   @@ -0,0 +1,67 @@
   +      'name' => 'Config factoryt test',
   

   %s/factoryt/factory

5. +++ b/core/tests/Drupal/Tests/Core/Config/ConfigFactoryTest.php
   @@ -0,0 +1,67 @@
   +   * Tests loading multiple with prime config names.
   

   suggest: Tests loading multiple config objects with primed config names?

Thanks for the review @larowlan!

1. Cleaned up imports
2. Removed @see reference to subscriber
3. I don't think the isCachedPrimed is used for anything but ensuring preventing a redirect loop. Maybe we can just rename to isCachingPriming to make more sense?
4. Typo fixed
5. Fixed comment

Re-roll

Can we come up with an initial list of names that should be preloaded. It seems silly for this to go in without any usage in core. Code looks good.

Agreed with everything in #112.

Only the silliest of silly nitpicks to be found:

1. +++ b/core/tests/Drupal/Tests/Core/Config/ConfigFactoryTest.php
   @@ -0,0 +1,67 @@
   +  public function testLoadMultipleWithCachePrimeNames() {
   +
   

   Extraneous newline.

2. +++ b/core/tests/Drupal/Tests/Core/Config/ConfigFactoryTest.php
   @@ -0,0 +1,67 @@
   +  }
   +}
   

   Missing newline.

Not sure why this is an event as opposed to either tagged services or just a single service implementing CacheCollector that could be replaced. Earlier patches were along those lines. Trying to think of a case where you'd need to have more than one active strategy for preloading - it seems like that could result in its own overhead if those in turn are caching lists of cids.

For 'more than one strategy', I mean:

- listener A adds configuration IDs by route

- listener B adds configuration IDs by role

or some other combination that different listeners try to do.

Need to think about this more but beejeebus explained the expectation was each module would provide a static list of configuration objects to pre-load via the event. I'm always very wary of anything opt-in for performance because it's extremely to ignore.

I've been assuming that we'd still try to come up with some kind of strategy like #35-38 - although admittedly that might not work at all, but it doesn't feel very compatible with the events.

Here's another reason why the current approach (preload on first load call) is problematic: It's useless for multilingual sites. All of it would need to be loaded again once the language negotiation happened.

We have a RoutePreloader now which is a request event subscriber, I think this should follow that model. Then it's a separate, optional step and the ConfigFactory doesn't need to be made more complicated due to it?

We will still need a solution for the handful of early bootstrap calls, but that could maybe just be a hardcoded list somewhere?

Attaching a simple proof of concept when doing it as a separate subscriber. I think that's much easier and decoupled, so that the config factory doesn't need to know about this.

We drifted away into a lot of more or less related discussions when looking at this, posting a few notes of things to do separately or improve here:

- Should we have multiple separate subscribers that load on their own or should we have one request subscriber that invokes the existing config event that we have right now in the previous patches? More complexity but it would allow us to group multiple strategies together as a single load.
- This happens after language negotiation, so we still need a solution for the few calls before there:

Without multilingual:
- system.module: This is a (we think flawed) attempt at trying to support multiple webheads and rebuilds on them, but there are many situations that we don't cover with this, like pushing new versions of modules, so we can just as well remove it
- system.performance: This is supposed to be overridable with settings, but it's broken => needs to be fixed.
- Then we have two config calls to system.filter and system.file.

With multilingual, we get 4 additional calls, and that's two to load the languages (I already have a patch to do it in a single call, will open an issue after this comment.) and we load the language types and language negotation settings. Not sure what to do about the remaining 3 after my issue.

Discussed this in depth with Alex Pott, Wim Leers, beejeebus and others at devdays.

The most difficult thing about getting preloading right is the following:

- When lots of configuration objects are used, you want to preload as many as possible.
- When there are render cache hits, less configuration objects will be needed.

Hence the better our overall, high-level caching, pre-loading can result in a lot of information loaded that's not actually required for the request.

Assuming #1880766: Preload configuration objects works and gets in, the individual configuration loads will result in more or less zero i/o (at least when using read-only PhpStorage, or compared to the db even with preloading). So if that gets in, we can skip preloading altogether and mark this duplicate. Postponing for now though.

There's another way of improving things which would to make bigger config files and having less files (for example put all of the system module in only one or maybe two files). The file number could be dramatically reduced. In most pages you are going to load almost everything of system module anyway ; I'm quite sure this is true for most modules. Having the file structure exposed at the API level was a very bad idea at the very beggining anyway and that's what is causing you the most trouble today. So just reduce the file number. But that probably belongs to another issue(s).

Sorry, which issue is this postponed on? #122 contains a self-reference. :)

I believe it's #2228261: Add a local, PhpStorage-based cache backend

@pounard while that might help a bit for static configuration, it would make no difference to configuration entities, which in the end will be the largest number of configuration objects on a site. Entity bundles, fields, instances, display modes, views, image styles etc. can't be put into a single file.

Also even one file per module for static configuration could result in over a hundred database queries where there's 100+ modules enabled.

Indeed.

Also #2248767: Use fast, local cache back-end (APCu, if available) for low-write caches (bootstrap, discovery, and config) and #2231595: Add a cache backend that checks an inconsistent cache, then falls back to a consistent cache backend.

Those are pretty far along, so I'm bumping this down to major/active.

Pre-loading could be useful to at least some extent even with those patches in, since with opt-in pre-loading it'd be possible to multiple load on cache misses. But that's much less of an issue with a local cache.

Is this still something we want to pursue given #2248767: Use fast, local cache back-end (APCu, if available) for low-write caches (bootstrap, discovery, and config)?

It's a duplicate of #2348245: Add a cache collector style backend to reduce the number of calls to cache backends if at all.