yiisoft / injector

PSR-11 compatible injector
https://www.yiiframework.com/
BSD 3-Clause "New" or "Revised" License
43 stars 18 forks source link

Cache reflections #52

Closed xepozz closed 1 year ago

xepozz commented 3 years ago
Q A
Is bugfix?
New feature? ✔️
Breaks BC?
Fixed issues

Added reflections cache Now the results on my computer are:

Subjects: 1, Assertions: 0, Failures: 0, Errors: 0
+-----------+-----------+----------------------------+-------+-----+----------+---------+--------+
| benchmark | subject   | set                        | revs  | its | mem_peak | mode    | rstdev |
+-----------+-----------+----------------------------+-------+-----+----------+---------+--------+
| MainBench | benchMake | without constructor        | 10000 | 10  | 6.310mb  | 1.249μs | ±7.01% |
| MainBench | benchMake | with empty constructor     | 10000 | 10  | 6.322mb  | 3.554μs | ±3.60% |
| MainBench | benchMake | with not empty constructor | 10000 | 10  | 6.329mb  | 8.747μs | ±4.62% |
+-----------+-----------+----------------------------+-------+-----+----------+---------+--------+

Without that PR the results are (see #51):

Subjects: 1, Assertions: 0, Failures: 0, Errors: 0
+-----------+-----------+----------------------------+-------+-----+----------+---------+--------+
| benchmark | subject   | set                        | revs  | its | mem_peak | mode    | rstdev |
+-----------+-----------+----------------------------+-------+-----+----------+---------+--------+
| MainBench | benchMake | without constructor        | 10000 | 10  | 6.310mb  | 1.596μs | ±4.71% |
| MainBench | benchMake | with empty constructor     | 10000 | 10  | 6.321mb  | 3.716μs | ±1.95% |
| MainBench | benchMake | with not empty constructor | 10000 | 10  | 6.329mb  | 8.871μs | ±2.26% |
+-----------+-----------+----------------------------+-------+-----+----------+---------+--------+
samdark commented 3 years ago

https://gist.github.com/mindplay-dk/3359812

samdark commented 3 years ago

Need to re-verify it.

samdark commented 3 years ago

My results:

Without cache:

+-----------+-----------+----------------------------+-------+-----+----------+---------+--------+
| benchmark | subject   | set                        | revs  | its | mem_peak | mode    | rstdev |
+-----------+-----------+----------------------------+-------+-----+----------+---------+--------+
| MainBench | benchMake | without constructor        | 10000 | 10  | 4.115mb  | 0.771μs | ±2.96% |
| MainBench | benchMake | with empty constructor     | 10000 | 10  | 4.118mb  | 2.144μs | ±3.06% |
| MainBench | benchMake | with not empty constructor | 10000 | 10  | 4.124mb  | 5.031μs | ±2.22% |
+-----------+-----------+----------------------------+-------+-----+----------+---------+--------+

With cache:

+-----------+-----------+----------------------------+-------+-----+----------+---------+--------+
| benchmark | subject   | set                        | revs  | its | mem_peak | mode    | rstdev |
+-----------+-----------+----------------------------+-------+-----+----------+---------+--------+
| MainBench | benchMake | without constructor        | 10000 | 10  | 4.116mb  | 0.568μs | ±1.73% |
| MainBench | benchMake | with empty constructor     | 10000 | 10  | 4.118mb  | 1.991μs | ±2.41% |
| MainBench | benchMake | with not empty constructor | 10000 | 10  | 4.125mb  | 4.719μs | ±2.91% |
+-----------+-----------+----------------------------+-------+-----+----------+---------+--------+
samdark commented 3 years ago

@xepozz would you please add a line for changelog?

@vjik worth checking if Factory performance could be improved the same way. Usage pattern is similar.

samdark commented 3 years ago

We need to check if the change slows down creating 1000 of different objects.

vjik commented 1 year ago

@vjik worth checking if Factory performance could be improved the same way. Usage pattern is similar.

Definitions caches reflection already: https://github.com/yiisoft/definitions/blob/master/src/Helpers/DefinitionExtractor.php#L37

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (e3968b3) 100.00% compared to head (3ca4d05) 100.00%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #52 +/- ## =========================================== Coverage 100.00% 100.00% - Complexity 85 88 +3 =========================================== Files 3 3 Lines 203 211 +8 =========================================== + Hits 203 211 +8 ``` | [Files Changed](https://app.codecov.io/gh/yiisoft/injector/pull/52?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yiisoft) | Coverage Δ | | |---|---|---| | [src/Injector.php](https://app.codecov.io/gh/yiisoft/injector/pull/52?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yiisoft#diff-c3JjL0luamVjdG9yLnBocA==) | `100.00% <100.00%> (ø)` | |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

vjik commented 1 year ago

Decided to replace constructor parameter to method withCacheReflections(bool)