Commit 284241ec authored by webchick's avatar webchick

Issue #2162013 by alexpott, Berdir, sun, pwolanin: Critical performance...

Issue #2162013 by alexpott, Berdir, sun, pwolanin: Critical performance regression due to config schema + TypedData lookups in installer.
parent 360c0c07
...@@ -107,7 +107,7 @@ services: ...@@ -107,7 +107,7 @@ services:
class: Drupal\Core\Config\InstallStorage class: Drupal\Core\Config\InstallStorage
config.typed: config.typed:
class: Drupal\Core\Config\TypedConfigManager class: Drupal\Core\Config\TypedConfigManager
arguments: ['@config.storage', '@config.storage.schema'] arguments: ['@config.storage', '@config.storage.schema', '@cache.config']
database: database:
class: Drupal\Core\Database\Connection class: Drupal\Core\Database\Connection
factory_class: Drupal\Core\Database\Database factory_class: Drupal\Core\Database\Database
......
...@@ -390,7 +390,8 @@ function install_begin_request(&$install_state) { ...@@ -390,7 +390,8 @@ function install_begin_request(&$install_state) {
$container->register('config.typed', 'Drupal\Core\Config\TypedConfigManager') $container->register('config.typed', 'Drupal\Core\Config\TypedConfigManager')
->addArgument(new Reference('config.storage')) ->addArgument(new Reference('config.storage'))
->addArgument(new Reference('config.storage.schema')); ->addArgument(new Reference('config.storage.schema'))
->addArgument(new Reference('cache.config'));
$container->register('config.factory', 'Drupal\Core\Config\ConfigFactory') $container->register('config.factory', 'Drupal\Core\Config\ConfigFactory')
->addArgument(new Reference('config.storage')) ->addArgument(new Reference('config.storage'))
......
...@@ -11,6 +11,7 @@ ...@@ -11,6 +11,7 @@
use Drupal\Component\Utility\String; use Drupal\Component\Utility\String;
use Drupal\Core\Config\ConfigNameException; use Drupal\Core\Config\ConfigNameException;
use Drupal\Core\Config\Context\ContextInterface; use Drupal\Core\Config\Context\ContextInterface;
use Drupal\Core\Config\Schema\SchemaIncompleteException;
use Drupal\Core\TypedData\PrimitiveInterface; use Drupal\Core\TypedData\PrimitiveInterface;
use Drupal\Core\TypedData\Type\FloatInterface; use Drupal\Core\TypedData\Type\FloatInterface;
use Drupal\Core\TypedData\Type\IntegerInterface; use Drupal\Core\TypedData\Type\IntegerInterface;
...@@ -516,37 +517,6 @@ protected function getSchemaWrapper() { ...@@ -516,37 +517,6 @@ protected function getSchemaWrapper() {
return $this->schemaWrapper; return $this->schemaWrapper;
} }
/**
* Gets the definition for the configuration key.
*
* @param string $key
* A string that maps to a key within the configuration data.
*
* @return \Drupal\Core\Config\Schema\Element
*
* @throws \Drupal\Core\Config\ConfigException
* Thrown when schema is incomplete.
*/
protected function getSchemaForKey($key) {
$parts = explode('.', $key);
$schema_wrapper = $this->getSchemaWrapper();
if (count($parts) == 1) {
$schema = $schema_wrapper->get($key);
}
else {
$schema = clone $schema_wrapper;
foreach ($parts as $nested_key) {
if (!is_object($schema) || !method_exists($schema, 'get')) {
throw new ConfigException(String::format("Incomplete schema for !key key in configuration object !name.", array('!name' => $this->name, '!key' => $key)));
}
else {
$schema = $schema->get($nested_key);
}
}
}
return $schema;
}
/** /**
* Casts the value to correct data type using the configuration schema. * Casts the value to correct data type using the configuration schema.
* *
...@@ -564,7 +534,7 @@ protected function castValue($key, $value) { ...@@ -564,7 +534,7 @@ protected function castValue($key, $value) {
} }
elseif (is_scalar($value)) { elseif (is_scalar($value)) {
try { try {
$element = $this->getSchemaForKey($key); $element = $this->getSchemaWrapper()->get($key);
if ($element instanceof PrimitiveInterface) { if ($element instanceof PrimitiveInterface) {
// Special handling for integers and floats since the configuration // Special handling for integers and floats since the configuration
// system is primarily concerned with saving values from the Form API // system is primarily concerned with saving values from the Form API
...@@ -586,7 +556,7 @@ protected function castValue($key, $value) { ...@@ -586,7 +556,7 @@ protected function castValue($key, $value) {
$value = $element->getString(); $value = $element->getString();
} }
} }
catch (\Exception $e) { catch (SchemaIncompleteException $e) {
// @todo throw an exception due to an incomplete schema. Only possible // @todo throw an exception due to an incomplete schema. Only possible
// once https://drupal.org/node/1910624 is complete. // once https://drupal.org/node/1910624 is complete.
} }
......
...@@ -7,10 +7,8 @@ ...@@ -7,10 +7,8 @@
namespace Drupal\Core\Config\Schema; namespace Drupal\Core\Config\Schema;
use Drupal\Component\Utility\NestedArray;
use Drupal\Core\TypedData\ComplexDataInterface; use Drupal\Core\TypedData\ComplexDataInterface;
use Drupal\Core\TypedData\TypedDataInterface; use Drupal\Component\Utility\String;
use \InvalidArgumentException;
/** /**
* Defines a mapping configuration element. * Defines a mapping configuration element.
...@@ -36,17 +34,29 @@ protected function parse() { ...@@ -36,17 +34,29 @@ protected function parse() {
/** /**
* Implements Drupal\Core\TypedData\ComplexDataInterface::get(). * Implements Drupal\Core\TypedData\ComplexDataInterface::get().
*
* Since all configuration objects are mappings the function will except a dot
* delimited key to access nested values, for example, 'page.front'.
*/ */
public function get($property_name) { public function get($property_name) {
$parts = explode('.', $property_name);
$root_key = array_shift($parts);
$elements = $this->getElements(); $elements = $this->getElements();
if (isset($elements[$property_name])) { if (isset($elements[$root_key])) {
return $elements[$property_name]; $element = $elements[$root_key];
} }
else { else {
throw new InvalidArgumentException(format_string("The configuration property @key doesn't exist.", array( throw new SchemaIncompleteException(String::format("The configuration property @key doesn't exist.", array('@key' => $property_name)));
'@key' => $property_name,
)));
} }
// If $property_name contained a dot recurse into the keys.
foreach ($parts as $key) {
if (!is_object($element) || !method_exists($element, 'get')) {
throw new SchemaIncompleteException(String::format("The configuration property @key does not exist.", array('@key' => $property_name)));
}
$element = $element->get($key);
}
return $element;
} }
/** /**
......
<?php
/**
* @file
* Contains \Drupal\Core\Config\Schema\SchemaIncompleteException.
*/
namespace Drupal\Core\Config\Schema;
/**
* An exception thrown when a config schema is incomplete.
*/
class SchemaIncompleteException extends \RuntimeException {
}
...@@ -18,7 +18,7 @@ class Sequence extends ArrayElement implements ListInterface { ...@@ -18,7 +18,7 @@ class Sequence extends ArrayElement implements ListInterface {
* Overrides ArrayElement::parse() * Overrides ArrayElement::parse()
*/ */
protected function parse() { protected function parse() {
$definition = $definition = $this->getItemDefinition(); $definition = $this->getItemDefinition();
$elements = array(); $elements = array();
foreach ($this->value as $key => $value) { foreach ($this->value as $key => $value) {
$elements[$key] = $this->parseElement($key, $value, $definition); $elements[$key] = $this->parseElement($key, $value, $definition);
...@@ -60,7 +60,7 @@ public function onChange($delta) { ...@@ -60,7 +60,7 @@ public function onChange($delta) {
* Typed configuration element. * Typed configuration element.
*/ */
public function get($key) { public function get($key) {
$elements = $this->parse(); $elements = $this->getElements();
return $elements[$key]; return $elements[$key];
} }
......
...@@ -11,13 +11,20 @@ ...@@ -11,13 +11,20 @@
use Drupal\Component\Plugin\PluginManagerBase; use Drupal\Component\Plugin\PluginManagerBase;
use Drupal\Component\Utility\NestedArray; use Drupal\Component\Utility\NestedArray;
use Drupal\Component\Utility\String; use Drupal\Component\Utility\String;
use Drupal\Core\Config\TypedConfigManagerInterface; use Drupal\Core\Cache\CacheBackendInterface;
/** /**
* Manages config type plugins. * Manages config type plugins.
*/ */
class TypedConfigManager extends PluginManagerBase implements TypedConfigManagerInterface { class TypedConfigManager extends PluginManagerBase implements TypedConfigManagerInterface {
/**
* The cache ID for the definitions.
*
* @var string
*/
const CACHE_ID = 'typed_config_definitions';
/** /**
* A storage controller instance for reading configuration data. * A storage controller instance for reading configuration data.
* *
...@@ -39,6 +46,13 @@ class TypedConfigManager extends PluginManagerBase implements TypedConfigManager ...@@ -39,6 +46,13 @@ class TypedConfigManager extends PluginManagerBase implements TypedConfigManager
*/ */
protected $definitions; protected $definitions;
/**
* Cache backend for the definitions.
*
* @var \Drupal\Core\Cache\CacheBackendInterface
*/
protected $cache;
/** /**
* Creates a new typed configuration manager. * Creates a new typed configuration manager.
* *
...@@ -46,10 +60,13 @@ class TypedConfigManager extends PluginManagerBase implements TypedConfigManager ...@@ -46,10 +60,13 @@ class TypedConfigManager extends PluginManagerBase implements TypedConfigManager
* The storage controller object to use for reading schema data * The storage controller object to use for reading schema data
* @param \Drupal\Core\Config\StorageInterface $schemaStorage * @param \Drupal\Core\Config\StorageInterface $schemaStorage
* The storage controller object to use for reading schema data * The storage controller object to use for reading schema data
* @param \Drupal\Core\Cache\CacheBackendInterface $cache
* The cache backend to use for caching the definitions.
*/ */
public function __construct(StorageInterface $configStorage, StorageInterface $schemaStorage) { public function __construct(StorageInterface $configStorage, StorageInterface $schemaStorage, CacheBackendInterface $cache) {
$this->configStorage = $configStorage; $this->configStorage = $configStorage;
$this->schemaStorage = $schemaStorage; $this->schemaStorage = $schemaStorage;
$this->cache = $cache;
} }
/** /**
...@@ -157,22 +174,34 @@ public function getDefinition($base_plugin_id) { ...@@ -157,22 +174,34 @@ public function getDefinition($base_plugin_id) {
} }
/** /**
* Implements Drupal\Component\Plugin\Discovery\DiscoveryInterface::getDefinitions(). * {@inheritdoc}
*/ */
public function getDefinitions() { public function getDefinitions() {
if (!isset($this->definitions)) { if (!isset($this->definitions)) {
$this->definitions = array(); if ($cache = $this->cache->get($this::CACHE_ID)) {
foreach ($this->schemaStorage->listAll() as $name) { $this->definitions = $cache->data;
if ($schema = $this->schemaStorage->read($name)) { }
else {
$this->definitions = array();
foreach ($this->schemaStorage->readMultiple($this->schemaStorage->listAll()) as $schema) {
foreach ($schema as $type => $definition) { foreach ($schema as $type => $definition) {
$this->definitions[$type] = $definition; $this->definitions[$type] = $definition;
} }
} }
$this->cache->set($this::CACHE_ID, $this->definitions);
} }
} }
return $this->definitions; return $this->definitions;
} }
/**
* {@inheritdoc}
*/
public function clearCachedDefinitions() {
$this->definitions = NULL;
$this->cache->delete($this::CACHE_ID);
}
/** /**
* Gets fallback metadata name. * Gets fallback metadata name.
* *
......
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
namespace Drupal\locale; namespace Drupal\locale;
use Drupal\Core\Cache\CacheBackendInterface;
use Drupal\Core\Language\Language; use Drupal\Core\Language\Language;
use Drupal\Core\Config\TypedConfigManager; use Drupal\Core\Config\TypedConfigManager;
use Drupal\Core\Config\StorageInterface; use Drupal\Core\Config\StorageInterface;
...@@ -47,10 +48,12 @@ class LocaleConfigManager extends TypedConfigManager { ...@@ -47,10 +48,12 @@ class LocaleConfigManager extends TypedConfigManager {
* data. * data.
* @param \Drupal\locale\StringStorageInterface $localeStorage * @param \Drupal\locale\StringStorageInterface $localeStorage
* The locale storage to use for reading string translations. * The locale storage to use for reading string translations.
* @param \Drupal\Core\Cache\CacheBackendInterface $cache
* The cache backend to use for caching the definitions.
*/ */
public function __construct(StorageInterface $configStorage, StorageInterface $schemaStorage, StorageInterface $installStorage, StringStorageInterface $localeStorage) { public function __construct(StorageInterface $configStorage, StorageInterface $schemaStorage, StorageInterface $installStorage, StringStorageInterface $localeStorage, CacheBackendInterface $cache) {
// Note we use the install storage for the parent constructor. // Note we use the install storage for the parent constructor.
parent::__construct($configStorage, $schemaStorage); parent::__construct($configStorage, $schemaStorage, $cache);
$this->installStorage = $installStorage; $this->installStorage = $installStorage;
$this->localeStorage = $localeStorage; $this->localeStorage = $localeStorage;
} }
......
...@@ -46,7 +46,8 @@ public function testHasTranslation() { ...@@ -46,7 +46,8 @@ public function testHasTranslation() {
$this->container->get('config.storage.installer'), $this->container->get('config.storage.installer'),
$this->container->get('config.storage.schema'), $this->container->get('config.storage.schema'),
$this->container->get('config.storage.installer'), $this->container->get('config.storage.installer'),
$this->container->get('locale.storage') $this->container->get('locale.storage'),
$this->container->get('cache.config')
); );
$language = new Language(array('id' => 'de')); $language = new Language(array('id' => 'de'));
......
...@@ -11,7 +11,7 @@ services: ...@@ -11,7 +11,7 @@ services:
arguments: ['@entity.manager'] arguments: ['@entity.manager']
locale.config.typed: locale.config.typed:
class: Drupal\locale\LocaleConfigManager class: Drupal\locale\LocaleConfigManager
arguments: ['@config.storage', '@config.storage.schema', '@config.storage.installer', '@locale.storage'] arguments: ['@config.storage', '@config.storage.schema', '@config.storage.installer', '@locale.storage', '@cache.config']
locale.storage: locale.storage:
class: Drupal\locale\StringDatabaseStorage class: Drupal\locale\StringDatabaseStorage
arguments: ['@database'] arguments: ['@database']
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment