Improve performance#114
Conversation
There was a problem hiding this comment.
Pull request overview
This PR focuses on performance improvements in the definitions normalization/validation path by introducing caching and fast-path checks, and adds PhpBench benchmarks to measure typical usage scenarios.
Changes:
- Added multiple caches and fast-path branches in
Normalizer,ArrayDefinition,CallableDefinition, andDefinitionStorageto reduce repeated parsing/reflection work. - Expanded unit tests to cover the new caching/fast-path behaviors (normalization, validator callable handling, build stack clearing).
- Introduced a PhpBench benchmark suite and configuration, plus a dev dependency to run it.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/Helpers/Normalizer.php |
Adds static caches and fast-path logic for classes/references/callables/values during normalization. |
src/ArrayDefinition.php |
Caches parsed config keys and reuses prepared “class-only” definitions; avoids some getter calls. |
src/CallableDefinition.php |
Caches extracted callable dependencies and invokes via prepared closure. |
src/DefinitionStorage.php |
Adds fast-path for explicit definitions and strict-mode misses; reduces build stack resets. |
src/Helpers/DefinitionValidator.php |
Adds coverage-ignore markers for PHP 8.4+ asymmetric visibility modifiers branch. |
tests/Unit/Helpers/NormalizerTest.php |
Adds tests for caching behavior and autoload avoidance for plain references. |
tests/Unit/Helpers/DefinitionValidatorTest.php |
Adds a test ensuring callable array definitions validate successfully. |
tests/Unit/DefinitionStorageTest.php |
Adds a test ensuring explicit lookups clear previous build stack state. |
tests/Unit/ArrayDefinitionTest.php |
Adds coverage for prepared-data arguments and ignored config keys. |
tests/Benchmark/TypicalUsageBench.php |
Adds a benchmark for common resolution patterns (array defs, callable defs, references). |
tests/Benchmark/NormalizerBench.php |
Adds a benchmark for normalization hot paths across multiple input types. |
tests/Benchmark/DefinitionStorageBench.php |
Adds a benchmark for storage construction and lookup patterns (strict/non-strict). |
phpbench.json |
Adds PhpBench configuration (bootstrap, paths, CSV output). |
composer.json |
Adds phpbench/phpbench to require-dev and suggests it for running benchmarks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
# Conflicts: # src/ArrayDefinition.php
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Tigrov
left a comment
There was a problem hiding this comment.
There were more optimizations, were they ineffective?
|
Yes. I've updated the benchmark to match real usage, and the majority of optimizations revealed to be either not necessary or making things worse. |
Benchmark results against master after removing optimizations that did not show a reliable gain.
The suite uses 15 iterations now. Very small subjects use higher rev counts, and the table includes relative standard deviation. Deltas inside the measured deviation should be treated as noise rather than a reliable gain or regression.
Retained changes:
Not retained as optimizations:
ArrayDefinitionmethod dependency cache.DefinitionStorage::has()explicit-definition and strict-mode fast paths.CallableDefinitiondependency cache and direct callable invocation.Normalizercaches.Note: Yii DI services are shared, so definition resolution normally happens once per service per container lifetime. The benchmark data does not show a meaningful typical request-time performance gain from this PR.