laminas / laminas-di

Automated dependency injection for PSR-11 containers
https://docs.laminas.dev/laminas-di/
BSD 3-Clause "New" or "Revised" License
36 stars 20 forks source link

Update `ClassDefinition#getParameters()` to use a more efficient constructor lookup #66

Closed sjokkateer closed 2 years ago

sjokkateer commented 2 years ago

Signed-off-by: Remy Bos 27890746+sjokkateer@users.noreply.github.com

Q A
Documentation no
Bugfix no
BC Break no
New Feature no
RFC no
QA no

Description

This PR changes the internals of ClassDefinition::reflectParameters() to call the ReflectionClass::getConstructor() method directly rather than obtaining the constructor through getMethod('__construct'). Intuitively the ReflectionClass::getConstructor() method could be faster.

Ocramius commented 2 years ago

could be faster.

Now that you have a benchmark suite to try this on, I think this requires a benchmark to be written 😛

EDIT: never mind, you did in #67!

sjokkateer commented 2 years ago

could be faster.

Now that you have a benchmark suite to try this on, I think this requires a benchmark to be written 😛

EDIT: never mind, you did in #67!

Do you prefer me to put those benchmarks in the same PR or like I do right now such that you can first compare branches?

Ocramius commented 2 years ago

No, I think the approach is fine, we just need to add the benchmark results here though (comment suffices)

sjokkateer commented 2 years ago
./vendor/bin/phpbench run --warmup=2 --revs=1000 --iterations=20 --retry-threshold=5 --tag=before_patch_66
PHPBench (1.2.5) running benchmarks... #standwithukraine
with configuration file: /laminas-di/phpbench.json
with PHP version 7.4.29, xdebug ❌, opcache ❌

\LaminasBench\Di\ClassDefinitionBench

    benchGetParametersBeforeTransparentCach.R1 I18 - Mo0.134μs (±2.13%)
    benchGetParametersBeforeTransparentCach.R1 I2 - Mo0.214μs (±1.27%)
    benchGetParametersBeforeTransparentCach.R1 I8 - Mo0.695μs (±1.58%)
    benchGetParametersConstructorNoOtherMet.R2 I13 - Mo0.214μs (±0.79%)
    benchGetParametersConstructorMultipleMe.R2 I13 - Mo0.215μs (±1.29%)
    benchGetParametersConstructorInheritedN.R1 I9 - Mo0.213μs (±1.15%)

Subjects: 6, Assertions: 0, Failures: 0, Errors: 0
Storing results ... OK
Run: 1348ad5e086b0610999cd3815663fe377d41888f
./vendor/bin/phpbench run --warmup=2 --revs=1000 --iterations=20 --retry-threshold=5 --tag=after_patch_66
PHPBench (1.2.5) running benchmarks... #standwithukraine
with configuration file: /laminas-di/phpbench.json
with PHP version 7.4.29, xdebug ❌, opcache ❌

\LaminasBench\Di\ClassDefinitionBench

    benchGetParametersBeforeTransparentCach.R1 I18 - Mo0.114μs (±1.40%)
    benchGetParametersBeforeTransparentCach.R1 I1 - Mo0.160μs (±1.83%)
    benchGetParametersBeforeTransparentCach.R1 I19 - Mo0.643μs (±1.23%)
    benchGetParametersConstructorNoOtherMet.R1 I19 - Mo0.161μs (±0.84%)
    benchGetParametersConstructorMultipleMe.R1 I19 - Mo0.161μs (±0.94%)
    benchGetParametersConstructorInheritedN.R1 I19 - Mo0.160μs (±0.93%)

Subjects: 6, Assertions: 0, Failures: 0, Errors: 0
Storing results ... OK
Run: 1348ad57413179ddd376e116c7f2101b58c88221
./vendor/bin/phpbench report --ref=before_patch_66 --ref=after_patch_66 --report=aggregate
+----------------------+-------------------------------------------------------------------------+-----+------+-----+----------+---------+--------+
| benchmark            | subject                                                                 | set | revs | its | mem_peak | mode    | rstdev |
+----------------------+-------------------------------------------------------------------------+-----+------+-----+----------+---------+--------+
| ClassDefinitionBench | benchGetParametersBeforeTransparentCachingNoConstructor                 |     | 1000 | 20  | 1.710mb  | 0.134μs | ±2.13% |
| ClassDefinitionBench | benchGetParametersBeforeTransparentCachingNoConstructorParameters       |     | 1000 | 20  | 1.710mb  | 0.214μs | ±1.27% |
| ClassDefinitionBench | benchGetParametersBeforeTransparentCachingMultipleConstructorParameters |     | 1000 | 20  | 1.710mb  | 0.695μs | ±1.58% |
| ClassDefinitionBench | benchGetParametersConstructorNoOtherMethods                             |     | 1000 | 20  | 1.710mb  | 0.214μs | ±0.79% |
| ClassDefinitionBench | benchGetParametersConstructorMultipleMethods                            |     | 1000 | 20  | 1.710mb  | 0.215μs | ±1.29% |
| ClassDefinitionBench | benchGetParametersConstructorInheritedNoOtherMethods                    |     | 1000 | 20  | 1.710mb  | 0.213μs | ±1.15% |
+----------------------+-------------------------------------------------------------------------+-----+------+-----+----------+---------+--------+
./vendor/bin/phpbench report --ref=after_patch_66 --report=aggregate
+----------------------+-------------------------------------------------------------------------+-----+------+-----+----------+---------+--------+
| benchmark            | subject                                                                 | set | revs | its | mem_peak | mode    | rstdev |
+----------------------+-------------------------------------------------------------------------+-----+------+-----+----------+---------+--------+
| ClassDefinitionBench | benchGetParametersBeforeTransparentCachingNoConstructor                 |     | 1000 | 20  | 1.710mb  | 0.114μs | ±1.40% |
| ClassDefinitionBench | benchGetParametersBeforeTransparentCachingNoConstructorParameters       |     | 1000 | 20  | 1.710mb  | 0.160μs | ±1.83% |
| ClassDefinitionBench | benchGetParametersBeforeTransparentCachingMultipleConstructorParameters |     | 1000 | 20  | 1.710mb  | 0.643μs | ±1.23% |
| ClassDefinitionBench | benchGetParametersConstructorNoOtherMethods                             |     | 1000 | 20  | 1.710mb  | 0.161μs | ±0.84% |
| ClassDefinitionBench | benchGetParametersConstructorMultipleMethods                            |     | 1000 | 20  | 1.710mb  | 0.161μs | ±0.94% |
| ClassDefinitionBench | benchGetParametersConstructorInheritedNoOtherMethods                    |     | 1000 | 20  | 1.710mb  | 0.160μs | ±0.93% |
+----------------------+-------------------------------------------------------------------------+-----+------+-----+----------+---------+--------+
Ocramius commented 2 years ago

That is a nice improvement 💪