From b30518d1c06ad22fa81bf679290e0fc8186e4393 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Sat, 15 Jul 2023 16:41:16 +0200 Subject: [PATCH] Optimize TypeRegistry::lookupName() from O(N) to O(1) (#6082) | Q | A |------------- | ----------- | Type | performance issue | Fixed issues | n/a #### Summary We use a lot of custom types and migration from `Type::getName()` to `TypeRegistry->lookupName()` discovered us the reverse lookup was very slow (O(N)). This PR makes it O(1). ~Index is initialized lazily on the 1st reverse lookup.~ The type registry is expected to not hold duplicate type instances and this PR fixes it, in https://github.com/doctrine/dbal/pull/6083#discussion_r1261812593 I was requested to fix it in this PR instead of a separate one. --- src/Types/TypeRegistry.php | 36 ++++++++++++++++++-------------- tests/Types/TypeRegistryTest.php | 8 +++++++ 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/src/Types/TypeRegistry.php b/src/Types/TypeRegistry.php index b5e800ad1b4..9b64c6faf9a 100644 --- a/src/Types/TypeRegistry.php +++ b/src/Types/TypeRegistry.php @@ -6,8 +6,7 @@ use Doctrine\DBAL\Exception; -use function array_search; -use function in_array; +use function spl_object_id; /** * The type registry is responsible for holding a map of all known DBAL types. @@ -17,11 +16,17 @@ final class TypeRegistry { /** @var array Map of type names and their corresponding flyweight objects. */ private array $instances; + /** @var array */ + private array $instancesReverseIndex; /** @param array $instances */ public function __construct(array $instances = []) { - $this->instances = $instances; + $this->instances = []; + $this->instancesReverseIndex = []; + foreach ($instances as $name => $type) { + $this->register($name, $type); + } } /** @@ -31,11 +36,12 @@ public function __construct(array $instances = []) */ public function get(string $name): Type { - if (! isset($this->instances[$name])) { + $type = $this->instances[$name] ?? null; + if ($type === null) { throw Exception::unknownColumnType($name); } - return $this->instances[$name]; + return $type; } /** @@ -77,7 +83,8 @@ public function register(string $name, Type $type): void throw Exception::typeAlreadyRegistered($type); } - $this->instances[$name] = $type; + $this->instances[$name] = $type; + $this->instancesReverseIndex[spl_object_id($type)] = $name; } /** @@ -87,15 +94,18 @@ public function register(string $name, Type $type): void */ public function override(string $name, Type $type): void { - if (! isset($this->instances[$name])) { + $origType = $this->instances[$name] ?? null; + if ($origType === null) { throw Exception::typeNotFound($name); } - if (! in_array($this->findTypeName($type), [$name, null], true)) { + if (($this->findTypeName($type) ?? $name) !== $name) { throw Exception::typeAlreadyRegistered($type); } - $this->instances[$name] = $type; + unset($this->instancesReverseIndex[spl_object_id($origType)]); + $this->instances[$name] = $type; + $this->instancesReverseIndex[spl_object_id($type)] = $name; } /** @@ -112,12 +122,6 @@ public function getMap(): array private function findTypeName(Type $type): ?string { - $name = array_search($type, $this->instances, true); - - if ($name === false) { - return null; - } - - return $name; + return $this->instancesReverseIndex[spl_object_id($type)] ?? null; } } diff --git a/tests/Types/TypeRegistryTest.php b/tests/Types/TypeRegistryTest.php index 42d57b69a56..36c3d7949a6 100644 --- a/tests/Types/TypeRegistryTest.php +++ b/tests/Types/TypeRegistryTest.php @@ -99,6 +99,14 @@ public function testRegisterWithAlreadyRegisteredInstance(): void $this->registry->register('other', $newType); } + public function testConstructorWithDuplicateInstance(): void + { + $newType = new TextType(); + + $this->expectException(Exception::class); + new TypeRegistry(['a' => $newType, 'b' => $newType]); + } + public function testOverride(): void { $baseType = new TextType();