-
-
Notifications
You must be signed in to change notification settings - Fork 46
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Performance optimization #387
base: master
Are you sure you want to change the base?
Changes from 12 commits
3df3fb7
f18c1c6
f97bd53
3c367d3
ca8064e
bf3fadc
2f99923
72889e7
c890b27
77b916e
69a4841
316cb53
45df64a
c11b58f
0b13705
783df5f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,7 +19,6 @@ | |
use Yiisoft\Di\Helpers\DefinitionParser; | ||
use Yiisoft\Di\Reference\TagReference; | ||
|
||
use function array_key_exists; | ||
use function array_keys; | ||
use function implode; | ||
use function in_array; | ||
|
@@ -69,6 +68,16 @@ | |
private array $resetters = []; | ||
private bool $useResettersFromMeta = true; | ||
|
||
/** | ||
* @var array<string, mixed> Normalized definitions cache. | ||
*/ | ||
private array $normalizedDefinitions = []; | ||
|
||
/** | ||
* @var int Number of normalized definitions before the cache is cleared. | ||
*/ | ||
private const MAX_NORMALIZED_DEFINITIONS = 100; | ||
|
||
/** | ||
* @param ContainerConfigInterface $config Container configuration. | ||
* | ||
|
@@ -103,16 +112,20 @@ | |
*/ | ||
public function has(string $id): bool | ||
{ | ||
try { | ||
if ($this->definitions->has($id)) { | ||
return true; | ||
} | ||
} catch (CircularReferenceException) { | ||
return true; | ||
} | ||
|
||
if (TagReference::isTagAlias($id)) { | ||
$tag = TagReference::extractTagFromAlias($id); | ||
return isset($this->tags[$tag]); | ||
} | ||
|
||
try { | ||
return $this->definitions->has($id); | ||
} catch (CircularReferenceException) { | ||
return true; | ||
} | ||
return false; | ||
} | ||
|
||
/** | ||
|
@@ -136,65 +149,79 @@ | |
*/ | ||
public function get(string $id) | ||
{ | ||
if (!array_key_exists($id, $this->instances)) { | ||
try { | ||
try { | ||
$this->instances[$id] = $this->build($id); | ||
} catch (NotFoundExceptionInterface $exception) { | ||
if (!$this->delegates->has($id)) { | ||
if ($exception instanceof NotFoundException) { | ||
if ($id !== $exception->getId()) { | ||
$buildStack = $exception->getBuildStack(); | ||
array_unshift($buildStack, $id); | ||
throw new NotFoundException($exception->getId(), $buildStack); | ||
} | ||
throw $exception; | ||
} | ||
throw new NotFoundException($id, [$id], previous: $exception); | ||
} | ||
// Fast path: check if instance exists. | ||
if (isset($this->instances[$id])) { | ||
return $id === StateResetter::class ? $this->prepareStateResetter($id) : $this->instances[$id]; | ||
Check failure on line 154 in src/Container.php
|
||
samdark marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
/** @psalm-suppress MixedReturnStatement */ | ||
return $this->delegates->get($id); | ||
} | ||
} catch (Throwable $e) { | ||
if ($e instanceof ContainerExceptionInterface && !$e instanceof InvalidConfigException) { | ||
throw $e; | ||
} | ||
throw new BuildingException($id, $e, $this->definitions->getBuildStack(), $e); | ||
try { | ||
$this->instances[$id] = $this->build($id); | ||
} catch (NotFoundException $exception) { | ||
// Fast path: if the exception ID matches the requested ID, no need to modify stack. | ||
if ($exception->getId() === $id) { | ||
// Try delegates before giving up. | ||
return $this->delegates->has($id) ? $this->delegates->get($id) : throw $exception; | ||
Check failure on line 163 in src/Container.php
|
||
} | ||
|
||
// Add current ID to build stack for better error reporting. | ||
$buildStack = $exception->getBuildStack(); | ||
array_unshift($buildStack, $id); | ||
throw new NotFoundException($exception->getId(), $buildStack); | ||
} catch (NotFoundExceptionInterface $exception) { | ||
// Try delegates before giving up | ||
return $this->delegates->has($id) ? $this->delegates->get($id) : throw new NotFoundException($id, [$id], previous: $exception); | ||
Check failure on line 172 in src/Container.php
|
||
} catch (ContainerExceptionInterface $e) { | ||
if (!$e instanceof InvalidConfigException) { | ||
throw $e; | ||
} | ||
throw new BuildingException($id, $e, $this->definitions->getBuildStack(), $e); | ||
} catch (Throwable $e) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems we should catch any exceptions from previous "catch" blocks also (for example, exceptions from delegates). |
||
throw new BuildingException($id, $e, $this->definitions->getBuildStack(), $e); | ||
} | ||
|
||
// Handle StateResetter for newly built instances. | ||
if ($id === StateResetter::class) { | ||
$delegatesResetter = null; | ||
if ($this->delegates->has(StateResetter::class)) { | ||
$delegatesResetter = $this->delegates->get(StateResetter::class); | ||
} | ||
return $this->prepareStateResetter($id); | ||
Check failure on line 184 in src/Container.php
|
||
} | ||
|
||
/** @var StateResetter $mainResetter */ | ||
$mainResetter = $this->instances[$id]; | ||
/** @psalm-suppress MixedReturnStatement */ | ||
return $this->instances[$id]; | ||
} | ||
|
||
if ($this->useResettersFromMeta) { | ||
/** @var StateResetter[] $resetters */ | ||
$resetters = []; | ||
foreach ($this->resetters as $serviceId => $callback) { | ||
if (isset($this->instances[$serviceId])) { | ||
$resetters[$serviceId] = $callback; | ||
} | ||
} | ||
if ($delegatesResetter !== null) { | ||
$resetters[] = $delegatesResetter; | ||
} | ||
$mainResetter->setResetters($resetters); | ||
} elseif ($delegatesResetter !== null) { | ||
$resetter = new StateResetter($this->get(ContainerInterface::class)); | ||
$resetter->setResetters([$mainResetter, $delegatesResetter]); | ||
/** | ||
* @param string $id | ||
* @return mixed | ||
*/ | ||
private function prepareStateResetter(string $id) | ||
samdark marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ | ||
$delegatesResetter = null; | ||
if ($this->delegates->has(StateResetter::class)) { | ||
$delegatesResetter = $this->delegates->get(StateResetter::class); | ||
} | ||
|
||
/** @var StateResetter $mainResetter */ | ||
$mainResetter = $this->instances[$id]; | ||
|
||
return $resetter; | ||
if ($this->useResettersFromMeta) { | ||
/** @var StateResetter[] $resetters */ | ||
$resetters = []; | ||
foreach ($this->resetters as $serviceId => $callback) { | ||
if (isset($this->instances[$serviceId])) { | ||
$resetters[$serviceId] = $callback; | ||
} | ||
} | ||
if ($delegatesResetter !== null) { | ||
$resetters[] = $delegatesResetter; | ||
} | ||
$mainResetter->setResetters($resetters); | ||
} elseif ($delegatesResetter !== null) { | ||
$resetter = new StateResetter($this->get(ContainerInterface::class)); | ||
$resetter->setResetters([$mainResetter, $delegatesResetter]); | ||
|
||
return $resetter; | ||
} | ||
|
||
/** @psalm-suppress MixedReturnStatement */ | ||
return $this->instances[$id]; | ||
return $mainResetter; | ||
} | ||
|
||
/** | ||
|
@@ -212,12 +239,16 @@ | |
[$definition, $meta] = DefinitionParser::parse($definition); | ||
if ($this->validate) { | ||
$this->validateDefinition($definition, $id); | ||
$this->validateMeta($meta); | ||
// Only validate meta if it's not empty. | ||
if ($meta !== []) { | ||
samdark marked this conversation as resolved.
Show resolved
Hide resolved
|
||
$this->validateMeta($meta); | ||
} | ||
} | ||
/** | ||
* @psalm-var array{reset?:Closure,tags?:string[]} $meta | ||
*/ | ||
|
||
// Process meta only if it has tags or reset callback. | ||
if (isset($meta[self::META_TAGS])) { | ||
$this->setDefinitionTags($id, $meta[self::META_TAGS]); | ||
} | ||
|
@@ -226,6 +257,7 @@ | |
} | ||
|
||
unset($this->instances[$id]); | ||
|
||
$this->addDefinitionToStorage($id, $definition); | ||
} | ||
|
||
|
@@ -295,26 +327,24 @@ | |
*/ | ||
private function validateDefinition(mixed $definition, ?string $id = null): void | ||
{ | ||
if (is_array($definition) && isset($definition[DefinitionParser::IS_PREPARED_ARRAY_DEFINITION_DATA])) { | ||
$class = $definition['class']; | ||
$constructorArguments = $definition['__construct()']; | ||
|
||
/** | ||
* @var array $methodsAndProperties Is always array for prepared array definition data. | ||
* | ||
* @see DefinitionParser::parse() | ||
*/ | ||
$methodsAndProperties = $definition['methodsAndProperties']; | ||
|
||
$definition = array_merge( | ||
$class === null ? [] : [ArrayDefinition::CLASS_NAME => $class], | ||
[ArrayDefinition::CONSTRUCTOR => $constructorArguments], | ||
// extract only value from parsed definition method | ||
array_map(static fn (array $data): mixed => $data[2], $methodsAndProperties), | ||
); | ||
// Skip validation for common simple cases. | ||
if (is_string($definition) || $definition instanceof ContainerInterface || $definition instanceof Closure) { | ||
samdark marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return; | ||
} | ||
|
||
if ($definition instanceof ExtensibleService) { | ||
if (is_array($definition)) { | ||
if (isset($definition[DefinitionParser::IS_PREPARED_ARRAY_DEFINITION_DATA])) { | ||
$class = $definition['class']; | ||
$constructorArguments = $definition['__construct()']; | ||
$methodsAndProperties = $definition['methodsAndProperties']; | ||
|
||
$definition = array_merge( | ||
$class === null ? [] : [ArrayDefinition::CLASS_NAME => $class], | ||
[ArrayDefinition::CONSTRUCTOR => $constructorArguments], | ||
array_map(static fn (array $data): mixed => $data[2], $methodsAndProperties), | ||
Check failure on line 344 in src/Container.php
|
||
); | ||
} | ||
} elseif ($definition instanceof ExtensibleService) { | ||
throw new InvalidConfigException( | ||
'Invalid definition. ExtensibleService is only allowed in provider extensions.' | ||
); | ||
|
@@ -463,7 +493,7 @@ | |
/** | ||
* Creates new instance by either interface name or alias. | ||
* | ||
* @param string $id The interface or an alias name that was previously registered. | ||
* @param string $id The interface or the alias name that was previously registered. | ||
* | ||
* @throws InvalidConfigException | ||
* @throws NotFoundExceptionInterface | ||
|
@@ -475,10 +505,7 @@ | |
*/ | ||
private function build(string $id) | ||
{ | ||
if (TagReference::isTagAlias($id)) { | ||
return $this->getTaggedServices($id); | ||
} | ||
|
||
// Fast path: check for circular reference first as it's the most critical. | ||
if (isset($this->building[$id])) { | ||
if ($id === ContainerInterface::class) { | ||
return $this; | ||
|
@@ -492,15 +519,28 @@ | |
); | ||
} | ||
|
||
// Less common case: tag alias. | ||
if (TagReference::isTagAlias($id)) { | ||
return $this->getTaggedServices($id); | ||
} | ||
|
||
// Check if the definition exists. | ||
if (!$this->definitions->has($id)) { | ||
throw new NotFoundException($id, $this->definitions->getBuildStack()); | ||
} | ||
|
||
$this->building[$id] = 1; | ||
try { | ||
if (!$this->definitions->has($id)) { | ||
throw new NotFoundException($id, $this->definitions->getBuildStack()); | ||
// Use cached normalized definition if available. | ||
if (!isset($this->normalizedDefinitions[$id])) { | ||
samdark marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// Clear cache if it gets too large to prevent memory issues. | ||
if (count($this->normalizedDefinitions) >= self::MAX_NORMALIZED_DEFINITIONS) { | ||
$this->normalizedDefinitions = []; | ||
} | ||
$this->normalizedDefinitions[$id] = DefinitionNormalizer::normalize($this->definitions->get($id), $id); | ||
} | ||
|
||
$definition = DefinitionNormalizer::normalize($this->definitions->get($id), $id); | ||
|
||
$object = $definition->resolve($this->get(ContainerInterface::class)); | ||
$object = $this->normalizedDefinitions[$id]->resolve($this->get(ContainerInterface::class)); | ||
Check failure on line 543 in src/Container.php
|
||
} finally { | ||
unset($this->building[$id]); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use
array_key_exists
instead ofisset
. Theoretically, result may benull
.