-
-
Notifications
You must be signed in to change notification settings - Fork 336
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
Faster AddReturnTypeDeclarationBasedOnParentClassMethodRector #4804
Conversation
|
||
use Rector\Tests\TypeDeclaration\Rector\ClassMethod\AddReturnTypeDeclarationBasedOnParentClassMethodRector\Source\SomeClassWithoutReturnType; | ||
|
||
class MyClass extends SomeClassWithoutReturnType |
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.
this test was asserting that no changes happen -> moved it to a "skip fixture"
return null; | ||
} | ||
|
||
$parentClassMethod = $this->astResolver->resolveClassMethodFromMethodReflection($parentMethodReflection); |
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.
astResolver->resolveClassMethodFromMethodReflection
is super in-efficient because it needs to parse the whole file into nodes
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.
Looking good, thank you @staabm
@TomasVotruba for some reason, this PR and #4805 is not mentioned in the latest release notes |
@staabm That's because I try to mention changes that might be interesting to the end user to avoid creating clutter news with all the merges. Tiny fixes and speed improvements depens on the impact. |
running
AddReturnTypeDeclarationBasedOnParentClassMethodRector
on a class which misses returnt types, and extends a class with several thousands lines of code (e.g. TCPDF) takes 45-50 seconds before this PR and 5 seconds after this PR.before this PR it took 768 MB of memory, after the PR it takes 256 MB
-> ~10 times faster
-> requires only 30% memory of previous approach