Skip to content

Made ParentClass reflection resolving lazier#2584

Merged
ondrejmirtes merged 2 commits intophpstan:1.10.xfrom
staabm:lazy-class-parent-reflection
Aug 25, 2023
Merged

Made ParentClass reflection resolving lazier#2584
ondrejmirtes merged 2 commits intophpstan:1.10.xfrom
staabm:lazy-class-parent-reflection

Conversation

@staabm
Copy link
Copy Markdown
Contributor

@staabm staabm commented Aug 25, 2023 •
edited
Loading

before this PR we looked up the whole parent class hierarchy, even though we work thru the results one by one and early exit on success. this means we are doing more reflection then necessary.

perf comparison

with 1.10.x@6754c2308

mstaab@NB-COMPLEX-61 MINGW64 /c/dvl/Workspace/phpstan-src-staabm (1.10.x)
$ php bin/phpstan clear-result-cache -c ../motiontm/app/partner/phpstan.neon.dist -q && time php bin/phpstan analyse -c ../motiontm/app/partner/phpstan.neon.dist -q

real    0m39.625s
user    0m0.031s
sys     0m0.046s

mstaab@NB-COMPLEX-61 MINGW64 /c/dvl/Workspace/phpstan-src-staabm (1.10.x)
$ php bin/phpstan clear-result-cache -c ../motiontm/app/partner/phpstan.neon.dist -q && time php bin/phpstan analyse -c ../motiontm/app/partner/phpstan.neon.dist -q

real    0m42.209s
user    0m0.000s
sys     0m0.046s

mstaab@NB-COMPLEX-61 MINGW64 /c/dvl/Workspace/phpstan-src-staabm (1.10.x)
$ php bin/phpstan clear-result-cache -c ../motiontm/app/partner/phpstan.neon.dist -q && time php bin/phpstan analyse -c ../motiontm/app/partner/phpstan.neon.dist -q

real    0m40.986s
user    0m0.000s
sys     0m0.031s

with this PR:

mstaab@NB-COMPLEX-61 MINGW64 /c/dvl/Workspace/phpstan-src-staabm (lazy-class-parent-reflection)
$ php bin/phpstan clear-result-cache -c ../motiontm/app/partner/phpstan.neon.dist -q && time php bin/phpstan analyse -c ../motiontm/app/partner/phpstan.neon.dist -q

real    0m39.760s
user    0m0.062s
sys     0m0.015s

mstaab@NB-COMPLEX-61 MINGW64 /c/dvl/Workspace/phpstan-src-staabm (lazy-class-parent-reflection)
$ php bin/phpstan clear-result-cache -c ../motiontm/app/partner/phpstan.neon.dist -q && time php bin/phpstan analyse -c ../motiontm/app/partner/phpstan.neon.dist -q

real    0m40.682s
user    0m0.000s
sys     0m0.015s

mstaab@NB-COMPLEX-61 MINGW64 /c/dvl/Workspace/phpstan-src-staabm (lazy-class-parent-reflection)
$ php bin/phpstan clear-result-cache -c ../motiontm/app/partner/phpstan.neon.dist -q && time php bin/phpstan analyse -c ../motiontm/app/partner/phpstan.neon.dist -q

real    0m38.821s
user    0m0.000s
sys     0m0.046s

its not a huge win, but I think the improvemnt adds up as more worker processes are started, as every worker has some overhead in doing too much reflection.

we could even consider marking getParents() as @deprecated, to make sure other 3rd party rules/extension don't do overaggressive reflection?

@staabm
Copy link
Copy Markdown
Contributor Author

staabm commented Aug 25, 2023

the actual diff is in dbee4b5

27f265c is only re-shuffling the code to improve readability

@ondrejmirtes ondrejmirtes merged commit 2d58174 into phpstan:1.10.x Aug 25, 2023
@ondrejmirtes
Copy link
Copy Markdown
Member

ondrejmirtes commented Aug 25, 2023

Thank you, this makes total sense.

I wouldn't go as far as deprecating the other method, it's okay to use if if you don't early-return from a foreach.

@staabm staabm deleted the lazy-class-parent-reflection branch August 25, 2023 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@staabm @ondrejmirtes