Skip to content
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 name resolving #4955

Merged
merged 4 commits into from Sep 9, 2023
Merged

Faster name resolving #4955

merged 4 commits into from Sep 9, 2023

Conversation

staabm
Copy link
Contributor

@staabm staabm commented Sep 9, 2023

in the worker profiles of mautic I could see these rectors spending a lot of time in repeated name resolving.

I moved the name determination before the loop and used a lower level name-resolver method for comparison.
I did so only in the rectors which were reported in profiles


running on mautic..

before this PR

time php ../rector-src/bin/rector.php --dry-run --no-progress-bar --no-diffs --clear-cache
                                                                                                                        
 [OK] 7 files would have changed (dry-run) by Rector                                                                    
                                                                                                                        
php ../rector-src/bin/rector.php --dry-run --no-progress-bar --no-diffs   478.62s user 8.69s system 873% cpu 55.776 total

with this PR

time php ../rector-src/bin/rector.php --dry-run --no-progress-bar --no-diffs --clear-cache
                                                                                                                        
 [OK] 7 files would have changed (dry-run) by Rector                                                                    
                                                                                                                        
php ../rector-src/bin/rector.php --dry-run --no-progress-bar --no-diffs   396.58s user 9.82s system 817% cpu 49.711 total

-> 80 seconds faster

@samsonasik
Copy link
Member

iirc, the total time is that should be seen, which 6 seconds faster instead of 80 seconds faster, you may looking using parallel so it collected for all processes time.

That still improvement tho :)

@staabm staabm marked this pull request as ready for review September 9, 2023 17:59
@staabm
Copy link
Contributor Author

staabm commented Sep 9, 2023

any suggestions regarding the last phpstan error?

@samsonasik
Copy link
Member

move complex logic to private method should can reduce complexity

@staabm
Copy link
Contributor Author

staabm commented Sep 9, 2023

I looked at the wrong method and was wondering what PHPStan is complaining about ;)

Copy link
Member

@samsonasik samsonasik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets give it a try, thank you @staabm

@samsonasik samsonasik merged commit 474b3a2 into rectorphp:main Sep 9, 2023
38 checks passed
@staabm staabm deleted the name branch September 9, 2023 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants