Issue #3327856 by alexpott, catch, zcht, longwave, znerol, Berdir, BramDriesen, Spokje, callen321, andypost, neclimdul, acbramley, mstrelan: Performance regression introduced by container serialization solution

merge-requests/2984/head
Lee Rowlands 2022-12-23 15:25:14 +10:00
parent 581813e76f
commit aaf6426b64
No known key found for this signature in database
GPG Key ID: 2B829A3DF9204DC4
13 changed files with 233 additions and 35 deletions

View File

@ -386,6 +386,8 @@ services:
class: Drupal\Core\Plugin\Context\LazyContextRepository
arguments: ['@service_container']
Drupal\Core\Plugin\Context\ContextRepositoryInterface: '@context.repository'
Drupal\Component\DependencyInjection\ReverseContainer:
arguments: [ '@service_container' ]
cron:
class: Drupal\Core\Cron
arguments: ['@module_handler', '@lock', '@queue', '@state', '@account_switcher', '@logger.channel.cron', '@plugin.manager.queue_worker', '@datetime.time']

View File

@ -25,6 +25,11 @@ interface ContainerInterface extends BaseContainerInterface {
*
* @return array
* Service ids keyed by a unique hash.
*
* @deprecated in drupal:9.5.1 and is removed from drupal:11.0.0. Use the
* 'Drupal\Component\DependencyInjection\ReverseContainer' service instead.
*
* @see https://www.drupal.org/node/3327942
*/
public function getServiceIdMappings(): array;
@ -36,6 +41,11 @@ interface ContainerInterface extends BaseContainerInterface {
*
* @return string
* A unique hash identifying the object.
*
* @deprecated in drupal:9.5.1 and is removed from drupal:11.0.0. Use the
* 'Drupal\Component\DependencyInjection\ReverseContainer' service instead.
*
* @see https://www.drupal.org/node/3327942
*/
public function generateServiceIdHash(object $object): string;

View File

@ -0,0 +1,98 @@
<?php
namespace Drupal\Component\DependencyInjection;
use Symfony\Component\DependencyInjection\Container as SymfonyContainer;
use Symfony\Component\DependencyInjection\ContainerInterface as SymfonyContainerInterface;
/**
* Retrieves service IDs from the container for public services.
*
* Heavily inspired by \Symfony\Component\DependencyInjection\ReverseContainer.
*/
final class ReverseContainer {
/**
* A closure on the container that can search for services.
*
* @var \Closure
*/
private \Closure $getServiceId;
/**
* A static map of services to a hash.
*
* @var array
*/
private static array $recordedServices = [];
/**
* Constructs a ReverseContainer object.
*
* @param \Drupal\Component\DependencyInjection\Container|\Symfony\Component\DependencyInjection\Container $serviceContainer
* The service container.
*/
public function __construct(private readonly Container|SymfonyContainer $serviceContainer) {
$this->getServiceId = \Closure::bind(function ($service): ?string {
/** @phpstan-ignore-next-line */
return array_search($service, $this->services, TRUE) ?: NULL;
}, $serviceContainer, $serviceContainer);
}
/**
* Returns the ID of the passed object when it exists as a service.
*
* To be reversible, services need to be public.
*
* @param object $service
* The service to find the ID for.
*/
public function getId(object $service): ?string {
if ($this->serviceContainer === $service || $service instanceof SymfonyContainerInterface) {
return 'service_container';
}
$hash = $this->generateServiceIdHash($service);
$id = self::$recordedServices[$hash] ?? ($this->getServiceId)($service);
if ($id !== NULL && $this->serviceContainer->has($id)) {
self::$recordedServices[$hash] = $id;
return $id;
}
return NULL;
}
/**
* Records a map of the container's services.
*
* This method is used so that stale services can be serialized after a
* container has been re-initialized.
*/
public function recordContainer(): void {
$service_recorder = \Closure::bind(function () : array {
/** @phpstan-ignore-next-line */
return $this->services;
}, $this->serviceContainer, $this->serviceContainer);
self::$recordedServices = array_merge(self::$recordedServices, array_flip(array_map([$this, 'generateServiceIdHash'], $service_recorder())));
}
/**
* Generates an identifier for a service based on the object class and hash.
*
* @param object $object
* The object to generate an identifier for.
*
* @return string
* The object's class and hash concatenated together.
*/
private function generateServiceIdHash(object $object): string {
// Include class name as an additional namespace for the hash since
// spl_object_hash's return can be recycled. This still is not a 100%
// guarantee to be unique but makes collisions incredibly difficult and even
// then the interface would be preserved.
// @see https://php.net/spl_object_hash#refsect1-function.spl-object-hash-notes
return get_class($object) . spl_object_hash($object);
}
}

View File

@ -5,7 +5,10 @@ namespace Drupal\Component\DependencyInjection;
/**
* A trait for service id hashing implementations.
*
* Handles delayed cache tag invalidations.
* @deprecated in drupal:9.5.1 and is removed from drupal:11.0.0. Use the
* 'Drupal\Component\DependencyInjection\ReverseContainer' service instead.
*
* @see https://www.drupal.org/node/3327942
*/
trait ServiceIdHashTrait {
@ -13,6 +16,7 @@ trait ServiceIdHashTrait {
* Implements \Drupal\Component\DependencyInjection\ContainerInterface::getServiceIdMappings()
*/
public function getServiceIdMappings(): array {
@trigger_error(__METHOD__ . "() is deprecated in drupal:9.5.1 and is removed from drupal:11.0.0. Use the 'Drupal\Component\DependencyInjection\ReverseContainer' service instead. See https://www.drupal.org/node/3327942", E_USER_DEPRECATED);
$mapping = [];
foreach ($this->getServiceIds() as $service_id) {
if ($this->initialized($service_id) && $service_id !== 'service_container') {
@ -26,6 +30,7 @@ trait ServiceIdHashTrait {
* Implements \Drupal\Component\DependencyInjection\ContainerInterface::generateServiceIdHash()
*/
public function generateServiceIdHash(object $object): string {
@trigger_error(__METHOD__ . "() is deprecated in drupal:9.5.1 and is removed from drupal:11.0.0. Use the 'Drupal\Component\DependencyInjection\ReverseContainer' service instead. See https://www.drupal.org/node/3327942", E_USER_DEPRECATED);
// Include class name as an additional namespace for the hash since
// spl_object_hash's return can be recycled. This still is not a 100%
// guarantee to be unique but makes collisions incredibly difficult and even

View File

@ -2,9 +2,9 @@
namespace Drupal\Core\DependencyInjection;
use Drupal\Component\DependencyInjection\ReverseContainer;
use Drupal\Core\Entity\EntityStorageInterface;
use Symfony\Component\DependencyInjection\ContainerInterface as SymfonyContainerInterface;
use Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException;
use Drupal\Core\StringTranslation\TranslatableMarkup;
/**
* Provides dependency injection friendly methods for serialization.
@ -34,8 +34,12 @@ trait DependencySerializationTrait {
$vars = get_object_vars($this);
try {
$container = \Drupal::getContainer();
$mapping = \Drupal::service('kernel')->getServiceIdMapping();
$reverse_container = $container->get(ReverseContainer::class);
foreach ($vars as $key => $value) {
if (!is_object($value) || $value instanceof TranslatableMarkup) {
// Ignore properties that cannot be services.
continue;
}
if ($value instanceof EntityStorageInterface) {
// If a class member is an entity storage, only store the entity type
// ID the storage is for, so it can be used to get a fresh object on
@ -47,34 +51,18 @@ trait DependencySerializationTrait {
$this->_entityStorages[$key] = $value->getEntityTypeId();
unset($vars[$key]);
}
elseif (is_object($value)) {
$service_id = FALSE;
// Special case the container.
if ($value instanceof SymfonyContainerInterface) {
$service_id = 'service_container';
}
else {
$id = $container->generateServiceIdHash($value);
if (isset($mapping[$id])) {
$service_id = $mapping[$id];
}
}
if ($service_id) {
// If a class member was instantiated by the dependency injection
// container, only store its ID so it can be used to get a fresh object
// on unserialization.
$this->_serviceIds[$key] = $service_id;
unset($vars[$key]);
}
elseif ($service_id = $reverse_container->getId($value)) {
// If a class member was instantiated by the dependency injection
// container, only store its ID so it can be used to get a fresh object
// on unserialization.
$this->_serviceIds[$key] = $service_id;
unset($vars[$key]);
}
}
}
catch (ContainerNotInitializedException $e) {
// No container, no problem.
}
catch (ServiceNotFoundException $e) {
// No kernel, very strange, but still no problem.
}
return array_keys($vars);
}

View File

@ -10,6 +10,7 @@ use Drupal\Core\Cache\DatabaseBackend;
use Drupal\Core\Config\BootstrapConfigStorageFactory;
use Drupal\Core\Config\NullStorage;
use Drupal\Core\DependencyInjection\ContainerBuilder;
use Drupal\Component\DependencyInjection\ReverseContainer;
use Drupal\Core\DependencyInjection\ServiceModifierInterface;
use Drupal\Core\DependencyInjection\ServiceProviderInterface;
use Drupal\Core\DependencyInjection\YamlFileLoader;
@ -238,6 +239,11 @@ class DrupalKernel implements DrupalKernelInterface, TerminableInterface {
/**
* A mapping from service classes to service IDs.
*
* @deprecated in drupal:9.5.1 and is removed from drupal:11.0.0. Use the
* 'Drupal\Component\DependencyInjection\ReverseContainer' service instead.
*
* @see https://www.drupal.org/node/3327942
*/
protected $serviceIdMapping = [];
@ -795,8 +801,14 @@ class DrupalKernel implements DrupalKernelInterface, TerminableInterface {
*
* @return string
* A unique hash value.
*
* @deprecated in drupal:9.5.1 and is removed from drupal:11.0.0. Use the
* 'Drupal\Component\DependencyInjection\ReverseContainer' service instead.
*
* @see https://www.drupal.org/node/3327942
*/
public static function generateServiceIdHash($object) {
@trigger_error(__METHOD__ . "() is deprecated in drupal:9.5.1 and is removed from drupal:11.0.0. Use the 'Drupal\Component\DependencyInjection\ReverseContainer' service instead. See https://www.drupal.org/node/3327942", E_USER_DEPRECATED);
// Include class name as an additional namespace for the hash since
// spl_object_hash's return can be recycled. This still is not a 100%
// guarantee to be unique but makes collisions incredibly difficult and even
@ -809,6 +821,7 @@ class DrupalKernel implements DrupalKernelInterface, TerminableInterface {
* {@inheritdoc}
*/
public function getServiceIdMapping() {
@trigger_error(__METHOD__ . "() is deprecated in drupal:9.5.1 and is removed from drupal:11.0.0. Use the 'Drupal\Component\DependencyInjection\ReverseContainer' service instead. See https://www.drupal.org/node/3327942", E_USER_DEPRECATED);
$this->collectServiceIdMapping();
return $this->serviceIdMapping;
}
@ -858,8 +871,12 @@ class DrupalKernel implements DrupalKernelInterface, TerminableInterface {
if ($this->container->initialized('current_user')) {
$current_user_id = $this->container->get('current_user')->id();
}
// Save the current services.
$this->collectServiceIdMapping();
// After rebuilding the container some objects will have stale services.
// Record a map of objects to service IDs prior to rebuilding the
// container in order to ensure
// \Drupal\Core\DependencyInjection\DependencySerializationTrait works as
// expected.
$this->container->get(ReverseContainer::class)->recordContainer();
// If there is a session, close and save it.
if ($this->container->initialized('session')) {
@ -1574,8 +1591,14 @@ class DrupalKernel implements DrupalKernelInterface, TerminableInterface {
/**
* Collect a mapping between service to ids.
*
* @deprecated in drupal:9.5.1 and is removed from drupal:11.0.0. Use the
* 'Drupal\Component\DependencyInjection\ReverseContainer' service instead.
*
* @see https://www.drupal.org/node/3327942
*/
protected function collectServiceIdMapping() {
@trigger_error(__METHOD__ . "() is deprecated in drupal:9.5.1 and is removed from drupal:11.0.0. Use the 'Drupal\Component\DependencyInjection\ReverseContainer' service instead. See https://www.drupal.org/node/3327942", E_USER_DEPRECATED);
if (isset($this->container)) {
foreach ($this->container->getServiceIdMappings() as $hash => $service_id) {
$this->serviceIdMapping[$hash] = $service_id;

View File

@ -139,6 +139,11 @@ interface DrupalKernelInterface extends HttpKernelInterface, ContainerAwareInter
/**
* Get a mapping from service hashes to service IDs.
*
* @deprecated in drupal:9.5.1 and is removed from drupal:11.0.0. Use the
* 'Drupal\Component\DependencyInjection\ReverseContainer' service instead.
*
* @see https://www.drupal.org/node/3327942
*/
public function getServiceIdMapping();

View File

@ -3,6 +3,7 @@
namespace Drupal\Core\Test;
use Drupal\Core\DependencyInjection\ContainerBuilder;
use Drupal\Component\DependencyInjection\ReverseContainer;
use Drupal\Core\DrupalKernel;
/**
@ -38,6 +39,7 @@ class TestKernel extends DrupalKernel {
// inside those objects.
$kernel->container = $container;
$container->set('kernel', $kernel);
$container->set(ReverseContainer::class, new ReverseContainer($container));
\Drupal::setContainer($container);
return $container;
}

View File

@ -2,6 +2,7 @@
namespace Drupal\Tests\system\Kernel;
use Drupal\Component\DependencyInjection\ReverseContainer;
use Drupal\decorated_service_test\TestServiceDecorator;
use Drupal\KernelTests\KernelTestBase;
@ -22,16 +23,12 @@ class DecoratedServiceTest extends KernelTestBase {
public function testDecoratedServiceId() {
// Service decorated once.
$test_service = $this->container->get('test_service');
$hash = $this->container->generateServiceIdHash($test_service);
$mappings = $this->container->getServiceIdMappings();
$this->assertEquals('test_service', $mappings[$hash]);
$this->assertEquals('test_service', $this->container->get(ReverseContainer::class)->getId($test_service));
$this->assertInstanceOf(TestServiceDecorator::class, $test_service);
// Service decorated twice.
$test_service2 = $this->container->get('test_service2');
$hash = $this->container->generateServiceIdHash($test_service2);
$mappings = $this->container->getServiceIdMappings();
$this->assertEquals('test_service2', $mappings[$hash]);
$this->assertEquals('test_service2', $this->container->get(ReverseContainer::class)->getId($test_service2));
$this->assertInstanceOf(TestServiceDecorator::class, $test_service2);
}

View File

@ -10,6 +10,7 @@ namespace Drupal\Tests\Component\DependencyInjection;
use Drupal\Component\Utility\Crypt;
use PHPUnit\Framework\TestCase;
use Prophecy\PhpUnit\ProphecyTrait;
use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
use Symfony\Component\DependencyInjection\Exception\LogicException;
@ -24,7 +25,7 @@ use Prophecy\Argument;
* @group DependencyInjection
*/
class ContainerTest extends TestCase {
use ExpectDeprecationTrait;
use ProphecyTrait;
/**
@ -688,8 +689,12 @@ class ContainerTest extends TestCase {
/**
* @covers \Drupal\Component\DependencyInjection\ServiceIdHashTrait::getServiceIdMappings
* @covers \Drupal\Component\DependencyInjection\ServiceIdHashTrait::generateServiceIdHash
*
* @group legacy
*/
public function testGetServiceIdMappings() {
$this->expectDeprecation("Drupal\Component\DependencyInjection\ServiceIdHashTrait::generateServiceIdHash() is deprecated in drupal:9.5.1 and is removed from drupal:11.0.0. Use the 'Drupal\Component\DependencyInjection\ReverseContainer' service instead. See https://www.drupal.org/node/3327942");
$this->expectDeprecation("Drupal\Component\DependencyInjection\ServiceIdHashTrait::getServiceIdMappings() is deprecated in drupal:9.5.1 and is removed from drupal:11.0.0. Use the 'Drupal\Component\DependencyInjection\ReverseContainer' service instead. See https://www.drupal.org/node/3327942");
$this->assertEquals([], $this->container->getServiceIdMappings());
$s1 = $this->container->get('other.service');
$s2 = $this->container->get('late.service');

View File

@ -0,0 +1,58 @@
<?php
namespace Drupal\Tests\Component\DependencyInjection;
use Drupal\Component\DependencyInjection\ReverseContainer;
use PHPUnit\Framework\TestCase;
use Symfony\Component\DependencyInjection\ContainerBuilder;
/**
* @runTestsInSeparateProcesses
* The reverse container uses a static to maintain information across
* container rebuilds.
*
* @coversDefaultClass \Drupal\Component\DependencyInjection\ReverseContainer
* @group DependencyInjection
*/
class ReverseContainerTest extends TestCase {
/**
* @covers ::getId
*/
public function testGetId(): void {
$container = new ContainerBuilder();
$service = new \stdClass();
$container->set('bar', $service);
$reverse_container = new ReverseContainer($container);
$this->assertSame('bar', $reverse_container->getId($service));
$non_service = new \stdClass();
$this->assertNull($reverse_container->getId($non_service));
$this->assertSame('service_container', $reverse_container->getId($container));
}
/**
* @covers ::recordContainer
*/
public function testRecordContainer(): void {
$container = new ContainerBuilder();
$service = new \stdClass();
$container->set('bar', $service);
$reverse_container = new ReverseContainer($container);
$reverse_container->recordContainer();
$container = new ContainerBuilder();
$reverse_container = new ReverseContainer($container);
// New container does not have a bar service.
$this->assertNull($reverse_container->getId($service));
// Add the bar service to make the lookup based on the old object work as
// expected.
$container->set('bar', new \stdClass());
$this->assertSame('bar', $reverse_container->getId($service));
}
}

View File

@ -7,6 +7,7 @@
namespace Drupal\Tests\Core\DependencyInjection;
use Drupal\Component\DependencyInjection\ReverseContainer;
use Drupal\Core\DependencyInjection\DependencySerializationTrait;
use Drupal\Core\Test\TestKernel;
use Drupal\Tests\UnitTestCase;
@ -37,6 +38,7 @@ class DependencySerializationTest extends UnitTestCase {
/** @var \Drupal\Tests\Core\DependencyInjection\DependencySerializationTestDummy $dependencySerialization */
$dependencySerialization = unserialize($string);
$this->assertTrue($container->has(ReverseContainer::class));
$this->assertSame($service, $dependencySerialization->service);
$this->assertSame($container, $dependencySerialization->container);
$this->assertEmpty($dependencySerialization->getServiceIds());

View File

@ -140,8 +140,11 @@ EOD;
/**
* @covers ::getServiceIdMapping
* @group legacy
*/
public function testGetServiceIdMapping() {
$this->expectDeprecation("Drupal\Core\DrupalKernel::getServiceIdMapping() is deprecated in drupal:9.5.1 and is removed from drupal:11.0.0. Use the 'Drupal\Component\DependencyInjection\ReverseContainer' service instead. See https://www.drupal.org/node/3327942");
$this->expectDeprecation("Drupal\Core\DrupalKernel::collectServiceIdMapping() is deprecated in drupal:9.5.1 and is removed from drupal:11.0.0. Use the 'Drupal\Component\DependencyInjection\ReverseContainer' service instead. See https://www.drupal.org/node/3327942");
$service = new BarClass();
$container = TestKernel::setContainerWithKernel();
$container->set('bar', $service);