Issue Summary

The current configuration system results in a lot of queries to the database for the same information. This is especially true of config information that is used a lot. For example, in Drupal's cache system the DatabaseBackend class methods garbageCollection() and prepareItem() both use similar code to get the cache_lifetime config.

$cache_lifetime = config('system.performance')->get('cache_lifetime');

Each time a fresh instance of the config system is instantiated and a fresh call to the database to the get the config is made.

Proposed solution

Implement the following architectural changes to CMI:

  • Make the DrupalConfig class responsible for handling the in-memory aspects for CMI - managing the data array and using drupal_static to cache reads.
  • Create a new ConfigStore class that is responsible for handling the storage layers - which will only instantiate StorageInterface classes as needed.
  • Implement a three layer storage system: cache, database, and file.
  • Using a standard Drupal cache bin for the layer making it easy to swap this for other supported Cache backends
  • Change the database layer to store config by config name and key instead of just config name to resolve @beejeebus's fear's of race conditions in writing config.
  • Make every storage layer of the ConfigStore implement StorageInterface and allow the classes used for each layer to be overridden.

A start on all this has been made in the CMI sandbox http://drupalcode.org/sandbox/heyrocker/1145636.git/shortlog/refs/heads/...

Initial results from a script that does the following shows the value of this approach (i think):

$loop = 100;
for ($i=1; $i < $loop; $i++) {
  $cached = cache('bootstrap')->get('system_list');
}

Attached are xhprof results for this code.

Current 8.x code Config cache rearch
Calls to db_query 297 99
config() iwall % 40.6% 0.8%
Total time 1,085,076 microsecs 229,841 microsecs
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alexpott’s picture

Status: Active » Needs review
FileSize
16.65 KB

The branch currently covers everything apart from:

  • Change the database layer to store config by config name and key instead of just config name to resolve @beejeebus's fear's of race conditions in writing config.

Which one could argue is a separate issue from tackling config performance.

Status: Needs review » Needs work

The last submitted patch, 1605124-config-cache.patch, failed testing.

alexpott’s picture

Status: Needs work » Needs review
FileSize
3.81 KB
19.6 KB

New patch fixes image style test case by clearing the new config static cache. Additionally it fixes the issues with module installation by reworking config_install_default_config() to use the new ConfigStore class.

sun’s picture

Status: Needs review » Postponed

Thanks for working on this!

But, hrm. Sorry, this issue tries to change way too many architectural things at once, and it won't be possible to discuss all of the individual changes properly. Let's follow the roadmap outlined on #1560060: [meta] Configuration system roadmap and architecture clean-up and get back to this when the prerequisites have been pulled off the table, so we can focus on performance benchmarks/profiling and possible performance optimizations in here only.

gdd’s picture

Status: Postponed » Closed (duplicate)

This discussion has pretty much been happening over at #1187726: Add caching for configuration / rework config object loading (Was: Memory usage and i/o from config objects), and I'd like to keep it there. So I'm closing this one as a dupe.